Add /*verilator public[flat|flat_rd|flat_rw| ]*/ metacomments (#3894)

docs/CONTRIBUTORS

@@ -65,6 +65,7 @@ Joey Liu
 John Coiner
 John Demme
 Jonathan Drolet
+Joseph Nwabueze
 Josh Redford
 Julie Schwartz
 Julien Margetts

docs/guide/extensions.rst

@@ -446,6 +446,31 @@ or "`ifdef`"'s may break other tools.
 
    Same as :option:`public_flat_rw` configuration file option.
 
+.. option:: /*verilator&32;public_[|flat|flat_rd|flat_rw]_on [@(<edge_list>)]*/ (as scope)
+
+   Used to wrap multiple signals and parameters with the respective public attribute.
+   See attribute above for their respective behavior. Cannot be nested. e.g:
+
+      .. code-block:: sv
+         /*verilator public_flat_rw_on*/
+         logic clk;
+         logic rst;
+         parameter width = 8;
+         /* verilator public_off*/
+         logic data;
+
+   Is equivalent to:
+
+      .. code-block:: sv
+         logic clk /*verilator public_flat_rw*/;
+         logic rst /*verilator public_flat_rw*/;
+         parameter width /*verilator public_flat_rw*/ = 8;
+         logic data;
+
+.. option:: /*verilator&32;public_off*/
+
+   Terminates the previous `/*verilator public*_on*/` directive; see above.
+
 .. option:: /*verilator&32;public_module*/
 
    Used after a module statement to indicate the module should not be

src/V3Ast.h

@@ -805,6 +805,10 @@ public:
     bool isTemp() const {
         return (m_e == BLOCKTEMP || m_e == MODULETEMP || m_e == STMTTEMP || m_e == XTEMP);
     }
+    bool isVPIAccessible() const {
+        return (m_e == VAR || m_e == GPARAM || m_e == LPARAM || m_e == PORT || m_e == WIRE
+                || m_e == TRI0 || m_e == TRI1);
+    }
 };
 constexpr bool operator==(const VVarType& lhs, const VVarType& rhs) VL_MT_SAFE {
     return lhs.m_e == rhs.m_e;

src/V3Error.h

@@ -187,7 +187,7 @@ public:
             "IMPERFECTSCH", "IMPLICIT", "IMPLICITSTATIC", "IMPORTSTAR", "IMPURE",
             "INCABSPATH", "INFINITELOOP", "INITIALDLY", "INSECURE",
             "LATCH", "LITENDIAN", "MINTYPMAXDLY", "MODDUP",
-            "MULTIDRIVEN", "MULTITOP","NOLATCH", "NULLPORT", "PINCONNECTEMPTY",
+            "MULTIDRIVEN", "MULTITOP", "NOLATCH", "NULLPORT", "PINCONNECTEMPTY",
             "PINMISSING", "PINNOCONNECT",  "PINNOTFOUND", "PKGNODECL", "PROCASSWIRE",
             "PROFOUTOFDATE", "PROTECTED", "RANDC", "REALCVT", "REDEFMACRO", "RISEFALLDLY",
             "SELRANGE", "SHORTREAL", "SPLITVAR", "STMTDLY", "SYMRSVDWORD", "SYNCASYNCNET",

src/V3ParseGrammar.cpp

@@ -240,6 +240,9 @@ AstVar* V3ParseGrammar::createVariable(FileLine* fileline, const string& name,
     } else {
         nodep->trace(allTracingOn(nodep->fileline()));
     }
+    if (nodep->varType().isVPIAccessible()) {
+        nodep->addAttrsp(GRAMMARP->cloneScopedSigAttr());
+    }
 
     // Remember the last variable created, so we can attach attributes to it in later parsing
     GRAMMARP->m_varAttrp = nodep;

src/V3PreProc.cpp

@@ -472,12 +472,16 @@ void V3PreProcImp::comment(const string& text) {
             // else ignore the comment we don't recognize
         }  // else no assertions
     } else if (vlcomment) {
-        string::size_type pos;
+        if (VString::startsWith(cmd, "public_flat_rw")) {
-        if ((pos = cmd.find("public_flat_rw")) != string::npos) {
             // "/*verilator public_flat_rw @(foo) */" -> "/*verilator public_flat_rw*/ @(foo)"
-            cmd = cmd.substr(pos + std::strlen("public_flat_rw"));
+            string::size_type endOfCmd = std::strlen("public_flat_rw");
-            while (isspace(cmd[0])) cmd = cmd.substr(1);
+            while (VString::isWordChar(cmd[endOfCmd])) ++endOfCmd;
-            if (!printed) insertUnreadback("/*verilator public_flat_rw*/ " + cmd + " /**/");
+            string baseCmd = cmd.substr(0, endOfCmd);
+            string arg = cmd.substr(endOfCmd);
+            while (isspace(arg[0])) arg = arg.substr(1);
+            if (arg.size() && baseCmd == "public_flat_rw_on")
+                baseCmd += "_sns"; // different cmd for applying sensitivity
+            if (!printed) insertUnreadback("/*verilator " + baseCmd + "*/ " + arg + " /**/");
         } else {
             if (!printed) insertUnreadback("/*verilator " + cmd + "*/");
         }

src/V3String.cpp

@@ -176,16 +176,14 @@ double VString::parseDouble(const string& str, bool* successp) {
     return d;
 }
 
-static bool isWordChar(char c) { return isalnum(c) || c == '_'; }
-
 string VString::replaceWord(const string& str, const string& from, const string& to) {
     string result = str;
     const size_t len = from.size();
     UASSERT_STATIC(len > 0, "Cannot replace empty string");
     for (size_t pos = 0; (pos = result.find(from, pos)) != string::npos; pos += len) {
         // Only replace whole words
-        if (((pos > 0) && isWordChar(result[pos - 1])) ||  //
+        if (((pos > 0) && VString::isWordChar(result[pos - 1])) ||  //
-            ((pos + len < result.size()) && isWordChar(result[pos + len]))) {
+            ((pos + len < result.size()) && VString::isWordChar(result[pos + len]))) {
             continue;
         }
         result.replace(pos, len, to);

src/V3String.h

@@ -122,6 +122,8 @@ public:
     static bool startsWith(const string& str, const string& prefix);
     // Predicate to check if 'str' ends with 'suffix'
     static bool endsWith(const string& str, const string& suffix);
+    // Return true if char is valid character in word
+    static bool isWordChar(char c) { return isalnum(c) || c == '_'; }
 };
 
 //######################################################################

src/verilog.l

@@ -757,9 +757,15 @@ vnum    {vnum1}|{vnum2}|{vnum3}|{vnum4}|{vnum5}
   "/*verilator parallel_case*/"         { FL; return yVL_PARALLEL_CASE; }
   "/*verilator public*/"                { FL; return yVL_PUBLIC; }
   "/*verilator public_flat*/"           { FL; return yVL_PUBLIC_FLAT; }
+  "/*verilator public_flat_on*/"        { FL; return yVL_PUBLIC_FLAT_ON; }
   "/*verilator public_flat_rd*/"        { FL; return yVL_PUBLIC_FLAT_RD; }
+  "/*verilator public_flat_rd_on*/"     { FL; return yVL_PUBLIC_FLAT_RD_ON; }
   "/*verilator public_flat_rw*/"        { FL; return yVL_PUBLIC_FLAT_RW; }  // The @(edge) is converted by the preproc
+  "/*verilator public_flat_rw_on*/"     { FL; return yVL_PUBLIC_FLAT_RW_ON; }
+  "/*verilator public_flat_rw_on_sns*/" { FL; return yVL_PUBLIC_FLAT_RW_ON_SNS; }
   "/*verilator public_module*/"         { FL; return yVL_PUBLIC_MODULE; }
+  "/*verilator public_on*/"             { FL; return yVL_PUBLIC_ON; }
+  "/*verilator public_off*/"            { FL; return yVL_PUBLIC_OFF; }  // terminates previous 'verilator public*_on'
   "/*verilator sc_bv*/"                 { FL; return yVL_SC_BV; }
   "/*verilator sc_clock*/"              { FL; yylval.fl->v3warn(DEPRECATED, "sc_clock is ignored"); FL_BRK; }
   "/*verilator sformat*/"               { FL; return yVL_SFORMAT; }

src/verilog.y

@@ -78,6 +78,7 @@ class V3ParseGrammar {
 public:
     AstVar* m_varAttrp = nullptr;  // Current variable for attribute adding
     AstRange* m_gateRangep = nullptr;  // Current range for gate declarations
+    AstNode* m_scopedSigAttr = nullptr;  // Pointer to default signal attribute
     AstCase* m_caseAttrp = nullptr;  // Current case statement for attribute adding
     AstNodeDType* m_varDTypep = nullptr;  // Pointer to data type for next signal declaration
     AstNodeDType* m_memDTypep = nullptr;  // Pointer to data type for next member declaration
@@ -270,6 +271,19 @@ public:
         }
         return itemsp;
     }
+
+    void setScopedSigAttr(AstNode* attrsp) {
+        if (m_scopedSigAttr) { // clearing set attribute
+                VL_DO_DANGLING(m_scopedSigAttr->deleteTree(), m_scopedSigAttr);
+        }
+        m_scopedSigAttr = attrsp;
+    }
+
+    void createScopedSigAttr(VAttrType vattrT) {
+        setScopedSigAttr(new AstAttrOf{PARSEP->lexFileline(), vattrT});
+    }
+
+    AstNode* cloneScopedSigAttr() const { return AstNode::cloneTreeNull(m_scopedSigAttr, true); }
 };
 
 const VBasicDTypeKwd LOGIC = VBasicDTypeKwd::LOGIC;  // Shorthand "LOGIC"
@@ -951,8 +965,14 @@ BISONPRE_VERSION(3.7,%define api.header.include {"V3ParseBison.h"})
 %token<fl>              yVL_PARALLEL_CASE         "/*verilator parallel_case*/"
 %token<fl>              yVL_PUBLIC                "/*verilator public*/"
 %token<fl>              yVL_PUBLIC_FLAT           "/*verilator public_flat*/"
+%token<fl>              yVL_PUBLIC_FLAT_ON        "/*verilator public_flat_on*/"
 %token<fl>              yVL_PUBLIC_FLAT_RD        "/*verilator public_flat_rd*/"
+%token<fl>              yVL_PUBLIC_FLAT_RD_ON     "/*verilator public_flat_rd_on*/"
 %token<fl>              yVL_PUBLIC_FLAT_RW        "/*verilator public_flat_rw*/"
+%token<fl>              yVL_PUBLIC_FLAT_RW_ON     "/*verilator public_flat_rw_on*/"
+%token<fl>              yVL_PUBLIC_FLAT_RW_ON_SNS "/*verilator public_flat_rw_on_sns*/"
+%token<fl>              yVL_PUBLIC_ON             "/*verilator public_on*/"
+%token<fl>              yVL_PUBLIC_OFF            "/*verilator public_off*/"
 %token<fl>              yVL_PUBLIC_MODULE         "/*verilator public_module*/"
 %token<fl>              yVL_SC_BV                 "/*verilator sc_bv*/"
 %token<fl>              yVL_SFORMAT               "/*verilator sformat*/"
@@ -1233,6 +1253,7 @@ package_item<nodep>:            // ==IEEE: package_item
         |       anonymous_program                       { $$ = $1; }
         |       package_export_declaration              { $$ = $1; }
         |       timeunits_declaration                   { $$ = $1; }
+        |       sigAttrScope                            { $$ = nullptr; }
         ;
 
 package_or_generate_item_declaration<nodep>:    // ==IEEE: package_or_generate_item_declaration
@@ -1344,6 +1365,7 @@ modFront<nodeModulep>:
                           PARSEP->rootp()->addModulesp($$);
                           SYMP->pushNew($$);
                           GRAMMARP->m_modp = $$; }
+        |       modFront sigAttrScope                   { $$ = $1; }
         ;
 
 importsAndParametersE<nodep>:   // IEEE: common part of module_declaration, interface_declaration, program_declaration
@@ -1402,13 +1424,18 @@ parameter_port_listE<nodep>:    // IEEE: parameter_port_list + empty == paramete
 paramPortDeclOrArgList<nodep>:  // IEEE: list_of_param_assignments + { parameter_port_declaration }
                 paramPortDeclOrArg                              { $$ = $1; }
         |       paramPortDeclOrArgList ',' paramPortDeclOrArg   { $$ = $1->addNext($3); }
+        |       paramPortDeclOrArgList sigAttrScope {$$ = $1;}
         ;
 
 paramPortDeclOrArg<nodep>:      // IEEE: param_assignment + parameter_port_declaration
         //                      // We combine the two as we can't tell which follows a comma
+                paramPortDeclOrArgSub     {$$ = $1;}
+        |       vlTag                                   { $$ = nullptr; }
+        ;
+paramPortDeclOrArgSub<nodep>:
                 parameter_port_declarationFrontE param_assignment       { $$ = $2; }
         |       parameter_port_declarationTypeFrontE type_assignment    { $$ = $2; }
-        |       vlTag                                   { $$ = nullptr; }
+        |       sigAttrScope paramPortDeclOrArgSub {$$ = $2; }
         ;
 
 portsStarE<nodep>:              // IEEE: .* + list_of_ports + list_of_port_declarations + empty
@@ -1444,11 +1471,13 @@ portAndTagE<nodep>:
                           $$ = $$->addNext(varp);
                           $$->v3warn(NULLPORT, "Null port on module (perhaps extraneous comma)"); }
         |       portAndTag                              { $$ = $1; }
