diff --git a/include/slang/ast/symbols/CompilationUnitSymbols.h b/include/slang/ast/symbols/CompilationUnitSymbols.h index c4ec4a822..9f9ed41b4 100644 --- a/include/slang/ast/symbols/CompilationUnitSymbols.h +++ b/include/slang/ast/symbols/CompilationUnitSymbols.h @@ -246,6 +246,9 @@ struct SLANG_EXPORT ConfigRule { /// The source range where this rule was declared. SourceRange sourceRange; + + /// A flag that marks whether this rule has been used during elaboration. + mutable bool isUsed = false; }; /// Contains information about a resolved configuration rule @@ -283,6 +286,9 @@ class SLANG_EXPORT ConfigBlockSymbol : public Symbol, public Scope { ConfigRule* rule = nullptr; }; + /// A flag that marks whether this config block has been used during elaboration. + mutable bool isUsed = false; + ConfigBlockSymbol(Compilation& compilation, std::string_view name, SourceLocation loc) : Symbol(SymbolKind::ConfigBlock, name, loc), Scope(compilation, this) {} @@ -310,6 +316,8 @@ class SLANG_EXPORT ConfigBlockSymbol : public Symbol, public Scope { return instanceOverrides; } + void checkUnusedRules() const; + static ConfigBlockSymbol& fromSyntax(const Scope& scope, const syntax::ConfigDeclarationSyntax& syntax); diff --git a/scripts/diagnostics.txt b/scripts/diagnostics.txt index b80667ca1..99175b09d 100644 --- a/scripts/diagnostics.txt +++ b/scripts/diagnostics.txt @@ -526,6 +526,8 @@ warning invalid-pulsestyle InvalidPulseStyle "invalid use of {} for '{}' since i warning implicit-port-type-mismatch ImplicitNamedPortTypeMismatch "implicit named port '{}' of type {} connects to value of inequivalent type {}" warning unknown-library WarnUnknownLibrary "unknown library '{}'" warning dup-config-rule DupConfigRule "duplicate config rule for {} -- ignoring this one" +warning unused-config-cell UnusedConfigCell "unused cell config rule" +warning unused-config-instance UnusedConfigInstance "unused config rule -- instance path does not exist in the design" subsystem Expressions error BadUnaryExpression "invalid operand type {} to unary expression" @@ -1066,7 +1068,7 @@ group default = { real-underflow real-overflow vector-overflow int-overflow unco raw-protect-eof protected-envelope specify-param dup-timing-path invalid-pulsestyle negative-timing-limit bad-procedural-force duplicate-defparam implicit-port-type-mismatch split-distweight-op dpi-pure-task multibit-edge unknown-sys-name unknown-library - dup-config-rule } + dup-config-rule unused-config-cell unused-config-instance } group extra = { empty-member empty-stmt dup-import pointless-void-cast case-gen-none case-gen-dup unused-result format-real ignored-slice task-ignored width-trunc dup-attr event-const @@ -1082,4 +1084,4 @@ group conversion = { width-trunc width-expand port-width-trunc port-width-expand group unused = { unused-def unused-net unused-implicit-net unused-variable undriven-net unassigned-variable unused-but-set-net unused-but-set-variable unused-port undriven-port unused-argument unused-parameter unused-type-parameter unused-typedef unused-genvar unused-assertion-decl - unused-but-set-port } + unused-but-set-port unused-config-cell unused-config-instance } diff --git a/scripts/warning_docs.txt b/scripts/warning_docs.txt index 2a8f08270..a127537c7 100644 --- a/scripts/warning_docs.txt +++ b/scripts/warning_docs.txt @@ -1162,3 +1162,29 @@ config cfg; instance d.foo use baz; endconfig ``` + +-Wunused-config-cell +A configuration cell rule is unused, which may mean that there's a typo in the target name. +@options --top=cfg +``` +config cfg; + design top; + cell foo liblist l; +endconfig + +module top; +endmodule +``` + +-Wunused-config-instance +A configuration instance rule is unused, which means that target path does not exist in the design. +@options --top=cfg +``` +config cfg; + design top; + instance top.foo liblist l; +endconfig + +module top; +endmodule +``` diff --git a/source/ast/Compilation.cpp b/source/ast/Compilation.cpp index ae88a043b..fc809536d 100644 --- a/source/ast/Compilation.cpp +++ b/source/ast/Compilation.cpp @@ -471,6 +471,7 @@ const RootSymbol& Compilation::getRoot(bool skipDefParamsAndBinds) { } if (foundConf) { + foundConf->isUsed = true; for (auto& cell : foundConf->getTopCells()) { selectedConfigs.emplace(foundConf); topDefs.push_back({&cell.definition, foundConf, cell.rule}); @@ -1350,6 +1351,16 @@ const Diagnostics& Compilation::getSemanticDiagnostics() { } } + // Report on unused config rules. + if (!configBlocks.empty()) { + for (auto& [name, confList] : configBlocks) { + for (auto config : confList) { + if (config->isUsed) + config->checkUnusedRules(); + } + } + } + if (!hasFlag(CompilationFlags::AllowTopLevelIfacePorts)) { // Top level instances cannot have interface or ref ports. for (auto inst : getRoot().topInstances) { @@ -2292,6 +2303,7 @@ auto findDefByLib(TDefList& defList, const SourceLibrary& target) std::pair Compilation::resolveConfigRule( const Scope& scope, const ConfigRule& rule) const { + rule.isUsed = true; auto& id = rule.useCell; SLANG_ASSERT(!id.name.empty()); @@ -2324,6 +2336,7 @@ std::pair Compilation::resolveConfigR if (auto configIt = configBlocks.find(id.name); configIt != configBlocks.end()) { auto result = findDefByLib(configIt->second, *overrideLib); if (result) { + result->isUsed = true; auto topCells = result->getTopCells(); if (topCells.size() != 1) { auto syntax = result->getSyntax(); @@ -2373,6 +2386,7 @@ std::pair Compilation::resolveConfigR if (auto& id = rule->useCell; !id.name.empty()) return resolveConfigRule(scope, *rule); + rule->isUsed = true; if (rule->liblist) liblist = *rule->liblist; } diff --git a/source/ast/symbols/CompilationUnitSymbols.cpp b/source/ast/symbols/CompilationUnitSymbols.cpp index 4dd2b71ff..ad850387c 100644 --- a/source/ast/symbols/CompilationUnitSymbols.cpp +++ b/source/ast/symbols/CompilationUnitSymbols.cpp @@ -373,6 +373,34 @@ ConfigBlockSymbol& ConfigBlockSymbol::fromSyntax(const Scope& scope, return *result; } +static void checkRuleUnused(const Scope& scope, const ConfigRule& rule, bool isCell) { + if (!rule.isUsed) { + scope.addDiag(isCell ? diag::UnusedConfigCell : diag::UnusedConfigInstance, + rule.sourceRange); + } +} + +static void checkNodeUnused(const Scope& scope, const ConfigBlockSymbol::InstanceOverride& node) { + if (node.rule) + checkRuleUnused(scope, *node.rule, false); + + for (auto& [name, child] : node.childNodes) + checkNodeUnused(scope, child); +} + +void ConfigBlockSymbol::checkUnusedRules() const { + for (auto& [_, cellOverride] : getCellOverrides()) { + if (cellOverride.defaultRule) + checkRuleUnused(*this, *cellOverride.defaultRule, true); + + for (auto& [lib, rule] : cellOverride.specificLibRules) + checkRuleUnused(*this, *rule, true); + } + + for (auto& [_, instOverride] : getInstanceOverrides()) + checkNodeUnused(*this, instOverride); +} + void ConfigBlockSymbol::resolve() const { SLANG_ASSERT(!resolved); resolved = true; diff --git a/source/ast/symbols/InstanceSymbols.cpp b/source/ast/symbols/InstanceSymbols.cpp index be0a2a1a0..0d807e08a 100644 --- a/source/ast/symbols/InstanceSymbols.cpp +++ b/source/ast/symbols/InstanceSymbols.cpp @@ -273,8 +273,11 @@ InstanceSymbol& InstanceSymbol::createDefault(Compilation& comp, const Definitio if (configBlock) { auto rc = comp.emplace(*configBlock, result); - if (configRule && configRule->liblist) - rc->liblist = *configRule->liblist; + if (configRule) { + configRule->isUsed = true; + if (configRule->liblist) + rc->liblist = *configRule->liblist; + } result.resolvedConfig = rc; } @@ -527,6 +530,7 @@ void InstanceSymbol::fromSyntax(Compilation& comp, const HierarchyInstantiationS if (confRule) { SLANG_ASSERT(resolvedConfig); auto rc = comp.emplace(*resolvedConfig); + confRule->isUsed = true; if (confRule->liblist) rc->liblist = *confRule->liblist; localConfig = rc; diff --git a/tests/unittests/ast/ConfigTests.cpp b/tests/unittests/ast/ConfigTests.cpp index 5bc84a17e..15784fc92 100644 --- a/tests/unittests/ast/ConfigTests.cpp +++ b/tests/unittests/ast/ConfigTests.cpp @@ -550,15 +550,17 @@ endmodule compilation.addSyntaxTree(tree); auto& diags = compilation.getAllDiagnostics(); - REQUIRE(diags.size() == 8); + REQUIRE(diags.size() == 10); CHECK(diags[0].code == diag::ConfigDupTop); CHECK(diags[1].code == diag::WarnUnknownLibrary); - CHECK(diags[2].code == diag::DupConfigRule); + CHECK(diags[2].code == diag::UnusedConfigCell); CHECK(diags[3].code == diag::DupConfigRule); - CHECK(diags[4].code == diag::ConfigInstanceWrongTop); - CHECK(diags[5].code == diag::ConfigParamsIgnored); - CHECK(diags[6].code == diag::WarnUnknownLibrary); - CHECK(diags[7].code == diag::NestedConfigMultipleTops); + CHECK(diags[4].code == diag::UnusedConfigInstance); + CHECK(diags[5].code == diag::DupConfigRule); + CHECK(diags[6].code == diag::ConfigInstanceWrongTop); + CHECK(diags[7].code == diag::ConfigParamsIgnored); + CHECK(diags[8].code == diag::WarnUnknownLibrary); + CHECK(diags[9].code == diag::NestedConfigMultipleTops); } TEST_CASE("Config rules with param overrides") { @@ -786,7 +788,10 @@ endconfig Compilation compilation(options); compilation.addSyntaxTree(tree2); compilation.addSyntaxTree(tree1); - NO_COMPILATION_ERRORS; + + auto& diags = compilation.getAllDiagnostics(); + REQUIRE(diags.size() == 1); + CHECK(diags[0].code == diag::UnusedConfigCell); auto getParam = [&](std::string_view name) { auto& param = compilation.getRoot().lookupName(name);