diff --git a/src/V3Param.cpp b/src/V3Param.cpp index 7cced105d..89df4b371 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -71,7 +71,9 @@ class ParameterizedHierBlocks final { typedef HierBlockOptsByOrigName::const_iterator HierMapIt; typedef std::map HierBlockModMap; typedef std::map> ParamConstMap; + typedef std::map GParamsMap; // key:parameter name value:parameter typedef std::map HierParamsMap; + typedef std::map ModParamsMap; // key:module name // MEMBERS // key:Original module name, value:HiearchyBlockOption* @@ -82,6 +84,7 @@ class ParameterizedHierBlocks final { HierBlockModMap m_hierBlockMod; // Overridden parameters of the hierarchical block HierParamsMap m_hierParams; + ModParamsMap m_modParams; // Parameter variables of hierarchical blocks // METHODS VL_DEBUG_FUNC; // Declare debug() @@ -101,12 +104,23 @@ public: const bool inserted = consts.emplace(pIt->first, std::move(constp)).second; UASSERT(inserted, pIt->first << " is already added"); } + // origName may be already registered, but it's fine. + m_modParams.insert({hierOpt.second.origName(), {}}); } for (AstNodeModule* modp = nodep->modulesp(); modp; modp = VN_CAST(modp->nextp(), NodeModule)) { if (hierOpts.find(modp->prettyName()) != hierOpts.end()) { m_hierBlockMod.emplace(modp->name(), modp); } + const auto defParamIt = m_modParams.find(modp->name()); + if (defParamIt != m_modParams.end()) { + // modp is the original of parameterized hierarchical block + for (AstNode* stmtp = modp->stmtsp(); stmtp; stmtp = stmtp->nextp()) { + if (AstVar* varp = VN_CAST(stmtp, Var)) { + if (varp->isGParam()) defParamIt->second.emplace(varp->name(), varp); + } + } + } } } AstNodeModule* findByParams(const string& origName, AstPin* firstPinp, @@ -117,6 +131,8 @@ public: // This module is a hierarchical block. Need to replace it by the protect-lib wrapper. const std::pair candidates = m_hierBlockOptsByOrigName.equal_range(origName); + const auto paramsIt = m_modParams.find(origName); + UASSERT_OBJ(paramsIt != m_modParams.end(), modp, origName << " must be registered"); HierMapIt hierIt; for (hierIt = candidates.first; hierIt != candidates.second; ++hierIt) { bool found = true; @@ -131,6 +147,14 @@ public: AstConst* constp = VN_CAST(pinp->exprp(), Const); UASSERT_OBJ(constp, pinp, "parameter for a hierarchical block must have been constified"); + const auto paramIt = paramsIt->second.find(modvarp->name()); + UASSERT_OBJ(paramIt != paramsIt->second.end(), modvarp, "must be registered"); + AstConst* defValuep = VN_CAST(paramIt->second->valuep(), Const); + if (defValuep && areSame(constp, defValuep)) { + UINFO(5, "Setting default value of " << constp << " to " << modvarp + << std::endl); + continue; // Skip this parameter because setting the same value + } const auto pIt = vlstd::as_const(params).find(modvarp->name()); UINFO(5, "Comparing " << modvarp->name() << " " << constp << std::endl); if (pIt == params.end() || paramIdx >= params.size() @@ -239,6 +263,10 @@ class ParamProcessor final { // Database to get protect-lib wrapper that matches parameters in hierarchical Verilation ParameterizedHierBlocks m_hierBlocks; + // Default parameter values key:parameter name, value:default value (can be nullptr) + typedef std::map DefaultValueMap; + // Default parameter values of hierarchical blocks + std::map m_defaultParameterValues; // METHODS VL_DEBUG_FUNC; // Declare debug() @@ -302,20 +330,16 @@ class ParamProcessor final { } return string("z") + cvtToStr(num); } - string moduleCalcName(AstNodeModule* srcModp, AstPin* paramsp, const string& longname) { + string moduleCalcName(AstNodeModule* srcModp, const string& longname) { string newname = longname; - if (longname.length() > 30 || srcModp->hierBlock()) { + if (longname.length() > 30) { const auto iter = m_longMap.find(longname); if (iter != m_longMap.end()) { newname = iter->second; } else { - if (srcModp->hierBlock()) { - newname = parametrizedHierBlockName(srcModp, paramsp); - } else { - newname = srcModp->name(); - // We use all upper case above, so lower here can't conflict - newname += "__pi" + cvtToStr(++m_longId); - } + newname = srcModp->name(); + // We use all upper case above, so lower here can't conflict + newname += "__pi" + cvtToStr(++m_longId); m_longMap.emplace(longname, newname); } } @@ -416,17 +440,61 @@ class ParamProcessor final { return false; } - string parametrizedHierBlockName(const AstNodeModule* modp, AstPin* paramPinsp) const { - VHashSha256 hash; - // Calculate hash using module name, parameter name, and parameter value - // The hash is used as the module suffix to find a module name that is unique in the design - hash.insert(modp->name()); + string parameterizedHierBlockName(AstNodeModule* modp, AstPin* paramPinsp) { + // Create a unique name in the following steps + // - Make a long name that includes all parameters, that appear + // in the alphabetical order. + // - Hash the long name to get valid Verilog symbol + UASSERT_OBJ(modp->hierBlock(), modp, "should be used for hierarchical block"); + + std::map pins; for (AstPin* pinp = paramPinsp; pinp; pinp = VN_CAST(pinp->nextp(), Pin)) { - if (AstVar* varp = pinp->modVarp()) hash.insert(varp->name()); - if (AstConst* constp = VN_CAST(pinp->exprp(), Const)) { - hash.insert(constp->num().ascii(false)); + checkSupportedParam(modp, pinp); + if (AstVar* varp = pinp->modVarp()) { + if (!pinp->exprp()) continue; + if (varp->isGParam()) { + AstConst* constp = VN_CAST(pinp->exprp(), Const); + pins.emplace(varp->name(), constp); + } } } + + auto paramsIt = m_defaultParameterValues.find(modp); + if (paramsIt == m_defaultParameterValues.end()) { // Not cached yet, so check parameters + // Using map with key=string so that we can scan it in deterministic order + DefaultValueMap params; + for (AstNode* stmtp = modp->stmtsp(); stmtp; stmtp = stmtp->nextp()) { + if (AstVar* varp = VN_CAST(stmtp, Var)) { + if (varp->isGParam()) { + AstConst* constp = VN_CAST(varp->valuep(), Const); + // constp can be nullptr if the parameter is not used to instantiate sub + // module. varp->valuep() is not contified yet in the case. + // nullptr means that the parameter is using some default value. + params.emplace(varp->name(), constp); + } + } + } + paramsIt = m_defaultParameterValues.emplace(modp, std::move(params)).first; + } + if (paramsIt->second.empty()) return modp->name(); // modp has no parameter + + string longname = modp->name(); + for (auto&& defaultValue : paramsIt->second) { + auto pinIt = pins.find(defaultValue.first); + AstConst* constp = pinIt == pins.end() ? defaultValue.second : pinIt->second; + // This longname is not valid as verilog symbol, but ok, because it will be hashed + longname += "_" + defaultValue.first + "="; + // constp can be nullptr + if (constp) longname += constp->num().ascii(false); + } + + auto iter = m_longMap.find(longname); + if (iter != m_longMap.end()) return iter->second; // Already calculated + + VHashSha256 hash; + // Calculate hash using longname + // The hash is used as the module suffix to find a module name that is unique in the design + hash.insert(longname); while (true) { // Copy VHashSha256 just in case of hash collision VHashSha256 hashStrGen = hash; @@ -437,7 +505,10 @@ class ParamProcessor final { // 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); - if (!moduleExists(newName)) return newName; + if (!moduleExists(newName)) { + m_longMap.emplace(longname, newName); + return newName; + } } // Hash collision. maybe just v3error is practically enough hash.insert(V3Os::trueRandom(64)); @@ -496,15 +567,18 @@ class ParamProcessor final { // DOES clone() so must be finished with module clonep() before here for (AstPin* pinp = paramsp; pinp; pinp = VN_CAST(pinp->nextp(), Pin)) { if (pinp->exprp()) { - if (newmodp->hierBlock()) checkSupportedParam(newmodp, pinp); if (AstVar* modvarp = pinp->modVarp()) { AstNode* newp = pinp->exprp(); // Const or InitArray + AstConst* exprp = VN_CAST(newp, Const); + AstConst* origp = VN_CAST(modvarp->valuep(), Const); + const bool overridden + = !(origp && ParameterizedHierBlocks::areSame(exprp, origp)); // Remove any existing parameter if (modvarp->valuep()) modvarp->valuep()->unlinkFrBack()->deleteTree(); // Set this parameter to value requested by cell UINFO(9, " set param " << modvarp << " = " << newp << endl); modvarp->valuep(newp->cloneTree(false)); - modvarp->overriddenParam(true); + modvarp->overriddenParam(overridden); } else if (AstParamTypeDType* modptp = pinp->modPTypep()) { AstNodeDType* dtypep = VN_CAST(pinp->exprp(), NodeDType); UASSERT_OBJ(dtypep, pinp, "unlinked param dtype"); @@ -680,8 +754,13 @@ public: if (nodep->recursive()) any_overrides = true; if (debug() > 8) nodep->paramsp()->dumpTreeAndNext(cout, "-cellparams: "); - for (AstPin* pinp = nodep->paramsp(); pinp; pinp = VN_CAST(pinp->nextp(), Pin)) { - cellPinCleanup(nodep, pinp, srcModp, longname /*ref*/, any_overrides /*ref*/); + if (srcModp->hierBlock()) { + longname = parameterizedHierBlockName(srcModp, nodep->paramsp()); + any_overrides = longname != srcModp->name(); + } else { + for (AstPin* pinp = nodep->paramsp(); pinp; pinp = VN_CAST(pinp->nextp(), Pin)) { + cellPinCleanup(nodep, pinp, srcModp, longname /*ref*/, any_overrides /*ref*/); + } } IfaceRefRefs ifaceRefRefs; cellInterfaceCleanup(nodep, srcModp, longname /*ref*/, any_overrides /*ref*/, @@ -697,7 +776,7 @@ public: // We need to relink the pins to the new module relinkPinsByName(nodep->pinsp(), paramedModp); } else { - string newname = moduleCalcName(srcModp, nodep->paramsp(), longname); + string newname = srcModp->hierBlock() ? longname : moduleCalcName(srcModp, longname); const ModInfo* modInfop = moduleFindOrClone(srcModp, nodep, nodep->paramsp(), newname, ifaceRefRefs); // Have child use this module instead. diff --git a/test_regress/t/t_hier_block.pl b/test_regress/t/t_hier_block.pl index 5fdce6ee9..fe743cb94 100755 --- a/test_regress/t/t_hier_block.pl +++ b/test_regress/t/t_hier_block.pl @@ -32,7 +32,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, 11); +file_grep($Self->{stats}, qr/HierBlock,\s+Hierarchical blocks\s+(\d+)/i, 13); file_grep($Self->{run_log_filename}, qr/MACRO:(\S+) is defined/i, "cplusplus"); ok(1); diff --git a/test_regress/t/t_hier_block.v b/test_regress/t/t_hier_block.v index d122a20ad..7c8ee9e15 100644 --- a/test_regress/t/t_hier_block.v +++ b/test_regress/t/t_hier_block.v @@ -291,6 +291,26 @@ module sub5 (input wire clk, input wire [127:0] in[2][3], output logic [7:0] out end end + wire [7:0] val0[2]; + wire [7:0] val1[2]; + wire [7:0] val2[2]; + wire [7:0] val3[2]; + sub6 i_sub0(.out(val0)); + sub6 #(.P0(1)) i_sub1(.out(val1)); // Setting the default value + sub6 #(.P0(1), .P1(2)) i_sub2(.out(val2)); // Setting the default value + sub6 #(.P0(1), .P1(3)) i_sub3(.out(val3)); + + always @(posedge clk) begin + if (val0[0] != 1 || val0[1] != 2) $stop; + if (val1[0] != 1 || val1[1] != 2) $stop; + if (val2[0] != 1 || val2[1] != 2) $stop; + if (val3[0] != 1 || val3[1] != 3) $stop; + end +endmodule + +module sub6 #(parameter P0 = 1, parameter P1 = 2) (output wire [7:0] out[2]); `HIER_BLOCK + assign out[0] = 8'(P0); + assign out[1] = 8'(P1); endmodule module delay #( diff --git a/test_regress/t/t_hier_block_cmake.pl b/test_regress/t/t_hier_block_cmake.pl index 87a83bf39..0517b3a3b 100755 --- a/test_regress/t/t_hier_block_cmake.pl +++ b/test_regress/t/t_hier_block_cmake.pl @@ -33,7 +33,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, 11); + file_grep($target_dir . 'Vt_hier_block__stats.txt', qr/HierBlock,\s+Hierarchical blocks\s+(\d+)/i, 13); file_grep($Self->{obj_dir} . '/run.log', qr/MACRO:(\S+) is defined/i, "cplusplus"); } diff --git a/test_regress/t/t_hier_block_sc.pl b/test_regress/t/t_hier_block_sc.pl index c24a54efa..139b280b9 100755 --- a/test_regress/t/t_hier_block_sc.pl +++ b/test_regress/t/t_hier_block_sc.pl @@ -34,7 +34,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, 11); +file_grep($Self->{stats}, qr/HierBlock,\s+Hierarchical blocks\s+(\d+)/i, 13); file_grep($Self->{run_log_filename}, qr/MACRO:(\S+) is defined/i, "cplusplus"); ok(1); diff --git a/test_regress/t/t_hier_block_vlt.pl b/test_regress/t/t_hier_block_vlt.pl index 8a7857a11..904182717 100755 --- a/test_regress/t/t_hier_block_vlt.pl +++ b/test_regress/t/t_hier_block_vlt.pl @@ -34,7 +34,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, 11); +file_grep($Self->{stats}, qr/HierBlock,\s+Hierarchical blocks\s+(\d+)/i, 13); file_grep($Self->{run_log_filename}, qr/MACRO:(\S+) is defined/i, "cplusplus"); ok(1);