From bfad52e44aae1cdb60e5d7b2b437dcc80ab1f9a5 Mon Sep 17 00:00:00 2001 From: MikePopoloski Date: Thu, 7 Mar 2024 20:29:08 -0500 Subject: [PATCH] Add compat flag to allow multi-driven subroutine local variables --- bindings/python/CompBindings.cpp | 4 ++-- docs/command-line-ref.dox | 6 +++++ include/slang/ast/Compilation.h | 7 ++++-- source/ast/expressions/CallExpression.cpp | 21 +++++++++++++---- source/driver/Driver.cpp | 6 ++++- tests/unittests/ast/ExpressionTests.cpp | 28 +++++++++++++++++++++++ 6 files changed, 63 insertions(+), 9 deletions(-) diff --git a/bindings/python/CompBindings.cpp b/bindings/python/CompBindings.cpp index 9ae2d9900..f07d8059b 100644 --- a/bindings/python/CompBindings.cpp +++ b/bindings/python/CompBindings.cpp @@ -54,8 +54,8 @@ void registerCompilation(py::module_& m) { .value("RelaxStringConversions", CompilationFlags::RelaxStringConversions) .value("AllowRecursiveImplicitCall", CompilationFlags::AllowRecursiveImplicitCall) .value("AllowBareValParamAssignment", CompilationFlags::AllowBareValParamAssignment) - .value("AllowSelfDeterminedStreamConcat", - CompilationFlags::AllowSelfDeterminedStreamConcat); + .value("AllowSelfDeterminedStreamConcat", CompilationFlags::AllowSelfDeterminedStreamConcat) + .value("AllowMultiDrivenLocals", CompilationFlags::AllowMultiDrivenLocals); py::class_(m, "CompilationOptions") .def(py::init<>()) diff --git a/docs/command-line-ref.dox b/docs/command-line-ref.dox index 9ec63b6ba..68aa6c775 100644 --- a/docs/command-line-ref.dox +++ b/docs/command-line-ref.dox @@ -337,6 +337,12 @@ Allow self-determined streaming concatenation expressions. SystemVerilog only pe expressions in assignment-like contexts but some tools allow it anyway so this flag can be used for compatibility purposes. +`--allow-multi-driven-locals` + +Allow subroutine local variables to be driven from multiple always_comb/always_ff blocks. +The LRM does not make an exception for local variables in assignment rules but +most tools allow it anyway so this flag exists for compatibility with those tools. + `--strict-driver-checking` Perform strict driver checking, which currently means disabling procedural 'for' @ref loop-unroll diff --git a/include/slang/ast/Compilation.h b/include/slang/ast/Compilation.h index 3a18702bf..2bff6a937 100644 --- a/include/slang/ast/Compilation.h +++ b/include/slang/ast/Compilation.h @@ -121,9 +121,12 @@ enum class SLANG_EXPORT CompilationFlags { /// Allow self-determined streaming concatenation expressions; normally these /// can only be used in specific assignment-like contexts. - AllowSelfDeterminedStreamConcat = 1 << 12 + AllowSelfDeterminedStreamConcat = 1 << 12, + + /// Allow multi-driven subroutine local variables. + AllowMultiDrivenLocals = 1 << 13 }; -SLANG_BITMASK(CompilationFlags, AllowSelfDeterminedStreamConcat) +SLANG_BITMASK(CompilationFlags, AllowMultiDrivenLocals) /// Contains various options that can control compilation behavior. struct SLANG_EXPORT CompilationOptions { diff --git a/source/ast/expressions/CallExpression.cpp b/source/ast/expressions/CallExpression.cpp index 2faec9091..4779e9001 100644 --- a/source/ast/expressions/CallExpression.cpp +++ b/source/ast/expressions/CallExpression.cpp @@ -879,13 +879,26 @@ class DriverVisitor : public ASTVisitor { } void handle(const ValueExpressionBase& expr) { - if (!visitedValues.emplace(&expr.symbol).second) + auto& sym = expr.symbol; + if (!visitedValues.emplace(&sym).second) return; + if (sub.getCompilation().hasFlag(CompilationFlags::AllowMultiDrivenLocals)) { + auto scope = sym.getParentScope(); + while (scope && scope->asSymbol().kind == SymbolKind::StatementBlock) + scope = scope->asSymbol().getParentScope(); + + if (scope == &sub) { + // This is a local variable of the subroutine, + // so don't do driver checking. + return; + } + } + // If the target symbol is driven by the subroutine we're inspecting, // add another driver for the procedure we're originally called from. SmallVector> drivers; - auto range = expr.symbol.drivers(); + auto range = sym.drivers(); for (auto it = range.begin(); it != range.end(); ++it) { if ((*it)->containingSymbol == &sub) drivers.push_back({it.bounds(), *it}); @@ -894,8 +907,8 @@ class DriverVisitor : public ASTVisitor { // This needs to be a separate loop to avoid mutating the driver map // while iterating over it. for (auto [bounds, driver] : drivers) { - expr.symbol.addDriver(DriverKind::Procedural, bounds, *driver->prefixExpression, - procedure, callExpr); + sym.addDriver(DriverKind::Procedural, bounds, *driver->prefixExpression, procedure, + callExpr); } } }; diff --git a/source/driver/Driver.cpp b/source/driver/Driver.cpp index 798e976e8..54cd3ba29 100644 --- a/source/driver/Driver.cpp +++ b/source/driver/Driver.cpp @@ -172,6 +172,9 @@ void Driver::addStandardArgs() { addCompFlag(CompilationFlags::AllowSelfDeterminedStreamConcat, "--allow-self-determined-stream-concat", "Allow self-determined streaming concatenation expressions"); + addCompFlag( + CompilationFlags::AllowMultiDrivenLocals, "--allow-multi-driven-locals", + "Allow subroutine local variables to be driven from multiple always_comb/_ff blocks"); addCompFlag(CompilationFlags::StrictDriverChecking, "--strict-driver-checking", "Perform strict driver checking, which currently means disabling " "procedural 'for' loop unrolling."); @@ -419,7 +422,8 @@ bool Driver::processOptions() { CompilationFlags::RelaxStringConversions, CompilationFlags::AllowRecursiveImplicitCall, CompilationFlags::AllowBareValParamAssignment, - CompilationFlags::AllowSelfDeterminedStreamConcat}; + CompilationFlags::AllowSelfDeterminedStreamConcat, + CompilationFlags::AllowMultiDrivenLocals}; for (auto flag : vcsCompatFlags) { auto& option = options.compilationFlags.at(flag); diff --git a/tests/unittests/ast/ExpressionTests.cpp b/tests/unittests/ast/ExpressionTests.cpp index b34ab851d..1d0712bb2 100644 --- a/tests/unittests/ast/ExpressionTests.cpp +++ b/tests/unittests/ast/ExpressionTests.cpp @@ -3219,3 +3219,31 @@ endmodule REQUIRE(diags.size() == 1); CHECK(diags[0].code == diag::MultipleAlwaysAssigns); } + +TEST_CASE("Multi-driven subroutine local var option to allow") { + auto tree = SyntaxTree::fromText(R"( +module top(input clk, input reset); + function logic m(logic d); + logic c; + c = d; + return c; + endfunction + + logic a, b; + always_ff @(posedge clk) begin + a <= m(a); + end + + always @(posedge reset) begin + b <= m(a); + end +endmodule +)"); + + CompilationOptions options; + options.flags |= CompilationFlags::AllowMultiDrivenLocals; + + Compilation compilation(options); + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; +}