Fix handling of static keyword in methods (verilator#4649)

antmicro · Oct 31, 2023 · a764c70 · a764c70
1 parent a27abab
commit a764c70
Show file tree

Hide file tree

Showing 14 changed files with 60 additions and 34 deletions.
diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h
@@ -84,10 +84,11 @@ class AstNodeFTask VL_NOT_FINAL : public AstNode {
     bool m_dpiPure : 1;  // DPI import pure (vs. virtual pure)
     bool m_pureVirtual : 1;  // Pure virtual
     bool m_recursive : 1;  // Recursive or part of recursion
+    bool m_static : 1;  // Static method in class
     bool m_underGenerate : 1;  // Under generate (for warning)
     bool m_virtual : 1;  // Virtual method in class
     bool m_needProcess : 1;  // Needs access to VlProcess of the caller
-    VLifetime m_lifetime;  // Lifetime
+    VLifetime m_lifetime;  // Default lifetime of local vars
     VIsCached m_purity;  // Pure state
 
 protected:
@@ -112,6 +113,7 @@ class AstNodeFTask VL_NOT_FINAL : public AstNode {
         , m_dpiPure{false}
         , m_pureVirtual{false}
         , m_recursive{false}
+        , m_static{false}
         , m_underGenerate{false}
         , m_virtual{false}
         , m_needProcess{false} {
@@ -173,6 +175,8 @@ class AstNodeFTask VL_NOT_FINAL : public AstNode {
     bool pureVirtual() const { return m_pureVirtual; }
     void recursive(bool flag) { m_recursive = flag; }
     bool recursive() const { return m_recursive; }
+    void isStatic(bool flag) { m_static = flag; }
+    bool isStatic() const { return m_static; }
     void underGenerate(bool flag) { m_underGenerate = flag; }
     bool underGenerate() const { return m_underGenerate; }
     void isVirtual(bool flag) { m_virtual = flag; }

diff --git a/src/V3Class.cpp b/src/V3Class.cpp
@@ -118,7 +118,7 @@ class ClassVisitor final : public VNVisitor {
     void visit(AstVar* nodep) override {
         iterateChildren(nodep);
         if (m_packageScopep) {
-            if (m_ftaskp && m_ftaskp->lifetime().isStatic()) {
+            if (m_ftaskp && m_ftaskp->isStatic()) {
                 // Move later, or we wouldn't keep iterating the class
                 // We're really moving the VarScope but we might not
                 // have a pointer to it yet
@@ -143,7 +143,7 @@ class ClassVisitor final : public VNVisitor {
         {
             m_ftaskp = nodep;
             iterateChildren(nodep);
-            if (m_packageScopep && nodep->lifetime().isStatic()) {
+            if (m_packageScopep && nodep->isStatic()) {
                 m_toScopeMoves.emplace_back(nodep, m_packageScopep);
             }
         }

diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp
@@ -3185,7 +3185,7 @@ class LinkDotResolveVisitor final : public VNVisitor {
             AstNodeFTask* const taskp
                 = foundp ? VN_CAST(foundp->nodep(), NodeFTask) : nullptr;  // Maybe nullptr
             if (taskp) {
-                if (staticAccess && !taskp->lifetime().isStatic()) {
+                if (staticAccess && !taskp->isStatic()) {
                     // TODO bug4077
                     // nodep->v3error("Static access to non-static task/function "
                     //                << taskp->prettyNameQ() << endl);
@@ -3358,6 +3358,7 @@ class LinkDotResolveVisitor final : public VNVisitor {
                 // External definition cannot have any specifiers, so no value will be overwritten.
                 nodep->isHideLocal(funcProtop->isHideLocal());
                 nodep->isHideProtected(funcProtop->isHideProtected());
+                nodep->isStatic(funcProtop->isStatic());
                 nodep->isVirtual(funcProtop->isVirtual());
                 nodep->lifetime(funcProtop->lifetime());
             } else {

diff --git a/src/V3LinkParse.cpp b/src/V3LinkParse.cpp
@@ -292,19 +292,10 @@ class LinkParseVisitor final : public VNVisitor {
             nodep->v3warn(STATICVAR, "Static variable with assignment declaration declared in a "
                                      "loop converted to automatic");
         }
-        if (m_ftaskp) {
-            bool classMethod = m_ftaskp->classMethod();
-            if (!classMethod) {
-                AstClassOrPackageRef* const pkgrefp
-                    = VN_CAST(m_ftaskp->classOrPackagep(), ClassOrPackageRef);
-                if (pkgrefp && VN_IS(pkgrefp->classOrPackagep(), Class)) classMethod = true;
-            }
-            if (classMethod && nodep->lifetime().isNone()) {
-                nodep->lifetime(VLifetime::AUTOMATIC);
-            }
-        }
-        if (nodep->lifetime().isNone() && nodep->varType() != VVarType::PORT) {
-            nodep->lifetime(m_lifetime);
+        if (nodep->varType() != VVarType::PORT) {
+            if (nodep->lifetime().isNone()) nodep->lifetime(m_lifetime);
+        } else if (m_ftaskp) {
+            nodep->lifetime(VLifetime::AUTOMATIC);
         }
 
         if (nodep->isGParam() && !nodep->isAnsi()) {  // shadow some parameters into localparams

diff --git a/src/V3ParseImp.h b/src/V3ParseImp.h
@@ -54,9 +54,8 @@ struct VMemberQualifiers {
             uint32_t m_rand : 1;  // Rand property/member qualifier
             uint32_t m_randc : 1;  // Randc property/member qualifier (ignored until supported)
             uint32_t m_virtual : 1;  // Virtual property/method qualifier
-            uint32_t m_automatic : 1;  // Automatic property/method qualifier
             uint32_t m_const : 1;  // Const property/method qualifier
-            uint32_t m_static : 1;  // Static class method
+            uint32_t m_static : 1;  // Static class member
         };
     };
     static VMemberQualifiers none() {
@@ -73,9 +72,8 @@ struct VMemberQualifiers {
         for (AstNodeFTask* nodep = nodesp; nodep; nodep = VN_AS(nodep->nextp(), NodeFTask)) {
             if (m_local) nodep->isHideLocal(true);
             if (m_protected) nodep->isHideProtected(true);
+            if (m_static) nodep->isStatic(true);
             if (m_virtual) nodep->isVirtual(true);
-            if (m_automatic) nodep->lifetime(VLifetime::AUTOMATIC);
-            if (m_static) nodep->lifetime(VLifetime::STATIC);
             if (m_const || m_rand || m_randc) {
                 nodep->v3error("Syntax error: 'const'/'rand'/'randc' not allowed before "
                                "function/task declaration");
@@ -88,7 +86,6 @@ struct VMemberQualifiers {
             if (m_randc) nodep->isRandC(true);
             if (m_local) nodep->isHideLocal(true);
             if (m_protected) nodep->isHideProtected(true);
-            if (m_automatic) nodep->lifetime(VLifetime::AUTOMATIC);
             if (m_static) nodep->lifetime(VLifetime::STATIC);
             if (m_const) nodep->isConst(true);
             if (m_virtual) {

diff --git a/src/V3Task.cpp b/src/V3Task.cpp
@@ -1693,7 +1693,7 @@ V3TaskConnects V3Task::taskConnects(AstNodeFTaskRef* nodep, AstNode* taskStmtsp,
                 newvaluep = new AstConst{nodep->fileline(), AstConst::Unsized32{}, 0};
             } else if (AstFuncRef* const funcRefp = VN_CAST(portp->valuep(), FuncRef)) {
                 const AstNodeFTask* const funcp = funcRefp->taskp();
-                if (funcp->classMethod() && funcp->lifetime().isStatic()) newvaluep = funcRefp;
+                if (funcp->classMethod() && funcp->isStatic()) newvaluep = funcRefp;
             } else if (AstConst* const constp = VN_CAST(portp->valuep(), Const)) {
                 newvaluep = constp;
             }

diff --git a/src/V3Width.cpp b/src/V3Width.cpp
@@ -3529,7 +3529,7 @@ class WidthVisitor final : public VNVisitor {
             if (AstNodeFTask* const ftaskp
                 = VN_CAST(m_memberMap.findMember(classp, nodep->name()), NodeFTask)) {
                 userIterate(ftaskp, nullptr);
-                if (ftaskp->lifetime().isStatic()) {
+                if (ftaskp->isStatic()) {
                     AstNodeExpr* argsp = nullptr;
                     if (nodep->pinsp()) argsp = nodep->pinsp()->unlinkFrBackWithNext();
                     AstNodeFTaskRef* newp = nullptr;

diff --git a/src/verilog.y b/src/verilog.y
@@ -7100,8 +7100,6 @@ memberQualOne<qualifiers>:                      // IEEE: property_qualifier + me
         |       yVIRTUAL__ETC                           { $$ = VMemberQualifiers::none(); $$.m_virtual = true; }
         //                      // Part of property_qualifier only
         |       random_qualifier                        { $$ = $1; }
-        //                      // Part of lifetime, but here as ySTATIC can be in different positions
-        |       yAUTOMATIC                              { $$ = VMemberQualifiers::none(); $$.m_automatic = true; }
         //                      // Part of data_declaration, but not in data_declarationVarFrontClass
         |       yCONST__ETC                             { $$ = VMemberQualifiers::none(); $$.m_const = true; }
         ;

diff --git a/test_regress/t/t_class_unsup_bad.out b/test_regress/t/t_class_unsup_bad.out
@@ -1,4 +1,4 @@
-%Error: t/t_class_unsup_bad.v:29:24: Syntax error: 'const'/'rand'/'randc' not allowed before function/task declaration
-   29 |    const function void func_const; endfunction
+%Error: t/t_class_unsup_bad.v:28:24: Syntax error: 'const'/'rand'/'randc' not allowed before function/task declaration
+   28 |    const function void func_const; endfunction
       |                        ^~~~~~~~~~
 %Error: Exiting due to
diff --git a/test_regress/t/t_class_unsup_bad.v b/test_regress/t/t_class_unsup_bad.v
@@ -25,7 +25,6 @@ class C #(parameter P=1);
    function int classfunc; endfunction
    virtual function void func_virtual; endfunction
    pure virtual function void func_pure_virtual;
-   automatic function void func_automatic; endfunction
    const function void func_const; endfunction
    extern task exttask;
 endclass

diff --git a/test_regress/t/t_func_defaults.v b/test_regress/t/t_func_defaults.v
@@ -21,7 +21,7 @@ endfunction
 
 class Foo;
    static int x;
-   function static int get_x;
+   static function int get_x;
       return x;
    endfunction
 endclass

diff --git a/test_regress/t/t_struct_cons_cast.v b/test_regress/t/t_struct_cons_cast.v
@@ -27,15 +27,15 @@ class Cls;
 
    state_info_t m_recur_states/*[uvm_object][uvm_object]*/[uvm_recursion_policy_enum];
 
-   automatic function uvm_recursion_policy_enum get_recursion_policy();
+   function uvm_recursion_policy_enum get_recursion_policy();
       return UVM_DEEP;
    endfunction
 
-   automatic function bit get_ret_val();
+   function bit get_ret_val();
       return $c(1);
    endfunction
 
-   automatic function void test();
+   function void test();
       bit ret_val;
       ret_val = $c1(1);
       // See issue #4568

diff --git a/test_regress/t/t_var_local.v b/test_regress/t/t_var_local.v
@@ -4,6 +4,21 @@
 // any use, without warranty, 2003 by Wilson Snyder.
 // SPDX-License-Identifier: CC0-1.0
 
+class cls;
+  static task automatic tsk1;
+     integer task_assign = 1;
+     if (task_assign != 1) $stop;
+     task_assign = 2;
+     if (task_assign != 2) $stop;
+  endtask
+  static task tsk2;
+     integer task_assign = 1;
+     if (task_assign != 1) $stop;
+     task_assign = 2;
+     if (task_assign != 2) $stop;
+  endtask
+endclass
+
 module t;
 
    integer top;
@@ -43,8 +58,13 @@ module t;
             end
          end
       end
+      // Repeat task calls to ensure we reinit the initial value
+      tsk;
       tsk;
-      tsk; // Second time to ensure we reinit the initial value
+      cls::tsk1();
+      cls::tsk1();
+      cls::tsk2();
+      cls::tsk2();
       $write("*-* All Finished *-*\n");
       $finish;
    end

diff --git a/test_regress/t/t_var_static.v b/test_regress/t/t_var_static.v
@@ -16,6 +16,17 @@ package my_pkg;
    endfunction
 endpackage
 
+class my_cls;
+   static function int get_cnt1;
+      static int cnt = 0;
+      return ++cnt;
+   endfunction
+   static function static int get_cnt2;
+      int cnt = 0;
+      return ++cnt;
+   endfunction
+endclass
+
 module t (/*AUTOARG*/
    // Inputs
    clk
@@ -89,6 +100,11 @@ module t (/*AUTOARG*/
       v = my_pkg::f_no_st_pkg(); `checkh(v, 1);
       v = my_pkg::f_no_st_pkg(); `checkh(v,   2);
       //
+      v = my_cls::get_cnt1(); `checkh(v,   1);
+      v = my_cls::get_cnt1(); `checkh(v,   2);
+      v = my_cls::get_cnt2(); `checkh(v,   1);
+      v = my_cls::get_cnt2(); `checkh(v,   2);
+      //
    end
 
    int cyc = 0;