From 665879f83954857d1d56092cf69a1b3eeb867e4f Mon Sep 17 00:00:00 2001 From: MikePopoloski Date: Thu, 22 Feb 2024 20:41:38 -0500 Subject: [PATCH] Allow hierarchically inheriting configuration specified liblists --- include/slang/ast/Compilation.h | 6 +- .../ast/symbols/CompilationUnitSymbols.h | 4 + include/slang/ast/symbols/InstanceSymbols.h | 5 + source/ast/Compilation.cpp | 99 +++++++++---------- source/ast/symbols/InstanceSymbols.cpp | 14 ++- tests/unittests/ast/ConfigTests.cpp | 39 ++++++++ 6 files changed, 107 insertions(+), 60 deletions(-) diff --git a/include/slang/ast/Compilation.h b/include/slang/ast/Compilation.h index ea6e1397a..69007ef83 100644 --- a/include/slang/ast/Compilation.h +++ b/include/slang/ast/Compilation.h @@ -680,8 +680,7 @@ class SLANG_EXPORT Compilation : public BumpAllocator { std::span instTargets, const DefinitionSymbol* defTarget); std::pair resolveConfigRules(std::string_view name, const Scope& scope, - const ConfigBlockSymbol* config, - const ConfigRule* configRule, + const ConfigRule& configRule, const std::vector& defList) const; Diagnostic* errorMissingDef(std::string_view name, const Scope& scope, SourceRange sourceRange, DiagCode code) const; @@ -769,9 +768,6 @@ class SLANG_EXPORT Compilation : public BumpAllocator { // A list of all created definitions, as storage for their memory. std::vector> definitionMemory; - // A map of config blocks to use for a given scope. - flat_hash_map configForScope; - // A map from diag code + location to the diagnostics that have occurred at that location. // This is used to collapse duplicate diagnostics across instantiations into a single report. using DiagMap = flat_hash_map, std::vector>; diff --git a/include/slang/ast/symbols/CompilationUnitSymbols.h b/include/slang/ast/symbols/CompilationUnitSymbols.h index 29745fba8..fef04ba37 100644 --- a/include/slang/ast/symbols/CompilationUnitSymbols.h +++ b/include/slang/ast/symbols/CompilationUnitSymbols.h @@ -17,6 +17,7 @@ class SyntaxTree; namespace slang::ast { +class ConfigBlockSymbol; class Expression; class InstanceSymbol; class Type; @@ -242,6 +243,9 @@ struct ConfigRule { /// A specific cell to use for this instance or definition lookup. ConfigCellId useCell; + /// A specific config block to use for this instance and child instances. + const ConfigBlockSymbol* useConfig = nullptr; + /// The source range where this rule was declared. SourceRange sourceRange; }; diff --git a/include/slang/ast/symbols/InstanceSymbols.h b/include/slang/ast/symbols/InstanceSymbols.h index db201a833..fbaf67253 100644 --- a/include/slang/ast/symbols/InstanceSymbols.h +++ b/include/slang/ast/symbols/InstanceSymbols.h @@ -30,6 +30,7 @@ class PortConnection; class PortSymbol; class PrimitiveSymbol; class TimingControl; +struct ConfigRule; struct HierarchyOverrideNode; enum class DriveStrength : int; @@ -59,6 +60,10 @@ class SLANG_EXPORT InstanceSymbol : public InstanceSymbolBase { public: const InstanceBodySymbol& body; + /// A config rule that applies to this instance, or a pointer to + /// the parent instance's config rule if there is one up the stack. + const ConfigRule* configRule = nullptr; + InstanceSymbol(std::string_view name, SourceLocation loc, InstanceBodySymbol& body); InstanceSymbol(Compilation& compilation, std::string_view name, SourceLocation loc, diff --git a/source/ast/Compilation.cpp b/source/ast/Compilation.cpp index 5c9058185..de5065bb2 100644 --- a/source/ast/Compilation.cpp +++ b/source/ast/Compilation.cpp @@ -553,8 +553,11 @@ const RootSymbol& Compilation::getRoot(bool skipDefParamsAndBinds) { root->addMember(instance); topList.push_back(&instance); - if (config) - configForScope.emplace(&instance.body, config); + if (config) { + auto rule = emplace(); + rule->useConfig = config; + instance.configRule = rule; + } } if (!hasFlag(CompilationFlags::SuppressUnused) && topDefs.empty()) @@ -585,27 +588,17 @@ const CompilationUnitSymbol* Compilation::getCompilationUnit( const Symbol* Compilation::tryGetDefinition(std::string_view lookupName, const Scope& scope) const { // Try to find a config block for this scope to help choose the right definition. - const ConfigBlockSymbol* config = nullptr; - if (!configForScope.empty()) { - auto searchScope = &scope; - do { - auto it = configForScope.find(searchScope); - if (it != configForScope.end()) { - config = it->second; - break; - } - - searchScope = searchScope->asSymbol().getParentScope(); - } while (searchScope); - } + const ConfigRule* configRule = nullptr; + if (auto inst = scope.getContainingInstance(); inst && inst->parentInstance) + configRule = inst->parentInstance->configRule; // Always search in the root scope to start. Most definitions are global. auto it = definitionMap.find({lookupName, root.get()}); if (it == definitionMap.end()) { // If there's a config it might be able to provide an // override for this cell name. - if (config) - return resolveConfigRules(lookupName, scope, config, nullptr, {}).first; + if (configRule) + return resolveConfigRules(lookupName, scope, *configRule, {}).first; return nullptr; } @@ -625,8 +618,8 @@ const Symbol* Compilation::tryGetDefinition(std::string_view lookupName, const S } auto& defList = it->second.first; - if (config) - return resolveConfigRules(lookupName, scope, config, nullptr, defList).first; + if (configRule) + return resolveConfigRules(lookupName, scope, *configRule, defList).first; // If there is a global priority list try to use that. for (auto lib : defaultLiblist) { @@ -679,9 +672,9 @@ const Symbol* Compilation::getDefinition(std::string_view lookupName, const Scop DiagCode code) const { std::pair result; if (auto it = definitionMap.find({lookupName, root.get()}); it != definitionMap.end()) - result = resolveConfigRules(lookupName, scope, nullptr, &configRule, it->second.first); + result = resolveConfigRules(lookupName, scope, configRule, it->second.first); else - result = resolveConfigRules(lookupName, scope, nullptr, &configRule, {}); + result = resolveConfigRules(lookupName, scope, configRule, {}); if (!result.first && !result.second) { // No definition found and no error issued, so issue one ourselves. @@ -2280,8 +2273,9 @@ void Compilation::resolveDefParamsAndBinds() { } std::pair Compilation::resolveConfigRules( - std::string_view lookupName, const Scope& scope, const ConfigBlockSymbol* config, - const ConfigRule* rule, const std::vector& defList) const { + std::string_view lookupName, const Scope& scope, const ConfigRule& initialRule, + const std::vector& defList) const { + auto findDefByLib = [](auto& defList, const SourceLibrary* target) -> Symbol* { for (auto def : defList) { if (def->getSourceLibrary() == target) @@ -2290,10 +2284,15 @@ std::pair Compilation::resolveConfigRules( return nullptr; }; - std::span liblist; - if (!rule) { - SLANG_ASSERT(config); + auto rule = &initialRule; + auto liblist = rule->liblist; + + // The config rule might specify that we use a specific + // configuration for this instance. + if (auto config = initialRule.useConfig) { + // By default we use the liblist from this config block. liblist = config->defaultLiblist; + if (auto overrideIt = config->cellOverrides.find(lookupName); overrideIt != config->cellOverrides.end()) { @@ -2305,41 +2304,37 @@ std::pair Compilation::resolveConfigRules( continue; rule = &cellOverride.rule; + liblist = rule->liblist; break; } } } - if (rule) { - auto& id = rule->useCell; - if (!id.name.empty()) { - auto overrideDefIt = definitionMap.find({id.name, root.get()}); - if (overrideDefIt == definitionMap.end()) { - errorMissingDef(id.name, *root, id.sourceRange, diag::UnknownModule); - return {nullptr, true}; - } + // The rule can specify a specific definition to use. + if (auto& id = rule->useCell; !id.name.empty()) { + auto overrideDefIt = definitionMap.find({id.name, root.get()}); + if (overrideDefIt == definitionMap.end()) { + errorMissingDef(id.name, *root, id.sourceRange, diag::UnknownModule); + return {nullptr, true}; + } - const SourceLibrary* overrideLib = nullptr; - 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 {nullptr, true}; - } + const SourceLibrary* overrideLib = nullptr; + 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 {nullptr, true}; } - - auto result = findDefByLib(overrideDefIt->second.first, overrideLib); - if (!result) - errorMissingDef(id.name, *root, id.sourceRange, diag::UnknownModule); - return {result, true}; } - // No name, so this is just a custom liblist -- search through it below. - liblist = rule->liblist; + auto result = findDefByLib(overrideDefIt->second.first, overrideLib); + if (!result) + errorMissingDef(id.name, *root, id.sourceRange, diag::UnknownModule); + return {result, true}; } // This search is O(n^2) but both lists should be small diff --git a/source/ast/symbols/InstanceSymbols.cpp b/source/ast/symbols/InstanceSymbols.cpp index dc1054720..fe4e41d7b 100644 --- a/source/ast/symbols/InstanceSymbols.cpp +++ b/source/ast/symbols/InstanceSymbols.cpp @@ -56,10 +56,12 @@ class InstanceBuilder { public: InstanceBuilder(const ASTContext& context, const DefinitionSymbol& definition, ParameterBuilder& paramBuilder, const HierarchyOverrideNode* parentOverrideNode, - std::span attributes, bool isFromBind) : + std::span attributes, + const ConfigRule* configRule, bool isFromBind) : compilation(context.getCompilation()), context(context), definition(definition), paramBuilder(paramBuilder), - parentOverrideNode(parentOverrideNode), attributes(attributes), isFromBind(isFromBind) {} + parentOverrideNode(parentOverrideNode), attributes(attributes), configRule(configRule), + isFromBind(isFromBind) {} Symbol* create(const HierarchicalInstanceSyntax& syntax) { path.clear(); @@ -96,6 +98,7 @@ class InstanceBuilder { ParameterBuilder& paramBuilder; const HierarchyOverrideNode* parentOverrideNode; std::span attributes; + const ConfigRule* configRule; bool isFromBind; Symbol* createInstance(const HierarchicalInstanceSyntax& syntax, @@ -106,6 +109,7 @@ class InstanceBuilder { paramBuilder, /* isUninstantiated */ false, isFromBind); + inst->configRule = configRule; inst->arrayPath = path.copy(compilation); inst->setSyntax(syntax); inst->setAttributes(*context.scope, attributes); @@ -454,8 +458,12 @@ void InstanceSymbol::fromSyntax(Compilation& comp, const HierarchyInstantiationS if (syntax.parameters) paramBuilder.setAssignments(*syntax.parameters); + auto instConfigRule = configRule; + if (!configRule && parentInst && parentInst->parentInstance) + instConfigRule = parentInst->parentInstance->configRule; + InstanceBuilder builder(context, definition, paramBuilder, parentOverrideNode, - syntax.attributes, isFromBind); + syntax.attributes, instConfigRule, isFromBind); if (specificInstance) { createImplicitNets(*specificInstance, context, netType, implicitNetNames, implicitNets); diff --git a/tests/unittests/ast/ConfigTests.cpp b/tests/unittests/ast/ConfigTests.cpp index e5bff82ea..84f3b94b7 100644 --- a/tests/unittests/ast/ConfigTests.cpp +++ b/tests/unittests/ast/ConfigTests.cpp @@ -391,3 +391,42 @@ endinterface CHECK(diags[3].code == diag::UnknownModule); CHECK(diags[4].code == diag::InvalidInstanceForParent); } + +TEST_CASE("Config inherited liblist") { + auto lib1 = std::make_unique("lib1", 1); + auto lib2 = std::make_unique("lib2", 2); + + auto tree1 = SyntaxTree::fromText(R"( +module mod; +endmodule +)", + SyntaxTree::getDefaultSourceManager(), "source", "", {}, + lib1.get()); + + auto tree2 = SyntaxTree::fromText(R"( +module baz; + mod m(); +endmodule +)", + SyntaxTree::getDefaultSourceManager(), "source", "", {}, + lib2.get()); + + auto tree3 = SyntaxTree::fromText(R"( +config cfg1; + design top; + instance top.b liblist lib1 lib2; +endconfig + +module top; + baz b(); +endmodule +)"); + CompilationOptions options; + options.topModules.emplace("cfg1"); + + Compilation compilation(options); + compilation.addSyntaxTree(tree1); + compilation.addSyntaxTree(tree2); + compilation.addSyntaxTree(tree3); + NO_COMPILATION_ERRORS; +}