From 6e092d99441e6d5d44d7793a4ab62678a5ddef16 Mon Sep 17 00:00:00 2001 From: vbtcl Date: Thu, 15 Oct 2026 19:08:06 -0700 Subject: [PATCH 1/2] Memoize top-level constant function calls A constant function called from a parameter initializer is evaluated once per instance of the enclosing module, even when every instance passes the same arguments. Designs that compute large tables in package functions (e.g. Hsiao ECC matrices) spend most of their elaboration time redoing identical evaluations. Top-level calls are now memoized on the compilation, keyed on the subroutine, the evaluation and AST flags, and the exact argument values. A result is only recorded when the evaluation took enough steps to be worth it, produced no diagnostics, and did not depend on its call site (a non-package parameter checked against the call's lookup location). Calls made from within a package are never memoized. On a hit the original step count is replayed, and a call that would exceed the step limit is evaluated for real so that it fails exactly as before. Co-Authored-By: Claude Opus 5.5 --- include/slang/ast/Compilation.h | 39 ++++++ include/slang/ast/EvalContext.h | 27 ++++ .../slang/ast/expressions/CallExpression.h | 4 + source/ast/Compilation.cpp | 89 +++++++++++++ source/ast/EvalContext.cpp | 4 + source/ast/expressions/CallExpression.cpp | 72 ++++++++++ source/ast/expressions/MiscExpressions.cpp | 9 +- tests/unittests/ast/ParameterTests.cpp | 123 ++++++++++++++++++ 8 files changed, 366 insertions(+), 1 deletion(-) diff --git a/include/slang/ast/Compilation.h b/include/slang/ast/Compilation.h index 620f3b14ae..862acbdd54 100644 --- a/include/slang/ast/Compilation.h +++ b/include/slang/ast/Compilation.h @@ -7,7 +7,9 @@ //------------------------------------------------------------------------------ #pragma once +#include #include +#include #include "slang/ast/ASTDiagMap.h" #include "slang/ast/InstanceCacheKey.h" @@ -749,6 +751,29 @@ class SLANG_EXPORT Compilation : public BumpAllocator { return constantAllocator.emplace(std::move(value)); } + /// @} + /// @name Constant function call memoization + /// @{ + + /// Looks up the memoized result of an earlier top-level call of the constant + /// function @a subroutine with identical @a args and evaluation flags. + /// On a hit, copies the result to @a result, sets @a steps to the number of + /// evaluation steps the original call took, and returns true. + bool findConstCallResult(const SubroutineSymbol& subroutine, uint8_t evalFlags, + uint64_t astFlags, std::span args, + size_t argsHash, ConstantValue& result, uint32_t& steps) const; + + /// Records the result of a top-level call of the constant function @a subroutine, + /// for later lookup via @a findConstCallResult. The caller guarantees that the + /// evaluation produced no diagnostics and did not depend on its call site. + void addConstCallResult(const SubroutineSymbol& subroutine, uint8_t evalFlags, + uint64_t astFlags, std::span args, size_t argsHash, + const ConstantValue& result, uint32_t steps); + + /// @} + /// @name Allocation functions + /// @{ + /// Allocates a symbol map. SymbolMap* allocSymbolMap() { SLANG_ASSERT(!isFrozen()); @@ -896,6 +921,20 @@ class SLANG_EXPORT Compilation : public BumpAllocator { flat_hash_map, std::shared_ptr> methodMap; + // Memoized results of top-level constant function calls, keyed by a hash of + // the subroutine, the evaluation flags and the argument values. + struct ConstCallEntry { + const SubroutineSymbol* subroutine; + uint8_t evalFlags; + uint64_t astFlags; + std::vector args; + ConstantValue result; + uint32_t steps; + }; + mutable std::mutex constCallMutex; + std::deque constCallEntries; + flat_hash_map> constCallMap; + // Map from instance bodies to side effects applied by them. flat_hash_map> instanceSideEffectMap; diff --git a/include/slang/ast/EvalContext.h b/include/slang/ast/EvalContext.h index 9f8394f8a4..9901a2dda4 100644 --- a/include/slang/ast/EvalContext.h +++ b/include/slang/ast/EvalContext.h @@ -159,6 +159,31 @@ class SLANG_EXPORT EvalContext { /// Returns nullptr if there is no queue target active. const ConstantValue* getQueueTarget() const { return queueTarget; } + /// Gets the number of steps (statements) executed so far. + uint32_t getSteps() const { return steps; } + + /// Accounts for @a count steps executed elsewhere on behalf of this context, + /// e.g. by a memoized constant function call. The caller must have checked + /// that the step limit is not reached. + void addSteps(uint32_t count) { steps += count; } + + /// Gets the depth of the call stack. + size_t getStackDepth() const { return stack.size(); } + + /// Gets the total number of diagnostics ever recorded by this context. + /// Unlike the stored diagnostics, this count never decreases. + uint32_t getDiagCount() const { return diagCount; } + + /// Returns true if there are any lvalues on the lvalue stack. + bool hasLValues() const { return !lvalStack.empty(); } + + /// Records that the evaluation of the outermost function call on the stack + /// depends on the lookup location of its call site. + void noteCallSiteDependence() { callSiteDependent = true; } + + /// Gets and clears the call site dependence flag (see @a noteCallSiteDependence). + bool takeCallSiteDependence() { return std::exchange(callSiteDependent, false); } + /// Dumps the contents of the call stack to a string for debugging. std::string dumpStack() const; @@ -191,7 +216,9 @@ class SLANG_EXPORT EvalContext { Diagnostics diags; Diagnostics warnings; SourceRange disableRange; + uint32_t diagCount = 0; bool backtraceReported = false; + bool callSiteDependent = false; }; } // namespace slang::ast diff --git a/include/slang/ast/expressions/CallExpression.h b/include/slang/ast/expressions/CallExpression.h index 643c6a91df..3caafa47cc 100644 --- a/include/slang/ast/expressions/CallExpression.h +++ b/include/slang/ast/expressions/CallExpression.h @@ -176,6 +176,10 @@ class SLANG_EXPORT CallExpression final : public Expression { static bool checkConstant(EvalContext& context, const SubroutineSymbol& subroutine, SourceRange range); + bool isMemoizableCall(const EvalContext& context, const SubroutineSymbol& symbol) const; + ConstantValue evalCall(EvalContext& context, const SubroutineSymbol& symbol, + std::span args) const; + const Expression* thisClass_; std::span arguments_; LookupLocation lookupLocation; diff --git a/source/ast/Compilation.cpp b/source/ast/Compilation.cpp index 61c5080da2..cf49ce65eb 100644 --- a/source/ast/Compilation.cpp +++ b/source/ast/Compilation.cpp @@ -9,6 +9,7 @@ #include "ElabVisitors.h" #include "builtins/Builtins.h" +#include #include #include @@ -1693,6 +1694,94 @@ Diagnostic& Compilation::addDiag(Diagnostic diag) { return result; } +// Value identity for memoization keys: unlike operator==, integers must also +// agree on width and signedness, and reals on their exact bit patterns. +static bool identicalValues(const ConstantValue& lhs, const ConstantValue& rhs) { + auto& lv = lhs.getVariant(); + auto& rv = rhs.getVariant(); + if (lv.index() != rv.index()) + return false; + + auto identicalRange = [](auto&& l, auto&& r) { + return std::ranges::equal(l, r, [](const ConstantValue& a, const ConstantValue& b) { + return identicalValues(a, b); + }); + }; + + if (lhs.isInteger()) { + auto& l = lhs.integer(); + auto& r = rhs.integer(); + return l.getBitWidth() == r.getBitWidth() && l.isSigned() == r.isSigned() && + l.hasUnknown() == r.hasUnknown() && exactlyEqual(l, r); + } + if (lhs.isReal()) + return std::bit_cast(double(lhs.real())) == + std::bit_cast(double(rhs.real())); + if (lhs.isShortReal()) + return std::bit_cast(float(lhs.shortReal())) == + std::bit_cast(float(rhs.shortReal())); + if (lhs.isUnpacked()) + return identicalRange(lhs.elements(), rhs.elements()); + if (lhs.isString()) + return lhs.str() == rhs.str(); + if (lhs.isMap()) { + auto& l = *lhs.map(); + auto& r = *rhs.map(); + return l.size() == r.size() && identicalValues(l.defaultValue, r.defaultValue) && + std::ranges::equal(l, r, [](auto& a, auto& b) { + return identicalValues(a.first, b.first) && identicalValues(a.second, b.second); + }); + } + if (lhs.isQueue()) { + auto& l = *lhs.queue(); + auto& r = *rhs.queue(); + return l.maxBound == r.maxBound && identicalRange(l, r); + } + if (lhs.isUnion()) { + auto& l = *lhs.unionVal(); + auto& r = *rhs.unionVal(); + return l.activeMember == r.activeMember && identicalValues(l.value, r.value); + } + + // Bad, null and unbounded values carry no payload. + return true; +} + +bool Compilation::findConstCallResult(const SubroutineSymbol& subroutine, uint8_t evalFlags, + uint64_t astFlags, std::span args, + size_t argsHash, ConstantValue& result, + uint32_t& steps) const { + std::unique_lock lock(constCallMutex); + auto it = constCallMap.find(argsHash); + if (it == constCallMap.end()) + return false; + + for (auto entry : it->second) { + if (entry->subroutine == &subroutine && entry->evalFlags == evalFlags && + entry->astFlags == astFlags && std::ranges::equal(entry->args, args, identicalValues)) { + result = entry->result; + steps = entry->steps; + return true; + } + } + return false; +} + +void Compilation::addConstCallResult(const SubroutineSymbol& subroutine, uint8_t evalFlags, + uint64_t astFlags, std::span args, + size_t argsHash, const ConstantValue& result, uint32_t steps) { + // Bound the memory spent on the memo; the limit only costs speed, never results. + static constexpr size_t MaxEntries = 1 << 16; + + std::unique_lock lock(constCallMutex); + if (constCallEntries.size() >= MaxEntries) + return; + + auto& entry = constCallEntries.emplace_back(ConstCallEntry{ + &subroutine, evalFlags, astFlags, {args.begin(), args.end()}, result, steps}); + constCallMap[argsHash].push_back(&entry); +} + AssertionInstanceDetails* Compilation::allocAssertionDetails() { SLANG_ASSERT(!isFrozen()); return assertionDetailsAllocator.emplace(); diff --git a/source/ast/EvalContext.cpp b/source/ast/EvalContext.cpp index cb51a1276f..00226bf1c8 100644 --- a/source/ast/EvalContext.cpp +++ b/source/ast/EvalContext.cpp @@ -26,7 +26,9 @@ void EvalContext::reset() { diags.clear(); warnings.clear(); disableRange = {}; + diagCount = 0; backtraceReported = false; + callSiteDependent = false; } ConstantValue* EvalContext::createLocal(const ValueSymbol* symbol, ConstantValue value) { @@ -133,6 +135,7 @@ Diagnostics EvalContext::getAllDiagnostics() const { } Diagnostic& EvalContext::addDiag(DiagCode code, SourceLocation location) { + diagCount++; const bool isError = getDefaultSeverity(code) >= DiagnosticSeverity::Error; auto& diag = isError ? diags.add(code, location) : warnings.add(code, location); reportStack(diag); @@ -140,6 +143,7 @@ Diagnostic& EvalContext::addDiag(DiagCode code, SourceLocation location) { } Diagnostic& EvalContext::addDiag(DiagCode code, SourceRange range) { + diagCount++; const bool isError = getDefaultSeverity(code) >= DiagnosticSeverity::Error; auto& diag = isError ? diags.add(code, range) : warnings.add(code, range); reportStack(diag); diff --git a/source/ast/expressions/CallExpression.cpp b/source/ast/expressions/CallExpression.cpp index 18ed6845d8..574c8590b9 100644 --- a/source/ast/expressions/CallExpression.cpp +++ b/source/ast/expressions/CallExpression.cpp @@ -694,6 +694,78 @@ ConstantValue CallExpression::evalImpl(EvalContext& context) const { args.emplace_back(std::move(v)); } + // A top-level call (e.g. from a parameter initializer) of a function that is + // repeatedly called with the same arguments -- typically once per instance of + // the same module -- is evaluated only once; see isMemoizableCall. + if (isMemoizableCall(context, symbol)) { + auto& comp = context.getCompilation(); + const uint8_t evalFlags = (context.flags & ~EvalFlags::CacheResults).bits(); + const uint64_t astFlags = context.astCtx.flags.bits(); + size_t argsHash = size_t(&symbol); + hash_combine(argsHash, evalFlags, astFlags); + for (auto& arg : args) + hash_combine(argsHash, arg.hash()); + + ConstantValue memoResult; + uint32_t memoSteps = 0; + if (comp.findConstCallResult(symbol, evalFlags, astFlags, args, argsHash, memoResult, + memoSteps)) { + // Replaying the step count keeps the step limit exact. If the call + // would hit the limit, evaluate it for real so that it fails the same way. + if (uint64_t(context.getSteps()) + memoSteps < comp.getOptions().maxConstexprSteps) { + context.addSteps(memoSteps); + return memoResult; + } + return evalCall(context, symbol, args); + } + + const uint32_t stepsBefore = context.getSteps(); + const uint32_t diagsBefore = context.getDiagCount(); + context.takeCallSiteDependence(); + + ConstantValue result = evalCall(context, symbol, args); + + // Only remember calls that are expensive enough to be worth it and whose + // evaluation is fully determined by the subroutine and argument values: + // no diagnostics (those must be issued at every call site) and no + // dependence on the call site's lookup location. + static constexpr uint32_t MinMemoSteps = 64; + const uint32_t steps = context.getSteps() - stepsBefore; + if (result && steps >= MinMemoSteps && context.getDiagCount() == diagsBefore && + !context.takeCallSiteDependence()) { + comp.addConstCallResult(symbol, evalFlags, astFlags, args, argsHash, result, steps); + } + return result; + } + + return evalCall(context, symbol, args); +} + +bool CallExpression::isMemoizableCall(const EvalContext& context, + const SubroutineSymbol& symbol) const { + // Only calls made outside of any function body, with no evaluation state + // that the callee could observe. + if (context.inFunction() || context.getQueueTarget() || context.hasLValues()) + return false; + + // Class methods can read static class properties; tasks and subroutines with + // output arguments have effects beyond their return value. + if (symbol.subroutineKind != SubroutineKind::Function || symbol.thisVar || + symbol.hasOutputArgs() || + symbol.getParentScope()->asSymbol().kind == SymbolKind::ClassType) { + return false; + } + + // A parameter referenced by the function must be declared before its call + // site (see NamedValueExpression::checkConstant). That holds at every call + // site for package parameters, unless the call is made from within a package; + // any other parameter reference marks the evaluation as call site dependent. + auto scope = lookupLocation.getScope(); + return scope && scope->asSymbol().kind != SymbolKind::Package; +} + +ConstantValue CallExpression::evalCall(EvalContext& context, const SubroutineSymbol& symbol, + std::span args) const { // Push a new stack frame, push argument values as locals. if (!context.pushFrame(symbol, sourceRange.start(), lookupLocation)) return nullptr; diff --git a/source/ast/expressions/MiscExpressions.cpp b/source/ast/expressions/MiscExpressions.cpp index 6a39bae363..82cf937435 100644 --- a/source/ast/expressions/MiscExpressions.cpp +++ b/source/ast/expressions/MiscExpressions.cpp @@ -440,9 +440,16 @@ bool NamedValueExpression::checkConstant(EvalContext& context) const { // of the constant function. If the two locations are not in the same compilation // unit, assume that it's ok. Also if the reference is via a package import // that's fine too. + auto scope = symbol.getParentScope(); + if (context.getStackDepth() == 1 && + (!scope || scope->asSymbol().kind != SymbolKind::Package)) { + // The outcome depends on where the outermost call was made, so its + // result can't be reused at other call sites (see CallExpression::evalImpl). + context.noteCallSiteDependence(); + } + auto compare = symbol.isDeclaredBefore(frame.lookupLocation); if (!compare.value_or(true)) { - auto scope = symbol.getParentScope(); if (!scope || scope->asSymbol().kind != SymbolKind::Package || scope == frame.lookupLocation.getScope()) { diff --git a/tests/unittests/ast/ParameterTests.cpp b/tests/unittests/ast/ParameterTests.cpp index add58a1d19..8185d4b860 100644 --- a/tests/unittests/ast/ParameterTests.cpp +++ b/tests/unittests/ast/ParameterTests.cpp @@ -3,6 +3,7 @@ #include "Test.h" #include +#include #include "slang/ast/symbols/CompilationUnitSymbols.h" #include "slang/ast/symbols/InstanceSymbols.h" @@ -1572,3 +1573,125 @@ endmodule REQUIRE(diags.size() == 1); CHECK(diags[0].code == diag::InfoTask); } + +TEST_CASE("Memoized constant function calls across instances") { + auto tree = SyntaxTree::fromText(R"( +package p; + localparam int Scale = 3; + function automatic int sum(int n); + int s = 0; + for (int i = 0; i < n; i++) s += i * Scale; + return s; + endfunction + function automatic real inv(real x); + return 1.0 / x; + endfunction +endpackage + +module leaf #(parameter int N = 1, parameter real X = 1.0); + localparam int S = p::sum(N); + localparam int T = p::sum(N) + p::sum(N); + localparam real I = p::inv(X); +endmodule + +module top; + leaf #(.N(100), .X(0.0)) a(); + leaf #(.N(100), .X(-0.0)) b(); + leaf #(.N(200)) c(); + for (genvar g = 0; g < 4; g++) begin : gen + leaf #(.N(100)) d(); + end +endmodule +)"); + + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + + auto param = [&](std::string_view path) -> const ConstantValue& { + return compilation.getRoot().lookupName(path)->as().getValue(); + }; + CHECK(param("top.a.S").integer() == 14850); + CHECK(param("top.b.S").integer() == 14850); + CHECK(param("top.c.S").integer() == 59700); + CHECK(param("top.gen[3].d.S").integer() == 14850); + CHECK(param("top.a.T").integer() == 29700); + CHECK(param("top.c.T").integer() == 119400); + + // +0.0 and -0.0 compare equal but are different arguments. + CHECK(double(param("top.a.I").real()) == std::numeric_limits::infinity()); + CHECK(double(param("top.b.I").real()) == -std::numeric_limits::infinity()); +} + +TEST_CASE("Memoized constant function calls keep the step limit") { + auto tree = SyntaxTree::fromText(R"( +package p; + function automatic int spin(int n); + int s = 0; + for (int i = 0; i < n; i++) s++; + return s; + endfunction +endpackage + +module leaf; + localparam int A = p::spin(2000); + localparam int B = p::spin(2000) + p::spin(2000) + p::spin(2000) + p::spin(2000) + + p::spin(2000); +endmodule + +module top; + leaf l1(); + leaf l2(); +endmodule +)"); + + CompilationOptions co; + co.maxConstexprSteps = 8192; + + Bag options; + options.set(co); + + Compilation compilation(options); + compilation.addSyntaxTree(tree); + + // One call fits in the step budget, five in one expression don't, whether + // or not the calls after the first one are served from the memo. + auto& diags = compilation.getAllDiagnostics(); + REQUIRE(diags.size() == 1); + CHECK(diags[0].code == diag::ConstEvalExceededMaxSteps); + + auto& l2a = compilation.getRoot().lookupName("top.l2.A")->as(); + CHECK(l2a.getValue().integer() == 2000); +} + +TEST_CASE("Memoized constant function calls keep call site diagnostics") { + auto tree = SyntaxTree::fromText(R"( +module m; + localparam int A = f(10); + localparam int B = 5; + localparam int C = f(10); + + function automatic int f(int n); + int s = 0; + for (int i = 0; i < n * 10; i++) s += B; + return s; + endfunction +endmodule + +module top; + m m1(); + m m2(); +endmodule +)"); + + Compilation compilation; + compilation.addSyntaxTree(tree); + + // C's call is fine and A's call isn't, whichever is evaluated first. + auto& diags = compilation.getAllDiagnostics(); + REQUIRE(diags.size() == 1); + CHECK(diags[0].code == diag::ConstEvalIdUsedInCEBeforeDecl); + + auto& c = compilation.getRoot().lookupName("top.m2.C")->as(); + CHECK(c.getValue().integer() == 500); +} From 6b20f348c1f49687f9c179005db1653ae0869f07 Mon Sep 17 00:00:00 2001 From: vbtcl Date: Fri, 16 Oct 2026 00:18:16 -0700 Subject: [PATCH 2/2] Spell out unique_lock's mutex type; test memoized calls with aggregate arguments Co-Authored-By: Claude Opus 5.5 --- source/ast/Compilation.cpp | 4 +-- tests/unittests/ast/ParameterTests.cpp | 35 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/source/ast/Compilation.cpp b/source/ast/Compilation.cpp index cf49ce65eb..26512fde52 100644 --- a/source/ast/Compilation.cpp +++ b/source/ast/Compilation.cpp @@ -1751,7 +1751,7 @@ bool Compilation::findConstCallResult(const SubroutineSymbol& subroutine, uint8_ uint64_t astFlags, std::span args, size_t argsHash, ConstantValue& result, uint32_t& steps) const { - std::unique_lock lock(constCallMutex); + std::unique_lock lock(constCallMutex); auto it = constCallMap.find(argsHash); if (it == constCallMap.end()) return false; @@ -1773,7 +1773,7 @@ void Compilation::addConstCallResult(const SubroutineSymbol& subroutine, uint8_t // Bound the memory spent on the memo; the limit only costs speed, never results. static constexpr size_t MaxEntries = 1 << 16; - std::unique_lock lock(constCallMutex); + std::unique_lock lock(constCallMutex); if (constCallEntries.size() >= MaxEntries) return; diff --git a/tests/unittests/ast/ParameterTests.cpp b/tests/unittests/ast/ParameterTests.cpp index 8185d4b860..21cd5f6d46 100644 --- a/tests/unittests/ast/ParameterTests.cpp +++ b/tests/unittests/ast/ParameterTests.cpp @@ -1695,3 +1695,38 @@ endmodule auto& c = compilation.getRoot().lookupName("top.m2.C")->as(); CHECK(c.getValue().integer() == 500); } + +TEST_CASE("Memoized constant function calls with aggregate arguments") { + auto tree = SyntaxTree::fromText(R"( +package p; + function automatic int weigh(int a[3], string s, int q[$], int m[string], shortreal r); + int w = 0; + for (int i = 0; i < 40; i++) w += a[i % 3] + s.len() + q.size() + m.num() + int'(r); + return w; + endfunction +endpackage + +module leaf #(parameter int K = 0); + localparam int A[3] = '{K, 2, 3}; + localparam int Q[$] = '{1, 2}; + localparam int M[string] = '{"a": 1, "b": K}; + localparam int W = p::weigh(A, "xy", Q, M, 1.5); +endmodule + +module top; + leaf #(1) l1a(); + leaf #(1) l1b(); + leaf #(2) l2(); +endmodule +)"); + + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + + auto w = [&](std::string_view path) { + return compilation.getRoot().lookupName(path)->as().getValue().integer(); + }; + CHECK(w("top.l1a.W") == w("top.l1b.W")); + CHECK(w("top.l1a.W") != w("top.l2.W")); +}