From c39440adbf12fa5c7d9256d79862c8142d4fcc35 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Wed, 7 May 2025 13:35:38 -0700 Subject: [PATCH 1/7] Add labels in generated verilog for FSMs --- lib/src/finite_state_machine.dart | 37 ++++++++++++++------------ lib/src/modules/conditionals/case.dart | 10 ++++--- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/lib/src/finite_state_machine.dart b/lib/src/finite_state_machine.dart index 4bbd8baa1..98d025832 100644 --- a/lib/src/finite_state_machine.dart +++ b/lib/src/finite_state_machine.dart @@ -100,7 +100,7 @@ class FiniteStateMachine { /// /// If [asyncReset] is `true`, the [reset] signal is asynchronous. FiniteStateMachine.multi( - this._clks, this.reset, this.resetState, this._states, + this._clks, this.reset, this.resetState, this._states, //TODO: defaults {this.asyncReset = false}) : _stateWidth = _logBase(_states.length, 2), currentState = @@ -120,22 +120,25 @@ class FiniteStateMachine { currentState, _states .map((state) => CaseItem( - Const(_stateValueLookup[state], width: _stateWidth), [ - ...state.actions, - Case( - Const(1), - state.events.entries - .map((entry) => CaseItem(entry.key, [ - nextState < - _stateValueLookup[ - _stateLookup[entry.value]] - ])) - .toList(growable: false), - conditionalType: state.conditionalType, - defaultItem: [ - nextState < getStateIndex(state.defaultNextState), - ]) - ])) + label: state.identifier.toString(), + Const(_stateValueLookup[state], width: _stateWidth) + .named(state.identifier.toString()), + [ + ...state.actions, + Case( + Const(1), + state.events.entries + .map((entry) => CaseItem(entry.key, [ + nextState < + _stateValueLookup[ + _stateLookup[entry.value]] + ])) + .toList(growable: false), + conditionalType: state.conditionalType, + defaultItem: [ + nextState < getStateIndex(state.defaultNextState), + ]) + ])) .toList(growable: false), conditionalType: ConditionalType.unique, defaultItem: [ diff --git a/lib/src/modules/conditionals/case.dart b/lib/src/modules/conditionals/case.dart index 590437391..f45d0cc46 100644 --- a/lib/src/modules/conditionals/case.dart +++ b/lib/src/modules/conditionals/case.dart @@ -20,8 +20,11 @@ class CaseItem { /// A [List] of [Conditional]s to execute when [value] is matched. final List then; + /// An optional label for this case body. + final String? label; + /// Executes [then] when [value] matches. - CaseItem(this.value, this.then); + CaseItem(this.value, this.then, {this.label}); @override String toString() => '$value : $then'; @@ -267,14 +270,15 @@ class Case extends Conditional { final subPadding = Conditional.calcPadding(indent + 2); for (final item in items) { final conditionName = inputsNameMap[driverInput(item.value).name]; + final caseLabel = item.label == null ? '' : ' // ${item.label!}'; final caseContents = item.then .map((conditional) => conditional.verilogContents( indent + 4, inputsNameMap, outputsNameMap, assignOperator)) .join('\n'); verilog.write(''' -$subPadding$conditionName : begin +$subPadding$conditionName : begin$caseLabel $caseContents -${subPadding}end +${subPadding}end$caseLabel '''); } if (defaultItem != null) { From 5ad65a232b862c4bcc2be77723333e15d6073945 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Wed, 7 May 2025 13:41:26 -0700 Subject: [PATCH 2/7] Add setupActions for things like default values --- lib/src/finite_state_machine.dart | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/src/finite_state_machine.dart b/lib/src/finite_state_machine.dart index 98d025832..f7ae867ae 100644 --- a/lib/src/finite_state_machine.dart +++ b/lib/src/finite_state_machine.dart @@ -68,6 +68,11 @@ class FiniteStateMachine { /// bus. final Logic currentState; + /// A [List] of [Conditional] actions to perform at the beginning of the + /// evaluation of actions for the [FiniteStateMachine]. This is useful for + /// things like setting up default values for signals across all states. + final List setupActions; + /// The next state of the FSM. /// /// Use [getStateIndex] to map from a [StateIdentifier] to the value on this @@ -92,7 +97,9 @@ class FiniteStateMachine { StateIdentifier resetState, List> states, { bool asyncReset = false, - }) : this.multi([clk], reset, resetState, states, asyncReset: asyncReset); + List setupActions = const [], + }) : this.multi([clk], reset, resetState, states, + asyncReset: asyncReset, setupActions: setupActions); /// Creates an finite state machine for the specified list of [_states], with /// an initial state of [resetState] (when [reset] is high) and transitions on @@ -100,9 +107,13 @@ class FiniteStateMachine { /// /// If [asyncReset] is `true`, the [reset] signal is asynchronous. FiniteStateMachine.multi( - this._clks, this.reset, this.resetState, this._states, //TODO: defaults - {this.asyncReset = false}) - : _stateWidth = _logBase(_states.length, 2), + this._clks, + this.reset, + this.resetState, + this._states, { + this.asyncReset = false, + this.setupActions = const [], + }) : _stateWidth = _logBase(_states.length, 2), currentState = Logic(name: 'currentState', width: _logBase(_states.length, 2)), nextState = @@ -116,6 +127,7 @@ class FiniteStateMachine { } Combinational([ + ...setupActions, Case( currentState, _states From ba2f1be361cb32c7241203fca8d25bc6d6a52a84 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Wed, 7 May 2025 14:19:52 -0700 Subject: [PATCH 3/7] expose more from FSM so extension is easier --- lib/src/finite_state_machine.dart | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/src/finite_state_machine.dart b/lib/src/finite_state_machine.dart index f7ae867ae..24173e2e4 100644 --- a/lib/src/finite_state_machine.dart +++ b/lib/src/finite_state_machine.dart @@ -46,6 +46,12 @@ class FiniteStateMachine { return _stateValueLookup[_stateLookup[id]]; } + /// A [Map] from the [StateIdentifier]s to the internal index used to + /// represent that state in the state machine. + // TODO: should this be overrideable? + late final Map stateIndexLookup = UnmodifiableMapView( + _stateValueLookup.map((key, value) => MapEntry(key.identifier, value))); + /// The clock signal to the FSM (when only single-triggered). Otherwise, the /// first clock. /// @@ -83,7 +89,7 @@ class FiniteStateMachine { static int _logBase(num x, num base) => (log(x) / log(base)).ceil(); /// Width of the state. - final int _stateWidth; + final int stateWidth; /// If `true`, the [reset] signal is asynchronous. final bool asyncReset; @@ -113,7 +119,7 @@ class FiniteStateMachine { this._states, { this.asyncReset = false, this.setupActions = const [], - }) : _stateWidth = _logBase(_states.length, 2), + }) : stateWidth = _logBase(_states.length, 2), currentState = Logic(name: 'currentState', width: _logBase(_states.length, 2)), nextState = @@ -133,7 +139,7 @@ class FiniteStateMachine { _states .map((state) => CaseItem( label: state.identifier.toString(), - Const(_stateValueLookup[state], width: _stateWidth) + Const(_stateValueLookup[state], width: stateWidth) .named(state.identifier.toString()), [ ...state.actions, From b45485a9ec071a5ef36e2c09ccc1d82b57e55fc1 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Tue, 27 May 2025 15:47:39 -0700 Subject: [PATCH 4/7] initial testing for fsm upgrade with init actions --- test/fsm_test.dart | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/fsm_test.dart b/test/fsm_test.dart index 0d84ed270..633022700 100644 --- a/test/fsm_test.dart +++ b/test/fsm_test.dart @@ -122,7 +122,6 @@ class TrafficTestModule extends Module { TrafficPresence.isEastActive(traffic): LightStates.northSlowing, }, actions: [ northLight < LightColor.green.value, - eastLight < LightColor.red.value, ]), State( LightStates.northSlowing, @@ -130,7 +129,6 @@ class TrafficTestModule extends Module { defaultNextState: LightStates.eastFlowing, actions: [ northLight < LightColor.yellow.value, - eastLight < LightColor.red.value, ], ), State( @@ -139,7 +137,6 @@ class TrafficTestModule extends Module { TrafficPresence.isNorthActive(traffic): LightStates.eastSlowing, }, actions: [ - northLight < LightColor.red.value, eastLight < LightColor.green.value, ], ), @@ -148,7 +145,6 @@ class TrafficTestModule extends Module { events: {}, defaultNextState: LightStates.northFlowing, actions: [ - northLight < LightColor.red.value, eastLight < LightColor.yellow.value, ], ), @@ -159,6 +155,11 @@ class TrafficTestModule extends Module { reset, LightStates.northFlowing, states, + setupActions: [ + // by default, lights should be red + northLight < LightColor.red.value, + eastLight < LightColor.red.value, + ], ); if (!kIsWeb) { @@ -316,7 +317,7 @@ void main() { }) ]; await SimCompare.checkFunctionalVector(pipem, vectors); - SimCompare.checkIverilogVector(pipem, vectors); + SimCompare.checkIverilogVector(pipem, vectors, dontDeleteTmpFiles: true); verifyMermaidStateDiagram(_trafficFSMPath); }); From 41f8af8ec064b5bc60cea2bda91a8b63e973ad3e Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Tue, 24 Jun 2025 11:08:54 -0700 Subject: [PATCH 5/7] remove case labels (pending other PR) --- lib/src/finite_state_machine.dart | 1 - lib/src/modules/conditionals/case.dart | 10 +++------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/src/finite_state_machine.dart b/lib/src/finite_state_machine.dart index 24173e2e4..cf010cc52 100644 --- a/lib/src/finite_state_machine.dart +++ b/lib/src/finite_state_machine.dart @@ -138,7 +138,6 @@ class FiniteStateMachine { currentState, _states .map((state) => CaseItem( - label: state.identifier.toString(), Const(_stateValueLookup[state], width: stateWidth) .named(state.identifier.toString()), [ diff --git a/lib/src/modules/conditionals/case.dart b/lib/src/modules/conditionals/case.dart index f45d0cc46..590437391 100644 --- a/lib/src/modules/conditionals/case.dart +++ b/lib/src/modules/conditionals/case.dart @@ -20,11 +20,8 @@ class CaseItem { /// A [List] of [Conditional]s to execute when [value] is matched. final List then; - /// An optional label for this case body. - final String? label; - /// Executes [then] when [value] matches. - CaseItem(this.value, this.then, {this.label}); + CaseItem(this.value, this.then); @override String toString() => '$value : $then'; @@ -270,15 +267,14 @@ class Case extends Conditional { final subPadding = Conditional.calcPadding(indent + 2); for (final item in items) { final conditionName = inputsNameMap[driverInput(item.value).name]; - final caseLabel = item.label == null ? '' : ' // ${item.label!}'; final caseContents = item.then .map((conditional) => conditional.verilogContents( indent + 4, inputsNameMap, outputsNameMap, assignOperator)) .join('\n'); verilog.write(''' -$subPadding$conditionName : begin$caseLabel +$subPadding$conditionName : begin $caseContents -${subPadding}end$caseLabel +${subPadding}end '''); } if (defaultItem != null) { From 35cdfa5a28abf1fe5ad851fdd66f07a36d872f89 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Tue, 24 Jun 2025 11:27:57 -0700 Subject: [PATCH 6/7] add more tests and docs --- doc/user_guide/_docs/A15-fsm.md | 49 +++++++++++++++++++++ lib/src/finite_state_machine.dart | 6 +-- test/fsm_test.dart | 72 +++++++++++++++++++++---------- 3 files changed, 101 insertions(+), 26 deletions(-) diff --git a/doc/user_guide/_docs/A15-fsm.md b/doc/user_guide/_docs/A15-fsm.md index ecba06b84..256ba9abe 100644 --- a/doc/user_guide/_docs/A15-fsm.md +++ b/doc/user_guide/_docs/A15-fsm.md @@ -113,6 +113,55 @@ FiniteStateMachine( ); ``` +Notice, that in all states, one of the lights is red. We could simplify the FSM by removing all the actions to make a light red and setting up a default in `setupActions` for the FSM. That way, every state would default to both lights being red, and we'd only need to specify states when they are non-red. For example: + +```dart +final states = >[ + State(LightStates.northFlowing, events: { + TrafficPresence.isEastActive(traffic): LightStates.northSlowing, + }, actions: [ + northLight < LightColor.green.value, + ]), + State( + LightStates.northSlowing, + events: {}, + defaultNextState: LightStates.eastFlowing, + actions: [ + northLight < LightColor.yellow.value, + ], + ), + State( + LightStates.eastFlowing, + events: { + TrafficPresence.isNorthActive(traffic): LightStates.eastSlowing, + }, + actions: [ + eastLight < LightColor.green.value, + ], + ), + State( + LightStates.eastSlowing, + events: {}, + defaultNextState: LightStates.northFlowing, + actions: [ + eastLight < LightColor.yellow.value, + ], + ), +]; + +FiniteStateMachine( + clk, + reset, + LightStates.northFlowing, + states, + setupActions: [ + // by default, lights should be red + northLight < LightColor.red.value, + eastLight < LightColor.red.value, + ], +); +``` + This state machine is now functional and synthesizable into SystemVerilog! You can even generate a mermaid diagram for the state machine using the [`generateDiagram`](https://intel.github.io/rohd/rohd/FiniteStateMachine/generateDiagram.html) API. diff --git a/lib/src/finite_state_machine.dart b/lib/src/finite_state_machine.dart index cf010cc52..689f7fabc 100644 --- a/lib/src/finite_state_machine.dart +++ b/lib/src/finite_state_machine.dart @@ -48,7 +48,6 @@ class FiniteStateMachine { /// A [Map] from the [StateIdentifier]s to the internal index used to /// represent that state in the state machine. - // TODO: should this be overrideable? late final Map stateIndexLookup = UnmodifiableMapView( _stateValueLookup.map((key, value) => MapEntry(key.identifier, value))); @@ -118,8 +117,9 @@ class FiniteStateMachine { this.resetState, this._states, { this.asyncReset = false, - this.setupActions = const [], - }) : stateWidth = _logBase(_states.length, 2), + List setupActions = const [], + }) : setupActions = List.unmodifiable(setupActions), + stateWidth = _logBase(_states.length, 2), currentState = Logic(name: 'currentState', width: _logBase(_states.length, 2)), nextState = diff --git a/test/fsm_test.dart b/test/fsm_test.dart index 633022700..b5f010a56 100644 --- a/test/fsm_test.dart +++ b/test/fsm_test.dart @@ -180,23 +180,49 @@ void main() { }); test('zero-out receivers in default case', () async { - final pipem = TestModule(Logic(), Logic(), Logic()); - await pipem.build(); + final mod = TestModule(Logic(), Logic(), Logic()); + await mod.build(); - final sv = pipem.generateSynth(); + final sv = mod.generateSynth(); expect(sv, contains("b = 1'h0;")); }); test('conditional type is used', () async { - final pipem = TestModule(Logic(), Logic(), Logic()); - await pipem.build(); + final mod = TestModule(Logic(), Logic(), Logic()); + await mod.build(); - final sv = pipem.generateSynth(); + final sv = mod.generateSynth(); expect(sv, contains('priority case')); }); + test('label name included in generated SV', () async { + final mod = TestModule(Logic(), Logic(), Logic()); + await mod.build(); + + final sv = mod.generateSynth(); + + expect(sv, contains('MyStates_state1 : begin')); + }); + + test('state value lookup is correct', () async { + final mod = DefaultStateFsmMod(Logic()); + await mod.build(); + + expect(mod._fsm.stateWidth, 2); + + expect(mod._fsm.stateIndexLookup.length, MyStates.values.length); + expect( + MyStates.values.every((e) => mod._fsm.stateIndexLookup.containsKey(e)), + isTrue); + for (var i = 0; i < MyStates.values.length; i++) { + final stateEnum = MyStates.values[i]; + expect(mod._fsm.getStateIndex(stateEnum), i); + expect(mod._fsm.stateIndexLookup[stateEnum], i); + } + }); + group('fsm validation', () { test('duplicate state identifiers throws exception', () { expect( @@ -232,9 +258,9 @@ void main() { group('simcompare', () { test('simple fsm', () async { - final pipem = TestModule(Logic(), Logic(), Logic()); + final mod = TestModule(Logic(), Logic(), Logic()); - await pipem.build(); + await mod.build(); final vectors = [ Vector({'reset': 1, 'a': 0, 'c': 0}, {}), @@ -242,32 +268,32 @@ void main() { Vector({}, {'b': 1}), Vector({'c': 1}, {'b': 0}), ]; - await SimCompare.checkFunctionalVector(pipem, vectors); - SimCompare.checkIverilogVector(pipem, vectors); + await SimCompare.checkFunctionalVector(mod, vectors); + SimCompare.checkIverilogVector(mod, vectors); verifyMermaidStateDiagram(_simpleFSMPath); }); test('simple fsm async reset', () async { - final pipem = + final mod = TestModule(Logic(), Logic(), Logic(), testingAsyncReset: true); - await pipem.build(); + await mod.build(); final vectors = [ Vector({'reset': 0, 'a': 0, 'c': 0}, {}), Vector({'reset': 1}, {'b': 0}), ]; - await SimCompare.checkFunctionalVector(pipem, vectors); - SimCompare.checkIverilogVector(pipem, vectors); + await SimCompare.checkFunctionalVector(mod, vectors); + SimCompare.checkIverilogVector(mod, vectors); verifyMermaidStateDiagram(_simpleFSMPath); }); test('default next state fsm', () async { - final pipem = DefaultStateFsmMod(Logic()); + final mod = DefaultStateFsmMod(Logic()); - await pipem.build(); + await mod.build(); final vectors = [ Vector({'reset': 1}, {}), @@ -276,12 +302,12 @@ void main() { Vector({'reset': 0}, {'b': 4}), Vector({'reset': 0}, {'b': 4}), ]; - await SimCompare.checkFunctionalVector(pipem, vectors); - SimCompare.checkIverilogVector(pipem, vectors); + await SimCompare.checkFunctionalVector(mod, vectors); + SimCompare.checkIverilogVector(mod, vectors); if (!kIsWeb) { const fsmPath = '$_tmpDir/default_next_state_fsm.md'; - pipem._fsm.generateDiagram(outputPath: fsmPath); + mod._fsm.generateDiagram(outputPath: fsmPath); final mermaid = File(fsmPath).readAsStringSync(); expect(mermaid, contains('state2')); @@ -294,8 +320,8 @@ void main() { }); test('traffic light fsm', () async { - final pipem = TrafficTestModule(Logic(width: 2), Logic()); - await pipem.build(); + final mod = TrafficTestModule(Logic(width: 2), Logic()); + await mod.build(); final vectors = [ Vector({'reset': 1, 'traffic': 00}, {}), @@ -316,8 +342,8 @@ void main() { 'eastLight': LightColor.green.value }) ]; - await SimCompare.checkFunctionalVector(pipem, vectors); - SimCompare.checkIverilogVector(pipem, vectors, dontDeleteTmpFiles: true); + await SimCompare.checkFunctionalVector(mod, vectors); + SimCompare.checkIverilogVector(mod, vectors); verifyMermaidStateDiagram(_trafficFSMPath); }); From 1f1c7a225b93b7e9fc2512d7335adeadb8ada042 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Tue, 24 Jun 2025 11:32:21 -0700 Subject: [PATCH 7/7] lint cleanup --- .github/workflows/general.yml | 2 -- lib/src/utilities/simcompare.dart | 6 +++--- test/logic_structure_test.dart | 3 ++- test/simulator_test.dart | 6 +++--- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/.github/workflows/general.yml b/.github/workflows/general.yml index cffbcbf1f..48215e8e1 100644 --- a/.github/workflows/general.yml +++ b/.github/workflows/general.yml @@ -34,8 +34,6 @@ jobs: - name: Setup Dart uses: dart-lang/setup-dart@v1 - with: - sdk: 3.6.2 # downgrade pending https://github.com/dart-lang/dartdoc/issues/3996 - name: Setup Node uses: actions/setup-node@v4 diff --git a/lib/src/utilities/simcompare.dart b/lib/src/utilities/simcompare.dart index 594fe0950..771880f1b 100644 --- a/lib/src/utilities/simcompare.dart +++ b/lib/src/utilities/simcompare.dart @@ -160,7 +160,7 @@ abstract class SimCompare { } for (final vector in vectors) { - Simulator.registerAction(timestamp, () { + Simulator.registerAction(timestamp, () async { for (final signalName in vector.inputValues.keys) { final value = vector.inputValues[signalName]; (module.tryInput(signalName) ?? getIoInputDriver(signalName)) @@ -168,7 +168,7 @@ abstract class SimCompare { } if (enableChecking) { - Simulator.postTick.first.then((value) { + unawaited(Simulator.postTick.first.then((value) { for (final signalName in vector.expectedOutputValues.keys) { final value = vector.expectedOutputValues[signalName]; final o = @@ -207,7 +207,7 @@ abstract class SimCompare { (Object err, StackTrace stackTrace) { Simulator.throwException(err as Exception, stackTrace); }, - ); + )); } }); timestamp += Vector._period; diff --git a/test/logic_structure_test.dart b/test/logic_structure_test.dart index 6947b0957..e0fbba3e5 100644 --- a/test/logic_structure_test.dart +++ b/test/logic_structure_test.dart @@ -93,7 +93,6 @@ class StructModuleWithInstrumentation extends Module { ..isOutput ..changed ..glitch - ..nextChanged // ignore: deprecated_member_use_from_same_package ..hasValidValue() // ignore: deprecated_member_use_from_same_package @@ -102,6 +101,8 @@ class StructModuleWithInstrumentation extends Module { ..valueBigInt // ignore: deprecated_member_use_from_same_package ..valueInt; + + unawaited(MyStruct().nextChanged); } } diff --git a/test/simulator_test.dart b/test/simulator_test.dart index 222f1ab56..f30529334 100644 --- a/test/simulator_test.dart +++ b/test/simulator_test.dart @@ -377,10 +377,10 @@ void main() { test('end of tick makes a re-tick if it was missed', () async { var endOfTickActionTaken = false; - Simulator.registerAction(100, () { - Simulator.postTick.first.then((_) { + Simulator.registerAction(100, () async { + unawaited(Simulator.postTick.first.then((_) { Simulator.injectEndOfTickAction(() => endOfTickActionTaken = true); - }); + })); }); var numStartTicks = 0;