More tests and fixes for config override rules

include/slang/ast/symbols/CompilationUnitSymbols.h

@@ -217,33 +217,29 @@ class SLANG_EXPORT DefinitionSymbol : public Symbol {
     mutable bool instantiated = false;
 };
 
-/// Identifies a specific cell as part of a config rule.
-struct SLANG_EXPORT ConfigCellId {
-    /// The library containing the cell (or empty to specify
-    /// that other logic should be used to find the library).
-    std::string_view lib;
-
-    /// The name of the cell.
-    std::string_view name;
-
-    /// The source range where this cell id was declared.
-    SourceRange sourceRange;
-
-    /// If true, this ID targets a config block specifically.
-    bool targetConfig = false;
-
-    ConfigCellId() = default;
-    ConfigCellId(std::string_view lib, std::string_view name, SourceRange sourceRange) :
-        lib(lib), name(name), sourceRange(sourceRange) {}
-};
-
 /// A rule that controls how a specific cell or instance in the design is configured.
 struct SLANG_EXPORT ConfigRule {
     /// A list of libraries to use to look up definitions.
     std::optional<std::span<const SourceLibrary* const>> liblist;
 
+    /// Contains information about how to look up a specific cell for this rule.
+    struct CellId {
+        /// The library containing the cell (or empty to specify
+        /// that other logic should be used to find the library).
+        std::string_view lib;
+
+        /// The name of the cell.
+        std::string_view name;
+
+        /// The source range where this cell id was declared.
+        SourceRange sourceRange;
+
+        /// If true, this ID targets a config block specifically.
+        bool targetConfig = false;
+    };
+
     /// A specific cell to use for this instance or definition lookup.
-    ConfigCellId useCell;
+    CellId useCell;
 
     /// Parameter overrides to apply to the instance.
     const syntax::ParameterValueAssignmentSyntax* paramOverrides = nullptr;

scripts/diagnostics.txt

@@ -283,6 +283,7 @@ error UniquePriorityAfterElse "cannot use 'unique' or 'priority' keyword after '
 error MultipleDefaultRules "configuration cannot have more than one default rule"
 error ConfigMissingName "cannot use 'config' without also specifying a target name"
 error ConfigParamsOrdered "parameter assignments in configurations must use explicit name notation, not positional notation"
+error ConfigSpecificCellLiblist "cannot specify a liblist when using a cell rule that targets a specific library already"
 error NoCommaInList "this list is delimited by spaces, not commas"
 warning dpi-pure-task DPIPureTask "DPI tasks cannot be marked 'pure'"
 warning nonstandard-generate NonStandardGenBlock "standalone generate block without loop or condition is not allowed in SystemVerilog"
@@ -497,7 +498,8 @@ error NetAliasCommonNetType "all nets in a net alias statement must have a commo
 error MissingExternWildcardPorts "could not find extern declaration for '{}' from which to take port list"
 error ExternIfaceArrayMethod "cannot connect extern subroutine to interface array"
 error ConfigInstanceWrongTop "config instance path must start with one of the cells named in the 'design' statement"
-error ConfigDupTop "config design specifies more than one top module named '{}'"
+error ConfigDupTop "config design specifies more than one top cell named '{}'"
+error ConfigOverrideTop "config rule can't override a top cell with a different target"
 error FatalTask "$fatal encountered{}"
 error ErrorTask "$error encountered{}"
 error StaticAssert "static assertion failed{}"

source/ast/Compilation.cpp

@@ -904,8 +904,9 @@ const ConfigBlockSymbol& Compilation::createConfigBlock(const Scope& scope,
         configBlocks.emplace(config.name, std::vector<const ConfigBlockSymbol*>{&config});
     }
     else {
+        auto configLib = scope.asSymbol().getSourceLibrary();
         auto findIt = std::ranges::find_if(it->second, [&](const ConfigBlockSymbol* elem) {
-            return elem->getSourceLibrary() == config.getSourceLibrary();
+            return elem->getSourceLibrary() == configLib;
         });
 
         if (findIt != it->second.end()) {

source/ast/symbols/CompilationUnitSymbols.cpp

@@ -421,8 +421,9 @@ void ConfigBlockSymbol::resolve() const {
             auto& cuc = rule.as<ConfigUseClauseSyntax>();
             result.paramOverrides = cuc.paramAssignments;
             if (cuc.name && !cuc.name->cell.valueText().empty()) {
-                result.useCell = ConfigCellId(cuc.name->library.valueText(),
-                                              cuc.name->cell.valueText(), cuc.name->sourceRange());
+                result.useCell.lib = cuc.name->library.valueText();
+                result.useCell.name = cuc.name->cell.valueText();
+                result.useCell.sourceRange = cuc.name->sourceRange();
                 if (cuc.config)
                     result.useCell.targetConfig = true;
             }
@@ -541,12 +542,17 @@ void ConfigBlockSymbol::resolve() const {
         }
     }
 
+    auto checkTopOverride = [&](const ConfigRule& rule) {
+        if (!rule.useCell.name.empty())
+            scope->addDiag(diag::ConfigOverrideTop, rule.sourceRange);
+    };
+
     // Check if any overrides should apply to the root instances.
-    // TODO: check validity of overrides here as applied to a root instance
     for (auto& [cellName, instOverride] : instanceOverrides) {
         if (instOverride.rule) {
             auto it = topCellNames.find(cellName);
             SLANG_ASSERT(it != topCellNames.end());
+            checkTopOverride(*instOverride.rule);
             topCellsBuf[it->second].rule = instOverride.rule;
         }
     }
@@ -564,6 +570,9 @@ void ConfigBlockSymbol::resolve() const {
                 specificIt != specificLibRules.end()) {
                 topCell.rule = specificIt->second;
             }
+
+            if (topCell.rule)
+                checkTopOverride(*topCell.rule);
         }
     }
 

source/parsing/Parser_members.cpp

@@ -3452,6 +3452,11 @@ ConfigDeclarationSyntax& Parser::parseConfigDeclaration(AttrList attributes) {
             else
                 rule = &parseConfigLiblist();
 
+            if (!cellName.library.valueText().empty() && rule->kind == SyntaxKind::ConfigLiblist) {
+                addDiag(diag::ConfigSpecificCellLiblist, rule->sourceRange())
+                    << cellName.library.range();
+            }
+
             rules.push_back(
                 &factory.cellConfigRule(token, cellName, *rule, expect(TokenKind::Semicolon)));
         }

tests/unittests/ast/ConfigTests.cpp

@@ -524,6 +524,7 @@ config cfg1;
     instance top.blah1 use qq;
     instance top.blah2 use rr;
     instance top.blah2 use #(.A(1));
+    cell foo.bar use baz;
 endconfig
 
 module a; endmodule
@@ -549,14 +550,15 @@ endmodule
     compilation.addSyntaxTree(tree);
 
     auto& diags = compilation.getAllDiagnostics();
-    REQUIRE(diags.size() == 7);
+    REQUIRE(diags.size() == 8);
     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);
