From b989ac6db5abbfca2e048089e2de3f06191d26ec Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Mon, 3 Jan 2022 18:50:41 -0500 Subject: [PATCH] Internals: Support linking recursive function calls (but not later stages) --- src/V3Ast.h | 4 ++++ src/V3AstNodes.cpp | 7 ++++--- src/V3LinkDot.cpp | 9 ++++++--- src/V3Simulate.h | 7 +++++++ src/V3Width.cpp | 10 +++++++--- test_regress/t/t_func_recurse.out | 7 ++++--- test_regress/t/t_func_recurse2.out | 2 +- test_regress/t/t_func_recurse_param.out | 17 ++++++++++++++--- test_regress/t/t_func_recurse_param_bad.out | 12 +++++++++--- 9 files changed, 56 insertions(+), 19 deletions(-) diff --git a/src/V3Ast.h b/src/V3Ast.h index 490aa2e80..ae0d45178 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -2750,6 +2750,7 @@ private: bool m_isHideProtected : 1; // Verilog protected bool m_pure : 1; // DPI import pure (vs. virtual pure) bool m_pureVirtual : 1; // Pure virtual + bool m_recursive : 1; // Recusive or part of recursion bool m_underGenerate : 1; // Under generate (for warning) bool m_virtual : 1; // Virtual method in class VLifetime m_lifetime; // Lifetime @@ -2774,6 +2775,7 @@ protected: , m_isHideProtected{false} , m_pure{false} , m_pureVirtual{false} + , m_recursive{false} , m_underGenerate{false} , m_virtual{false} { addNOp3p(stmtsp); @@ -2843,6 +2845,8 @@ public: bool pure() const { return m_pure; } void pureVirtual(bool flag) { m_pureVirtual = flag; } bool pureVirtual() const { return m_pureVirtual; } + void recursive(bool flag) { m_recursive = flag; } + bool recursive() const { return m_recursive; } void underGenerate(bool flag) { m_underGenerate = flag; } bool underGenerate() const { return m_underGenerate; } void isVirtual(bool flag) { m_virtual = flag; } diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index dd05e1be1..0ec29945f 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -1802,12 +1802,13 @@ void AstNodeFTaskRef::dump(std::ostream& str) const { void AstNodeFTask::dump(std::ostream& str) const { this->AstNode::dump(str); if (classMethod()) str << " [METHOD]"; - if (taskPublic()) str << " [PUBLIC]"; - if (prototype()) str << " [PROTOTYPE]"; - if (dpiImport()) str << " [DPII]"; if (dpiExport()) str << " [DPIX]"; + if (dpiImport()) str << " [DPII]"; if (dpiOpenChild()) str << " [DPIOPENCHILD]"; if (dpiOpenParent()) str << " [DPIOPENPARENT]"; + if (prototype()) str << " [PROTOTYPE]"; + if (recursive()) str << " [RECURSIVE]"; + if (taskPublic()) str << " [PUBLIC]"; if ((dpiImport() || dpiExport()) && cname() != name()) str << " [c=" << cname() << "]"; } void AstNodeBlock::dump(std::ostream& str) const { diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index 18b684b64..7bec4d6b3 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -1860,7 +1860,7 @@ private: VSymEnt* m_pinSymp = nullptr; // SymEnt for pin lookups const AstCell* m_cellp = nullptr; // Current cell AstNodeModule* m_modp = nullptr; // Current module - const AstNodeFTask* m_ftaskp = nullptr; // Current function/task + AstNodeFTask* m_ftaskp = nullptr; // Current function/task int m_modportNum = 0; // Uniqueify modport numbers struct DotStates { @@ -2748,8 +2748,11 @@ private: if (foundp) { if (VN_IS(foundp->nodep(), Var) && m_ds.m_dotText == "" && m_ftaskp && m_ftaskp->name() == foundp->nodep()->name()) { - nodep->v3warn(E_UNSUPPORTED, "Unsupported: Recursive function call " - << nodep->prettyNameQ()); + // This is a recursive reference to the function itself, not to the var + nodep->taskp(m_ftaskp); + nodep->classOrPackagep(foundp->classOrPackagep()); + UINFO(7, " Resolved recursive " << nodep + << endl); // Also prints taskp } else { nodep->v3error("Found definition of '" << m_ds.m_dotText << (m_ds.m_dotText == "" ? "" : ".") diff --git a/src/V3Simulate.h b/src/V3Simulate.h index 30c5ed277..d9d4cb990 100644 --- a/src/V3Simulate.h +++ b/src/V3Simulate.h @@ -977,6 +977,13 @@ private: VL_DANGLING(funcp); // Make sure we've sized the function funcp = nodep->taskp(); UASSERT_OBJ(funcp, nodep, "Not linked"); + if (funcp->recursive()) { + // Because we attach values to nodes rather then making a stack, this is a mess + // When we do support this, we need a stack depth limit of 1K or something, + // and the t_func_recurse_param_bad.v test should check that limit's error message + clearOptimizable(funcp, "Unsupported: Recursive constant functions"); + return; + } // Apply function call values to function V3TaskConnects tconnects = V3Task::taskConnects(nodep, nodep->taskp()->stmtsp()); // Must do this in two steps, eval all params, then apply them diff --git a/src/V3Width.cpp b/src/V3Width.cpp index b616c10f6..4b934f2e3 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -4538,8 +4538,10 @@ private: // Grab width from the output variable (if it's a function) if (nodep->didWidth()) return; if (nodep->doingWidth()) { - nodep->v3warn(E_UNSUPPORTED, "Unsupported: Recursive function or task call"); - nodep->dtypeSetBit(); + UINFO(5, "Recursive function or task call: " << nodep); + nodep->v3warn(E_UNSUPPORTED, "Unsupported: Recursive function or task call: " + << nodep->prettyNameQ()); + nodep->recursive(true); nodep->didWidth(true); return; } @@ -4554,7 +4556,8 @@ private: // Would use user1 etc, but V3Width called from too many places to spend a user nodep->doingWidth(true); m_ftaskp = nodep; - userIterateChildren(nodep, nullptr); + // First width the function variable, as if is a recursive function we need data type + if (nodep->fvarp()) userIterate(nodep->fvarp(), nullptr); if (nodep->isConstructor()) { // Pretend it's void so less special casing needed when look at dtypes nodep->dtypeSetVoid(); @@ -4563,6 +4566,7 @@ private: UASSERT_OBJ(m_funcp, nodep, "FTask with function variable, but isn't a function"); nodep->dtypeFrom(nodep->fvarp()); // Which will get it from fvarp()->dtypep() } + userIterateChildren(nodep, nullptr); nodep->didWidth(true); nodep->doingWidth(false); m_funcp = nullptr; diff --git a/test_regress/t/t_func_recurse.out b/test_regress/t/t_func_recurse.out index 6d1b05195..81f9b9dde 100644 --- a/test_regress/t/t_func_recurse.out +++ b/test_regress/t/t_func_recurse.out @@ -1,5 +1,6 @@ -%Error-UNSUPPORTED: t/t_func_recurse.v:12:31: Unsupported: Recursive function call 'recurse_self' - 12 | else recurse_self = i + recurse_self(i - 1) * 2; - | ^~~~~~~~~~~~ +%Error-UNSUPPORTED: t/t_func_recurse.v:9:27: Unsupported: Recursive function or task call: 'recurse_self' + : ... In instance t + 9 | function automatic int recurse_self; + | ^~~~~~~~~~~~ ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest %Error: Exiting due to diff --git a/test_regress/t/t_func_recurse2.out b/test_regress/t/t_func_recurse2.out index 476d8791a..b93b57e47 100644 --- a/test_regress/t/t_func_recurse2.out +++ b/test_regress/t/t_func_recurse2.out @@ -1,4 +1,4 @@ -%Error-UNSUPPORTED: t/t_func_recurse2.v:9:27: Unsupported: Recursive function or task call +%Error-UNSUPPORTED: t/t_func_recurse2.v:9:27: Unsupported: Recursive function or task call: 'recurse_1' : ... In instance t 9 | function automatic int recurse_1; | ^~~~~~~~~ diff --git a/test_regress/t/t_func_recurse_param.out b/test_regress/t/t_func_recurse_param.out index f43ed155d..102d49d6f 100644 --- a/test_regress/t/t_func_recurse_param.out +++ b/test_regress/t/t_func_recurse_param.out @@ -1,5 +1,16 @@ -%Error-UNSUPPORTED: t/t_func_recurse_param.v:12:31: Unsupported: Recursive function call 'recurse_self' - 12 | else recurse_self = i + recurse_self(i - 1) * 2; - | ^~~~~~~~~~~~ +%Error-UNSUPPORTED: t/t_func_recurse_param.v:9:27: Unsupported: Recursive function or task call: 'recurse_self' + : ... In instance t + 9 | function automatic int recurse_self; + | ^~~~~~~~~~~~ ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest +%Error: t/t_func_recurse_param.v:15:26: Expecting expression to be constant, but can't determine constant for FUNCREF 'recurse_self' + : ... In instance t + t/t_func_recurse_param.v:9:27: ... Location of non-constant FUNC 'recurse_self': Unsupported: Recursive constant functions + 15 | localparam int ZERO = recurse_self(0); + | ^~~~~~~~~~~~ +%Error: t/t_func_recurse_param.v:16:28: Expecting expression to be constant, but can't determine constant for FUNCREF 'recurse_self' + : ... In instance t + t/t_func_recurse_param.v:9:27: ... Location of non-constant FUNC 'recurse_self': Unsupported: Recursive constant functions + 16 | localparam int ELEVEN = recurse_self(3); + | ^~~~~~~~~~~~ %Error: Exiting due to diff --git a/test_regress/t/t_func_recurse_param_bad.out b/test_regress/t/t_func_recurse_param_bad.out index 44cf3059a..121ee90f9 100644 --- a/test_regress/t/t_func_recurse_param_bad.out +++ b/test_regress/t/t_func_recurse_param_bad.out @@ -1,5 +1,11 @@ -%Error-UNSUPPORTED: t/t_func_recurse_param_bad.v:12:31: Unsupported: Recursive function call 'recurse_self' - 12 | else recurse_self = i + recurse_self(i - 1) * 2; - | ^~~~~~~~~~~~ +%Error-UNSUPPORTED: t/t_func_recurse_param_bad.v:9:27: Unsupported: Recursive function or task call: 'recurse_self' + : ... In instance t + 9 | function automatic int recurse_self; + | ^~~~~~~~~~~~ ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest +%Error: t/t_func_recurse_param_bad.v:15:26: Expecting expression to be constant, but can't determine constant for FUNCREF 'recurse_self' + : ... In instance t + t/t_func_recurse_param_bad.v:9:27: ... Location of non-constant FUNC 'recurse_self': Unsupported: Recursive constant functions + 15 | localparam int HUGE = recurse_self(10000); + | ^~~~~~~~~~~~ %Error: Exiting due to