Minor tweaks to speed up defparam and bind resolution pass

MikePopoloski · Nov 27, 2024 · 3f6542f · 3f6542f
1 parent 44434bc
commit 3f6542f
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 18 deletions.
diff --git a/include/slang/ast/Compilation.h b/include/slang/ast/Compilation.h
@@ -871,6 +871,7 @@ class SLANG_EXPORT Compilation : public BumpAllocator {
     bool finalized = false;
     bool finalizing = false; // to prevent reentrant calls to getRoot()
     bool anyElemsWithTimescales = false;
+    bool diagsDisabled = false;
     uint32_t typoCorrections = 0;
     int nextEnumSystemId = 1;
     int nextStructSystemId = 1;

diff --git a/source/ast/Compilation.cpp b/source/ast/Compilation.cpp
@@ -1295,14 +1295,16 @@ void Compilation::noteExternDefinition(const Scope& scope, const SyntaxNode& syn
 
 const SyntaxNode* Compilation::getExternDefinition(std::string_view name,
                                                    const Scope& scope) const {
-    const Scope* searchScope = &scope;
-    do {
-        auto it = externDefMap.find(std::make_tuple(name, searchScope));
-        if (it != externDefMap.end())
-            return it->second;
+    if (!externDefMap.empty()) {
+        const Scope* searchScope = &scope;
+        do {
+            auto it = externDefMap.find(std::make_tuple(name, searchScope));
+            if (it != externDefMap.end())
+                return it->second;
 
-        searchScope = searchScope->asSymbol().getParentScope();
-    } while (searchScope);
+            searchScope = searchScope->asSymbol().getParentScope();
+        } while (searchScope);
+    }
 
     return nullptr;
 }
@@ -1646,6 +1648,11 @@ void Compilation::addDiagnostics(const Diagnostics& diagnostics) {
 }
 
 Diagnostic& Compilation::addDiag(Diagnostic diag) {
+    if (diagsDisabled) {
+        tempDiag = std::move(diag);
+        return tempDiag;
+    }
+
     auto isSuppressed = [](const Symbol* symbol) {
         while (symbol) {
             if (symbol->kind == SymbolKind::GenerateBlock)
@@ -2260,6 +2267,7 @@ void Compilation::resolveDefParamsAndBinds() {
     };
 
     auto cloneInto = [&](Compilation& c) {
+        c.diagsDisabled = true;
         c.options = options;
         for (auto& tree : syntaxTrees)
             c.addSyntaxTree(tree);
@@ -2348,6 +2356,7 @@ void Compilation::resolveDefParamsAndBinds() {
     size_t generateLevel = 0;
     size_t numBlocksSeen = 0;
     size_t numBindsSeen = 0;
+    size_t numDefParamsSeen = 0;
     while (true) {
         // Traverse the design and find all defparams and their values.
         // defparam resolution happens in a cloned compilation unit because we will be
@@ -2357,16 +2366,47 @@ void Compilation::resolveDefParamsAndBinds() {
         Compilation initialClone({}, defaultLibPtr);
         cloneInto(initialClone);
 
-        DefParamVisitor initialVisitor(options.maxInstanceDepth, generateLevel);
-        initialClone.getRoot(/* skipDefParamsAndBinds */ true).visit(initialVisitor);
-        if (checkProblem(initialVisitor))
-            return;
+        size_t currBlocksSeen;
+        auto nextIt = [&] {
+            // If we haven't found any new blocks we're done iterating.
+            SLANG_ASSERT(currBlocksSeen >= numBlocksSeen);
+            if (currBlocksSeen == numBlocksSeen)
+                return true;
 
-        saveState(initialVisitor, initialClone);
+            // Otherwise advance into the next blocks and try again.
+            generateLevel++;
+            numBlocksSeen = currBlocksSeen;
+            return false;
+        };
+
+        while (true) {
+            DefParamVisitor v(options.maxInstanceDepth, generateLevel);
+            initialClone.getRoot(/* skipDefParamsAndBinds */ true).visit(v);
+            if (checkProblem(v))
+                return;
+
+            currBlocksSeen = v.numBlocksSeen;
+            if (v.found.size() > numDefParamsSeen ||
+                initialClone.bindDirectives.size() > numBindsSeen) {
+                numDefParamsSeen = v.found.size();
+                saveState(v, initialClone);
+                break;
+            }
+
+            // We didn't find any more binds or defparams so increase
+            // our generate level and try again.
+            if (nextIt()) {
+                saveState(v, initialClone);
+                break;
+            }
+        }
 
         // If we have found more binds, do another visit to let them be applied
         // and potentially add blocks and defparams to our set for this level.
         if (initialClone.bindDirectives.size() > numBindsSeen) {
+            // Reset the number of defparams seen to ensure that
+            // we re-resolve everything after the next iteration.
+            numDefParamsSeen = 0;
             numBindsSeen = initialClone.bindDirectives.size();
             continue;
         }
@@ -2432,12 +2472,8 @@ void Compilation::resolveDefParamsAndBinds() {
         if (!allSame)
             break;
 
-        SLANG_ASSERT(initialVisitor.numBlocksSeen >= numBlocksSeen);
-        if (initialVisitor.numBlocksSeen == numBlocksSeen)
+        if (nextIt())
             break;
-
-        generateLevel++;
-        numBlocksSeen = initialVisitor.numBlocksSeen;
     }
 
     // We have our final overrides; copy them into the main compilation unit.

diff --git a/source/ast/ElabVisitors.h b/source/ast/ElabVisitors.h
@@ -496,8 +496,10 @@ struct DefParamVisitor : public ASTVisitor<DefParamVisitor, false, false> {
     }
 
     void handle(const InstanceSymbol& symbol) {
-        if (symbol.body.flags.has(InstanceFlags::Uninstantiated) || hierarchyProblem)
+        if (symbol.name.empty() || symbol.body.flags.has(InstanceFlags::Uninstantiated) ||
+            hierarchyProblem) {
             return;
+        }
 
         // If we hit max depth we have a problem -- setting the hierarchyProblem
         // member will cause other functions to early out so that we complete

diff --git a/source/ast/symbols/InstanceSymbols.cpp b/source/ast/symbols/InstanceSymbols.cpp
@@ -745,6 +745,9 @@ void InstanceSymbol::fromFixupSyntax(Compilation& comp, const DefinitionSymbol&
     // for this fixup case.
     SmallVector<TokenOrSyntax, 4> instances;
     for (auto decl : syntax.declarators) {
+        if (decl->name.valueText().empty())
+            continue;
+
         auto loc = decl->name.location();
         if (!instances.empty())
             instances.push_back(missing(TokenKind::Comma, loc));