Repository: intel/rohd (dart)

Issue:
Support multiple triggers on flops
**Is your feature request related to a problem? Please describe.**
Support multiple triggers for flops, like in SystemVerilog:
```SystemVerilog
always_ff @(posedge clk or posedge reset)
```

**Describe the solution you'd like**
The `FF` and `FlipFlop` should have a syntax that accepts a list of signals instead of a single `clk` signal.

**Describe alternatives you've considered**
Using something like `clk | reset` is insufficient because there could be a rising edge of reset during a clock high period.

Pull request:
## Description & Motivation

- Rename `FF` to `Sequential`, since it's a better name.
- Add `Sequential.multi` for multiple triggers on one block (like `always_ff(@posedge clk or @posedge reset)`)

## Related Issue(s)

Implements a solution for #42 

## Testing

Modified a counter test to take advantage of `Sequential.multi`

## Backwards-compatibility

> Is this a breaking change that will not be backwards-compatible?  If yes, how so?

Yes
- The `clk` has been removed from the `Sequential` public API since it probably shouldn't have been exposed in the first place.
- `FF` has been deprecated, but not removed yet, to preserve backwards compatibility.

## Documentation

> Does the change require any updates to documentation?  If so, where?  Are they included?

Yes, API docs are updated

Patch:
diff --git a/README.md b/README.md
index 721bc2ffa..cf1133552 100644
--- a/README.md
+++ b/README.md
@@ -122,8 +122,8 @@ class Counter extends Module {
     // Assignment statement of nextVal to be val+1 (<= is the assignment operator)
     nextVal <= val + 1;
 
-    // `FF` is like SystemVerilog's always_ff, in this case trigger on the positive edge of clk
-    FF(clk, [
+    // `Sequential` is like SystemVerilog's always_ff, in this case trigger on the positive edge of clk
+    Sequential(clk, [
       // `If` is a conditional if statement, like `if` in SystemVerilog always blocks
       If(reset, then:[
         // the '<' operator is a conditional assignment
@@ -361,12 +361,12 @@ class MyModule extends Module {
 ```
 
 ### Sequentials
-ROHD has a basic [`FlipFlop`](https://intel.github.io/rohd/rohd/FlipFlop-class.html) module that can be used as a flip flop.  For more complex sequential logic, use the `FF` block described in the Conditionals section.
+ROHD has a basic [`FlipFlop`](https://intel.github.io/rohd/rohd/FlipFlop-class.html) module that can be used as a flip flop.  For more complex sequential logic, use the `Sequential` block described in the Conditionals section.
 
 Dart doesn't have a notion of certain signals being "clocks" vs. "not clocks".  You can use any signal as a clock input to sequential logic, and have as many clocks of as many frequencies as you want.
 
 ### Conditionals
-ROHD supports a variety of [`Conditional`](https://intel.github.io/rohd/rohd/Conditional-class.html) type statements that always must fall within a type of `_Always` block, similar to SystemVerilog.  There are two types of `_Always` blocks: [`FF`](https://intel.github.io/rohd/rohd/FF-class.html) and [`Combinational`](https://intel.github.io/rohd/rohd/Combinational-class.html), which map to SystemVerilog's `always_ff` and `always_comb`, respectively.  `Combinational` takes a list of `Conditional` statements.  Different kinds of `Conditional` statement, such as `If`, may be composed of more `Conditional` statements.  You can create `Conditional` composition chains as deep as you like.
+ROHD supports a variety of [`Conditional`](https://intel.github.io/rohd/rohd/Conditional-class.html) type statements that always must fall within a type of `_Always` block, similar to SystemVerilog.  There are two types of `_Always` blocks: [`Sequential`](https://intel.github.io/rohd/rohd/Sequential-class.html) and [`Combinational`](https://intel.github.io/rohd/rohd/Combinational-class.html), which map to SystemVerilog's `always_ff` and `always_comb`, respectively.  `Combinational` takes a list of `Conditional` statements.  Different kinds of `Conditional` statement, such as `If`, may be composed of more `Conditional` statements.  You can create `Conditional` composition chains as deep as you like.
 
 Conditional statements are executed imperatively and in order, just like the contents of `always` blocks in SystemVerilog.  `_Always` blocks in ROHD map 1-to-1 with SystemVerilog `always` statements when converted.
 
@@ -395,7 +395,7 @@ Combinational([
 #### `IfBlock`
 The [`IfBlock`](https://intel.github.io/rohd/rohd/IfBlock-class.html) makes syntax for long chains of if / else if / else chains nicer.  For example:
 ```dart
-FF(clk, [
+Sequential(clk, [
   IfBlock([
     // the first one must be Iff (yes, with 2 f's, to differentiate from If above)
     Iff(a & ~b, [
@@ -507,7 +507,7 @@ class Counter extends Module {
     // access signals directly from the interface
     nextVal <= intf.val + 1;
 
-    FF( SimpleClockGenerator(10).clk, [
+    Sequential( SimpleClockGenerator(10).clk, [
       If(intf.reset, then:[
         intf.val < 0
       ], orElse: [If(intf.en, then: [
diff --git a/example/example.dart b/example/example.dart
index 68d73523b..032fd1b15 100644
--- a/example/example.dart
+++ b/example/example.dart
@@ -35,8 +35,8 @@ class Counter extends Module {
     // Assignment statement of nextVal to be val+1 (<= is the assignment operator)
     nextVal <= val + 1;
 
-    // `FF` is like SystemVerilog's always_ff, in this case trigger on the positive edge of clk
-    FF(clk, [
+    // `Sequential` is like SystemVerilog's always_ff, in this case trigger on the positive edge of clk
+    Sequential(clk, [
       // `If` is a conditional if statement, like `if` in SystemVerilog always blocks
       If(reset, then: [
         // the '<' operator is a conditional assignment
diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart
index f62921d34..f2a838e25 100644
--- a/lib/src/modules/conditional.dart
+++ b/lib/src/modules/conditional.dart
@@ -93,7 +93,7 @@ abstract class _Always extends Module with CustomSystemVerilog {
 ///
 /// This is similar to an `always_comb` block in SystemVerilog.
 class Combinational extends _Always {
-  Combinational(List<Conditional> conditionals, {String name = 'always_comb'})
+  Combinational(List<Conditional> conditionals, {String name = 'combinational'})
       : super(conditionals, name: name) {
     _execute(); // for initial values
     for (var element in _assignedDriverToInputMap.keys) {
@@ -135,20 +135,32 @@ class Combinational extends _Always {
   String assignOperator() => '=';
 }
 
+@Deprecated('Use Sequential instead')
+typedef FF = Sequential;
+
 /// Represents a block of sequential logic.
 ///
-/// This is similar to an `always_ff` block in SystemVerilog.
-class FF extends _Always {
-  /// The name of the clock for this block.
-  final String _clk = Module.unpreferredName('clk');
-
-  /// The clock used in this block.
-  Logic get clk => input(_clk);
-
-  FF(Logic clk, List<Conditional> conditionals, {String name = 'always_ff'})
+/// This is similar to an `always_ff` block in SystemVerilog.  Positive edge triggered by
+/// either one trigger or multiple with [Sequential.multi].
+class Sequential extends _Always {
+  /// The input clocks used in this block.
+  final List<Logic> _clks = [];
+
+  /// Constructs a [Sequential] single-triggered by [clk].
+  Sequential(Logic clk, List<Conditional> conditionals,
+      {String name = 'sequential'})
+      : this.multi([clk], conditionals, name: name);
+
+  /// Constructs a [Sequential] multi-triggered by any of [clks].
+  Sequential.multi(List<Logic> clks, List<Conditional> conditionals,
+      {String name = 'sequential'})
       : super(conditionals, name: name) {
-    if (clk.width > 1) throw Exception('clk must be 1 bit');
-    addInput(_clk, clk);
+    for (var i = 0; i < clks.length; i++) {
+      var clk = clks[i];
+      if (clk.width > 1) throw Exception('clk must be 1 bit');
+      _clks.add(addInput(Module.unpreferredName('clk$i'), clk));
+      _preTickClkValues.add(null);
+    }
     _setup();
   }
 
@@ -156,7 +168,7 @@ class FF extends _Always {
   final Map<Logic, LogicValues> _inputToPreTickInputValuesMap = {};
 
   /// The value of the clock before the tick.
-  LogicValue? _preTickClkValue;
+  final List<LogicValue?> _preTickClkValues = [];
 
   /// Keeps track of whether the clock has glitched and an [_execute] is necessary.
   bool _pendingExecute = false;
@@ -174,7 +186,7 @@ class FF extends _Always {
       element._updateOverrideMap(_inputToPreTickInputValuesMap);
     }
 
-    // listen to every input of this FF for changes
+    // listen to every input of this `Sequential` for changes
     for (var driverInput in _assignedDriverToInputMap.values) {
       driverInput.glitch.listen((event) {
         if (Simulator.phase != SimulatorPhase.clksStable) {
@@ -199,40 +211,65 @@ class FF extends _Always {
     }
 
     // listen to every clock glitch to see if we need to execute
-    clk.glitch.listen((event) {
-      // we want the first previousValue from the first glitch of this tick
-      _preTickClkValue ??= event.previousValue.bit;
-      if (!_pendingExecute) {
-        Simulator.clkStable.first.then((value) {
-          // once the clocks are stable, execute the contents of the FF
-          _execute();
-          _preTickClkValue = null;
-          _pendingExecute = false;
-        });
-      }
-      _pendingExecute = true;
-    });
+    for (var i = 0; i < _clks.length; i++) {
+      var clk = _clks[i];
+      clk.glitch.listen((event) {
+        // we want the first previousValue from the first glitch of this tick
+        _preTickClkValues[i] ??= event.previousValue.bit;
+        if (!_pendingExecute) {
+          Simulator.clkStable.first.then((value) {
+            // once the clocks are stable, execute the contents of the FF
+            _execute();
+            _pendingExecute = false;
+          });
+        }
+        _pendingExecute = true;
+      });
+    }
   }
 
   void _execute() {
-    if (!clk.bit.isValid || !_preTickClkValue!.isValid) {
+    bool anyClkInvalid = false;
+    bool anyClkPosedge = false;
+    for (var i = 0; i < _clks.length; i++) {
+      // if the pre-tick value is null, then it should have the same value as it currently does
+      if (!_clks[i].bit.isValid || !(_preTickClkValues[i]?.isValid ?? true)) {
+        anyClkInvalid = true;
+        break;
+      } else if (_preTickClkValues[i] != null &&
+          LogicValue.isPosedge(_preTickClkValues[i]!, _clks[i].bit)) {
+        anyClkPosedge = true;
+        break;
+      }
+    }
+
+    if (anyClkInvalid) {
       for (var receiverOutput in _assignedReceiverToOutputMap.values) {
         receiverOutput.put(LogicValue.x);
       }
-    } else if (LogicValue.isPosedge(_preTickClkValue!, clk.bit)) {
+    } else if (anyClkPosedge) {
       var allDrivenSignals = <Logic>[];
       for (var element in conditionals) {
         allDrivenSignals.addAll(element.execute());
       }
       if (allDrivenSignals.length != allDrivenSignals.toSet().length) {
-        throw Exception('FF drove the same signal multiple times.');
+        throw Exception('Sequential drove the same signal multiple times.');
       }
     }
+
+    // clear out all the pre-tick value of clocks
+    for (var i = 0; i < _clks.length; i++) {
+      _preTickClkValues[i] = null;
+    }
   }
 
   @override
-  String alwaysVerilogStatement(Map<String, String> inputs) =>
-      'always_ff @(posedge ${inputs[_clk]})';
+  String alwaysVerilogStatement(Map<String, String> inputs) {
+    String triggers =
+        _clks.map((clk) => 'posedge ${inputs[clk.name]}').join(' or ');
+    return 'always_ff @($triggers)';
+  }
+
   @override
   String assignOperator() => '<=';
 }
@@ -247,7 +284,7 @@ abstract class Conditional {
 
   /// A [Map] of override [LogicValues]s for driver [Logic]s of this [Conditional].
   ///
-  /// This is used for things like [FF]'s pre-tick values.
+  /// This is used for things like [Sequential]'s pre-tick values.
   Map<Logic, LogicValues> _driverValueOverrideMap = {};
 
   /// Updates the values of [_assignedReceiverToOutputMap] and [_assignedDriverToInputMap] and
@@ -834,7 +871,7 @@ class FlipFlop extends Module with CustomSystemVerilog {
 
   /// Performs setup for custom functional behavior.
   void _setup() {
-    FF(clk, [q < d]);
+    Sequential(clk, [q < d]);
   }
 
   @override
diff --git a/lib/src/modules/pipeline.dart b/lib/src/modules/pipeline.dart
index 290272343..c1bddde57 100644
--- a/lib/src/modules/pipeline.dart
+++ b/lib/src/modules/pipeline.dart
@@ -184,7 +184,7 @@ class Pipeline {
         ])
       ];
     }
-    FF(clk, ffAssignsWithStall, name: 'ff_${newLogic.name}');
+    Sequential(clk, ffAssignsWithStall, name: 'ff_${newLogic.name}');
   }
 
   /// The stage input for a signal associated with [logic] to [stageIndex].
diff --git a/lib/src/utilities/simcompare.dart b/lib/src/utilities/simcompare.dart
index 88f288668..a23626fa3 100644
--- a/lib/src/utilities/simcompare.dart
+++ b/lib/src/utilities/simcompare.dart
@@ -77,12 +77,18 @@ class SimCompare {
         for (var signalName in vector.expectedOutputValues.keys) {
           var value = vector.expectedOutputValues[signalName];
           var o = module.output(signalName);
+          var errorReason =
+              'For vector #${vectors.indexOf(vector)} $vector, expected $o to be $value, but it was ${o.value}.';
           if (value is int) {
-            expect(o.valueInt, equals(value));
+            if (!o.value.isValid) {
+              // invalid value causes exception without helpful message, so throw it
+              throw Exception(errorReason);
+            }
+            expect(o.valueInt, equals(value), reason: errorReason);
           } else if (value is LogicValue &&
               (value == LogicValue.x || value == LogicValue.z)) {
             o.value.toList().forEach((element) {
-              expect(element, equals(value));
+              expect(element, equals(value), reason: errorReason);
             });
           } else {
             throw Exception(


Test patch:
diff --git a/test/conditionals_test.dart b/test/conditionals_test.dart
index f22e7029e..101b683b8 100644
--- a/test/conditionals_test.dart
+++ b/test/conditionals_test.dart
@@ -114,8 +114,8 @@ class CombModule extends Module {
   }
 }
 
-class FFModule extends Module {
-  FFModule(Logic a, Logic b, Logic d) : super(name: 'ffmodule') {
+class SequentialModule extends Module {
+  SequentialModule(Logic a, Logic b, Logic d) : super(name: 'ffmodule') {
     a = addInput('a', a);
     b = addInput('b', b);
     var y = addOutput('y');
@@ -125,7 +125,7 @@ class FFModule extends Module {
     d = addInput('d', d, width: d.width);
     var q = addOutput('q', width: d.width);
 
-    FF(SimpleClockGenerator(10).clk, [
+    Sequential(SimpleClockGenerator(10).clk, [
       If(a, then: [
         q < d,
         y < a,
@@ -209,7 +209,7 @@ void main() {
     });
 
     test('conditional ff', () async {
-      var mod = FFModule(Logic(), Logic(), Logic(width: 8));
+      var mod = SequentialModule(Logic(), Logic(), Logic(width: 8));
       await mod.build();
       var vectors = [
         Vector({'a': 1, 'd': 1}, {}),
diff --git a/test/counter_test.dart b/test/counter_test.dart
index 54041d350..a18da6bb5 100644
--- a/test/counter_test.dart
+++ b/test/counter_test.dart
@@ -8,12 +8,16 @@
 /// Author: Max Korbel <max.korbel@intel.com>
 ///
 
+import 'dart:async';
+// import 'dart:io';
+
 import 'package:rohd/rohd.dart';
 import 'package:test/test.dart';
 import 'package:rohd/src/utilities/simcompare.dart';
 
 class Counter extends Module {
   final int width;
+  Logic get val => output('val');
   Counter(Logic en, Logic reset, {this.width = 8}) : super(name: 'counter') {
     en = addInput('en', en);
     reset = addInput('reset', reset);
@@ -24,7 +28,10 @@ class Counter extends Module {
 
     nextVal <= val + 1;
 
-    FF((SimpleClockGenerator(10).clk | reset), [
+    Sequential.multi([
+      SimpleClockGenerator(10).clk,
+      reset
+    ], [
       If(reset, then: [
         val < 0
       ], orElse: [
@@ -41,10 +48,18 @@ void main() {
 
   group('simcompare', () {
     test('counter', () async {
-      var mod = Counter(Logic(), Logic());
-      await mod.build();
-      // WaveDumper(mod);
-      // File('tmp_counter.sv').writeAsStringSync(mod.generateSynth());
+      var reset = Logic();
+      var counter = Counter(Logic(), reset);
+      await counter.build();
+      // WaveDumper(counter);
+      // File('tmp_counter.sv').writeAsStringSync(counter.generateSynth());
+
+      // check that 1 timestep after reset, the value has reset properly
+      unawaited(reset.nextPosedge
+          .then((value) => Simulator.registerAction(Simulator.time + 1, () {
+                expect(counter.val.valueInt, equals(0));
+              })));
+
       var vectors = [
         Vector({'en': 0, 'reset': 0}, {}),
         Vector({'en': 0, 'reset': 1}, {'val': 0}),
@@ -58,9 +73,9 @@ void main() {
         Vector({'en': 1, 'reset': 0}, {'val': 4}),
         Vector({'en': 0, 'reset': 0}, {'val': 5}),
       ];
-      await SimCompare.checkFunctionalVector(mod, vectors);
+      await SimCompare.checkFunctionalVector(counter, vectors);
       var simResult = SimCompare.iverilogVector(
-          mod.generateSynth(), mod.runtimeType.toString(), vectors,
+          counter.generateSynth(), counter.runtimeType.toString(), vectors,
           signalToWidthMap: {'val': 8});
       expect(simResult, equals(true));
     });
diff --git a/test/counter_wintf_test.dart b/test/counter_wintf_test.dart
index a345d63cc..3231b4ee9 100644
--- a/test/counter_wintf_test.dart
+++ b/test/counter_wintf_test.dart
@@ -52,7 +52,7 @@ class Counter extends Module {
 
     nextVal <= intf.val + 1;
 
-    FF((SimpleClockGenerator(10).clk), [
+    Sequential((SimpleClockGenerator(10).clk), [
       If(intf.reset, then: [
         intf.val < 0
       ], orElse: [
diff --git a/test/translations_test.dart b/test/translations_test.dart
index 9a7459420..3257fd1ff 100644
--- a/test/translations_test.dart
+++ b/test/translations_test.dart
@@ -70,8 +70,8 @@ class FlopArray extends Module {
     var storageBank = List<Logic>.generate(
         numEntries, (i) => Logic(name: 'storageBank_$i', width: dwidth));
 
-    // FF(lclk, [  // normally this should be here
-    FF(SimpleClockGenerator(10).clk, [
+    // Sequential(lclk, [  // normally this should be here
+    Sequential(SimpleClockGenerator(10).clk, [
       //for testing purposes, easier to just plug a clock in here
       If(lrst, then: [
         ...storageBank