+        |       portAndTag sigAttrScope                 { $$ = $1; }
         ;
 
 portAndTag<nodep>:
                 port                                    { $$ = $1; }
         |       vlTag port                              { $$ = $2; }  // Tag will associate with previous port
+        |       sigAttrScope portAndTag                 { $$ = $2; }
         ;
 
 port<nodep>:                    // ==IEEE: port
@@ -1597,6 +1626,7 @@ intFront<nodeModulep>:
                           $$->lifetime($2);
                           PARSEP->rootp()->addModulesp($$);
                           SYMP->pushNew($$); }
+        |       intFront sigAttrScope                   { $$ = $1; }
         ;
 
 interface_itemListE<nodep>:
@@ -2590,6 +2620,7 @@ module_common_item<nodep>:      // ==IEEE: module_common_item
         |       loop_generate_construct                 { $$ = $1; }
         |       conditional_generate_construct          { $$ = $1; }
         |       elaboration_system_task                 { $$ = $1; }
+        |       sigAttrScope                            { $$ = nullptr; }
         //
         |       error ';'                               { $$ = nullptr; }
         ;
@@ -2920,7 +2951,20 @@ netId<strp>:
         |       idSVKwd                                 { $$ = $1; $<fl>$ = $<fl>1; }
         ;
 
