Repository: intel/rohd (dart)

Issue:
Add an enable signal to `FlipFlop`
### Motivation

Along the same lines as https://github.com/intel/rohd/issues/69

It would be nice to have an enable signal optionally provided as part of the built-in `FlipFlop`.

### Desired solution

Add an optional enable signal to `FlipFlop`.

### Alternatives considered

_No response_

### Additional details

_No response_

Pull request description:
<!-- Please make sure you check out the contribution guidelines before submitting a pull request! -->

## Description & Motivation

<!-- Description of changes, and motivation for adding them. -->
It would be nice to have an enable signal optionally provided as part of the built-in FlipFlop as per #334 

## Testing

<!-- Please describe how you tested your changes. -->
Update test/flop_test.dart for the same.

## Documentation

> Does the change require any updates to documentation? If so, where? Are they included?

<!-- Answer here. -->
Update required at official [documentation](https://intel.github.io/rohd/rohd/FlipFlop-class.html). Not included by me.

Patch:
diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart
index 3c4771dde..fd3677412 100644
--- a/lib/src/modules/conditional.dart
+++ b/lib/src/modules/conditional.dart
@@ -1278,6 +1278,9 @@ ${padding}end ''');
 
 /// Represents a single flip-flop with no reset.
 class FlipFlop extends Module with CustomSystemVerilog {
+  /// Name for the enable input of this flop
+  late final String _enName;
+
   /// Name for the clk of this flop.
   late final String _clkName;
 
@@ -1290,24 +1293,54 @@ class FlipFlop extends Module with CustomSystemVerilog {
   /// The clock, posedge triggered.
   late final Logic _clk = input(_clkName);
 
+  /// Optional enable input to the flop.
+  ///
+  /// If enable is  high or enable is not provided then flop output will vary
+  /// on the basis of clock [_clk] and input [_d]. If enable is low, then
+  /// output of the flop remains frozen irrespective of the input [_d].
+  late final Logic _en = input(_enName);
+
   /// The input to the flop.
   late final Logic _d = input(_dName);
 
   /// The output of the flop.
   late final Logic q = output(_qName);
 
+  /// To track if optional enable is provided or not.
+  late final bool _isEnableProvided;
+
   /// Constructs a flip flop which is positive edge triggered on [clk].
-  FlipFlop(Logic clk, Logic d, {super.name = 'flipflop'}) {
+  ///
+  /// When optional [en] is provided, an additional input will be created for
+  /// flop. If optional [en] is high or not provided, output will vary as per
+  /// input[d]. For low [en], output remains frozen irrespective of input [d]
+  FlipFlop(Logic clk, Logic d, {Logic? en, super.name = 'flipflop'}) {
     if (clk.width != 1) {
       throw Exception('clk must be 1 bit');
     }
+
     _clkName = Module.unpreferredName('clk');
     _dName = Module.unpreferredName('d');
     _qName = Module.unpreferredName('q');
+
     addInput(_clkName, clk);
     addInput(_dName, d, width: d.width);
     addOutput(_qName, width: d.width);
-    _setup();
+
+    if (en != null) {
+      if (en.width != 1) {
+        throw PortWidthMismatchException(en, 1);
+      }
+      _enName = Module.unpreferredName('en');
+      addInput(_enName, en);
+      _isEnableProvided = true;
+
+      _setupWithEnable();
+    } else {
+      _isEnableProvided = false;
+
+      _setup();
+    }
   }
 
   /// Performs setup for custom functional behavior.
@@ -1315,15 +1348,35 @@ class FlipFlop extends Module with CustomSystemVerilog {
     Sequential(_clk, [q < _d]);
   }
 
+  /// Performs setup for custom functional behavior with enable
+  void _setupWithEnable() {
+    Sequential(_clk, [
+      If(_en, then: [q < _d])
+    ]);
+  }
+
   @override
   String instantiationVerilog(String instanceType, String instanceName,
       Map<String, String> inputs, Map<String, String> outputs) {
-    if (inputs.length != 2 || outputs.length != 1) {
-      throw Exception('FlipFlop has exactly two inputs and one output.');
+    if (_isEnableProvided) {
+      if (inputs.length != 3 || outputs.length != 1) {
+        throw Exception('FlipFlop has exactly three inputs and one output.');
+      }
+    } else {
+      if (inputs.length != 2 || outputs.length != 1) {
+        throw Exception('FlipFlop has exactly two inputs and one output.');
+      }
     }
+
     final clk = inputs[_clkName]!;
     final d = inputs[_dName]!;
     final q = outputs[_qName]!;
-    return 'always_ff @(posedge $clk) $q <= $d;  // $instanceName';
+
+    if (_isEnableProvided) {
+      final en = inputs[_enName]!;
+      return 'always_ff @(posedge $clk) if($en) $q <= $d;  // $instanceName';
+    } else {
+      return 'always_ff @(posedge $clk) $q <= $d;  // $instanceName';
+    }
   }
 }


Test patch:
diff --git a/test/changed_test.dart b/test/changed_test.dart
index a86e4854a..28f434e62 100644
--- a/test/changed_test.dart
+++ b/test/changed_test.dart
@@ -114,6 +114,83 @@ void main() {
 
     expect(qHadPosedge, equals(true));
   });
+  test('injection triggers flop with enable logicvalue 1', () async {
+    final baseClk = SimpleClockGenerator(10).clk;
+
+    final clk = Logic();
+    final d = Logic();
+    final en = Logic();
+    final q = FlipFlop(clk, d, en: en).q;
+
+    var qHadPosedge = false;
+
+    Simulator.setMaxSimTime(100);
+
+    unawaited(q.nextPosedge.then((value) {
+      qHadPosedge = true;
+    }));
+
+    unawaited(Simulator.run());
+
+    await baseClk.nextPosedge;
+    en.inject(1);
+    clk.inject(0);
+    d.inject(0);
+    await baseClk.nextPosedge;
+    clk.inject(1);
+    await baseClk.nextPosedge;
+    expect(q.value, equals(LogicValue.zero));
+    en.inject(1);
+    clk.inject(0);
+    d.inject(1);
+    await baseClk.nextPosedge;
+    clk.inject(1);
+    await baseClk.nextPosedge;
+    expect(q.value, equals(LogicValue.one));
+
+    await Simulator.simulationEnded;
+
+    expect(qHadPosedge, equals(true));
+  });
+
+  test('injection triggers flop with enable', () async {
+    final baseClk = SimpleClockGenerator(10).clk;
+
+    final clk = Logic();
+    final d = Logic();
+    final en = Logic();
+    final q = FlipFlop(clk, d, en: en).q;
+
+    var qHadPosedge = false;
+
+    Simulator.setMaxSimTime(100);
+
+    unawaited(q.nextPosedge.then((value) {
+      qHadPosedge = true;
+    }));
+
+    unawaited(Simulator.run());
+
+    await baseClk.nextPosedge;
+    en.inject(1);
+    clk.inject(0);
+    d.inject(0);
+    await baseClk.nextPosedge;
+    clk.inject(1);
+    await baseClk.nextPosedge;
+    expect(q.value, equals(LogicValue.zero));
+    en.inject(0);
+    clk.inject(0);
+    d.inject(1);
+    await baseClk.nextPosedge;
+    clk.inject(1);
+    await baseClk.nextPosedge;
+    expect(q.value, equals(LogicValue.zero));
+
+    await Simulator.simulationEnded;
+
+    expect(qHadPosedge, equals(false));
+  });
 
   test('reconnected signal still hits changed events', () async {
     final a = Logic(name: 'a');
diff --git a/test/comb_sensitivity_test.dart b/test/comb_sensitivity_test.dart
index b9dc54486..876f5f8df 100644
--- a/test/comb_sensitivity_test.dart
+++ b/test/comb_sensitivity_test.dart
@@ -13,23 +13,33 @@ import 'package:test/test.dart';
 
 class SeqWrapper extends Module {
   Logic get fromFlop => output('fromFlop');
-  SeqWrapper(Logic toFlop, Logic toNothing) {
+  SeqWrapper(Logic toFlop, Logic toNothing, {Logic? en}) {
     toFlop = addInput('toFlop', toFlop);
     toNothing = addInput('toNothing', toNothing);
 
     addOutput('fromFlop');
 
-    fromFlop <=
-        FlipFlop(
-          SimpleClockGenerator(10).clk,
-          toFlop,
-        ).q;
+    if (en != null) {
+      en = addInput('en', en);
+      fromFlop <=
+          FlipFlop(
+            SimpleClockGenerator(10).clk,
+            toFlop,
+            en: en,
+          ).q;
+    } else {
+      fromFlop <=
+          FlipFlop(
+            SimpleClockGenerator(10).clk,
+            toFlop,
+          ).q;
+    }
   }
 }
 
 class TopMod extends Module {
   Logic get muxToFlop => output('muxToFlop');
-  TopMod(Logic theSource) {
+  TopMod(Logic theSource, {Logic? en}) {
     theSource = addInput('theSource', theSource);
 
     final control = Logic(name: 'control');
@@ -41,7 +51,12 @@ class TopMod extends Module {
       theSource,
     );
 
-    final seqWrapper = SeqWrapper(toFlop, toNothing);
+    final SeqWrapper seqWrapper;
+    if (en != null) {
+      seqWrapper = SeqWrapper(toFlop, toNothing, en: en);
+    } else {
+      seqWrapper = SeqWrapper(toFlop, toNothing);
+    }
 
     Combinational([
       control < theSource,
@@ -64,4 +79,30 @@ void main() async {
 
     expect(mod.muxToFlop.value.isValid, isTrue);
   });
+
+  test('false sensitivity does not cause false comb cycle (test with enable 1)',
+      () async {
+    final theSource = Logic(name: 'theSource')..put(0);
+    final en = Logic(name: 'en')..put(1);
+    final mod = TopMod(theSource, en: en);
+
+    await mod.build();
+
+    theSource.put(1);
+
+    expect(mod.muxToFlop.value.isValid, isTrue);
+  });
+
+  test('false sensitivity does not cause false comb cycle (test with enable 0)',
+      () async {
+    final theSource = Logic(name: 'theSource')..put(0);
+    final en = Logic(name: 'en')..put(0);
+    final mod = TopMod(theSource, en: en);
+
+    await mod.build();
+
+    theSource.put(1);
+
+    expect(mod.muxToFlop.value.isValid, isTrue);
+  });
 }
diff --git a/test/flop_test.dart b/test/flop_test.dart
index 64450cd6c..53f1d8d83 100644
--- a/test/flop_test.dart
+++ b/test/flop_test.dart
@@ -14,13 +14,22 @@ import 'package:test/test.dart';
 class FlopTestModule extends Module {
   Logic get a => input('a');
   Logic get y => output('y');
+  Logic get en => input('en');
 
-  FlopTestModule(Logic a) : super(name: 'floptestmodule') {
+  FlopTestModule(Logic a, {Logic? en}) : super(name: 'floptestmodule') {
     a = addInput('a', a, width: a.width);
-    final y = addOutput('y', width: a.width);
+    if (en != null) {
+      en = addInput('en', en, width: en.width);
+    }
 
+    final y = addOutput('y', width: a.width);
     final clk = SimpleClockGenerator(10).clk;
-    y <= FlipFlop(clk, a).q;
+
+    if (en != null) {
+      y <= FlipFlop(clk, a, en: en).q;
+    } else {
+      y <= FlipFlop(clk, a).q;
+    }
   }
 }
 
@@ -43,6 +52,30 @@ void main() {
       await SimCompare.checkFunctionalVector(ftm, vectors);
       final simResult = SimCompare.iverilogVector(ftm, vectors);
       expect(simResult, equals(true));
+      // expect(true, true);
+    });
+
+    test('flop bit with enable', () async {
+      final ftm = FlopTestModule(Logic(), en: Logic());
+      await ftm.build();
+      final vectors = [
+        Vector({'a': 0, 'en': 1}, {}),
+        Vector({'a': 1, 'en': 1}, {'y': 0}),
+        Vector({'a': 1, 'en': 1}, {'y': 1}),
+        Vector({'a': 0, 'en': 1}, {'y': 1}),
+        Vector({'a': 0, 'en': 1}, {'y': 0}),
+        Vector({'a': 1, 'en': 1}, {'y': 0}),
+        Vector({'a': 1, 'en': 0}, {'y': 1}),
+        Vector({'a': 0, 'en': 0}, {'y': 1}),
+        Vector({'a': 0, 'en': 1}, {'y': 1}),
+        Vector({'a': 1, 'en': 1}, {'y': 0}),
+        Vector({'a': 0, 'en': 0}, {'y': 1}),
+        Vector({'a': 1, 'en': 0}, {'y': 1}),
+      ];
+      await SimCompare.checkFunctionalVector(ftm, vectors);
+      final simResult = SimCompare.iverilogVector(ftm, vectors);
+      expect(simResult, equals(true));
+      // expect(true, true);
     });
 
     test('flop bus', () async {
@@ -59,5 +92,29 @@ void main() {
       final simResult = SimCompare.iverilogVector(ftm, vectors);
       expect(simResult, equals(true));
     });
+
+    test('flop bus with enable', () async {
+      final ftm = FlopTestModule(Logic(width: 8), en: Logic());
+      await ftm.build();
+      final vectors = [
+        Vector({'a': 0, 'en': 1}, {}),
+        Vector({'a': 0xff, 'en': 1}, {'y': 0}),
+        Vector({'a': 0xaa, 'en': 1}, {'y': 0xff}),
+        Vector({'a': 0x55, 'en': 1}, {'y': 0xaa}),
+        Vector({'a': 0x1, 'en': 1}, {'y': 0x55}),
+        Vector({'a': 0, 'en': 1}, {'y': 0x1}),
+        Vector({'a': 0xff, 'en': 1}, {'y': 0}),
+        Vector({'a': 0xaa, 'en': 1}, {'y': 0xff}),
+        Vector({'a': 0x55, 'en': 0}, {'y': 0xaa}),
+        Vector({'a': 0x1, 'en': 0}, {'y': 0xaa}),
+        Vector({'a': 0x55, 'en': 1}, {'y': 0xaa}),
+        Vector({'a': 0x1, 'en': 1}, {'y': 0x55}),
+        Vector({'a': 0x55, 'en': 0}, {'y': 0x1}),
+        Vector({'a': 0x1, 'en': 1}, {'y': 0x1}),
+      ];
+      await SimCompare.checkFunctionalVector(ftm, vectors);
+      final simResult = SimCompare.iverilogVector(ftm, vectors);
+      expect(simResult, equals(true));
+    });
   });
 }
diff --git a/test/multimodule2_test.dart b/test/multimodule2_test.dart
index bcfb91325..a62cf9a3b 100644
--- a/test/multimodule2_test.dart
+++ b/test/multimodule2_test.dart
@@ -15,30 +15,50 @@ import 'package:test/test.dart';
 
 class Doublesync extends Module {
   Logic get dso => output('dso');
-  Doublesync(Logic clk, Logic dds) : super(name: 'doublesync') {
+  Doublesync(Logic clk, Logic dds, {Logic? en}) : super(name: 'doublesync') {
     clk = addInput('clk', clk);
     dds = addInput('dds', dds, width: dds.width);
+
     addOutput('dso', width: dds.width);
 
-    dso <= FlipFlop(clk, FlipFlop(clk, dds, name: 'innerflop').q).q;
+    if (en != null) {
+      en = addInput('en', en, width: en.width);
+
+      dso <= FlipFlop(clk, FlipFlop(clk, dds, en: en, name: 'innerflop').q).q;
+    } else {
+      dso <= FlipFlop(clk, FlipFlop(clk, dds, name: 'innerflop').q).q;
+    }
   }
 }
 
 class DSWrap extends Module {
   Logic get sigSyncWrap => output('sig_sync_wrap');
-  DSWrap(Logic clk, Logic sig) : super(name: 'dswrap') {
+  DSWrap(Logic clk, Logic sig, {Logic? en}) : super(name: 'dswrap') {
     sig = addInput('sig', sig);
     clk = addInput('clk', clk);
     addOutput('sig_sync_wrap');
-    sigSyncWrap <= Doublesync(clk, sig).dso;
+
+    if (en != null) {
+      en = addInput('en', en);
+
+      sigSyncWrap <= Doublesync(clk, sig, en: en).dso;
+    } else {
+      sigSyncWrap <= Doublesync(clk, sig).dso;
+    }
   }
 }
 
 class TopModule extends Module {
-  TopModule(Logic sig) : super(name: 'top') {
+  TopModule(Logic sig, {Logic? en}) : super(name: 'top') {
     sig = addInput('sig', sig);
     final sigSync = addOutput('sig_sync');
-    sigSync <= DSWrap(SimpleClockGenerator(10).clk, sig).sigSyncWrap;
+    if (en != null) {
+      en = addInput('en', en);
+
+      sigSync <= DSWrap(SimpleClockGenerator(10).clk, sig, en: en).sigSyncWrap;
+    } else {
+      sigSync <= DSWrap(SimpleClockGenerator(10).clk, sig).sigSyncWrap;
+    }
   }
 }
 
@@ -64,5 +84,30 @@ void main() {
       final simResult = SimCompare.iverilogVector(ftm, vectors);
       expect(simResult, equals(true));
     });
+    test('multimodules2 with enable', () async {
+      final ftm = TopModule(Logic(), en: Logic());
+      await ftm.build();
+      final vectors = [
+        Vector({'sig': 0, 'en': 1}, {}),
+        Vector({'sig': 0, 'en': 1}, {}),
+        Vector({'sig': 0, 'en': 1}, {}),
+        Vector({'sig': 1, 'en': 1}, {'sig_sync': 0}),
+        Vector({'sig': 1, 'en': 1}, {'sig_sync': 0}),
+        Vector({'sig': 0, 'en': 0}, {'sig_sync': 1}),
+        Vector({'sig': 1, 'en': 1}, {'sig_sync': 1}),
+        Vector({'sig': 0, 'en': 1}, {'sig_sync': 1}),
+        Vector({'sig': 0, 'en': 0}, {'sig_sync': 1}),
+        Vector({'sig': 1, 'en': 0}, {'sig_sync': 0}),
+        Vector({'sig': 1, 'en': 0}, {'sig_sync': 0}),
+        Vector({'sig': 1, 'en': 1}, {'sig_sync': 0}),
+        Vector({'sig': 1, 'en': 1}, {'sig_sync': 0}),
+        Vector({'sig': 0, 'en': 1}, {'sig_sync': 1}),
+        Vector({'sig': 1, 'en': 1}, {'sig_sync': 1}),
+        Vector({'sig': 1, 'en': 1}, {'sig_sync': 0}),
+      ];
+      await SimCompare.checkFunctionalVector(ftm, vectors);
+      final simResult = SimCompare.iverilogVector(ftm, vectors);
+      expect(simResult, equals(true));
+    });
   });
 }