Repository: intel/rohd (dart)

Issue:
Allow multiple nonblocking assignments
### Motivation

As far as I understood, ROHD is capable of handling multiple consecutive nonblocking assignments to one variable (tried to comment out [this line](https://github.com/intel/rohd/blob/main/lib/src/modules/conditional.dart#L421) and everything worked). I found mentions of this (https://github.com/intel/rohd/issues/114) and cannot agree with the phrase

> It doesn't make sense for a signal to be driven by two drivers in a Sequential block.

Here is an example where I needed this feature:

```dart
Sequential(intf.clock, [
  If(~intf.block, then: [
    intf.branch < 0,
    trigger < 0,
    If(trigger, then: [
      IfBlock([
        Iff.s(
          action.eq(CFUInterface.actionAddressLowByte),
          intf.addressLowByte < intf.rfcuData,
        ),
        ElseIf.s(
          action.eq(CFUInterface.actionAddressHighByte),
          intf.addressHighByte < intf.rfcuData,
        ),
        ElseIf.s(
          action.eq(CFUInterface.actionValue),
          value < intf.rfcuData[0],
        ),
        Else.s(
            If(intf.rfcuData[0].eq(CFUInterface.operationSnapshot), then: [
          intf.addressLowByte < instructionAddress.slice(7, 0),
          intf.addressHighByte < instructionAddress.slice(15, 8),
        ], orElse: [
          If.s(value.eq(0), intf.branch < 1)
        ]))
      ]),
    ]),
    If(intf.enable, then: [
      action < intf.action,
      instructionAddress < intf.ifuInstructionAddress,
      trigger < 1,
    ]),
  ]),
]);
```
In this example, the module saves the control signals and processes them on the next cycle, when the data on the `intf.rfcuData` bus is ready. The `intf.branch` and `trigger` flags are set to zero by default, but this can be overridden depending on conditions. It seems to me that this way of using multiple consecutive non-blocking assignments simplifies the scheme.

---

Some links:

* http://www.sunburst-design.com/papers/CummingsSNUG2000SJ_NBA.pdf (page 21)
* https://stackoverflow.com/questions/15718192/verilog-sequence-of-non-blocking-assignments (examples of use are given and the peculiarity of using such a signal as a clock signal in modeling is indicated)



### Desired solution

It is proposed to add functionality for configuring `Simulator`, for example:

```dart
enum SimulatorConfigRestrictionLevel {disallow, notify, allow}

class Simulator {
  // ...

  static void config({SimulatorConfigRestrictionLevel multipleNonblockingAssignments = SimulatorConfigRestrictionLevel.disallow}) {
    // ...
  }

  // ...
}
```

Prefer default restrictive behavior.

### Alternatives considered

_No response_

### Additional details

I can do the implementation if the idea is accepted.

Pull request:
<!-- Please make sure you check out the contribution guidelines before submitting a pull request! -->

## Description & Motivation

Allow multiple non-blocking assignments in `Sequential`.  See #321.

Added a `allowMultipleAssignments` flag to `Sequential`, defaulting to `true`.

## Related Issue(s)

Fix #321 

## Testing

Updated existing tests for exception, and added new test allowing multiple assignments.

## Backwards-compatibility

> Is this a breaking change that will not be backwards-compatible? If yes, how so?

No, but default is now *less* strict

## Documentation

> Does the change require any updates to documentation? If so, where? Are they included?

API docs updated

Patch:
diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart
index d2857dbc9..616dc7489 100644
--- a/lib/src/modules/conditional.dart
+++ b/lib/src/modules/conditional.dart
@@ -378,6 +378,11 @@ class Sequential extends _Always {
   /// The input clocks used in this block.
   final List<Logic> _clks = [];
 
+  /// When `false`, an [SignalRedrivenException] will be thrown during
+  /// simulation if the same signal is driven multiple times within this
+  /// [Sequential].
+  final bool allowMultipleAssignments;
+
   /// Constructs a [Sequential] single-triggered by [clk].
   ///
   /// If `reset` is provided, then all signals driven by this block will be
@@ -387,12 +392,21 @@ class Sequential extends _Always {
   /// that value instead upon reset. If a signal is in `resetValues` but not
   /// driven by any other [Conditional] in this block, it will be driven to the
   /// specified reset value.
-  Sequential(Logic clk, List<Conditional> conditionals,
-      {Logic? reset,
-      Map<Logic, dynamic>? resetValues,
-      String name = 'sequential'})
-      : this.multi([clk], conditionals,
-            name: name, reset: reset, resetValues: resetValues);
+  Sequential(
+    Logic clk,
+    List<Conditional> conditionals, {
+    Logic? reset,
+    Map<Logic, dynamic>? resetValues,
+    bool allowMultipleAssignments = true,
+    String name = 'sequential',
+  }) : this.multi(
+          [clk],
+          conditionals,
+          name: name,
+          reset: reset,
+          resetValues: resetValues,
+          allowMultipleAssignments: allowMultipleAssignments,
+        );
 
   /// Constructs a [Sequential] multi-triggered by any of [clks].
   ///
@@ -403,8 +417,14 @@ class Sequential extends _Always {
   /// that value instead upon reset. If a signal is in `resetValues` but not
   /// driven by any other [Conditional] in this block, it will be driven to the
   /// specified reset value.
-  Sequential.multi(List<Logic> clks, super._conditionals,
-      {super.reset, super.resetValues, super.name = 'sequential'}) {
+  Sequential.multi(
+    List<Logic> clks,
+    super._conditionals, {
+    super.reset,
+    super.resetValues,
+    super.name = 'sequential',
+    this.allowMultipleAssignments = true,
+  }) {
     if (clks.isEmpty) {
       throw IllegalConfigurationException('Must provide at least one clock.');
     }
@@ -547,12 +567,18 @@ class Sequential extends _Always {
         receiverOutput.put(LogicValue.x);
       }
     } else if (anyClkPosedge) {
-      final allDrivenSignals = DuplicateDetectionSet<Logic>();
-      for (final element in conditionals) {
-        element.execute(allDrivenSignals, null);
-      }
-      if (allDrivenSignals.hasDuplicates) {
-        throw SignalRedrivenException(allDrivenSignals.duplicates);
+      if (allowMultipleAssignments) {
+        for (final element in conditionals) {
+          element.execute(null, null);
+        }
+      } else {
+        final allDrivenSignals = DuplicateDetectionSet<Logic>();
+        for (final element in conditionals) {
+          element.execute(allDrivenSignals, null);
+        }
+        if (allDrivenSignals.hasDuplicates) {
+          throw SignalRedrivenException(allDrivenSignals.duplicates);
+        }
       }
     }
 
@@ -642,7 +668,7 @@ abstract class Conditional {
   /// which consumes the current value of those drivers.  It is used to check
   /// that signals are not "written after read", for example.
   @protected
-  void execute(Set<Logic> drivenSignals, void Function(Logic toGuard)? guard);
+  void execute(Set<Logic>? drivenSignals, void Function(Logic toGuard)? guard);
 
   /// Lists *all* receivers, recursively including all sub-[Conditional]s
   /// receivers.
@@ -752,10 +778,11 @@ abstract class Conditional {
       {required int context});
 
   /// Drives X to all receivers.
-  void _driveX(Set<Logic> drivenSignals) {
+  void _driveX(Set<Logic>? drivenSignals) {
     for (final receiver in receivers) {
       receiverOutput(receiver).put(LogicValue.x);
-      if (!drivenSignals.contains(receiver) || receiver.value.isValid) {
+      if (drivenSignals != null &&
+          (!drivenSignals.contains(receiver) || receiver.value.isValid)) {
         drivenSignals.add(receiver);
       }
     }
@@ -794,7 +821,7 @@ class ConditionalGroup extends Conditional {
       [for (final conditional in conditionals) ...conditional.receivers];
 
   @override
-  void execute(Set<Logic> drivenSignals, void Function(Logic toGuard)? guard) {
+  void execute(Set<Logic>? drivenSignals, void Function(Logic toGuard)? guard) {
     for (final conditional in conditionals) {
       conditional.execute(drivenSignals, guard);
     }
@@ -850,7 +877,7 @@ class ConditionalAssign extends Conditional {
   late final _receiverOutput = receiverOutput(receiver);
 
   @override
-  void execute(Set<Logic> drivenSignals,
+  void execute(Set<Logic>? drivenSignals,
       [void Function(Logic toGuard)? guard]) {
     if (guard != null) {
       guard(driver);
@@ -863,7 +890,8 @@ class ConditionalAssign extends Conditional {
       _receiverOutput.put(currentValue);
     }
 
-    if (!drivenSignals.contains(receiver) || receiver.value.isValid) {
+    if (drivenSignals != null &&
+        (!drivenSignals.contains(receiver) || receiver.value.isValid)) {
       drivenSignals.add(receiver);
     }
   }
@@ -1053,7 +1081,7 @@ class Case extends Conditional {
   String get caseType => 'case';
 
   @override
-  void execute(Set<Logic> drivenSignals, [void Function(Logic)? guard]) {
+  void execute(Set<Logic>? drivenSignals, [void Function(Logic)? guard]) {
     if (guard != null) {
       guard(expression);
       for (final item in items) {
@@ -1397,7 +1425,7 @@ class If extends Conditional {
   }
 
   @override
-  void execute(Set<Logic> drivenSignals, [void Function(Logic)? guard]) {
+  void execute(Set<Logic>? drivenSignals, [void Function(Logic)? guard]) {
     if (guard != null) {
       for (final iff in iffs) {
         guard(iff.condition);


Test patch:
diff --git a/test/conditionals_test.dart b/test/conditionals_test.dart
index 01905c1c5..81f02c987 100644
--- a/test/conditionals_test.dart
+++ b/test/conditionals_test.dart
@@ -391,7 +391,8 @@ class SingleElseModule extends Module {
 }
 
 class SignalRedrivenSequentialModule extends Module {
-  SignalRedrivenSequentialModule(Logic a, Logic b, Logic d)
+  SignalRedrivenSequentialModule(Logic a, Logic b, Logic d,
+      {required bool allowRedrive})
       : super(name: 'ffmodule') {
     a = addInput('a', a);
     b = addInput('b', b);
@@ -400,13 +401,17 @@ class SignalRedrivenSequentialModule extends Module {
     d = addInput('d', d, width: d.width);
 
     final k = addOutput('k', width: 8);
-    Sequential(SimpleClockGenerator(10).clk, [
-      If(a, then: [
-        k < k,
-        q < k,
-        q < d,
-      ])
-    ]);
+    Sequential(
+      SimpleClockGenerator(10).clk,
+      [
+        If(a, then: [
+          k < k,
+          q < k,
+          q < d,
+        ])
+      ],
+      allowMultipleAssignments: allowRedrive,
+    );
   }
 }
 
@@ -419,10 +424,14 @@ class SignalRedrivenSequentialModuleWithX extends Module {
 
     final b = addOutput('b');
 
-    Sequential(SimpleClockGenerator(10).clk, [
-      If(a, then: [b < c]),
-      If(d, then: [b < c])
-    ]);
+    Sequential(
+      SimpleClockGenerator(10).clk,
+      [
+        If(a, then: [b < c]),
+        If(d, then: [b < c])
+      ],
+      allowMultipleAssignments: false,
+    );
   }
 }
 
@@ -712,9 +721,10 @@ void main() {
 
   test(
       'should return SignalRedrivenException when there are multiple drivers '
-      'for a flop.', () async {
-    final mod =
-        SignalRedrivenSequentialModule(Logic(), Logic(), Logic(width: 8));
+      'for a flop when redrive not allowed.', () async {
+    final mod = SignalRedrivenSequentialModule(
+        Logic(), Logic(), Logic(width: 8),
+        allowRedrive: false);
     await mod.build();
     final vectors = [
       Vector({'a': 1, 'd': 1}, {}),
@@ -729,6 +739,21 @@ void main() {
     }
   });
 
+  test('should allow redrive when allowed', () async {
+    final mod = SignalRedrivenSequentialModule(
+        Logic(), Logic(), Logic(width: 8),
+        allowRedrive: true);
+    await mod.build();
+    final vectors = [
+      Vector({'a': 1, 'd': 1}, {}),
+      Vector({'a': 1, 'b': 0, 'd': 2}, {'q': 1}),
+      Vector({'a': 1, 'b': 0, 'd': 3}, {'q': 2}),
+    ];
+
+    await SimCompare.checkFunctionalVector(mod, vectors);
+    SimCompare.checkIverilogVector(mod, vectors);
+  });
+
   test(
       'should return NonSupportedTypeException when '
       'simcompare expected output values has invalid runtime type. ', () async {
@@ -749,7 +774,7 @@ void main() {
 
   test(
       'should return SignalRedrivenException when driven with '
-      'x signals and valid signals.', () async {
+      'x signals and valid signals when redrive not allowed.', () async {
     final mod = SignalRedrivenSequentialModuleWithX(Logic(), Logic(), Logic());
     await mod.build();
     final vectors = [