-sigAttrListE<nodep>:
+sigAttrScope:
+                yVL_PUBLIC_FLAT_RW_ON_SNS attr_event_control
+                                                        { AstNode* sigAttrsp = new AstAttrOf{$1, VAttrType::VAR_PUBLIC_FLAT_RW};
+                                                          sigAttrsp->addNext(new AstAlwaysPublic{$1, $2, nullptr});
+                                                          GRAMMARP->setScopedSigAttr(sigAttrsp);
+                                                          v3Global.dpi(true); }
+        |       yVL_PUBLIC_ON                           { GRAMMARP->createScopedSigAttr(VAttrType::VAR_PUBLIC); }
+        |       yVL_PUBLIC_FLAT_ON                      { GRAMMARP->createScopedSigAttr(VAttrType::VAR_PUBLIC_FLAT); }
+        |       yVL_PUBLIC_FLAT_RD_ON                   { GRAMMARP->createScopedSigAttr(VAttrType::VAR_PUBLIC_FLAT_RD); }
+        |       yVL_PUBLIC_FLAT_RW_ON                   { GRAMMARP->createScopedSigAttr(VAttrType::VAR_PUBLIC_FLAT_RW); }
+        |       yVL_PUBLIC_OFF                          { GRAMMARP->setScopedSigAttr(nullptr); }
+        ;
+
+sigAttrListE<nodep>: // Scoped Attributes are added to explicit attributes
                 /* empty */                             { $$ = nullptr; }
         |       sigAttrList                             { $$ = $1; }
         ;

