Fix linking parameterized hierarchical blocks and recursive hierarchical blocks (#4654)

src/V3Config.cpp

@@ -610,7 +610,7 @@ void V3Config::applyIgnores(FileLine* filelinep) {
 }
 
 void V3Config::applyModule(AstNodeModule* modulep) {
-    const string& modname = modulep->name();
+    const string& modname = modulep->origName();
     V3ConfigModule* modp = V3ConfigResolver::s().modules().resolve(modname);
     if (modp) modp->apply(modulep);
 }

src/V3Param.cpp

@@ -74,6 +74,7 @@ class ParameterizedHierBlocks final {
     using GParamsMap = std::map<const std::string, AstVar*>;  // key:parameter name value:parameter
 
     // MEMBERS
+    const bool m_hierSubRun;  // Is in sub-run for hierarchical verilation
     // key:Original module name, value:HiearchyBlockOption*
     // If a module is parameterized, the module is uniquified to overridden parameters.
     // This is why HierBlockOptsByOrigName is multimap.
@@ -88,7 +89,8 @@ class ParameterizedHierBlocks final {
     // METHODS
 
 public:
-    ParameterizedHierBlocks(const V3HierBlockOptSet& hierOpts, AstNetlist* nodep) {
+    ParameterizedHierBlocks(const V3HierBlockOptSet& hierOpts, AstNetlist* nodep)
+        : m_hierSubRun(!v3Global.opt.hierBlocks().empty() || v3Global.opt.hierChild()) {
         for (const auto& hierOpt : hierOpts) {
             m_hierBlockOptsByOrigName.emplace(hierOpt.second.origName(), &hierOpt.second);
             const V3HierarchicalBlockOption::ParamStrMap& params = hierOpt.second.params();
@@ -109,7 +111,11 @@ public:
             if (hierOpts.find(modp->prettyName()) != hierOpts.end()) {
                 m_hierBlockMod.emplace(modp->name(), modp);
             }
-            const auto defParamIt = m_modParams.find(modp->name());
+            // Recursive hierarchical module may change its name, so we have to match its origName.
+            // Collect values from recursive cloned module as parameters in the top module could be
+            // overridden.
+            const string actualModName = modp->recursiveClone() ? modp->origName() : modp->name();
+            const auto defParamIt = m_modParams.find(actualModName);
             if (defParamIt != m_modParams.end()) {
                 // modp is the original of parameterized hierarchical block
                 for (AstNode* stmtp = modp->stmtsp(); stmtp; stmtp = stmtp->nextp()) {
@@ -120,11 +126,13 @@ public:
             }
         }
     }
+    bool hierSubRun() const { return m_hierSubRun; }
+    bool isHierBlock(const string& origName) const {
+        return m_hierBlockOptsByOrigName.find(origName) != m_hierBlockOptsByOrigName.end();
+    }
     AstNodeModule* findByParams(const string& origName, AstPin* firstPinp,
                                 const AstNodeModule* modp) {
-        if (m_hierBlockOptsByOrigName.find(origName) == m_hierBlockOptsByOrigName.end()) {
+        UASSERT(isHierBlock(origName), origName << " is not hierarchical block\n");
-            return nullptr;
-        }
         // This module is a hierarchical block. Need to replace it by the --lib-create wrapper.
         const std::pair<HierMapIt, HierMapIt> candidates
             = m_hierBlockOptsByOrigName.equal_range(origName);
@@ -510,9 +518,9 @@ class ParamProcessor final {
             pair.first->second = std::move(params);
         }
         const auto paramsIt = pair.first;
-        if (paramsIt->second.empty()) return modp->name();  // modp has no parameter
+        if (paramsIt->second.empty()) return modp->origName();  // modp has no parameter
 
-        string longname = modp->name();
+        string longname = modp->origName();
         for (auto&& defaultValue : paramsIt->second) {
             const auto pinIt = pins.find(defaultValue.first);
             const AstConst* const constp
@@ -536,7 +544,7 @@ class ParamProcessor final {
             // Hex string must be a safe suffix for any symbol
             const string hashStr = hashStrGen.digestHex();
             for (string::size_type i = 1; i < hashStr.size(); ++i) {
-                string newName = modp->name();
+                string newName = modp->origName();
                 // Don't use '__' not to be encoded when this module is loaded later by Verilator
                 if (newName.at(newName.size() - 1) != '_') newName += '_';
                 newName += hashStr.substr(0, i);
@@ -586,8 +594,6 @@ class ParamProcessor final {
         newmodp->user2(false);  // We need to re-recurse this module once changed
         newmodp->recursive(false);
         newmodp->recursiveClone(false);
-        // Only the first generation of clone holds this property
-        newmodp->hierBlock(srcModp->hierBlock() && !srcModp->recursiveClone());
         // Recursion may need level cleanups
         if (newmodp->level() <= m_modp->level()) newmodp->level(m_modp->level() + 1);
         if ((newmodp->level() - srcModp->level()) >= (v3Global.opt.moduleRecursionDepth() - 2)) {
@@ -851,7 +857,16 @@ class ParamProcessor final {
         cellInterfaceCleanup(pinsp, srcModpr, longname /*ref*/, any_overrides /*ref*/,
                              ifaceRefRefs /*ref*/);
 
-        if (!any_overrides) {
+        if (m_hierBlocks.hierSubRun() && m_hierBlocks.isHierBlock(srcModpr->origName())) {
+            AstNodeModule* const paramedModp
+                = m_hierBlocks.findByParams(srcModpr->origName(), paramsp, m_modp);
+            UASSERT_OBJ(paramedModp, nodep, "Failed to find sub-module for hierarchical block");
+            paramedModp->dead(false);
+            // We need to relink the pins to the new module
+            relinkPinsByName(pinsp, paramedModp);
+            srcModpr = paramedModp;
+            any_overrides = true;
+        } else if (!any_overrides) {
             UINFO(8, "Cell parameters all match original values, skipping expansion.\n");
             // If it's the first use of the default instance, create a copy and store it in user3p.
             // user3p will also be used to check if the default instance is used.
@@ -863,12 +878,6 @@ class ParamProcessor final {
                 srcModpr->user3p(classCopyp);
                 storeOriginalParams(classCopyp);
             }
-        } else if (AstNodeModule* const paramedModp
-                   = m_hierBlocks.findByParams(srcModpr->name(), paramsp, m_modp)) {
-            paramedModp->dead(false);
-            // We need to relink the pins to the new module
-            relinkPinsByName(pinsp, paramedModp);
-            srcModpr = paramedModp;
         } else {
             const string newname
                 = srcModpr->hierBlock() ? longname : moduleCalcName(srcModpr, longname);

test_regress/t/t_hier_block.pl

@@ -35,7 +35,7 @@ file_grep($Self->{obj_dir} . "/Vsub0/sub0.sv", qr/^\s+\/\/\s+timeprecision\s+(\d
 file_grep($Self->{obj_dir} . "/Vsub0/sub0.sv", /^module\s+(\S+)\s+/, "sub0");
 file_grep($Self->{obj_dir} . "/Vsub1/sub1.sv", /^module\s+(\S+)\s+/, "sub1");
 file_grep($Self->{obj_dir} . "/Vsub2/sub2.sv", /^module\s+(\S+)\s+/, "sub2");
-file_grep($Self->{stats}, qr/HierBlock,\s+Hierarchical blocks\s+(\d+)/i, 13);
+file_grep($Self->{stats}, qr/HierBlock,\s+Hierarchical blocks\s+(\d+)/i, 14);
 file_grep($Self->{run_log_filename}, qr/MACRO:(\S+) is defined/i, "cplusplus");
 
 ok(1);

test_regress/t/t_hier_block_cmake.pl

@@ -36,7 +36,7 @@ if (!$Self->have_cmake) {
     file_grep($target_dir . 'Vsub0/sub0.sv', /^module\s+(\S+)\s+/, "sub0");
     file_grep($target_dir . 'Vsub1/sub1.sv', /^module\s+(\S+)\s+/, "sub1");
     file_grep($target_dir . 'Vsub2/sub2.sv', /^module\s+(\S+)\s+/, "sub2");
-    file_grep($target_dir . 'Vt_hier_block__stats.txt', qr/HierBlock,\s+Hierarchical blocks\s+(\d+)/i, 13);
+    file_grep($target_dir . 'Vt_hier_block__stats.txt', qr/HierBlock,\s+Hierarchical blocks\s+(\d+)/i, 14);
     file_grep($Self->{obj_dir} . '/run.log', qr/MACRO:(\S+) is defined/i, "cplusplus");
 }
 

test_regress/t/t_hier_block_sc.pl

@@ -35,7 +35,7 @@ execute(
 file_grep($Self->{obj_dir} . "/Vsub0/sub0.sv", /^module\s+(\S+)\s+/, "sub0");
 file_grep($Self->{obj_dir} . "/Vsub1/sub1.sv", /^module\s+(\S+)\s+/, "sub1");
 file_grep($Self->{obj_dir} . "/Vsub2/sub2.sv", /^module\s+(\S+)\s+/, "sub2");
-file_grep($Self->{stats}, qr/HierBlock,\s+Hierarchical blocks\s+(\d+)/i, 13);
+file_grep($Self->{stats}, qr/HierBlock,\s+Hierarchical blocks\s+(\d+)/i, 14);
 file_grep($Self->{run_log_filename}, qr/MACRO:(\S+) is defined/i, "cplusplus");
 
 ok(1);

test_regress/t/t_hier_block_sc_trace_fst.pl

@@ -37,7 +37,7 @@ execute(
 file_grep($Self->{obj_dir} . "/Vsub0/sub0.sv", /^module\s+(\S+)\s+/, "sub0");
 file_grep($Self->{obj_dir} . "/Vsub1/sub1.sv", /^module\s+(\S+)\s+/, "sub1");
 file_grep($Self->{obj_dir} . "/Vsub2/sub2.sv", /^module\s+(\S+)\s+/, "sub2");
-file_grep($Self->{stats}, qr/HierBlock,\s+Hierarchical blocks\s+(\d+)/i, 13);
+file_grep($Self->{stats}, qr/HierBlock,\s+Hierarchical blocks\s+(\d+)/i, 14);
 file_grep($Self->{run_log_filename}, qr/MACRO:(\S+) is defined/i, "cplusplus");
 
 fst_identical($Self->trace_filename, $Self->{golden_filename});

test_regress/t/t_hier_block_sc_trace_vcd.pl

@@ -37,7 +37,7 @@ execute(
 file_grep($Self->{obj_dir} . "/Vsub0/sub0.sv", /^module\s+(\S+)\s+/, "sub0");
 file_grep($Self->{obj_dir} . "/Vsub1/sub1.sv", /^module\s+(\S+)\s+/, "sub1");
 file_grep($Self->{obj_dir} . "/Vsub2/sub2.sv", /^module\s+(\S+)\s+/, "sub2");
-file_grep($Self->{stats}, qr/HierBlock,\s+Hierarchical blocks\s+(\d+)/i, 13);
+file_grep($Self->{stats}, qr/HierBlock,\s+Hierarchical blocks\s+(\d+)/i, 14);
 file_grep($Self->{run_log_filename}, qr/MACRO:(\S+) is defined/i, "cplusplus");
 
 vcd_identical($Self->trace_filename, $Self->{golden_filename});

test_regress/t/t_hier_block_vlt.pl

@@ -35,7 +35,7 @@ file_grep($Self->{obj_dir} . "/Vsub0/sub0.sv", qr/^\s+timeprecision\s+(\d+)ps;/m
 file_grep($Self->{obj_dir} . "/Vsub0/sub0.sv", /^module\s+(\S+)\s+/, "sub0");
 file_grep($Self->{obj_dir} . "/Vsub1/sub1.sv", /^module\s+(\S+)\s+/, "sub1");
 file_grep($Self->{obj_dir} . "/Vsub2/sub2.sv", /^module\s+(\S+)\s+/, "sub2");
-file_grep($Self->{stats}, qr/HierBlock,\s+Hierarchical blocks\s+(\d+)/i, 13);
+file_grep($Self->{stats}, qr/HierBlock,\s+Hierarchical blocks\s+(\d+)/i, 14);
 file_grep($Self->{run_log_filename}, qr/MACRO:(\S+) is defined/i, "cplusplus");
 
 ok(1);

test_regress/t/t_hier_block_vlt.vlt

@@ -6,4 +6,4 @@
 
 `verilator_config
 hier_block -module "sub?"
-hier_block -module "delay"
+hier_block -module "delay"  // matches recursive modules