Skip to content

Commit

Permalink
Allow hierarchically inheriting configuration specified liblists
Browse files Browse the repository at this point in the history
  • Loading branch information
MikePopoloski committed Feb 23, 2024
1 parent f904c15 commit 665879f
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 60 deletions.
6 changes: 1 addition & 5 deletions include/slang/ast/Compilation.h
Original file line number Diff line number Diff line change
Expand Up @@ -680,8 +680,7 @@ class SLANG_EXPORT Compilation : public BumpAllocator {
std::span<const Symbol* const> instTargets,
const DefinitionSymbol* defTarget);
std::pair<const Symbol*, bool> resolveConfigRules(std::string_view name, const Scope& scope,
const ConfigBlockSymbol* config,
const ConfigRule* configRule,
const ConfigRule& configRule,
const std::vector<Symbol*>& defList) const;
Diagnostic* errorMissingDef(std::string_view name, const Scope& scope, SourceRange sourceRange,
DiagCode code) const;
Expand Down Expand Up @@ -769,9 +768,6 @@ class SLANG_EXPORT Compilation : public BumpAllocator {
// A list of all created definitions, as storage for their memory.
std::vector<std::unique_ptr<DefinitionSymbol>> definitionMemory;

// A map of config blocks to use for a given scope.
flat_hash_map<const Scope*, const ConfigBlockSymbol*> 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::tuple<DiagCode, SourceLocation>, std::vector<Diagnostic>>;
Expand Down
4 changes: 4 additions & 0 deletions include/slang/ast/symbols/CompilationUnitSymbols.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class SyntaxTree;

namespace slang::ast {

class ConfigBlockSymbol;
class Expression;
class InstanceSymbol;
class Type;
Expand Down Expand Up @@ -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;
};
Expand Down
5 changes: 5 additions & 0 deletions include/slang/ast/symbols/InstanceSymbols.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class PortConnection;
class PortSymbol;
class PrimitiveSymbol;
class TimingControl;
struct ConfigRule;
struct HierarchyOverrideNode;
enum class DriveStrength : int;

Expand Down Expand Up @@ -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,
Expand Down
99 changes: 47 additions & 52 deletions source/ast/Compilation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<ConfigRule>();
rule->useConfig = config;
instance.configRule = rule;
}
}

if (!hasFlag(CompilationFlags::SuppressUnused) && topDefs.empty())
Expand Down Expand Up @@ -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;
}

Expand All @@ -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) {
Expand Down Expand Up @@ -679,9 +672,9 @@ const Symbol* Compilation::getDefinition(std::string_view lookupName, const Scop
DiagCode code) const {
std::pair<const Symbol*, bool> 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.
Expand Down Expand Up @@ -2280,8 +2273,9 @@ void Compilation::resolveDefParamsAndBinds() {
}

std::pair<const Symbol*, bool> Compilation::resolveConfigRules(
std::string_view lookupName, const Scope& scope, const ConfigBlockSymbol* config,
const ConfigRule* rule, const std::vector<Symbol*>& defList) const {
std::string_view lookupName, const Scope& scope, const ConfigRule& initialRule,
const std::vector<Symbol*>& defList) const {

auto findDefByLib = [](auto& defList, const SourceLibrary* target) -> Symbol* {
for (auto def : defList) {
if (def->getSourceLibrary() == target)
Expand All @@ -2290,10 +2284,15 @@ std::pair<const Symbol*, bool> Compilation::resolveConfigRules(
return nullptr;
};

std::span<const SourceLibrary* const> 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()) {

Expand All @@ -2305,41 +2304,37 @@ std::pair<const Symbol*, bool> 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
Expand Down
14 changes: 11 additions & 3 deletions source/ast/symbols/InstanceSymbols.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,12 @@ class InstanceBuilder {
public:
InstanceBuilder(const ASTContext& context, const DefinitionSymbol& definition,
ParameterBuilder& paramBuilder, const HierarchyOverrideNode* parentOverrideNode,
std::span<const AttributeInstanceSyntax* const> attributes, bool isFromBind) :
std::span<const AttributeInstanceSyntax* const> 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();
Expand Down Expand Up @@ -96,6 +98,7 @@ class InstanceBuilder {
ParameterBuilder& paramBuilder;
const HierarchyOverrideNode* parentOverrideNode;
std::span<const AttributeInstanceSyntax* const> attributes;
const ConfigRule* configRule;
bool isFromBind;

Symbol* createInstance(const HierarchicalInstanceSyntax& syntax,
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
39 changes: 39 additions & 0 deletions tests/unittests/ast/ConfigTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<SourceLibrary>("lib1", 1);
auto lib2 = std::make_unique<SourceLibrary>("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;
}

0 comments on commit 665879f

Please sign in to comment.