From be2493264c413b44f497cbbbc88945b2e6b5e7a7 Mon Sep 17 00:00:00 2001 From: MikePopoloski Date: Sat, 2 Mar 2024 16:24:56 -0500 Subject: [PATCH] Support config cell overrides with specific target libs --- include/slang/ast/Compilation.h | 3 +- .../ast/symbols/CompilationUnitSymbols.h | 8 +- source/ast/Compilation.cpp | 177 ++++++++++-------- source/ast/symbols/CompilationUnitSymbols.cpp | 57 +++--- source/ast/symbols/InstanceSymbols.cpp | 8 +- tests/unittests/ast/ConfigTests.cpp | 33 ++++ 6 files changed, 165 insertions(+), 121 deletions(-) diff --git a/include/slang/ast/Compilation.h b/include/slang/ast/Compilation.h index 5fc9ac123..dadbeb2bc 100644 --- a/include/slang/ast/Compilation.h +++ b/include/slang/ast/Compilation.h @@ -46,7 +46,6 @@ class Symbol; class SystemSubroutine; class ValueDriver; struct AssertionInstanceDetails; -struct ConfigCellId; struct ConfigRule; struct ResolvedConfig; @@ -698,6 +697,8 @@ class SLANG_EXPORT Compilation : public BumpAllocator { void checkBindTargetParams(const syntax::BindDirectiveSyntax& syntax, const Scope& scope, std::span instTargets, const DefinitionSymbol* defTarget); + std::pair resolveConfigRule(const Scope& scope, + const ConfigRule& rule) const; std::pair resolveConfigRules( std::string_view name, const Scope& scope, const ResolvedConfig* parentConfig, const ConfigRule* configRule, const std::vector& defList) const; diff --git a/include/slang/ast/symbols/CompilationUnitSymbols.h b/include/slang/ast/symbols/CompilationUnitSymbols.h index a5e876e04..88eed05d1 100644 --- a/include/slang/ast/symbols/CompilationUnitSymbols.h +++ b/include/slang/ast/symbols/CompilationUnitSymbols.h @@ -278,8 +278,8 @@ class SLANG_EXPORT ConfigBlockSymbol : public Symbol, public Scope { }; struct CellOverride { - const SourceLibrary* specificLib = nullptr; - ConfigRule rule; + flat_hash_map specificLibRules; + ConfigRule* defaultRule = nullptr; }; struct InstanceOverride { @@ -302,7 +302,7 @@ class SLANG_EXPORT ConfigBlockSymbol : public Symbol, public Scope { return defaultLiblist; } - const flat_hash_map>& getCellOverrides() const { + const flat_hash_map& getCellOverrides() const { if (!resolved) resolve(); return cellOverrides; @@ -326,7 +326,7 @@ class SLANG_EXPORT ConfigBlockSymbol : public Symbol, public Scope { mutable std::span topCells; mutable std::span defaultLiblist; - mutable flat_hash_map> cellOverrides; + mutable flat_hash_map cellOverrides; mutable flat_hash_map instanceOverrides; mutable bool resolved = false; }; diff --git a/source/ast/Compilation.cpp b/source/ast/Compilation.cpp index 6433a89e5..74daf216a 100644 --- a/source/ast/Compilation.cpp +++ b/source/ast/Compilation.cpp @@ -2278,20 +2278,83 @@ void Compilation::resolveDefParamsAndBinds() { copyStateInto(*this, true); } +template +auto findDefByLib(TDefList& defList, const SourceLibrary& target) + -> std::remove_reference_t { + for (auto def : defList) { + if (def->getSourceLibrary() == &target) + return def; + } + return nullptr; +} + +std::pair Compilation::resolveConfigRule( + const Scope& scope, const ConfigRule& rule) const { + + auto& id = rule.useCell; + SLANG_ASSERT(!id.name.empty()); + + // Figure out the target library. + const SourceLibrary* overrideLib = defaultLibPtr; + if (id.lib.empty()) { + if (auto parentDef = scope.asSymbol().getDeclaringDefinition()) + overrideLib = &parentDef->sourceLibrary; + } + else { + overrideLib = getSourceLibrary(id.lib); + if (!overrideLib) { + root->addDiag(diag::UnknownLibrary, id.sourceRange) << id.lib; + return {{}, true}; + } + } + + if (!id.targetConfig) { + if (auto overrideDefIt = definitionMap.find({id.name, root.get()}); + overrideDefIt != definitionMap.end()) { + // There are definitions with this name; find the one that + // matches our target library. + auto result = findDefByLib(overrideDefIt->second.first, *overrideLib); + if (result) + return {{result, nullptr, &rule}, true}; + } + } + + // If we didn't find a target definition, try to look for a config. + if (auto configIt = configBlocks.find(id.name); configIt != configBlocks.end()) { + auto result = findDefByLib(configIt->second, *overrideLib); + if (result) { + auto topCells = result->getTopCells(); + if (topCells.size() != 1) { + auto syntax = result->getSyntax(); + SLANG_ASSERT(syntax); + + auto range = syntax->as().topCells.sourceRange(); + auto& diag = scope.addDiag(diag::NestedConfigMultipleTops, range); + diag << result->name; + diag.addNote(diag::NoteConfigRule, rule.sourceRange); + + return {{}, true}; + } + + if (rule.paramOverrides) { + scope.addDiag(diag::ConfigParamsIgnored, rule.paramOverrides->sourceRange()) + << result->name; + } + + return {{&topCells[0].definition, result, topCells[0].rule}, true}; + } + } + + // Otherwise we have an error. + errorMissingDef(id.name, *root, id.sourceRange, diag::UnknownModule); + return {{}, true}; +} + std::pair Compilation::resolveConfigRules( std::string_view lookupName, const Scope& scope, const ResolvedConfig* parentConfig, const ConfigRule* rule, const std::vector& defList) const { - auto findDefByLib = - [](auto& defList, - const SourceLibrary& target) -> std::remove_reference_t { - for (auto def : defList) { - if (def->getSourceLibrary() == &target) - return def; - } - return nullptr; - }; - + const ConfigBlockSymbol::CellOverride* cellOverride = nullptr; std::span liblist; if (parentConfig) { SLANG_ASSERT(!rule); @@ -2300,94 +2363,50 @@ std::pair Compilation::resolveConfigR auto& conf = parentConfig->useConfig; auto& overrides = conf.getCellOverrides(); if (auto overrideIt = overrides.find(lookupName); overrideIt != overrides.end()) { - // There is at least one cell override with this name; look through the - // list and take the first one that matches our library restrictions. - for (auto& cellOverride : overrideIt->second) { - // TODO: support this - if (cellOverride.specificLib) - continue; - - rule = &cellOverride.rule; - break; - } + cellOverride = &overrideIt->second; + rule = cellOverride->defaultRule; } } if (rule) { + if (auto& id = rule->useCell; !id.name.empty()) + return resolveConfigRule(scope, *rule); + if (rule->liblist) liblist = *rule->liblist; + } - if (auto& id = rule->useCell; !id.name.empty()) { - // Figure out the target library. - const SourceLibrary* overrideLib = defaultLibPtr; - if (id.lib.empty()) { - if (auto parentDef = scope.asSymbol().getDeclaringDefinition()) - overrideLib = &parentDef->sourceLibrary; - } - else { - overrideLib = getSourceLibrary(id.lib); - if (!overrideLib) { - root->addDiag(diag::UnknownLibrary, id.sourceRange) << id.lib; - return {{}, true}; - } - } - - if (!id.targetConfig) { - if (auto overrideDefIt = definitionMap.find({id.name, root.get()}); - overrideDefIt != definitionMap.end()) { - // There are definitions with this name; find the one that - // matches our target library. - auto result = findDefByLib(overrideDefIt->second.first, *overrideLib); - if (result) - return {{result, nullptr, rule}, true}; - } - } - - // If we didn't find a target definition, try to look for a config. - if (auto configIt = configBlocks.find(id.name); configIt != configBlocks.end()) { - auto result = findDefByLib(configIt->second, *overrideLib); - if (result) { - auto topCells = result->getTopCells(); - if (topCells.size() != 1) { - auto syntax = result->getSyntax(); - SLANG_ASSERT(syntax); - - auto range = syntax->as().topCells.sourceRange(); - auto& diag = scope.addDiag(diag::NestedConfigMultipleTops, range); - diag << result->name; - diag.addNote(diag::NoteConfigRule, rule->sourceRange); - - return {{}, true}; - } - - if (rule->paramOverrides) { - scope.addDiag(diag::ConfigParamsIgnored, - rule->paramOverrides->sourceRange()) - << result->name; - } - - return {{&topCells[0].definition, result, topCells[0].rule}, true}; - } + auto findDefWithOverride = [&](const SourceLibrary& targetLib) + -> std::optional> { + // If we have a cell override that specifically targets + // this lib then we should just return that directly. + if (cellOverride) { + if (auto it = cellOverride->specificLibRules.find(&targetLib); + it != cellOverride->specificLibRules.end()) { + return resolveConfigRule(scope, *it->second); } + } - // Otherwise we have an error. - errorMissingDef(id.name, *root, id.sourceRange, diag::UnknownModule); - return {{}, true}; + // Otherwise try to find the def in our list. + if (auto def = findDefByLib(defList, targetLib)) { + return std::pair{{def, nullptr, rule}, + false}; } - } + return std::nullopt; + }; if (!liblist.empty()) { // This search is O(n^2) but both lists should be small // (or even just one element each) in basically all cases. for (auto lib : liblist) { - if (auto def = findDefByLib(defList, *lib)) - return {{def, nullptr, rule}, false}; + if (auto result = findDefWithOverride(*lib)) + return *result; } } else if (auto parentDef = scope.asSymbol().getDeclaringDefinition()) { // Fall back to picking based on the parent instance's library. - if (auto def = findDefByLib(defList, parentDef->sourceLibrary)) - return {{def, nullptr, rule}, false}; + if (auto result = findDefWithOverride(parentDef->sourceLibrary)) + return *result; } return {{}, false}; diff --git a/source/ast/symbols/CompilationUnitSymbols.cpp b/source/ast/symbols/CompilationUnitSymbols.cpp index b7ff26975..81dd1e29e 100644 --- a/source/ast/symbols/CompilationUnitSymbols.cpp +++ b/source/ast/symbols/CompilationUnitSymbols.cpp @@ -8,6 +8,7 @@ #include "slang/ast/symbols/CompilationUnitSymbols.h" #include "ParameterBuilder.h" +#include #include "slang/ast/ASTSerializer.h" #include "slang/ast/Compilation.h" @@ -466,37 +467,30 @@ void ConfigBlockSymbol::resolve() const { if (cellName.empty()) break; - CellOverride co; + auto rule = comp.emplace(buildRule(*ccr.ruleClause)); + + auto& overrides = cellOverrides[cellName]; if (auto libName = ccr.name->library.valueText(); !libName.empty()) { - co.specificLib = comp.getSourceLibrary(libName); - if (!co.specificLib) { + auto specificLib = comp.getSourceLibrary(libName); + if (!specificLib) { scope->addDiag(diag::WarnUnknownLibrary, ccr.name->library.range()) << libName; break; } - } - - co.rule = buildRule(*ccr.ruleClause); - auto& overrides = cellOverrides[cellName]; - auto it = std::ranges::find_if(overrides, [&](auto& item) { - return item.specificLib == co.specificLib; - }); - - if (it != overrides.end()) { - mergeRules(it->rule, co.rule, [&] { - std::string name = "cell '"; - if (co.specificLib) { - name += ccr.name->library.valueText(); - name.push_back('.'); - } - name += cellName; - name.push_back('\''); - return name; - }); + auto [it, inserted] = overrides.specificLibRules.emplace(specificLib, rule); + if (!inserted) { + mergeRules(*it->second, *rule, [&] { + return fmt::format("cell '{}.{}'", specificLib->name, cellName); + }); + } + } + else if (overrides.defaultRule) { + mergeRules(*overrides.defaultRule, *rule, + [&] { return fmt::format("cell '{}'", cellName); }); } else { - overrides.push_back(co); + overrides.defaultRule = rule; } break; } @@ -564,19 +558,12 @@ void ConfigBlockSymbol::resolve() const { continue; if (auto it = cellOverrides.find(topCell.definition.name); it != cellOverrides.end()) { - CellOverride* defaultOverride = nullptr; - for (auto& co : it->second) { - if (!co.specificLib) { - defaultOverride = &co; - } - else if (co.specificLib == &topCell.definition.sourceLibrary) { - topCell.rule = &co.rule; - break; - } + topCell.rule = it->second.defaultRule; + auto& specificLibRules = it->second.specificLibRules; + if (auto specificIt = specificLibRules.find(&topCell.definition.sourceLibrary); + specificIt != specificLibRules.end()) { + topCell.rule = specificIt->second; } - - if (!topCell.rule && defaultOverride) - topCell.rule = &defaultOverride->rule; } } diff --git a/source/ast/symbols/InstanceSymbols.cpp b/source/ast/symbols/InstanceSymbols.cpp index aefd404c2..be0a2a1a0 100644 --- a/source/ast/symbols/InstanceSymbols.cpp +++ b/source/ast/symbols/InstanceSymbols.cpp @@ -271,8 +271,12 @@ InstanceSymbol& InstanceSymbol::createDefault(Compilation& comp, const Definitio /* isUninstantiated */ false, hierarchyOverrideNode, configBlock, configRule)); - if (configBlock) - result.resolvedConfig = comp.emplace(*configBlock, result); + if (configBlock) { + auto rc = comp.emplace(*configBlock, result); + if (configRule && configRule->liblist) + rc->liblist = *configRule->liblist; + result.resolvedConfig = rc; + } return result; } diff --git a/tests/unittests/ast/ConfigTests.cpp b/tests/unittests/ast/ConfigTests.cpp index 28196539d..e8e59a2d0 100644 --- a/tests/unittests/ast/ConfigTests.cpp +++ b/tests/unittests/ast/ConfigTests.cpp @@ -719,3 +719,36 @@ endmodule CHECK(getParam("top.foo.B").integer() == 5); } + +TEST_CASE("Config cell overrides with specific target lib") { + auto lib1 = std::make_unique("lib1", 1); + + auto tree1 = SyntaxTree::fromText(R"( +module qq; +endmodule +)", + SyntaxTree::getDefaultSourceManager(), "source", "", {}, + lib1.get()); + + auto tree2 = SyntaxTree::fromText(R"( +config cfg1; + design top; + cell lib1.f use baz; + cell top liblist lib1; +endconfig + +module baz; +endmodule + +module top; + f foo(); +endmodule +)"); + CompilationOptions options; + options.topModules.emplace("cfg1"); + + Compilation compilation(options); + compilation.addSyntaxTree(tree2); + compilation.addSyntaxTree(tree1); + NO_COMPILATION_ERRORS; +}