test_regress/t/t_vpi_var.cpp

@@ -19,8 +19,14 @@
 #include "verilated_vcd_c.h"
 #include "verilated_vpi.h"
 
+#ifdef T_VPI_VAR2
+#include "Vt_vpi_var2.h"
+#include "Vt_vpi_var2__Dpi.h"
+#else
 #include "Vt_vpi_var.h"
 #include "Vt_vpi_var__Dpi.h"
+#endif
+
 #include "svdpi.h"
 
 #endif
@@ -267,6 +273,25 @@ int _mon_check_var() {
     TestVpiHandle vh3 = vpi_handle_by_name((PLI_BYTE8*)"onebit", vh2);
     CHECK_RESULT_NZ(vh3);
 
+#ifdef T_VPI_VAR2
+    // test scoped attributes
+    TestVpiHandle vh_invisible1 = vpi_handle_by_name((PLI_BYTE8*)"invisible1", vh2);
+    CHECK_RESULT_Z(vh_invisible1);
+
+    TestVpiHandle vh_invisible2 = vpi_handle_by_name((PLI_BYTE8*)"invisible2", vh2);
+    CHECK_RESULT_Z(vh_invisible2);
+
+    TestVpiHandle vh_visibleParam1 = vpi_handle_by_name((PLI_BYTE8*)"visibleParam1", vh2);
+    CHECK_RESULT_NZ(vh_visibleParam1);
+
+    TestVpiHandle vh_invisibleParam1 = vpi_handle_by_name((PLI_BYTE8*)"invisibleParam1", vh2);
+    CHECK_RESULT_Z(vh_invisibleParam1);
+
+    TestVpiHandle vh_visibleParam2 = vpi_handle_by_name((PLI_BYTE8*)"visibleParam2", vh2);
+    CHECK_RESULT_NZ(vh_visibleParam2);
+
+#endif
+
     // onebit attributes
     PLI_INT32 d;
     d = vpi_get(vpiType, vh3);

test_regress/t/t_vpi_var2.pl

@@ -0,0 +1,31 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2023 by Wilson Snyder. This program is free software; you
+# can redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+scenarios(simulator => 1);
+
+pli_filename("t_vpi_var.cpp");
+compile(
+    make_top_shell => 0,
+    make_main => 0,
+    make_pli => 1,
+    sim_time => 2100,
+    iv_flags2 => ["-g2005-sv -D USE_VPI_NOT_DPI -DWAVES -DT_VPI_VAR2"],
+    v_flags2 => ["+define+USE_VPI_NOT_DPI"],
+    verilator_flags2 => ["--exe --vpi --no-l2name $Self->{t_dir}/t_vpi_var.cpp"],
+    );
+
+execute(
+    use_libvpi => 1,
+    check_finished => 1,
+    all_run_flags => ['+PLUS +INT=1234 +STRSTR']
+    );
+
+ok(1);
+1;

test_regress/t/t_vpi_var2.v

@@ -0,0 +1,158 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// Copyright 2023 by Wilson Snyder. This program is free software; you can
+// redistribute it and/or modify it under the terms of either the GNU
+// Lesser General Public License Version 3 or the Perl Artistic License
+// Version 2.0.
+// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+`ifdef USE_VPI_NOT_DPI
+//We call it via $c so we can verify DPI isn't required - see bug572
+`else
+import "DPI-C" context function int mon_check();
+`endif
+
+module t
+/* verilator public_flat_on */
+#(
+   parameter int visibleParam1 = 0,
+/* verilator public_off */
+   parameter int invisibleParam1 = 1,
+/* verilator public_on */
+   parameter int visibleParam2 = 2
+/* verilator public_off */
+)
+(/*AUTOARG*/
+   // Inputs
+   clk
+   );
+
+`ifdef VERILATOR
+`systemc_header
+extern "C" int mon_check();
+`verilog
+`endif
+
+   input clk;
+/*verilator public_flat_rw_on @(posedge clk)*/
+   reg          onebit;
+   reg [2:1]    twoone;
+   reg [2:1]    fourthreetwoone[4:3];
+   reg LONGSTART_a_very_long_name_which_will_get_hashed_a_very_long_name_which_will_get_hashed_a_very_long_name_which_will_get_hashed_a_very_long_name_which_will_get_hashed_LONGEND;
+   // verilator lint_off LITENDIAN
+   reg [0:61]   quads[2:3]      /*verilator public_flat_rw @(posedge clk)*/;
+/*verilator public_off*/
+   reg             invisible1;
+   // verilator lint_on LITENDIAN
+
+/*verilator public_flat_rd_on*/
+   reg [31:0]      count;
+   reg [31:0]      half_count;
+/*verilator public_off*/
+   reg             invisible2;
+
+/*verilator public_flat_rw_on @(posedge clk)*/
+   reg [7:0]       text_byte;
+   reg [15:0]      text_half;
+   reg [31:0]      text_word;
+   reg [63:0]      text_long;
+   reg [511:0]     text;
+/*verilator public_off*/
+   integer        status;
+
+   sub sub();
+
+   // Test loop
+   initial begin
+      count = 0;
+      onebit = 1'b0;
+      fourthreetwoone[3] = 0; // stop icarus optimizing away
+      text_byte = "B";
+      text_half = "Hf";
+      text_word = "Word";
+      text_long = "Long64b";
+      text = "Verilog Test module";
+`ifdef VERILATOR
+      status = $c32("mon_check()");
+`endif
+`ifdef IVERILOG
+      status = $mon_check();
+`endif
+`ifndef USE_VPI_NOT_DPI
+      status = mon_check();
+`endif
+      if (status!=0) begin
+         $write("%%Error: t_vpi_var.cpp:%0d: C Test failed\n", status);
+         $stop;
+      end
+      $write("%%Info: Checking results\n");
+      if (onebit != 1'b1) $stop;
+      if (quads[2] != 62'h12819213_abd31a1c) $stop;
+      if (quads[3] != 62'h1c77bb9b_3784ea09) $stop;
+      if (text_byte != "A") $stop;
+      if (text_half != "T2") $stop;
+      if (text_word != "Tree") $stop;
+      if (text_long != "44Four44") $stop;
+      if (text != "lorem ipsum") $stop;
+   end
+
+   always @(posedge clk) begin
+      count <= count + 2;
+      if (count[1])
+        half_count <= half_count + 2;
+
+      if (count == 1000) begin
+         $write("*-* All Finished *-*\n");
+         $finish;
+      end
+   end
+
+   genvar i;
+   generate
+   for (i=1; i<=6; i=i+1) begin : arr
+     arr #(.LENGTH(i)) arr();
+   end
+   endgenerate
+
+   genvar k;
+   generate
+   for (k=1; k<=6; k=k+1) begin : subs
+      sub subsub();
+   end
+   endgenerate
+
+endmodule : t
+
+module sub;
+   reg subsig1 /*verilator public_flat_rw*/;
+   reg subsig2 /*verilator public_flat_rd*/;
+`ifdef IVERILOG
+   // stop icarus optimizing signals away
+   wire redundant = subsig1 | subsig2;
+`endif
+endmodule : sub
+
+module arr;
+
+   parameter LENGTH = 1;
+
+/*verilator public_flat_rw_on*/
+   reg [LENGTH-1:0] sig;
+   reg [LENGTH-1:0] rfr;
+
+   reg            check;
+   reg          verbose;
+/*verilator public_off*/
+
+   initial begin
+      sig = {LENGTH{1'b0}};
+      rfr = {LENGTH{1'b0}};
+   end
+
+   always @(posedge check) begin
+     if (verbose) $display("%m : %x %x", sig, rfr);
+     if (check && sig != rfr) $stop;
+     check <= 0;
+   end
+
+endmodule : arr