diff --git a/include/slang/ast/symbols/CompilationUnitSymbols.h b/include/slang/ast/symbols/CompilationUnitSymbols.h index 070f08acf..a5e876e04 100644 --- a/include/slang/ast/symbols/CompilationUnitSymbols.h +++ b/include/slang/ast/symbols/CompilationUnitSymbols.h @@ -264,9 +264,6 @@ struct SLANG_EXPORT ResolvedConfig { /// A list of libraries to use to look up definitions. std::span liblist; - /// The original rule that was resolved. - const ConfigRule* configRule = nullptr; - ResolvedConfig(const ConfigBlockSymbol& useConfig, const InstanceSymbol& rootInstance); }; diff --git a/scripts/diagnostics.txt b/scripts/diagnostics.txt index fba88b868..52c9aa72a 100644 --- a/scripts/diagnostics.txt +++ b/scripts/diagnostics.txt @@ -1019,6 +1019,8 @@ error MissingExternModuleImpl "missing implementation for extern {} '{}'" error ExternDeclMismatchImpl "extern {} '{}' does not match implementation" error ExternDeclMismatchPrev "extern {} '{}' does not match previous extern definition" error UnknownLibrary "unknown library '{}'" +error NestedConfigMultipleTops "non-top config '{}' has more than one top cell specified" +error ConfigParamsIgnored "parameter overrides provided by config rule are ignored because the target instance is using a different nested config '{}'" warning unused-def UnusedDefinition "{} definition is unused" warning unused-net UnusedNet "unused net '{}'" warning unused-implicit-net UnusedImplicitNet "implicit net '{}' is unused elsewhere" diff --git a/source/ast/Compilation.cpp b/source/ast/Compilation.cpp index 4195b730a..6433a89e5 100644 --- a/source/ast/Compilation.cpp +++ b/source/ast/Compilation.cpp @@ -2349,11 +2349,24 @@ std::pair Compilation::resolveConfigR if (result) { auto topCells = result->getTopCells(); if (topCells.size() != 1) { - // TODO: error + 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}; } - return {{&topCells[0].definition, result, nullptr}, true}; + if (rule->paramOverrides) { + scope.addDiag(diag::ConfigParamsIgnored, + rule->paramOverrides->sourceRange()) + << result->name; + } + + return {{&topCells[0].definition, result, topCells[0].rule}, true}; } } diff --git a/source/ast/symbols/CompilationUnitSymbols.cpp b/source/ast/symbols/CompilationUnitSymbols.cpp index ea96c319e..b7ff26975 100644 --- a/source/ast/symbols/CompilationUnitSymbols.cpp +++ b/source/ast/symbols/CompilationUnitSymbols.cpp @@ -445,8 +445,13 @@ void ConfigBlockSymbol::resolve() const { curRule.paramOverrides = newRule.paramOverrides; if (newRule.liblist) curRule.liblist = newRule.liblist; - if (!newRule.useCell.name.empty()) + if (!newRule.useCell.name.empty()) { + // Note: prefer the source range to come from the rule that + // supplies a use cell name, to make other reported errors easier + // to understand. curRule.useCell = newRule.useCell; + curRule.sourceRange = newRule.sourceRange; + } } }; diff --git a/source/ast/symbols/InstanceSymbols.cpp b/source/ast/symbols/InstanceSymbols.cpp index e0731b497..aefd404c2 100644 --- a/source/ast/symbols/InstanceSymbols.cpp +++ b/source/ast/symbols/InstanceSymbols.cpp @@ -59,7 +59,7 @@ class InstanceBuilder { std::span attributes, const ResolvedConfig* resolvedConfig, const ConfigBlockSymbol* newConfigRoot, bool isFromBind) : - compilation(context.getCompilation()), + comp(context.getCompilation()), context(context), definition(definition), paramBuilder(paramBuilder), parentOverrideNode(parentOverrideNode), attributes(attributes), resolvedConfig(resolvedConfig), newConfigRoot(newConfigRoot), isFromBind(isFromBind) {} @@ -92,7 +92,7 @@ class InstanceBuilder { private: using DimIterator = std::span::iterator; - Compilation& compilation; + Compilation& comp; const ASTContext& context; const DefinitionSymbol& definition; SmallVector path; @@ -107,22 +107,17 @@ class InstanceBuilder { const HierarchyOverrideNode* overrideNode) { paramBuilder.setOverrides(overrideNode); auto [name, loc] = getNameLoc(syntax); - auto inst = compilation.emplace(compilation, name, loc, definition, - paramBuilder, /* isUninstantiated */ false, - isFromBind); - inst->arrayPath = path.copy(compilation); + auto inst = comp.emplace(comp, name, loc, definition, paramBuilder, + /* isUninstantiated */ false, isFromBind); + inst->arrayPath = path.copy(comp); inst->setSyntax(syntax); inst->setAttributes(*context.scope, attributes); if (resolvedConfig) { - if (newConfigRoot) { - auto rc = compilation.emplace(*newConfigRoot, *inst); - rc->configRule = resolvedConfig->configRule; - inst->resolvedConfig = rc; - } - else { + if (newConfigRoot) + inst->resolvedConfig = comp.emplace(*newConfigRoot, *inst); + else inst->resolvedConfig = resolvedConfig; - } } return inst; @@ -136,10 +131,10 @@ class InstanceBuilder { SLANG_ASSERT(syntax.decl); auto nameToken = syntax.decl->name; auto createEmpty = [&]() { - return compilation.emplace(compilation, nameToken.valueText(), - nameToken.location(), - std::span{}, - ConstantRange()); + return comp.emplace(comp, nameToken.valueText(), + nameToken.location(), + std::span{}, + ConstantRange()); }; auto& dimSyntax = **it; @@ -153,9 +148,9 @@ class InstanceBuilder { return createEmpty(); ConstantRange range = dim.range; - if (range.width() > compilation.getOptions().maxInstanceArray) { + if (range.width() > comp.getOptions().maxInstanceArray) { auto& diag = context.addDiag(diag::MaxInstanceArrayExceeded, dimSyntax.sourceRange()); - diag << definition.getKindString() << compilation.getOptions().maxInstanceArray; + diag << definition.getKindString() << comp.getOptions().maxInstanceArray; return createEmpty(); } @@ -176,9 +171,9 @@ class InstanceBuilder { elements.push_back(symbol); } - auto result = compilation.emplace(compilation, nameToken.valueText(), - nameToken.location(), - elements.copy(compilation), range); + auto result = comp.emplace(comp, nameToken.valueText(), + nameToken.location(), elements.copy(comp), + range); result->setSyntax(syntax); for (auto element : elements) @@ -528,7 +523,6 @@ void InstanceSymbol::fromSyntax(Compilation& comp, const HierarchyInstantiationS if (confRule) { SLANG_ASSERT(resolvedConfig); auto rc = comp.emplace(*resolvedConfig); - rc->configRule = confRule; if (confRule->liblist) rc->liblist = *confRule->liblist; localConfig = rc; diff --git a/tests/unittests/ast/ConfigTests.cpp b/tests/unittests/ast/ConfigTests.cpp index 4fb5a93a0..28196539d 100644 --- a/tests/unittests/ast/ConfigTests.cpp +++ b/tests/unittests/ast/ConfigTests.cpp @@ -511,7 +511,7 @@ endmodule CHECK(barA.getDefinition().name == "m2"); } -TEST_CASE("Config warning cases") { +TEST_CASE("Config warning / error cases") { auto tree = SyntaxTree::fromText(R"( config cfg1; design top top; @@ -521,9 +521,25 @@ config cfg1; instance top.foo use lib1.c; instance top.foo use d; instance foo.bar use e; + instance top.blah1 use qq; + instance top.blah2 use rr; + instance top.blah2 use #(.A(1)); +endconfig + +module a; endmodule +module b; endmodule + +config qq; + design a b; +endconfig + +config rr; + design a; endconfig module top; + asdf blah1(); + asdf blah2(); endmodule )"); CompilationOptions options; @@ -533,12 +549,14 @@ endmodule compilation.addSyntaxTree(tree); auto& diags = compilation.getAllDiagnostics(); - REQUIRE(diags.size() == 5); + REQUIRE(diags.size() == 7); CHECK(diags[0].code == diag::ConfigDupTop); CHECK(diags[1].code == diag::WarnUnknownLibrary); CHECK(diags[2].code == diag::DupConfigRule); CHECK(diags[3].code == diag::DupConfigRule); CHECK(diags[4].code == diag::ConfigInstanceWrongTop); + CHECK(diags[5].code == diag::ConfigParamsIgnored); + CHECK(diags[6].code == diag::NestedConfigMultipleTops); } TEST_CASE("Config rules with param overrides") { @@ -667,3 +685,37 @@ endmodule CHECK(getParam("top.A").integer() == 1); CHECK(getParam("top.foo.B").integer() == 2); } + +TEST_CASE("Config param overrides with nested config path") { + auto tree = SyntaxTree::fromText(R"( +config cfg1; + design top; + cell f use cfg2; +endconfig + +config cfg2; + design f; + cell f use #(.B(5)); +endconfig + +module f #(parameter int B); +endmodule + +module top; + f foo(); +endmodule +)"); + CompilationOptions options; + options.topModules.emplace("cfg1"); + + Compilation compilation(options); + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + + auto getParam = [&](std::string_view name) { + auto& param = compilation.getRoot().lookupName(name); + return param.getValue(); + }; + + CHECK(getParam("top.foo.B").integer() == 5); +}