+    CHECK(diags[6].code == diag::WarnUnknownLibrary);
+    CHECK(diags[7].code == diag::NestedConfigMultipleTops);
 }
 
 TEST_CASE("Config rules with param overrides") {
@@ -690,7 +692,7 @@ TEST_CASE("Config param overrides with nested config path") {
     auto tree = SyntaxTree::fromText(R"(
 config cfg1;
     design top;
-    cell f use cfg2;
+    cell f use cfg2 : config;
 endconfig
 
 config cfg2;
@@ -733,11 +735,12 @@ endmodule
     auto tree2 = SyntaxTree::fromText(R"(
 config cfg1;
     design top;
+    cell lib1.f use #(.A(4));
     cell lib1.f use baz;
     cell top liblist lib1;
 endconfig
 
-module baz;
+module baz #(parameter int A);
 endmodule
 
 module top;
@@ -751,4 +754,95 @@ endmodule
     compilation.addSyntaxTree(tree2);
     compilation.addSyntaxTree(tree1);
     NO_COMPILATION_ERRORS;
+
+    auto getParam = [&](std::string_view name) {
+        auto& param = compilation.getRoot().lookupName<ParameterSymbol>(name);
+        return param.getValue();
+    };
+
+    CHECK(getParam("top.foo.A").integer() == 4);
+}
+
+TEST_CASE("Config override top cell with specific target lib") {
+    auto lib1 = std::make_unique<SourceLibrary>("lib1", 1);
+
+    auto tree1 = SyntaxTree::fromText(R"(
+module qq #(parameter int A);
+endmodule
+)",
+                                      SyntaxTree::getDefaultSourceManager(), "source", "", {},
+                                      lib1.get());
+
+    auto tree2 = SyntaxTree::fromText(R"(
+config cfg1;
+    design lib1.qq;
+    cell lib1.qq use #(.A(4));
+    cell qq use #(.A(5));
+endconfig
+)");
+    CompilationOptions options;
+    options.topModules.emplace("cfg1");
+
+    Compilation compilation(options);
+    compilation.addSyntaxTree(tree2);
+    compilation.addSyntaxTree(tree1);
+    NO_COMPILATION_ERRORS;
+
+    auto getParam = [&](std::string_view name) {
+        auto& param = compilation.getRoot().lookupName<ParameterSymbol>(name);
+        return param.getValue();
+    };
+
+    CHECK(getParam("qq.A").integer() == 4);
+}
+
+TEST_CASE("Config invalid top cell override rule") {
+    auto tree = SyntaxTree::fromText(R"(
+config cfg1;
+    design top;
+    instance top use foo;
+endconfig
+
+config cfg2;
+    design top2;
+    cell top2 use foo;
+endconfig
+
+module top;
+endmodule
+
+module top2;
+endmodule
+)");
+    CompilationOptions options;
+    options.topModules.emplace("cfg1");
+    options.topModules.emplace("cfg2");
+
+    Compilation compilation(options);
+    compilation.addSyntaxTree(tree);
+
+    auto& diags = compilation.getAllDiagnostics();
+    REQUIRE(diags.size() == 2);
+    CHECK(diags[0].code == diag::ConfigOverrideTop);
+    CHECK(diags[1].code == diag::ConfigOverrideTop);
+}
+
+TEST_CASE("Duplicate config blocks") {
+    auto tree = SyntaxTree::fromText(R"(
+config cfg1;
+    design top;
+    instance top use foo;
+endconfig
+
+config cfg1;
+    design top;
+endconfig
+)");
+
+    Compilation compilation;
+    compilation.addSyntaxTree(tree);
+
+    auto& diags = compilation.getAllDiagnostics();
+    REQUIRE(diags.size() == 1);
+    CHECK(diags[0].code == diag::Redefinition);
 }

tests/unittests/parsing/MemberParsingTests.cpp

@@ -1182,16 +1182,18 @@ config cfg;
     instance foo.bar use #() : config;
     default liblist b;
     instance foo.bar use #(3, 4);
+    cell foo.bar liblist l;
 endconfig
 )";
 
     parseCompilationUnit(text);
 
-    REQUIRE(diagnostics.size() == 4);
+    REQUIRE(diagnostics.size() == 5);
     CHECK(diagnostics[0].code == diag::ExpectedIdentifier);
     CHECK(diagnostics[1].code == diag::ConfigMissingName);
     CHECK(diagnostics[2].code == diag::MultipleDefaultRules);
     CHECK(diagnostics[3].code == diag::ConfigParamsOrdered);
+    CHECK(diagnostics[4].code == diag::ConfigSpecificCellLiblist);
 }
 
 TEST_CASE("Config specific parse error for extraneous commas") {