Add compat flag to allow multi-driven subroutine local variables

MikePopoloski · Mar 8, 2024 · bfad52e · bfad52e
1 parent 97a57fb
commit bfad52e
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 9 deletions.
diff --git 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_<CompilationOptions>(m, "CompilationOptions")
         .def(py::init<>())

diff --git 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
@@ -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
@@ -879,13 +879,26 @@ class DriverVisitor : public ASTVisitor<DriverVisitor, true, true> {
     }
 
     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<std::pair<DriverBitRange, const ValueDriver*>> 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<DriverVisitor, true, true> {
         // 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
@@ -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
@@ -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;
+}