From a53bbb0f8c110400c45c43b642fcbfefbf8ba0e5 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Wed, 17 Apr 2024 06:09:16 -0700 Subject: [PATCH 01/28] Add support for negedge triggers on Sequential --- lib/src/modules/conditional.dart | 137 +++++++++++++++++++------------ 1 file changed, 86 insertions(+), 51 deletions(-) diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart index 1c9939d2b..9e19b1bad 100644 --- a/lib/src/modules/conditional.dart +++ b/lib/src/modules/conditional.dart @@ -405,20 +405,55 @@ class Combinational extends _Always { @Deprecated('Use Sequential instead') typedef FF = Sequential; +/// A tracking construct for triggers of [Sequential]s. +class _SequentialTrigger { + /// The signal for this trigger. + final Logic signal; + + /// Whether this triggers on a positive edge (`true`) or a negative edge + /// (`false`). + final bool isPosedge; + + /// The value of the trigger before the tick. + LogicValue? preTickValue; + + /// Creates a tracking object for triggers. + _SequentialTrigger(this.signal, {required this.isPosedge}) + : _edgeCheck = isPosedge ? LogicValue.isPosedge : LogicValue.isNegedge; + + /// The method for checking whether an edge has occurred. + final bool Function(LogicValue previousValue, LogicValue newValue) _edgeCheck; + + /// The previous value of the signal. + LogicValue get _previousValue => + // if the pre-tick value is null, then it should have the same value as + // it currently does + preTickValue ?? signal.value; + + /// Whether this trigger has been triggered. + bool get isTriggered => isValid && _edgeCheck(_previousValue, signal.value); + + /// Whether this trigger is valid. + bool get isValid => signal.value.isValid && _previousValue.isValid; + + /// The SystemVerilog keyword for this trigger. + String get verilogTriggerKeyword => isPosedge ? 'posedge' : 'negedge'; +} + /// Represents a block of sequential logic. /// /// 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 _clks = []; + /// The input edge triggers used in this block. + final List<_SequentialTrigger> _triggers = []; /// 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]. + /// Constructs a [Sequential] single-triggered by the positive edge of [clk]. /// /// If `reset` is provided, then all signals driven by this block will be /// conditionally reset when the signal is high. @@ -443,40 +478,53 @@ class Sequential extends _Always { allowMultipleAssignments: allowMultipleAssignments, ); - /// Constructs a [Sequential] multi-triggered by any of [clks]. + /// Constructs a [Sequential] multi-triggered by any of [posedgeTriggers] and + /// [negedgeTriggers] (on positive and negative edges, respectively). /// /// If `reset` is provided, then all signals driven by this block will be - /// conditionally reset when the signal is high. - /// The default reset value is to `0`, but if `resetValues` is provided then - /// the corresponding value associated with the driven signal will be set to - /// 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. + /// conditionally reset when the signal is high. The default reset value is to + /// `0`, but if `resetValues` is provided then the corresponding value + /// associated with the driven signal will be set to 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 clks, + List posedgeTriggers, super._conditionals, { super.reset, super.resetValues, super.name = 'sequential', this.allowMultipleAssignments = true, + List negedgeTriggers = const [], }) { - if (clks.isEmpty) { + if (posedgeTriggers.isEmpty) { throw IllegalConfigurationException('Must provide at least one clock.'); } - for (var i = 0; i < clks.length; i++) { - final clk = clks[i]; - if (clk.width > 1) { - throw Exception('Each clk must be 1 bit, but saw $clk.'); + _registerInputTriggers(posedgeTriggers, isPosedge: true); + _registerInputTriggers(negedgeTriggers, isPosedge: false); + + _setup(); + } + + /// Registers either positive or negative edge trigger inputs for + /// [providedTriggers] based on [isPosedge]. + void _registerInputTriggers(List providedTriggers, + {required bool isPosedge}) { + for (var i = 0; i < providedTriggers.length; i++) { + final trigger = providedTriggers[i]; + if (trigger.width != 1) { + throw Exception('Each clk or trigger must be 1 bit, but saw $trigger.'); } - _clks.add(addInput( - _portUniquifier.getUniqueName( - initialName: Sanitizer.sanitizeSV( - Naming.unpreferredName('clk${i}_${clk.name}'))), - clk)); - _preTickClkValues.add(null); + + _triggers.add(_SequentialTrigger( + addInput( + _portUniquifier.getUniqueName( + initialName: Sanitizer.sanitizeSV( + Naming.unpreferredName('clk${i}_${trigger.name}'))), + trigger), + isPosedge: isPosedge)); } - _setup(); } /// A map from input [Logic]s to the values that should be used for @@ -484,9 +532,6 @@ class Sequential extends _Always { final Map _inputToPreTickInputValuesMap = HashMap(); - /// The value of the clock before the tick. - final List _preTickClkValues = []; - /// Keeps track of whether the clock has glitched and an [_execute] is /// necessary. bool _pendingExecute = false; @@ -548,11 +593,10 @@ class Sequential extends _Always { } // listen to every clock glitch to see if we need to execute - for (var i = 0; i < _clks.length; i++) { - final clk = _clks[i]; - clk.glitch.listen((event) async { + for (final trigger in _triggers) { + trigger.signal.glitch.listen((event) async { // we want the first previousValue from the first glitch of this tick - _preTickClkValues[i] ??= event.previousValue; + trigger.preTickValue ??= event.previousValue; if (!_pendingExecute) { unawaited(Simulator.clkStable.first.then((value) { // once the clocks are stable, execute the contents of the FF @@ -581,27 +625,16 @@ class Sequential extends _Always { } void _execute() { - var anyClkInvalid = false; - var anyClkPosedge = false; + //TODO: test what if n'th triggered, n+1'th invalid, still treats as invalid - 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].value.isValid || !(_preTickClkValues[i]?.isValid ?? true)) { - anyClkInvalid = true; - break; - } else if (_preTickClkValues[i] != null && - LogicValue.isPosedge(_preTickClkValues[i]!, _clks[i].value)) { - anyClkPosedge = true; - break; - } - } + final anyTriggered = _triggers.any((t) => t.isTriggered); + final anyTriggerInvalid = _triggers.any((t) => !t.isValid); - if (anyClkInvalid) { + if (anyTriggerInvalid) { for (final receiverOutput in _assignedReceiverToOutputMap.values) { receiverOutput.put(LogicValue.x); } - } else if (anyClkPosedge) { + } else if (anyTriggered) { if (allowMultipleAssignments) { for (final element in conditionals) { element.execute(null, null); @@ -618,16 +651,18 @@ class Sequential extends _Always { } // clear out all the pre-tick value of clocks - for (var i = 0; i < _clks.length; i++) { - _preTickClkValues[i] = null; + for (final trigger in _triggers) { + trigger.preTickValue = null; } } @override String alwaysVerilogStatement(Map inputs) { - final triggers = - _clks.map((clk) => 'posedge ${inputs[clk.name]}').join(' or '); - return 'always_ff @($triggers)'; + final svTriggers = _triggers + .map((trigger) => + '${trigger.verilogTriggerKeyword} ${inputs[trigger.signal.name]}') + .join(' or '); + return 'always_ff @($svTriggers)'; } @override From 8aa53cd967220c641b29b1e47d0cae790a9a1cd2 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Wed, 17 Apr 2024 06:20:19 -0700 Subject: [PATCH 02/28] test for negedge trigger --- lib/src/modules/conditional.dart | 10 ++++----- test/sequential_test.dart | 36 +++++++++++++++++++++++++++++--- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart index 9e19b1bad..8df035b01 100644 --- a/lib/src/modules/conditional.dart +++ b/lib/src/modules/conditional.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2023 Intel Corporation +// Copyright (C) 2021-2024 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // conditional.dart @@ -497,13 +497,13 @@ class Sequential extends _Always { this.allowMultipleAssignments = true, List negedgeTriggers = const [], }) { - if (posedgeTriggers.isEmpty) { - throw IllegalConfigurationException('Must provide at least one clock.'); - } - _registerInputTriggers(posedgeTriggers, isPosedge: true); _registerInputTriggers(negedgeTriggers, isPosedge: false); + if (_triggers.isEmpty) { + throw IllegalConfigurationException('Must provide at least one clock.'); + } + _setup(); } diff --git a/test/sequential_test.dart b/test/sequential_test.dart index 834494c86..73baf475b 100644 --- a/test/sequential_test.dart +++ b/test/sequential_test.dart @@ -1,5 +1,5 @@ // SPDX-License-Identifier: BSD-3-Clause -// Copyright (C) 2021-2023 Intel Corporation +// Copyright (C) 2021-2024 Intel Corporation // // sequential_test.dart // Unit test for Sequential @@ -125,6 +125,20 @@ class SeqResetValTypes extends Module { } } +class NegedgeTriggeredSeq extends Module { + NegedgeTriggeredSeq(Logic a) { + a = addInput('a', a); + final b = addOutput('b'); + final clk = SimpleClockGenerator(10).clk; + + Sequential.multi( + [], + negedgeTriggers: [~clk], + [b < a], + ); + } +} + void main() { tearDown(() async { await Simulator.reset(); @@ -152,8 +166,7 @@ void main() { Vector({}, {'out': 5}), ]; await SimCompare.checkFunctionalVector(dut, vectors); - final simResult = SimCompare.iverilogVector(dut, vectors); - expect(simResult, equals(true)); + SimCompare.checkIverilogVector(dut, vectors); }); group('shorthand with sequential', () { @@ -208,4 +221,21 @@ void main() { await SimCompare.checkFunctionalVector(dut, vectors); SimCompare.checkIverilogVector(dut, vectors); }); + + test('negedge triggered flop', () async { + final mod = NegedgeTriggeredSeq(Logic()); + await mod.build(); + + final sv = mod.generateSynth(); + expect(sv, contains('always_ff @(negedge')); + + final vectors = [ + Vector({'a': 0}, {}), + Vector({'a': 1}, {'b': 0}), + Vector({'a': 0}, {'b': 1}), + ]; + + await SimCompare.checkFunctionalVector(mod, vectors); + SimCompare.checkIverilogVector(mod, vectors); + }); } From adb290919949c35f0c2906188f4ecb83af52a270 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Wed, 17 Apr 2024 06:27:37 -0700 Subject: [PATCH 03/28] add some extra tests --- lib/src/modules/conditional.dart | 2 -- test/sequential_test.dart | 35 ++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart index 8df035b01..b6ecd840f 100644 --- a/lib/src/modules/conditional.dart +++ b/lib/src/modules/conditional.dart @@ -625,8 +625,6 @@ class Sequential extends _Always { } void _execute() { - //TODO: test what if n'th triggered, n+1'th invalid, still treats as invalid - final anyTriggered = _triggers.any((t) => t.isTriggered); final anyTriggerInvalid = _triggers.any((t) => !t.isValid); diff --git a/test/sequential_test.dart b/test/sequential_test.dart index 73baf475b..20be814fb 100644 --- a/test/sequential_test.dart +++ b/test/sequential_test.dart @@ -238,4 +238,39 @@ void main() { await SimCompare.checkFunctionalVector(mod, vectors); SimCompare.checkIverilogVector(mod, vectors); }); + + test('negedge trigger actually occurs on negedge', () async { + final clk = Logic()..put(0); + + final a = Logic()..put(1); + final b = Logic(); + + Sequential.multi([], negedgeTriggers: [clk], [b < a]); + + Simulator.registerAction(20, () => clk.put(1)); + + Simulator.registerAction(30, () => expect(b.value, LogicValue.z)); + + Simulator.registerAction(40, () => clk.put(0)); + + Simulator.registerAction(50, () => expect(b.value, LogicValue.one)); + + await Simulator.run(); + }); + + test('invalid trigger after valid trigger still causes x prop', () async { + final c1 = Logic()..put(0); + final c2 = Logic()..put(LogicValue.x); + + final a = Logic()..put(1); + final b = Logic(); + + Sequential.multi([c1, c2], [b < a]); + + Simulator.registerAction(20, () => c1.put(1)); + + Simulator.registerAction(30, () => expect(b.value, LogicValue.x)); + + await Simulator.run(); + }); } From 67f94a7c38c10cbb5d5d23ab7beb41ddef31c4a8 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Wed, 17 Apr 2024 06:31:26 -0700 Subject: [PATCH 04/28] Test flop with both edge triggers --- test/sequential_test.dart | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/sequential_test.dart b/test/sequential_test.dart index 20be814fb..ade256cf3 100644 --- a/test/sequential_test.dart +++ b/test/sequential_test.dart @@ -139,6 +139,21 @@ class NegedgeTriggeredSeq extends Module { } } +class BothTriggeredSeq extends Module { + BothTriggeredSeq(Logic reset) { + reset = addInput('reset', reset); + final b = addOutput('b', width: 8); + final clk = SimpleClockGenerator(10).clk; + + Sequential.multi( + [clk], + reset: reset, + negedgeTriggers: [clk], + [b < b + 1], + ); + } +} + void main() { tearDown(() async { await Simulator.reset(); @@ -239,6 +254,23 @@ void main() { SimCompare.checkIverilogVector(mod, vectors); }); + test('multiple triggers, both edges', () async { + final mod = BothTriggeredSeq(Logic()); + await mod.build(); + + final vectors = [ + Vector({'reset': 1}, {}), + Vector({'reset': 1}, {'b': 0}), + Vector({'reset': 0}, {'b': 0}), + Vector({}, {'b': 2}), + Vector({}, {'b': 4}), + Vector({}, {'b': 6}), + ]; + + await SimCompare.checkFunctionalVector(mod, vectors); + SimCompare.checkIverilogVector(mod, vectors); + }); + test('negedge trigger actually occurs on negedge', () async { final clk = Logic()..put(0); From a5dc0de6717adb779b67e0380e47a5b3fb2bd6e1 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Tue, 4 Jun 2024 11:19:27 -0700 Subject: [PATCH 05/28] add notes to update pipeline and fsm --- lib/src/finite_state_machine.dart | 1 + lib/src/modules/pipeline.dart | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/src/finite_state_machine.dart b/lib/src/finite_state_machine.dart index 9847b288b..6c7c61cca 100644 --- a/lib/src/finite_state_machine.dart +++ b/lib/src/finite_state_machine.dart @@ -90,6 +90,7 @@ class FiniteStateMachine { List> states, ) : this.multi([clk], reset, resetState, states); + // TODO: allow FSM to support async reset /// Creates an finite state machine for the specified list of [_states], with /// an initial state of [resetState] (when synchronous [reset] is high) and /// transitions on positive edges of any of [_clks]. diff --git a/lib/src/modules/pipeline.dart b/lib/src/modules/pipeline.dart index 06231cca9..46dbfab80 100644 --- a/lib/src/modules/pipeline.dart +++ b/lib/src/modules/pipeline.dart @@ -154,6 +154,7 @@ class Pipeline { resetValues: resetValues, reset: reset); + // TODO: allow Pipeline to support async reset /// Constructs a [Pipeline] with multiple triggers on any of [_clks]. Pipeline.multi(this._clks, {List Function(PipelineStageInfo p)> stages = const [], From 166ab18d19c053d6e0f0cee37bd827649606c411 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Tue, 3 Dec 2024 13:03:59 -0800 Subject: [PATCH 06/28] added asyncReset flag to places, started with failing async reset test --- lib/src/finite_state_machine.dart | 15 +++++-- lib/src/modules/conditional.dart | 74 ++++++++++++++++++++++++------- lib/src/modules/pipeline.dart | 60 +++++++++++++++---------- test/net_bus_test.dart | 15 +++++++ test/pipeline_test.dart | 2 + 5 files changed, 122 insertions(+), 44 deletions(-) diff --git a/lib/src/finite_state_machine.dart b/lib/src/finite_state_machine.dart index 9847b288b..16f06697f 100644 --- a/lib/src/finite_state_machine.dart +++ b/lib/src/finite_state_machine.dart @@ -80,6 +80,9 @@ class FiniteStateMachine { /// Width of the state. final int _stateWidth; + /// If `true`, the [reset] signal is asynchronous. + final bool asyncReset; + /// Creates an finite state machine for the specified list of [_states], with /// an initial state of [resetState] (when synchronous [reset] is high) and /// transitions on positive [clk] edges. @@ -87,14 +90,18 @@ class FiniteStateMachine { Logic clk, Logic reset, StateIdentifier resetState, - List> states, - ) : this.multi([clk], reset, resetState, states); + List> states, { + bool asyncReset = false, + }) : this.multi([clk], reset, resetState, states, asyncReset: asyncReset); + + //TODO: test FSMs with async reset /// Creates an finite state machine for the specified list of [_states], with /// an initial state of [resetState] (when synchronous [reset] is high) and /// transitions on positive edges of any of [_clks]. FiniteStateMachine.multi( - this._clks, this.reset, this.resetState, this._states) + this._clks, this.reset, this.resetState, this._states, + {this.asyncReset = false}) : _stateWidth = _logBase(_states.length, 2), currentState = Logic(name: 'currentState', width: _logBase(_states.length, 2)), @@ -147,7 +154,7 @@ class FiniteStateMachine { ]) ]); - Sequential.multi(_clks, reset: reset, resetValues: { + Sequential.multi(_clks, reset: reset, asyncReset: asyncReset, resetValues: { currentState: _stateValueLookup[_stateLookup[resetState]] }, [ currentState < nextState, diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart index bf9d5d880..0999d98f5 100644 --- a/lib/src/modules/conditional.dart +++ b/lib/src/modules/conditional.dart @@ -433,20 +433,29 @@ class Sequential extends _Always { /// [Sequential]. final bool allowMultipleAssignments; + /// Indicates whether provided `reset` signals should be treated as an async + /// reset. If no `reset` is provided, this will have no effect. + final bool asyncReset; + /// Constructs a [Sequential] single-triggered by [clk]. /// /// If `reset` is provided, then all signals driven by this block will be - /// conditionally reset when the signal is high. - /// The default reset value is to `0`, but if `resetValues` is provided then - /// the corresponding value associated with the driven signal will be set to - /// 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. + /// conditionally reset when the signal is high. The default reset value is to + /// `0`, but if `resetValues` is provided then the corresponding value + /// associated with the driven signal will be set to 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. + /// + /// If [asyncReset] is true, the [reset] signal (if provided) will be treated + /// as an async reset. If [asyncReset] is false, the reset signal will be + /// treated as synchronous. Sequential( Logic clk, List conditionals, { Logic? reset, Map? resetValues, + bool asyncReset = false, bool allowMultipleAssignments = true, String name = 'sequential', }) : this.multi( @@ -454,6 +463,7 @@ class Sequential extends _Always { conditionals, name: name, reset: reset, + asyncReset: asyncReset, resetValues: resetValues, allowMultipleAssignments: allowMultipleAssignments, ); @@ -461,20 +471,25 @@ class Sequential extends _Always { /// Constructs a [Sequential] multi-triggered by any of [clks]. /// /// If `reset` is provided, then all signals driven by this block will be - /// conditionally reset when the signal is high. - /// The default reset value is to `0`, but if `resetValues` is provided then - /// the corresponding value associated with the driven signal will be set to - /// 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. + /// conditionally reset when the signal is high. The default reset value is to + /// `0`, but if `resetValues` is provided then the corresponding value + /// associated with the driven signal will be set to 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. + /// + /// If [asyncReset] is true, the [reset] signal (if provided) will be treated + /// as an async reset. If [asyncReset] is false, the reset signal will be + /// treated as synchronous. Sequential.multi( List clks, super._conditionals, { - super.reset, + Logic? reset, super.resetValues, + this.asyncReset = false, super.name = 'sequential', this.allowMultipleAssignments = true, - }) { + }) : super(reset: reset) { if (clks.isEmpty) { throw IllegalConfigurationException('Must provide at least one clock.'); } @@ -491,6 +506,12 @@ class Sequential extends _Always { clk)); _preTickClkValues.add(null); } + + if (reset != null) { + _clks.add(_assignedDriverToInputMap[reset]!); + _preTickClkValues.add(null); + } + _setup(); } @@ -1700,6 +1721,10 @@ class FlipFlop extends Module with SystemVerilog { /// Only initialized if a constant value is provided. late LogicValue _resetValueConst; + /// Indicates whether provided `reset` signals should be treated as an async + /// reset. If no `reset` is provided, this will have no effect. + final bool asyncReset; + /// Constructs a flip flop which is positive edge triggered on [clk]. /// /// When optional [en] is provided, an additional input will be created for @@ -1711,12 +1736,17 @@ class FlipFlop extends Module with SystemVerilog { /// it will reset to the provided [resetValue]. The type of [resetValue] must /// be a valid driver of a [ConditionalAssign] (e.g. [Logic], [LogicValue], /// [int], etc.). + /// + /// If [asyncReset] is true, the [reset] signal (if provided) will be treated + /// as an async reset. If [asyncReset] is false, the reset signal will be + /// treated as synchronous. FlipFlop( Logic clk, Logic d, { Logic? en, Logic? reset, dynamic resetValue, + this.asyncReset = false, super.name = 'flipflop', }) { if (clk.width != 1) { @@ -1756,6 +1786,7 @@ class FlipFlop extends Module with SystemVerilog { _clk, contents, reset: _reset, + asyncReset: asyncReset, resetValues: _reset != null ? {q: _resetValuePort ?? _resetValueConst} : null, ); @@ -1781,22 +1812,31 @@ class FlipFlop extends Module with SystemVerilog { final clk = ports[_clkName]!; final d = ports[_dName]!; final q = ports[_qName]!; + final en = _en != null ? ports[_enName]! : null; + final reset = _reset != null ? ports[_resetName]! : null; - final svBuffer = StringBuffer('always_ff @(posedge $clk) '); + final triggerString = [ + clk, + if (reset != null) reset, + ].map((e) => 'posedge $e').join(' or '); + + final svBuffer = StringBuffer('always_ff @($triggerString) '); if (_reset != null) { final resetValueString = _resetValuePort != null ? ports[_resetValueName]! : _resetValueConst.toString(); - svBuffer.write('if(${ports[_resetName]}) $q <= $resetValueString; else '); + svBuffer.write('if(${reset!}) $q <= $resetValueString; else '); } if (_en != null) { - svBuffer.write('if(${ports[_enName]!}) '); + svBuffer.write('if(${en!}) '); } svBuffer.write('$q <= $d; // $instanceName'); return svBuffer.toString(); } + + //TODO: test sequential, sequential.multi, flipflop, and flop all! SV too! } diff --git a/lib/src/modules/pipeline.dart b/lib/src/modules/pipeline.dart index 06231cca9..591b306ea 100644 --- a/lib/src/modules/pipeline.dart +++ b/lib/src/modules/pipeline.dart @@ -94,6 +94,9 @@ class Pipeline { /// An optional reset signal for all pipelined signals. final Logic? reset; + /// If `true`, then [reset] is asynchronous, if provided. + final bool asyncReset; + /// All the [_PipeStage]s for this [Pipeline] late final List<_PipeStage> _stages; @@ -146,13 +149,15 @@ class Pipeline { List? stalls, List signals = const [], Map resetValues = const {}, - Logic? reset}) + Logic? reset, + bool asyncReset = false}) : this.multi([clk], stages: stages, stalls: stalls, signals: signals, resetValues: resetValues, - reset: reset); + reset: reset, + asyncReset: asyncReset); /// Constructs a [Pipeline] with multiple triggers on any of [_clks]. Pipeline.multi(this._clks, @@ -160,7 +165,8 @@ class Pipeline { List? stalls, List signals = const [], Map resetValues = const {}, - this.reset}) { + this.reset, + this.asyncReset = false}) { _stages = stages.map(_PipeStage.new).toList(); _stages.add(_PipeStage((p) => [])); // output stage @@ -230,10 +236,10 @@ class Pipeline { /// Adds a new signal to be pipelined across all stages. void _add(Logic newLogic) { - dynamic resetValue; - if (_resetValues.containsKey(newLogic)) { - resetValue = _resetValues[newLogic]; - } + // dynamic resetValue; + // if (_resetValues.containsKey(newLogic)) { + // resetValue = _resetValues[newLogic]; + // } for (var i = 0; i < _stages.length; i++) { _stages[i]._addLogic(newLogic, i); @@ -245,7 +251,7 @@ class Pipeline { ffAssigns.add(_i(newLogic, i) < _o(newLogic, i - 1)); } - var ffAssignsWithStall = + final ffAssignsWithStall = List.generate(stageCount - 1, (index) { final stall = _stages[index].stall; final ffAssign = ffAssigns[index] as ConditionalAssign; @@ -255,21 +261,29 @@ class Pipeline { return ffAssign.receiver < driver; }); - if (reset != null) { - ffAssignsWithStall = [ - If.block([ - Iff( - reset!, - ffAssigns.map((conditional) { - conditional as ConditionalAssign; - return conditional.receiver < (resetValue ?? 0); - }).toList(growable: false), - ), - Else(ffAssignsWithStall) - ]) - ]; - } - Sequential.multi(_clks, ffAssignsWithStall, name: 'ff_${newLogic.name}'); + //TODO: can we just delete this?? + // if (reset != null) { + // ffAssignsWithStall = [ + // If.block([ + // Iff( + // reset!, + // ffAssigns.map((conditional) { + // conditional as ConditionalAssign; + // return conditional.receiver < (resetValue ?? 0); + // }).toList(growable: false), + // ), + // Else(ffAssignsWithStall) + // ]) + // ]; + // } + + Sequential.multi( + _clks, + reset: reset, + resetValues: _resetValues, + asyncReset: asyncReset, + ffAssignsWithStall, + name: 'ff_${newLogic.name}'); } /// The stage input for a signal associated with [logic] to [stageIndex]. diff --git a/test/net_bus_test.dart b/test/net_bus_test.dart index bfaa917c8..773c27b79 100644 --- a/test/net_bus_test.dart +++ b/test/net_bus_test.dart @@ -293,6 +293,21 @@ void main() { expect(a.value, LogicValue.zero); }); + test('circular blasted connection', () { + final a = LogicNet(width: 8); + + final b = a.getRange(0, 4); + + final c = [b, b].swizzle(); + + a <= c; + c <= a; + + a.put(0x33); + + expect(c.value.toInt(), 0x33); + }); + group('simple', () { test('double passthrough', () async { final dut = DoubleNetPassthrough(LogicNet(width: 8), LogicNet(width: 8)); diff --git a/test/pipeline_test.dart b/test/pipeline_test.dart index f00e28bfa..a70d7815b 100644 --- a/test/pipeline_test.dart +++ b/test/pipeline_test.dart @@ -202,6 +202,8 @@ void main() { final pipem = SimplePipelineModule(Logic(width: 8)); await pipem.build(); + //TODO: test that reset works! + //TODO: test that reset value works! final vectors = [ Vector({'a': 1}, {}), Vector({'a': 2}, {}), From 9904eb96c493deca1ad3b94d189ff46f8fc2da42 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Tue, 3 Dec 2024 14:45:48 -0800 Subject: [PATCH 07/28] merge works with async and negedge --- lib/src/modules/conditional.dart | 15 +++++++------ test/async_reset_test.dart | 36 ++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 8 deletions(-) create mode 100644 test/async_reset_test.dart diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart index 35633facb..5e34864a0 100644 --- a/lib/src/modules/conditional.dart +++ b/lib/src/modules/conditional.dart @@ -527,18 +527,17 @@ class Sequential extends _Always { super.name = 'sequential', this.allowMultipleAssignments = true, List negedgeTriggers = const [], - }) { + }) : super(reset: reset) { _registerInputTriggers(posedgeTriggers, isPosedge: true); _registerInputTriggers(negedgeTriggers, isPosedge: false); - if (_triggers.isEmpty) { - throw IllegalConfigurationException('Must provide at least one clock.'); + if (reset != null && asyncReset) { + _triggers.add(_SequentialTrigger(_assignedDriverToInputMap[reset]!, + isPosedge: true)); } - //TODO: trash cleanup - if (reset != null) { - _clks.add(_assignedDriverToInputMap[reset]!); - _preTickClkValues.add(null); + if (_triggers.isEmpty) { + throw IllegalConfigurationException('Must provide at least one trigger.'); } _setup(); @@ -558,7 +557,7 @@ class Sequential extends _Always { addInput( _portUniquifier.getUniqueName( initialName: Sanitizer.sanitizeSV( - Naming.unpreferredName('clk${i}_${trigger.name}'))), + Naming.unpreferredName('trigger${i}_${trigger.name}'))), trigger), isPosedge: isPosedge)); } diff --git a/test/async_reset_test.dart b/test/async_reset_test.dart new file mode 100644 index 000000000..c23a6bb95 --- /dev/null +++ b/test/async_reset_test.dart @@ -0,0 +1,36 @@ +import 'package:rohd/rohd.dart'; +import 'package:test/test.dart'; + +void main() { + test('async reset samples correct reset value', () async { + final clk = Logic(); + final reset = Logic(); + final val = Logic(); + + reset.inject(0); + clk.inject(0); + + //TODO: try different ways to define the reset as async + Sequential.multi( + [clk, reset], + reset: reset, + [ + val < 1, + ], + ); + + Simulator.registerAction(10, () { + clk.inject(1); + }); + + Simulator.registerAction(14, () { + reset.inject(1); + }); + + Simulator.registerAction(15, () { + expect(val.value.toInt(), 0); + }); + + await Simulator.run(); + }); +} From f7cfa69a11c0d1daf6e9d14678c68c1f1742c463 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Wed, 4 Dec 2024 10:48:39 -0800 Subject: [PATCH 08/28] async reset sampling works --- lib/src/modules/conditional.dart | 41 +++++++++++++++++++++----------- test/async_reset_test.dart | 12 ++++++---- 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart index 5e34864a0..a01a04e29 100644 --- a/lib/src/modules/conditional.dart +++ b/lib/src/modules/conditional.dart @@ -25,7 +25,7 @@ abstract class _Always extends Module with SystemVerilog { /// A [List] of the [Conditional]s to execute. List get conditionals => UnmodifiableListView(_conditionals); - late List _conditionals; + List _conditionals; /// A mapping from internal receiver signals to designated [Module] outputs. final Map _assignedReceiverToOutputMap = @@ -528,14 +528,12 @@ class Sequential extends _Always { this.allowMultipleAssignments = true, List negedgeTriggers = const [], }) : super(reset: reset) { - _registerInputTriggers(posedgeTriggers, isPosedge: true); + _registerInputTriggers([ + ...posedgeTriggers, + if (reset != null && asyncReset) reset, + ], isPosedge: true); _registerInputTriggers(negedgeTriggers, isPosedge: false); - if (reset != null && asyncReset) { - _triggers.add(_SequentialTrigger(_assignedDriverToInputMap[reset]!, - isPosedge: true)); - } - if (_triggers.isEmpty) { throw IllegalConfigurationException('Must provide at least one trigger.'); } @@ -553,6 +551,10 @@ class Sequential extends _Always { throw Exception('Each clk or trigger must be 1 bit, but saw $trigger.'); } + if (_assignedDriverToInputMap.containsKey(trigger)) { + _driverInputsThatAreTriggers.add(_assignedDriverToInputMap[trigger]!); + } + _triggers.add(_SequentialTrigger( addInput( _portUniquifier.getUniqueName( @@ -579,6 +581,19 @@ class Sequential extends _Always { /// Keeps track of whether values need to be updated post-tick. bool _pendingPostUpdate = false; + /// All [input]s which are also triggers. + final Set _driverInputsThatAreTriggers = {}; + + /// Updates the [_inputToPreTickInputValuesMap], if appropriate. + void _updateInputToPreTickInputValue(Logic driverInput) { + if (_driverInputsThatAreTriggers.contains(driverInput)) { + // triggers should be sampled at the new value, not the previous value + return; + } + + _inputToPreTickInputValuesMap[driverInput] = driverInput.value; + } + /// Performs setup steps for custom functional behavior of this block. void _setup() { // one time is enough, it's a final map @@ -590,13 +605,13 @@ class Sequential extends _Always { for (final driverInput in _assignedDriverToInputMap.values) { // pre-fill the _inputToPreTickInputValuesMap so that nothing ever // uses values directly - _inputToPreTickInputValuesMap[driverInput] = driverInput.value; + _updateInputToPreTickInputValue(driverInput); driverInput.glitch.listen((event) async { if (Simulator.phase != SimulatorPhase.clkStable) { // if the change happens not when the clocks are stable, immediately // update the map - _inputToPreTickInputValuesMap[driverInput] = driverInput.value; + _updateInputToPreTickInputValue(driverInput); } else { // if this is during stable clocks, it's probably another flop // driving it, so hold onto it for later @@ -607,11 +622,9 @@ class Sequential extends _Always { (value) { // once the tick has completed, // we can update the override maps - for (final driverInput in _driverInputsPendingPostUpdate) { - _inputToPreTickInputValuesMap[driverInput] = - driverInput.value; - } - _driverInputsPendingPostUpdate.clear(); + _driverInputsPendingPostUpdate + ..forEach(_updateInputToPreTickInputValue) + ..clear(); _pendingPostUpdate = false; }, ).catchError( diff --git a/test/async_reset_test.dart b/test/async_reset_test.dart index c23a6bb95..8f6bc98b0 100644 --- a/test/async_reset_test.dart +++ b/test/async_reset_test.dart @@ -3,9 +3,11 @@ import 'package:test/test.dart'; void main() { test('async reset samples correct reset value', () async { - final clk = Logic(); - final reset = Logic(); - final val = Logic(); + final clk = Logic(name: 'clk'); + final reset = Logic(name: 'reset'); + final val = Logic(name: 'val'); + + reset.glitch.listen((x) => print('reset: $x')); reset.inject(0); clk.inject(0); @@ -20,11 +22,11 @@ void main() { ); Simulator.registerAction(10, () { - clk.inject(1); + clk.put(1); }); Simulator.registerAction(14, () { - reset.inject(1); + reset.put(1); }); Simulator.registerAction(15, () { From 7d3db7b77867a9256594de7aef1a7277a6e2a14d Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Wed, 4 Dec 2024 11:02:32 -0800 Subject: [PATCH 09/28] test different asyncreset mechanisms work --- lib/src/modules/conditional.dart | 6 ++ test/async_reset_test.dart | 103 +++++++++++++++++++++---------- 2 files changed, 77 insertions(+), 32 deletions(-) diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart index a01a04e29..52cf2c893 100644 --- a/lib/src/modules/conditional.dart +++ b/lib/src/modules/conditional.dart @@ -1705,12 +1705,17 @@ ${padding}end '''); /// When the optional [reset] is provided, the flop will be reset (active-high). /// If no [resetValue] is provided, the reset value is always `0`. Otherwise, /// it will reset to the provided [resetValue]. +/// +/// If [asyncReset] is true, the [reset] signal (if provided) will be treated +/// as an async reset. If [asyncReset] is false, the reset signal will be +/// treated as synchronous. Logic flop( Logic clk, Logic d, { Logic? en, Logic? reset, dynamic resetValue, + bool asyncReset = false, }) => FlipFlop( clk, @@ -1718,6 +1723,7 @@ Logic flop( en: en, reset: reset, resetValue: resetValue, + asyncReset: asyncReset, ).q; /// Represents a single flip-flop with no reset. diff --git a/test/async_reset_test.dart b/test/async_reset_test.dart index 8f6bc98b0..2875974f9 100644 --- a/test/async_reset_test.dart +++ b/test/async_reset_test.dart @@ -2,37 +2,76 @@ import 'package:rohd/rohd.dart'; import 'package:test/test.dart'; void main() { - test('async reset samples correct reset value', () async { - final clk = Logic(name: 'clk'); - final reset = Logic(name: 'reset'); - final val = Logic(name: 'val'); - - reset.glitch.listen((x) => print('reset: $x')); - - reset.inject(0); - clk.inject(0); - - //TODO: try different ways to define the reset as async - Sequential.multi( - [clk, reset], - reset: reset, - [ - val < 1, - ], - ); - - Simulator.registerAction(10, () { - clk.put(1); - }); - - Simulator.registerAction(14, () { - reset.put(1); - }); - - Simulator.registerAction(15, () { - expect(val.value.toInt(), 0); - }); - - await Simulator.run(); + tearDown(() async { + await Simulator.reset(); }); + + group('async reset samples correct reset value', () { + final seqMechanism = { + 'Sequential.multi': (Logic clk, Logic reset, Logic val) => + Sequential.multi( + [clk, reset], + reset: reset, + [ + val < 1, + ], + ), + 'Sequential with asyncReset': (Logic clk, Logic reset, Logic val) => + Sequential( + clk, + reset: reset, + [ + val < 1, + ], + asyncReset: true, + ), + 'FlipFlop with asyncReset': (Logic clk, Logic reset, Logic val) { + val <= + FlipFlop( + clk, + reset: reset, + val, + asyncReset: true, + ).q; + }, + 'flop with asyncReset': (Logic clk, Logic reset, Logic val) { + val <= + flop( + clk, + reset: reset, + val, + asyncReset: true, + ); + }, + }; + + for (final mechanism in seqMechanism.entries) { + test('using ${mechanism.key}', () async { + final clk = Logic(name: 'clk'); + final reset = Logic(name: 'reset'); + final val = Logic(name: 'val'); + + reset.inject(0); + clk.inject(0); + + mechanism.value(clk, reset, val); + + Simulator.registerAction(10, () { + clk.put(1); + }); + + Simulator.registerAction(14, () { + reset.put(1); + }); + + Simulator.registerAction(15, () { + expect(val.value.toInt(), 0); + }); + + await Simulator.run(); + }); + } + }); + + //TODO: test async reset with clocks too } From 6cf153c0ba95c34f2b72c7fd17a97ed19ae172d9 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Thu, 5 Dec 2024 09:23:05 -0800 Subject: [PATCH 10/28] test for non-identical trigger --- .../name/invalid_reserved_name_exception.dart | 1 + test/async_reset_test.dart | 40 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/lib/src/exceptions/name/invalid_reserved_name_exception.dart b/lib/src/exceptions/name/invalid_reserved_name_exception.dart index ae7e2493b..f908468d7 100644 --- a/lib/src/exceptions/name/invalid_reserved_name_exception.dart +++ b/lib/src/exceptions/name/invalid_reserved_name_exception.dart @@ -18,4 +18,5 @@ class InvalidReservedNameException extends RohdException { [super.message = 'Reserved Name need to follow proper naming ' 'convention if reserved' ' name set to true']); + //TODO: make this error message better (what's the name, fix grammar) } diff --git a/test/async_reset_test.dart b/test/async_reset_test.dart index 2875974f9..d3147abb9 100644 --- a/test/async_reset_test.dart +++ b/test/async_reset_test.dart @@ -1,6 +1,26 @@ import 'package:rohd/rohd.dart'; +import 'package:rohd/src/utilities/simcompare.dart'; import 'package:test/test.dart'; +class NonIdenticalTriggerSeq extends Module { + NonIdenticalTriggerSeq(Logic trigger) { + final clk = Logic(name: 'clk'); + trigger = addInput('trigger', trigger); + + final innerTrigger = Logic(name: 'innerTrigger', naming: Naming.reserved); + innerTrigger <= trigger; + + final result = addOutput('result'); + + Sequential.multi([ + clk, + innerTrigger + ], [ + result < trigger, + ]); + } +} + void main() { tearDown(() async { await Simulator.reset(); @@ -45,6 +65,11 @@ void main() { }, }; + //TODO: what if there's another (connected) version of that signal that's the trigger?? but not exactly the same logic? + //TODO: how to deal with injects that trigger edges?? + + //TODO: doc clearly the behavior of sampling async triggersl + for (final mechanism in seqMechanism.entries) { test('using ${mechanism.key}', () async { final clk = Logic(name: 'clk'); @@ -73,5 +98,20 @@ void main() { } }); + test('non-identical signal trigger', () async { + final mod = NonIdenticalTriggerSeq(Logic()); + + await mod.build(); + + final vectors = [ + Vector({'trigger': 0}, {}), + Vector({'trigger': 1}, {'result': 1}), + ]; + + // await SimCompare.checkFunctionalVector(mod, vectors); //TODO fix + SimCompare.checkIverilogVector(mod, vectors, + dontDeleteTmpFiles: true, dumpWaves: true); + }); + //TODO: test async reset with clocks too } From 3b7f4e527e3b38d7f6e5d45b2ea5a4dc40c29601 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Mon, 9 Dec 2024 17:08:41 -0800 Subject: [PATCH 11/28] wip async checks --- lib/src/modules/conditional.dart | 10 ++ lib/src/simulator.dart | 6 + test/async_reset_test.dart | 207 ++++++++++++++++++++++++++++--- test/changed_test.dart | 26 ++++ 4 files changed, 235 insertions(+), 14 deletions(-) diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart index 52cf2c893..fb9296bc6 100644 --- a/lib/src/modules/conditional.dart +++ b/lib/src/modules/conditional.dart @@ -601,6 +601,13 @@ class Sequential extends _Always { element._updateOverrideMap(_inputToPreTickInputValuesMap); } + // TODO: Maybe the right recipe: + // - keep track of WHAT triggered this flop each time + // - if any other incoming signal changes (trigger or otherwise), then that's BAD (race condition) + // - actually, multiple triggers are ok? that's fine to evaluate? + // Ok, another attempt: + // - if a trigger AND a non-trigger toggle when NOT clksStable + // listen to every input of this `Sequential` for changes for (final driverInput in _assignedDriverToInputMap.values) { // pre-fill the _inputToPreTickInputValuesMap so that nothing ever @@ -612,6 +619,7 @@ class Sequential extends _Always { // if the change happens not when the clocks are stable, immediately // update the map _updateInputToPreTickInputValue(driverInput); + print('non-clock stable change: ${driverInput.name} $event'); } else { // if this is during stable clocks, it's probably another flop // driving it, so hold onto it for later @@ -646,6 +654,8 @@ class Sequential extends _Always { trigger.signal.glitch.listen((event) async { // we want the first previousValue from the first glitch of this tick trigger.preTickValue ??= event.previousValue; + print( + 'trigger glitched: ${trigger.signal.name} $event ${Simulator.phase}'); if (!_pendingExecute) { unawaited(Simulator.clkStable.first.then((value) { // once the clocks are stable, execute the contents of the FF diff --git a/lib/src/simulator.dart b/lib/src/simulator.dart index 64476c903..74c5b94ef 100644 --- a/lib/src/simulator.dart +++ b/lib/src/simulator.dart @@ -327,6 +327,12 @@ abstract class Simulator { while (_pendingList.isNotEmpty) { await _pendingList.removeFirst()(); } + + //TODO: is this appropriate to do? if so, make it a function! + while (_injectedActions.isNotEmpty) { + final injectedFunction = _injectedActions.removeFirst(); + await injectedFunction(); + } } /// Executes the clkStable phase diff --git a/test/async_reset_test.dart b/test/async_reset_test.dart index d3147abb9..4b992c2e2 100644 --- a/test/async_reset_test.dart +++ b/test/async_reset_test.dart @@ -1,22 +1,56 @@ +import 'dart:async'; + import 'package:rohd/rohd.dart'; import 'package:rohd/src/utilities/simcompare.dart'; import 'package:test/test.dart'; class NonIdenticalTriggerSeq extends Module { - NonIdenticalTriggerSeq(Logic trigger) { + /// If [triggerAfterSampledUpdate] is `true`, then the trigger for the + /// sequential block happens *afer* the signal being sampled updates. If + /// [triggerAfterSampledUpdate] is `false`, then the trigger for the + /// sequential block happens *before* the signal being sampled updates. + NonIdenticalTriggerSeq( + Logic trigger, { + bool invert = false, + bool triggerAfterSampledUpdate = true, + }) { final clk = Logic(name: 'clk'); trigger = addInput('trigger', trigger); final innerTrigger = Logic(name: 'innerTrigger', naming: Naming.reserved); - innerTrigger <= trigger; + innerTrigger <= (invert ? ~trigger : trigger); final result = addOutput('result'); Sequential.multi([ clk, - innerTrigger + if (triggerAfterSampledUpdate) innerTrigger else trigger, ], [ - result < trigger, + result < (triggerAfterSampledUpdate ? trigger : innerTrigger), + ]); + } +} + +class MultipleTriggerSeq extends Module { + MultipleTriggerSeq(Logic trigger1, Logic trigger2) { + final clk = Logic(); //SimpleClockGenerator(10).clk; + trigger1 = addInput('trigger1', trigger1); + trigger2 = addInput('trigger2', trigger2); + + final result = addOutput('result', width: 8); + + Sequential.multi([ + clk, + trigger1, + trigger2 + ], [ + If.block([ + Iff(trigger1 & ~trigger2, [result < 0xa]), + ElseIf(~trigger1 & trigger2, [result < 0xb]), + // ElseIf(trigger1 & trigger2, [result < 0xc]), + ElseIf(~trigger1 & ~trigger2, [result < 0xd]), + ]), + // result < (trigger1 & trigger2), ]); } } @@ -98,20 +132,165 @@ void main() { } }); - test('non-identical signal trigger', () async { - final mod = NonIdenticalTriggerSeq(Logic()); + test('async reset triggered via injection after clk edge still triggers', + () async { + final clk = SimpleClockGenerator(10).clk; + final reset = Logic(name: 'reset')..inject(0); + final val = Logic(name: 'val'); + + Sequential(clk, reset: reset, asyncReset: true, [ + val < 1, + ]); + + Simulator.setMaxSimTime(1000); + unawaited(Simulator.run()); + + await clk.nextPosedge; + await clk.nextPosedge; + // reset.inject(1); //TODO + Simulator.injectAction(() { + // print('asdf1'); + reset.put(1); + }); + + Simulator.registerAction(Simulator.time + 1, () { + // print('asdf'); + expect(val.value.toInt(), 0); + }); + + // one more edge so sim doesnt end immediately + await clk.nextPosedge; + + await Simulator.endSimulation(); + }); + + group('non-identical signal trigger', () { + test('normal', () async { + final mod = NonIdenticalTriggerSeq(Logic()); + + await mod.build(); + + final vectors = [ + Vector({'trigger': 0}, {}), + Vector({'trigger': 1}, {'result': 1}), + ]; + + await SimCompare.checkFunctionalVector(mod, vectors); //TODO fix + SimCompare.checkIverilogVector(mod, vectors); + }); + + test('inverted', () async { + final mod = NonIdenticalTriggerSeq(Logic(), invert: true); + + await mod.build(); + + final vectors = [ + Vector({'trigger': 1}, {}), + Vector({'trigger': 0}, {'result': 0}), + ]; - await mod.build(); + // await SimCompare.checkFunctionalVector(mod, vectors); //TODO fix + SimCompare.checkIverilogVector(mod, vectors); + }); - final vectors = [ - Vector({'trigger': 0}, {}), - Vector({'trigger': 1}, {'result': 1}), - ]; + test('trigger earlier inverted', () async { + final mod = NonIdenticalTriggerSeq(Logic(), + invert: true, triggerAfterSampledUpdate: false); - // await SimCompare.checkFunctionalVector(mod, vectors); //TODO fix - SimCompare.checkIverilogVector(mod, vectors, - dontDeleteTmpFiles: true, dumpWaves: true); + await mod.build(); + + final vectors = [ + Vector({'trigger': 0}, {}), + // in this case, the trigger happened before the sampled value updated + Vector({'trigger': 1}, {'result': 1}), + ]; + + // await SimCompare.checkFunctionalVector(mod, vectors); //TODO fix + SimCompare.checkIverilogVector(mod, vectors); + }); + + test('trigger earlier normal', () async { + final mod = + NonIdenticalTriggerSeq(Logic(), triggerAfterSampledUpdate: false); + + await mod.build(); + + final vectors = [ + Vector({'trigger': 0}, {}), + // in this case, the two signals are "identical", so there is no "later" + Vector({'trigger': 1}, {'result': 1}), + ]; + + // await SimCompare.checkFunctionalVector(mod, vectors); //TODO fix + SimCompare.checkIverilogVector(mod, vectors); + }); }); //TODO: test async reset with clocks too + + //TODO: test if registerAction (same tick) with clk and reset both edges at the same time + // what should that even do?? error?? what if two signals other than clk (like different resets) + // that need to be mutually exclusive happen? + // always@(clk, r1, r2) if(r1 & ~r2) ... if(r2 & ~r1) ... if(r1 & r2) ... if(~r1 & ~r2) ... + + group('multiple trigger races', () { + test('two resets simulatenously', () async { + final mod = MultipleTriggerSeq(Logic(), Logic()); + + await mod.build(); + + final vectors = [ + Vector({'trigger1': 0, 'trigger2': 0}, {}), + // Vector({'trigger1': 0, 'trigger2': 0}, {'result': 0xd}), + Vector({'trigger1': 1, 'trigger2': 1}, {}), + Vector({'trigger1': 1, 'trigger2': 1}, {'result': 0xd}), + ]; + + //TODO fix, should this fail? + // await SimCompare.checkFunctionalVector(mod, vectors); + SimCompare.checkIverilogVector(mod, vectors); + }); + + test('one then another trigger post-changed', () async { + final a = Logic()..put(0); + final b = Logic()..put(0); + + final result = Logic(width: 8); + + Sequential.multi([ + a, + b + ], [ + If.block([ + Iff(a & ~b, [result < 0xa]), + ElseIf(~a & b, [result < 0xb]), + Else([result < 0xc]), + ]), + ]); + + Simulator.registerAction(10, () { + a.put(1); + }); + + a.changed.listen((_) { + Simulator.injectAction(() { + a.put(0); + b.put(1); + }); + }); + + final seenValues = []; + + result.changed.listen((_) { + expect(Simulator.time, 10); + seenValues.add(result.value); + }); + + await Simulator.run(); + + expect(seenValues.length, 2); + expect(seenValues[0].toInt(), 0xa); + expect(seenValues[1].toInt(), 0xb); + }); + }); } diff --git a/test/changed_test.dart b/test/changed_test.dart index 808d59403..89e11de57 100644 --- a/test/changed_test.dart +++ b/test/changed_test.dart @@ -150,6 +150,32 @@ void main() { expect(numPosedges, equals(1)); }); + test('injection can trigger multiple changed events if post tick', () async { + final a = Logic(width: 8)..put(0); + + Simulator.registerAction(10, () { + a + ..put(1) + ..inject(2); + }); + + a.changed.listen((_) { + a.inject(a.value.toInt() | (1 << 4)); + }); + + final seenValues = []; + + a.changed.listen((_) { + seenValues.add(a.value); + }); + + await Simulator.run(); + + expect(seenValues.length, 2); + expect(seenValues[0].toInt(), 2); + expect(seenValues[1].toInt(), 2 | (1 << 4)); + }); + group('injection triggers flop', () { Future injectionTriggersFlop({required bool useArrays}) async { final baseClk = SimpleClockGenerator(10).clk; From e3663a9387a4069422fa14e1a4d5deb442472097 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Tue, 10 Dec 2024 15:13:29 -0800 Subject: [PATCH 12/28] check is printing at maybe the right time --- lib/src/modules/conditional.dart | 108 ++++++++++++++++++++++++++++--- test/async_reset_test.dart | 8 +-- 2 files changed, 104 insertions(+), 12 deletions(-) diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart index fb9296bc6..65db12f5e 100644 --- a/lib/src/modules/conditional.dart +++ b/lib/src/modules/conditional.dart @@ -453,6 +453,37 @@ class _SequentialTrigger { /// The SystemVerilog keyword for this trigger. String get verilogTriggerKeyword => isPosedge ? 'posedge' : 'negedge'; + + @override + String toString() => '@$verilogTriggerKeyword ${signal.name}'; +} + +class _SequentialTriggerRaceTracker { + bool _triggerOccurred = false; + bool _nonTriggerOccurred = false; + bool _registeredPostTick = false; + + void triggered() { + _triggerOccurred = true; + _registerPostTick(); + } + + void nonTriggered() { + _nonTriggerOccurred = true; + _registerPostTick(); + } + + bool get isInViolation => _triggerOccurred && _nonTriggerOccurred; + + void _registerPostTick() async { + unawaited(Simulator.postTick.first.then((value) { + _registeredPostTick = false; + _triggerOccurred = false; + _nonTriggerOccurred = false; + })); + + _registeredPostTick = true; + } } /// Represents a block of sequential logic. @@ -472,7 +503,6 @@ class Sequential extends _Always { /// reset. If no `reset` is provided, this will have no effect. final bool asyncReset; - /// Constructs a [Sequential] single-triggered by [clk]. /// Constructs a [Sequential] single-triggered by the positive edge of [clk]. /// /// If `reset` is provided, then all signals driven by this block will be @@ -585,15 +615,23 @@ class Sequential extends _Always { final Set _driverInputsThatAreTriggers = {}; /// Updates the [_inputToPreTickInputValuesMap], if appropriate. - void _updateInputToPreTickInputValue(Logic driverInput) { + /// + /// Returns `true` only if the map was updated. If `false`, then the input + /// was a trigger. + bool _updateInputToPreTickInputValue(Logic driverInput) { if (_driverInputsThatAreTriggers.contains(driverInput)) { // triggers should be sampled at the new value, not the previous value - return; + return false; } _inputToPreTickInputValuesMap[driverInput] = driverInput.value; + return true; } + //TODO: doc, and is this the right line to put this? + final _SequentialTriggerRaceTracker _raceTracker = + _SequentialTriggerRaceTracker(); + /// Performs setup steps for custom functional behavior of this block. void _setup() { // one time is enough, it's a final map @@ -606,7 +644,25 @@ class Sequential extends _Always { // - if any other incoming signal changes (trigger or otherwise), then that's BAD (race condition) // - actually, multiple triggers are ok? that's fine to evaluate? // Ok, another attempt: - // - if a trigger AND a non-trigger toggle when NOT clksStable + // - if a trigger AND a non-trigger toggle when NOT clksStable (in MainTick) + // ok, more formally: + // - if a non-trigger toggles in !clkStable, AND + // a trigger toggles, + // BEFORE/RESETTING at postTick, + // THEN drive and X on all outputs + + // var glitchedTrigger = false; + // var glitchedInput = false; + // void checkGlitchRules() { + // if (glitchedInput && glitchedTrigger) { + // print('Both trigger and input glitched in the same tick'); + // } + // } + + // Simulator.postTick.listen((_) { + // glitchedInput = false; + // glitchedTrigger = false; + // }); // listen to every input of this `Sequential` for changes for (final driverInput in _assignedDriverToInputMap.values) { @@ -618,8 +674,24 @@ class Sequential extends _Always { if (Simulator.phase != SimulatorPhase.clkStable) { // if the change happens not when the clocks are stable, immediately // update the map - _updateInputToPreTickInputValue(driverInput); - print('non-clock stable change: ${driverInput.name} $event'); + final didUpdate = _updateInputToPreTickInputValue(driverInput); + + // if (didUpdate && Simulator.phase == SimulatorPhase.mainTick) { + // glitchedInput = true; + // print( + // '@${Simulator.time} input glitch on mainTick: ${driverInput.name} $event'); + // checkGlitchRules(); + // } + if (didUpdate) { + // print( + // '@${Simulator.time} input glitch: ${driverInput.name} ${Simulator.phase} $event'); + _raceTracker.nonTriggered(); + } + + // if (_pendingExecute && didUpdate) { + // print( + // 'input glitch while pending execute out of clks stable: ${driverInput.name} $event'); + // } } else { // if this is during stable clocks, it's probably another flop // driving it, so hold onto it for later @@ -634,6 +706,8 @@ class Sequential extends _Always { ..forEach(_updateInputToPreTickInputValue) ..clear(); _pendingPostUpdate = false; + + // glitchedInput = false; }, ).catchError( test: (error) => error is Exception, @@ -654,10 +728,23 @@ class Sequential extends _Always { trigger.signal.glitch.listen((event) async { // we want the first previousValue from the first glitch of this tick trigger.preTickValue ??= event.previousValue; - print( - 'trigger glitched: ${trigger.signal.name} $event ${Simulator.phase}'); + + // if (_driverInputsPendingPostUpdate.isNotEmpty) { + // print( + // 'trigger glitched when pending input changes: ${trigger.signal.name} $event ${Simulator.phase}'); + // } + // glitchedTrigger = true; + _raceTracker.triggered(); + + // print( + // '@${Simulator.time} trigger glitch ${trigger.signal.name} ${Simulator.phase} $event'); + // checkGlitchRules(); + if (!_pendingExecute) { unawaited(Simulator.clkStable.first.then((value) { + // checkGlitchRules(); //TODO: this should only be IF TRIGGERED! + // glitchedTrigger = false; + // once the clocks are stable, execute the contents of the FF _execute(); _pendingExecute = false; @@ -688,10 +775,15 @@ class Sequential extends _Always { final anyTriggerInvalid = _triggers.any((t) => !t.isValid); if (anyTriggerInvalid) { + //TODO: use _driveX here? for (final receiverOutput in _assignedReceiverToOutputMap.values) { receiverOutput.put(LogicValue.x); } } else if (anyTriggered) { + if (_raceTracker.isInViolation) { + print('violation!'); + } + if (allowMultipleAssignments) { for (final element in conditionals) { element.execute(null, null); diff --git a/test/async_reset_test.dart b/test/async_reset_test.dart index 4b992c2e2..5dabb06d1 100644 --- a/test/async_reset_test.dart +++ b/test/async_reset_test.dart @@ -14,7 +14,7 @@ class NonIdenticalTriggerSeq extends Module { bool invert = false, bool triggerAfterSampledUpdate = true, }) { - final clk = Logic(name: 'clk'); + final clk = Logic(name: 'clk')..gets(Const(0)); trigger = addInput('trigger', trigger); final innerTrigger = Logic(name: 'innerTrigger', naming: Naming.reserved); @@ -189,7 +189,7 @@ void main() { Vector({'trigger': 0}, {'result': 0}), ]; - // await SimCompare.checkFunctionalVector(mod, vectors); //TODO fix + await SimCompare.checkFunctionalVector(mod, vectors); //TODO fix SimCompare.checkIverilogVector(mod, vectors); }); @@ -205,7 +205,7 @@ void main() { Vector({'trigger': 1}, {'result': 1}), ]; - // await SimCompare.checkFunctionalVector(mod, vectors); //TODO fix + await SimCompare.checkFunctionalVector(mod, vectors); //TODO fix SimCompare.checkIverilogVector(mod, vectors); }); @@ -221,7 +221,7 @@ void main() { Vector({'trigger': 1}, {'result': 1}), ]; - // await SimCompare.checkFunctionalVector(mod, vectors); //TODO fix + await SimCompare.checkFunctionalVector(mod, vectors); //TODO fix SimCompare.checkIverilogVector(mod, vectors); }); }); From c6a760f9df66432fd3daad224f9b7bb5709d87d6 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Thu, 12 Dec 2024 09:46:33 -0800 Subject: [PATCH 13/28] fix examples with new restriction --- example/fir_filter.dart | 49 ++++++++++-------- example/oven_fsm.dart | 85 +++++++++++++++++++------------ lib/src/interfaces/interface.dart | 2 +- lib/src/modules/conditional.dart | 19 +++++-- test/example_test.dart | 2 + 5 files changed, 96 insertions(+), 61 deletions(-) diff --git a/example/fir_filter.dart b/example/fir_filter.dart index dc0c2042a..1f17f0d3d 100644 --- a/example/fir_filter.dart +++ b/example/fir_filter.dart @@ -10,6 +10,7 @@ // allow `print` messages (disable lint): // ignore_for_file: avoid_print +import 'dart:async'; import 'dart:io'; import 'package:rohd/rohd.dart'; @@ -85,7 +86,7 @@ Future main({bool noPrint = false}) async { // Generate a simple clock. This will run along by itself as // the Simulator goes. - final clk = SimpleClockGenerator(5).clk; + final clk = SimpleClockGenerator(10).clk; // 4-cycle delay coefficients. final firFilter = @@ -105,39 +106,43 @@ Future main({bool noPrint = false}) async { // Now let's try simulating! - // Let's set the initial setting. - en.put(0); - resetB.put(0); - inputVal.put(1); - // Attach a waveform dumper. if (!noPrint) { WaveDumper(firFilter); } - // Raise enable at time 5. - Simulator.registerAction(5, () => en.put(1)); + // Let's set the initial setting. + en.inject(0); + resetB.inject(0); + inputVal.inject(1); + + // Set a maximum time for the simulation so it doesn't keep running forever. + Simulator.setMaxSimTime(200); + + // Kick off the simulation. + unawaited(Simulator.run()); + + await clk.nextPosedge; - // Raise resetB at time 10. - Simulator.registerAction(10, () => resetB.put(1)); + // Raise enable + await clk.nextPosedge; + en.inject(1); + + // Raise resetB + await clk.nextPosedge; + resetB.inject(1); // Plan the input sequence. for (var i = 1; i < 10; i++) { - Simulator.registerAction(5 + i * 4, () => inputVal.put(i)); + await clk.nextPosedge; + inputVal.inject(i); } // Print a message when we're done with the simulation! - Simulator.registerAction(100, () { - if (!noPrint) { - print('Simulation completed!'); - } - }); - - // Set a maximum time for the simulation so it doesn't keep running forever. - Simulator.setMaxSimTime(100); - - // Kick off the simulation. - await Simulator.run(); + await Simulator.endSimulation(); + if (!noPrint) { + print('Simulation completed!'); + } // We can take a look at the waves now. if (!noPrint) { diff --git a/example/oven_fsm.dart b/example/oven_fsm.dart index 27da69737..2788baa55 100644 --- a/example/oven_fsm.dart +++ b/example/oven_fsm.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2023 Intel Corporation +// Copyright (C) 2023-2024 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // oven_fsm.dart @@ -10,6 +10,8 @@ // ignore_for_file: avoid_print // Import the ROHD package. +import 'dart:async'; + import 'package:rohd/rohd.dart'; // Import the counter module implement in example.dart. @@ -58,17 +60,15 @@ class OvenModule extends Module { Logic get led => output('led'); // This oven module receives a `button` and a `reset` input from runtime. - OvenModule(Logic button, Logic reset) : super(name: 'OvenModule') { + OvenModule(Logic button, Logic reset, Logic clk) : super(name: 'OvenModule') { // Register inputs and outputs of the module in the constructor. // Module logic must consume registered inputs and output to registered // outputs. `led` output also added as the output port. button = addInput('button', button, width: button.width); reset = addInput('reset', reset); + clk = addInput('clk', clk); final led = addOutput('led', width: button.width); - // An internal clock generator. - final clk = SimpleClockGenerator(10).clk; - // Register local signals, `counterReset` and `en` // for Counter module. final counterReset = Logic(name: 'counter_reset'); @@ -184,16 +184,26 @@ class OvenModule extends Module { FiniteStateMachine get ovenStateMachine => _oven; } +/// A helper function to wait for a number of cycles. +Future waitCycles(Logic clk, int numCycles) async { + for (var i = 0; i < numCycles; i++) { + await clk.nextPosedge; + } +} + Future main({bool noPrint = false}) async { // Signals `button` and `reset` that mimic user's behaviour of button pressed // and reset. // - // Width of button is 2 because button is represent by 2-bits signal. + // Width of button is 2 because button is represented by a 2-bit signal. final button = Logic(name: 'button', width: 2); final reset = Logic(name: 'reset'); + // A clock generator. + final clk = SimpleClockGenerator(10).clk; + // Build an Oven Module and passed the `button` and `reset`. - final oven = OvenModule(button, reset); + final oven = OvenModule(button, reset, clk); // Generate a Mermaid FSM diagram and save as the name `oven_fsm.md`. // Note that the extension of the files is recommend as .md or .mmd. @@ -210,14 +220,22 @@ Future main({bool noPrint = false}) async { // Now let's try simulating! - // Let's start off with asserting reset to Oven. - reset.inject(1); + // Set a maximum time for the simulation so it doesn't keep running forever. + Simulator.setMaxSimTime(300); // Attach a waveform dumper so we can see what happens. if (!noPrint) { WaveDumper(oven, outputPath: 'oven.vcd'); } + // Kick off the simulation. + unawaited(Simulator.run()); + + await clk.nextPosedge; + + // Let's start off with asserting reset to Oven. + reset.inject(1); + if (!noPrint) { // We can listen to the streams on LED light changes based on time. oven.led.changed.listen((event) { @@ -227,38 +245,39 @@ Future main({bool noPrint = false}) async { // Print the Simulator time when the LED light changes. print('@t=${Simulator.time}, LED changed to: $ledVal'); }); + + button.changed.listen((event) { + final buttonVal = Button.values[event.newValue.toInt()].name; + print('@t=${Simulator.time}, Button changed to: $buttonVal'); + }); } - // Drop reset at time 25. - Simulator.registerAction(25, () => reset.put(0)); + await waitCycles(clk, 2); - // Press button start => `00` at time 25. - Simulator.registerAction(25, () { - button.put(Button.start.value); - }); + // Drop reset + reset.inject(0); - // Press button pause => `01` at time 50. - Simulator.registerAction(50, () { - button.put(Button.pause.value); - }); + // Press button start => `00` + button.inject(Button.start.value); - // Press button resume => `10` at time 70. - Simulator.registerAction(70, () { - button.put(Button.resume.value); - }); + await waitCycles(clk, 3); - // Print a message when we're done with the simulation! - Simulator.registerAction(120, () { - if (!noPrint) { - print('Simulation completed!'); - } - }); + // Press button pause => `01` + button.inject(Button.pause.value); - // Set a maximum time for the simulation so it doesn't keep running forever. - Simulator.setMaxSimTime(120); + await waitCycles(clk, 3); - // Kick off the simulation. - await Simulator.run(); + // Press button resume => `10` + button.inject(Button.resume.value); + + await waitCycles(clk, 8); + + await Simulator.endSimulation(); + + // Print a message when we're done with the simulation! + if (!noPrint) { + print('Simulation completed!'); + } // We can take a look at the waves now if (!noPrint) { diff --git a/lib/src/interfaces/interface.dart b/lib/src/interfaces/interface.dart index 4a439a7e7..f7e9c5963 100644 --- a/lib/src/interfaces/interface.dart +++ b/lib/src/interfaces/interface.dart @@ -47,7 +47,7 @@ class Interface { Logic port(String name) => _ports.containsKey(name) ? _ports[name]! : throw PortDoesNotExistException( - 'Port named "$name" not found on this interface.'); + 'Port named "$name" not found on this interface: $this.'); /// Provides the [port] named [name] if it exists, otherwise `null`. Logic? tryPort(String name) => _ports[name]; diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart index 65db12f5e..ebecd7ad4 100644 --- a/lib/src/modules/conditional.dart +++ b/lib/src/modules/conditional.dart @@ -475,7 +475,7 @@ class _SequentialTriggerRaceTracker { bool get isInViolation => _triggerOccurred && _nonTriggerOccurred; - void _registerPostTick() async { + void _registerPostTick() { unawaited(Simulator.postTick.first.then((value) { _registeredPostTick = false; _triggerOccurred = false; @@ -490,6 +490,8 @@ class _SequentialTriggerRaceTracker { /// /// This is similar to an `always_ff` block in SystemVerilog. Positive edge /// triggered by either one trigger or multiple with [Sequential.multi]. +/// +/// TODO: write about race conditions class Sequential extends _Always { /// The input edge triggers used in this block. final List<_SequentialTrigger> _triggers = []; @@ -683,9 +685,11 @@ class Sequential extends _Always { // checkGlitchRules(); // } if (didUpdate) { - // print( - // '@${Simulator.time} input glitch: ${driverInput.name} ${Simulator.phase} $event'); - _raceTracker.nonTriggered(); + if (Simulator.phase != SimulatorPhase.outOfTick) { + // print( + // '@${Simulator.time} input glitch: ${driverInput.name} ${Simulator.phase} $event'); + _raceTracker.nonTriggered(); + } } // if (_pendingExecute && didUpdate) { @@ -734,7 +738,12 @@ class Sequential extends _Always { // 'trigger glitched when pending input changes: ${trigger.signal.name} $event ${Simulator.phase}'); // } // glitchedTrigger = true; - _raceTracker.triggered(); + + if (Simulator.phase != SimulatorPhase.outOfTick) { + // print( + // '@${Simulator.time} trigger glitch ${trigger.signal.name} ${Simulator.phase} $event'); + _raceTracker.triggered(); + } // print( // '@${Simulator.time} trigger glitch ${trigger.signal.name} ${Simulator.phase} $event'); diff --git a/test/example_test.dart b/test/example_test.dart index 4b3c15b19..14e11685e 100644 --- a/test/example_test.dart +++ b/test/example_test.dart @@ -23,9 +23,11 @@ void main() { test('counter example', () async { await counter.main(noPrint: true); }); + test('tree example', () async { await tree.main(noPrint: true); }); + test('fir filter example', () async { await fir_filter.main(noPrint: true); }); From 7adb1d9e26260c8be90070b0e9d817f20ee1cc4b Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Thu, 12 Dec 2024 10:46:42 -0800 Subject: [PATCH 14/28] making progress on getting tests workign with new rules --- lib/src/modules/conditional.dart | 53 +++++++++++++++------------ test/async_reset_test.dart | 62 ++++++++++++++++++++++---------- 2 files changed, 74 insertions(+), 41 deletions(-) diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart index ebecd7ad4..ed515a1aa 100644 --- a/lib/src/modules/conditional.dart +++ b/lib/src/modules/conditional.dart @@ -476,13 +476,15 @@ class _SequentialTriggerRaceTracker { bool get isInViolation => _triggerOccurred && _nonTriggerOccurred; void _registerPostTick() { - unawaited(Simulator.postTick.first.then((value) { - _registeredPostTick = false; - _triggerOccurred = false; - _nonTriggerOccurred = false; - })); + if (!_registeredPostTick) { + unawaited(Simulator.postTick.first.then((value) { + _registeredPostTick = false; + _triggerOccurred = false; + _nonTriggerOccurred = false; + })); - _registeredPostTick = true; + _registeredPostTick = true; + } } } @@ -779,31 +781,36 @@ class Sequential extends _Always { } } + /// Drives [LogicValue.x] on all outputs of this [Sequential]. + void _driveX() { + for (final receiverOutput in _assignedReceiverToOutputMap.values) { + receiverOutput.put(LogicValue.x); + } + } + void _execute() { final anyTriggered = _triggers.any((t) => t.isTriggered); final anyTriggerInvalid = _triggers.any((t) => !t.isValid); if (anyTriggerInvalid) { - //TODO: use _driveX here? - for (final receiverOutput in _assignedReceiverToOutputMap.values) { - receiverOutput.put(LogicValue.x); - } + _driveX(); } else if (anyTriggered) { if (_raceTracker.isInViolation) { - print('violation!'); - } - - if (allowMultipleAssignments) { - for (final element in conditionals) { - element.execute(null, null); - } + // print('violation!'); //TODO + _driveX(); } else { - final allDrivenSignals = DuplicateDetectionSet(); - 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(); + for (final element in conditionals) { + element.execute(allDrivenSignals, null); + } + if (allDrivenSignals.hasDuplicates) { + throw SignalRedrivenException(allDrivenSignals.duplicates); + } } } } diff --git a/test/async_reset_test.dart b/test/async_reset_test.dart index 5dabb06d1..0ff03a999 100644 --- a/test/async_reset_test.dart +++ b/test/async_reset_test.dart @@ -33,7 +33,7 @@ class NonIdenticalTriggerSeq extends Module { class MultipleTriggerSeq extends Module { MultipleTriggerSeq(Logic trigger1, Logic trigger2) { - final clk = Logic(); //SimpleClockGenerator(10).clk; + final clk = Logic(); trigger1 = addInput('trigger1', trigger1); trigger2 = addInput('trigger2', trigger2); @@ -47,10 +47,9 @@ class MultipleTriggerSeq extends Module { If.block([ Iff(trigger1 & ~trigger2, [result < 0xa]), ElseIf(~trigger1 & trigger2, [result < 0xb]), - // ElseIf(trigger1 & trigger2, [result < 0xc]), + ElseIf(trigger1 & trigger2, [result < 0xc]), ElseIf(~trigger1 & ~trigger2, [result < 0xd]), ]), - // result < (trigger1 & trigger2), ]); } } @@ -170,13 +169,20 @@ void main() { await mod.build(); - final vectors = [ + final vectorsRohd = [ + Vector({'trigger': 0}, {}), + // ROHD catches a bad race condition + Vector({'trigger': 1}, {'result': 'x'}), + ]; + + await SimCompare.checkFunctionalVector(mod, vectorsRohd); + + final vectorsSv = [ Vector({'trigger': 0}, {}), Vector({'trigger': 1}, {'result': 1}), ]; - await SimCompare.checkFunctionalVector(mod, vectors); //TODO fix - SimCompare.checkIverilogVector(mod, vectors); + SimCompare.checkIverilogVector(mod, vectorsSv); }); test('inverted', () async { @@ -184,13 +190,20 @@ void main() { await mod.build(); - final vectors = [ + final vectorsRohd = [ + Vector({'trigger': 1}, {}), + // ROHD catches a bad race condition + Vector({'trigger': 0}, {'result': 'x'}), + ]; + + await SimCompare.checkFunctionalVector(mod, vectorsRohd); + + final vectorsSv = [ Vector({'trigger': 1}, {}), Vector({'trigger': 0}, {'result': 0}), ]; - await SimCompare.checkFunctionalVector(mod, vectors); //TODO fix - SimCompare.checkIverilogVector(mod, vectors); + SimCompare.checkIverilogVector(mod, vectorsSv); }); test('trigger earlier inverted', () async { @@ -199,14 +212,21 @@ void main() { await mod.build(); - final vectors = [ + final vectorsRohd = [ + Vector({'trigger': 0}, {}), + // ROHD catches a bad race condition + Vector({'trigger': 1}, {'result': 'x'}), + ]; + + await SimCompare.checkFunctionalVector(mod, vectorsRohd); + + final vectorsSv = [ Vector({'trigger': 0}, {}), // in this case, the trigger happened before the sampled value updated Vector({'trigger': 1}, {'result': 1}), ]; - await SimCompare.checkFunctionalVector(mod, vectors); //TODO fix - SimCompare.checkIverilogVector(mod, vectors); + SimCompare.checkIverilogVector(mod, vectorsSv); }); test('trigger earlier normal', () async { @@ -215,14 +235,21 @@ void main() { await mod.build(); - final vectors = [ + final vectorsRohd = [ + Vector({'trigger': 0}, {}), + // ROHD catches a bad race condition + Vector({'trigger': 1}, {'result': 'x'}), + ]; + + await SimCompare.checkFunctionalVector(mod, vectorsRohd); //TODO fix + + final vectorsSv = [ Vector({'trigger': 0}, {}), // in this case, the two signals are "identical", so there is no "later" Vector({'trigger': 1}, {'result': 1}), ]; - await SimCompare.checkFunctionalVector(mod, vectors); //TODO fix - SimCompare.checkIverilogVector(mod, vectors); + SimCompare.checkIverilogVector(mod, vectorsSv); }); }); @@ -241,13 +268,12 @@ void main() { final vectors = [ Vector({'trigger1': 0, 'trigger2': 0}, {}), - // Vector({'trigger1': 0, 'trigger2': 0}, {'result': 0xd}), Vector({'trigger1': 1, 'trigger2': 1}, {}), - Vector({'trigger1': 1, 'trigger2': 1}, {'result': 0xd}), + Vector({'trigger1': 1, 'trigger2': 1}, {'result': 0xc}), ]; //TODO fix, should this fail? - // await SimCompare.checkFunctionalVector(mod, vectors); + await SimCompare.checkFunctionalVector(mod, vectors); SimCompare.checkIverilogVector(mod, vectors); }); From a05693e807c9508b99aabd4ace8ad764ceb71d5e Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Thu, 12 Dec 2024 11:21:26 -0800 Subject: [PATCH 15/28] fix multi trigger test, add some doc --- lib/src/modules/conditional.dart | 22 +++++++++++++++------- test/async_reset_test.dart | 22 +++++++++++++++------- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart index ed515a1aa..2c0527031 100644 --- a/lib/src/modules/conditional.dart +++ b/lib/src/modules/conditional.dart @@ -490,10 +490,8 @@ class _SequentialTriggerRaceTracker { /// Represents a block of sequential logic. /// -/// This is similar to an `always_ff` block in SystemVerilog. Positive edge -/// triggered by either one trigger or multiple with [Sequential.multi]. -/// -/// TODO: write about race conditions +/// This is similar to an `always_ff` block in SystemVerilog. Edge triggered by +/// either one trigger or multiple with [Sequential.multi]. class Sequential extends _Always { /// The input edge triggers used in this block. final List<_SequentialTrigger> _triggers = []; @@ -552,6 +550,16 @@ class Sequential extends _Always { /// If [asyncReset] is true, the [reset] signal (if provided) will be treated /// as an async reset. If [asyncReset] is false, the reset signal will be /// treated as synchronous. + /// + /// If a trigger is sampled within the `conditionals`, the value will be the + /// "new" value of that trigger. This is meant to help model how an + /// asynchronous trigger (e.g. async reset) could affect the behavior of the + /// sequential elements implied. One must be careful to describe logic which + /// is synthesizable. The [Sequential] will attempt to drive `X` in scenarios + /// it can detect may not simulate and synthesize the same way, but it cannot + /// guarantee it. If both a trigger and an input that is not a trigger glitch + /// simultaneously during the phases of the [Simulator], then all outputs of + /// this [Sequential] will be driven to [LogicValue.x]. Sequential.multi( List posedgeTriggers, super._conditionals, { @@ -688,8 +696,8 @@ class Sequential extends _Always { // } if (didUpdate) { if (Simulator.phase != SimulatorPhase.outOfTick) { - // print( - // '@${Simulator.time} input glitch: ${driverInput.name} ${Simulator.phase} $event'); + print( + '@${Simulator.time} input glitch: ${driverInput.name} ${Simulator.phase} $event'); _raceTracker.nonTriggered(); } } @@ -796,7 +804,7 @@ class Sequential extends _Always { _driveX(); } else if (anyTriggered) { if (_raceTracker.isInViolation) { - // print('violation!'); //TODO + // print('@${Simulator.time} violation!'); //TODO _driveX(); } else { if (allowMultipleAssignments) { diff --git a/test/async_reset_test.dart b/test/async_reset_test.dart index 0ff03a999..5a8cb4c11 100644 --- a/test/async_reset_test.dart +++ b/test/async_reset_test.dart @@ -33,7 +33,7 @@ class NonIdenticalTriggerSeq extends Module { class MultipleTriggerSeq extends Module { MultipleTriggerSeq(Logic trigger1, Logic trigger2) { - final clk = Logic(); + final clk = Logic(name: 'clk')..gets(Const(0)); trigger1 = addInput('trigger1', trigger1); trigger2 = addInput('trigger2', trigger2); @@ -44,11 +44,20 @@ class MultipleTriggerSeq extends Module { trigger1, trigger2 ], [ - If.block([ - Iff(trigger1 & ~trigger2, [result < 0xa]), - ElseIf(~trigger1 & trigger2, [result < 0xb]), - ElseIf(trigger1 & trigger2, [result < 0xc]), - ElseIf(~trigger1 & ~trigger2, [result < 0xd]), + // this is bad style and possibly won't synthesize properly, but helps + // test some of the checking in `Sequential`s + If(trigger1, then: [ + If(trigger2, then: [ + result < 0xc, + ], orElse: [ + result < 0xa, + ]), + ], orElse: [ + If(trigger2, then: [ + result < 0xb, + ], orElse: [ + result < 0xd, + ]), ]), ]); } @@ -272,7 +281,6 @@ void main() { Vector({'trigger1': 1, 'trigger2': 1}, {'result': 0xc}), ]; - //TODO fix, should this fail? await SimCompare.checkFunctionalVector(mod, vectors); SimCompare.checkIverilogVector(mod, vectors); }); From 8ff97b13391929ff03c23ff5fac4b489a1002ee3 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Thu, 12 Dec 2024 11:37:31 -0800 Subject: [PATCH 16/28] fix another test with order of multiple triggers --- test/async_reset_test.dart | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/test/async_reset_test.dart b/test/async_reset_test.dart index 5a8cb4c11..42cfd05c9 100644 --- a/test/async_reset_test.dart +++ b/test/async_reset_test.dart @@ -295,11 +295,21 @@ void main() { a, b ], [ - If.block([ - Iff(a & ~b, [result < 0xa]), - ElseIf(~a & b, [result < 0xb]), - Else([result < 0xc]), - ]), + // this is bad style and possibly won't synthesize properly, but helps + // test some of the checking in `Sequential`s + If.s( + a, + If.s( + b, + result < 0xc, + result < 0xa, + ), + If.s( + b, + result < 0xb, + result < 0xc, + ), + ), ]); Simulator.registerAction(10, () { From 806ccc836c7ad3ab26e292952046f5b198f0c758 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Thu, 12 Dec 2024 11:44:35 -0800 Subject: [PATCH 17/28] test with and without clock presence --- test/async_reset_test.dart | 45 ++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/test/async_reset_test.dart b/test/async_reset_test.dart index 42cfd05c9..1644bc4f2 100644 --- a/test/async_reset_test.dart +++ b/test/async_reset_test.dart @@ -32,12 +32,14 @@ class NonIdenticalTriggerSeq extends Module { } class MultipleTriggerSeq extends Module { - MultipleTriggerSeq(Logic trigger1, Logic trigger2) { - final clk = Logic(name: 'clk')..gets(Const(0)); + Logic get result => output('result'); + MultipleTriggerSeq(Logic trigger1, Logic trigger2, {bool withClock = false}) { + final clk = Logic(name: 'clk') + ..gets(withClock ? SimpleClockGenerator(10).clk : Const(0)); trigger1 = addInput('trigger1', trigger1); trigger2 = addInput('trigger2', trigger2); - final result = addOutput('result', width: 8); + addOutput('result', width: 8); Sequential.multi([ clk, @@ -262,27 +264,32 @@ void main() { }); }); - //TODO: test async reset with clocks too + group('multiple trigger races', () { + group('two resets simulatenously', () { + for (final withClock in [true, false]) { + test('withClock=$withClock', () async { + final mod = + MultipleTriggerSeq(Logic(), Logic(), withClock: withClock); - //TODO: test if registerAction (same tick) with clk and reset both edges at the same time - // what should that even do?? error?? what if two signals other than clk (like different resets) - // that need to be mutually exclusive happen? - // always@(clk, r1, r2) if(r1 & ~r2) ... if(r2 & ~r1) ... if(r1 & r2) ... if(~r1 & ~r2) ... + await mod.build(); - group('multiple trigger races', () { - test('two resets simulatenously', () async { - final mod = MultipleTriggerSeq(Logic(), Logic()); + WaveDumper(mod); - await mod.build(); + final vectors = [ + Vector({'trigger1': 0, 'trigger2': 0}, {}), + Vector({'trigger1': 1, 'trigger2': 1}, {}), + Vector({'trigger1': 1, 'trigger2': 1}, {'result': 0xc}), + ]; - final vectors = [ - Vector({'trigger1': 0, 'trigger2': 0}, {}), - Vector({'trigger1': 1, 'trigger2': 1}, {}), - Vector({'trigger1': 1, 'trigger2': 1}, {'result': 0xc}), - ]; + // make sure change happend right on the edges + Simulator.registerAction(12, () { + expect(mod.result.value.toInt(), 0xc); + }); - await SimCompare.checkFunctionalVector(mod, vectors); - SimCompare.checkIverilogVector(mod, vectors); + await SimCompare.checkFunctionalVector(mod, vectors); + SimCompare.checkIverilogVector(mod, vectors); + }); + } }); test('one then another trigger post-changed', () async { From 1aebf3499199df89d1647398f2b2d1eb75e674ed Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Thu, 12 Dec 2024 14:52:04 -0800 Subject: [PATCH 18/28] settling on solution that works --- lib/src/modules/conditional.dart | 64 ++++---------------------------- lib/src/simulator.dart | 23 +++++++----- test/async_reset_test.dart | 29 ++++++++++++--- 3 files changed, 45 insertions(+), 71 deletions(-) diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart index 2c0527031..e95964084 100644 --- a/lib/src/modules/conditional.dart +++ b/lib/src/modules/conditional.dart @@ -458,6 +458,7 @@ class _SequentialTrigger { String toString() => '@$verilogTriggerKeyword ${signal.name}'; } +//TODO: doc stuff class _SequentialTriggerRaceTracker { bool _triggerOccurred = false; bool _nonTriggerOccurred = false; @@ -651,31 +652,6 @@ class Sequential extends _Always { element._updateOverrideMap(_inputToPreTickInputValuesMap); } - // TODO: Maybe the right recipe: - // - keep track of WHAT triggered this flop each time - // - if any other incoming signal changes (trigger or otherwise), then that's BAD (race condition) - // - actually, multiple triggers are ok? that's fine to evaluate? - // Ok, another attempt: - // - if a trigger AND a non-trigger toggle when NOT clksStable (in MainTick) - // ok, more formally: - // - if a non-trigger toggles in !clkStable, AND - // a trigger toggles, - // BEFORE/RESETTING at postTick, - // THEN drive and X on all outputs - - // var glitchedTrigger = false; - // var glitchedInput = false; - // void checkGlitchRules() { - // if (glitchedInput && glitchedTrigger) { - // print('Both trigger and input glitched in the same tick'); - // } - // } - - // Simulator.postTick.listen((_) { - // glitchedInput = false; - // glitchedTrigger = false; - // }); - // listen to every input of this `Sequential` for changes for (final driverInput in _assignedDriverToInputMap.values) { // pre-fill the _inputToPreTickInputValuesMap so that nothing ever @@ -688,24 +664,12 @@ class Sequential extends _Always { // update the map final didUpdate = _updateInputToPreTickInputValue(driverInput); - // if (didUpdate && Simulator.phase == SimulatorPhase.mainTick) { - // glitchedInput = true; - // print( - // '@${Simulator.time} input glitch on mainTick: ${driverInput.name} $event'); - // checkGlitchRules(); - // } - if (didUpdate) { - if (Simulator.phase != SimulatorPhase.outOfTick) { - print( - '@${Simulator.time} input glitch: ${driverInput.name} ${Simulator.phase} $event'); - _raceTracker.nonTriggered(); - } + if (didUpdate && Simulator.phase != SimulatorPhase.outOfTick) { + //TODO + // print( + // '@${Simulator.time} input glitch: ${driverInput.name} ${Simulator.phase} $event'); + _raceTracker.nonTriggered(); } - - // if (_pendingExecute && didUpdate) { - // print( - // 'input glitch while pending execute out of clks stable: ${driverInput.name} $event'); - // } } else { // if this is during stable clocks, it's probably another flop // driving it, so hold onto it for later @@ -720,8 +684,6 @@ class Sequential extends _Always { ..forEach(_updateInputToPreTickInputValue) ..clear(); _pendingPostUpdate = false; - - // glitchedInput = false; }, ).catchError( test: (error) => error is Exception, @@ -743,27 +705,15 @@ class Sequential extends _Always { // we want the first previousValue from the first glitch of this tick trigger.preTickValue ??= event.previousValue; - // if (_driverInputsPendingPostUpdate.isNotEmpty) { - // print( - // 'trigger glitched when pending input changes: ${trigger.signal.name} $event ${Simulator.phase}'); - // } - // glitchedTrigger = true; - if (Simulator.phase != SimulatorPhase.outOfTick) { + //TODO // print( // '@${Simulator.time} trigger glitch ${trigger.signal.name} ${Simulator.phase} $event'); _raceTracker.triggered(); } - // print( - // '@${Simulator.time} trigger glitch ${trigger.signal.name} ${Simulator.phase} $event'); - // checkGlitchRules(); - if (!_pendingExecute) { unawaited(Simulator.clkStable.first.then((value) { - // checkGlitchRules(); //TODO: this should only be IF TRIGGERED! - // glitchedTrigger = false; - // once the clocks are stable, execute the contents of the FF _execute(); _pendingExecute = false; diff --git a/lib/src/simulator.dart b/lib/src/simulator.dart index 74c5b94ef..aac67d104 100644 --- a/lib/src/simulator.dart +++ b/lib/src/simulator.dart @@ -328,11 +328,8 @@ abstract class Simulator { await _pendingList.removeFirst()(); } - //TODO: is this appropriate to do? if so, make it a function! - while (_injectedActions.isNotEmpty) { - final injectedFunction = _injectedActions.removeFirst(); - await injectedFunction(); - } + //TODO: is this appropriate to do? + await _executeInjectedActions(); } /// Executes the clkStable phase @@ -343,15 +340,20 @@ abstract class Simulator { _clkStableController.add(null); } + /// Executes all the injected actions. + static Future _executeInjectedActions() async { + while (_injectedActions.isNotEmpty) { + final injectedFunction = _injectedActions.removeFirst(); + await injectedFunction(); + } + } + /// Executes the outOfTick phase //// /// Just before we end the current tick, we execute the injected actions, /// removing them from [_injectedActions] as we go. static Future _outOfTick() async { - while (_injectedActions.isNotEmpty) { - final injectedFunction = _injectedActions.removeFirst(); - await injectedFunction(); - } + await _executeInjectedActions(); _phase = SimulatorPhase.outOfTick; @@ -387,6 +389,9 @@ abstract class Simulator { ' To run a new simulation, use Simulator.reset().'); } + // Yield execution to the event loop before beginning the simulation. + await Future(() {}); + while (hasStepsRemaining() && _simExceptions.isEmpty && !_simulationEndRequested && diff --git a/test/async_reset_test.dart b/test/async_reset_test.dart index 1644bc4f2..8d0e2b235 100644 --- a/test/async_reset_test.dart +++ b/test/async_reset_test.dart @@ -109,9 +109,6 @@ void main() { }, }; - //TODO: what if there's another (connected) version of that signal that's the trigger?? but not exactly the same logic? - //TODO: how to deal with injects that trigger edges?? - //TODO: doc clearly the behavior of sampling async triggersl for (final mechanism in seqMechanism.entries) { @@ -273,8 +270,6 @@ void main() { await mod.build(); - WaveDumper(mod); - final vectors = [ Vector({'trigger1': 0, 'trigger2': 0}, {}), Vector({'trigger1': 1, 'trigger2': 1}, {}), @@ -344,4 +339,28 @@ void main() { expect(seenValues[1].toInt(), 0xb); }); }); + + test('put before trigger changed does not cause race x generation', () async { + final d = Logic(); + final clk = SimpleClockGenerator(10).clk; + + final q = flop(clk, d); + + unawaited(Simulator.run()); + + d.put(1); + + await clk.nextPosedge; + + expect(q.value.isValid, isTrue); + expect(q.value.toInt(), 1); + + d.put(0); + + await clk.nextPosedge; + + expect(q.value.toInt(), 0); + + await Simulator.endSimulation(); + }); } From 2ffcde7e9e050ee3b6a395442b1dc05da7446347 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Thu, 12 Dec 2024 16:26:44 -0800 Subject: [PATCH 19/28] async reset simcompare with different flop approaches --- lib/src/finite_state_machine.dart | 7 ++- lib/src/modules/conditional.dart | 2 - test/async_reset_test.dart | 93 +++++++++++++++++++++---------- 3 files changed, 69 insertions(+), 33 deletions(-) diff --git a/lib/src/finite_state_machine.dart b/lib/src/finite_state_machine.dart index a44e4db24..6e169e04f 100644 --- a/lib/src/finite_state_machine.dart +++ b/lib/src/finite_state_machine.dart @@ -96,10 +96,11 @@ class FiniteStateMachine { //TODO: test FSMs with async reset - // TODO: allow FSM to support async reset /// Creates an finite state machine for the specified list of [_states], with - /// an initial state of [resetState] (when synchronous [reset] is high) and - /// transitions on positive edges of any of [_clks]. + /// an initial state of [resetState] (when [reset] is high) and transitions on + /// positive edges of any of [_clks]. + /// + /// If [asyncReset] is `true`, the [reset] signal is asynchronous. FiniteStateMachine.multi( this._clks, this.reset, this.resetState, this._states, {this.asyncReset = false}) diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart index e95964084..0e1f62d2e 100644 --- a/lib/src/modules/conditional.dart +++ b/lib/src/modules/conditional.dart @@ -1965,6 +1965,4 @@ class FlipFlop extends Module with SystemVerilog { return svBuffer.toString(); } - - //TODO: test sequential, sequential.multi, flipflop, and flop all! SV too! } diff --git a/test/async_reset_test.dart b/test/async_reset_test.dart index 8d0e2b235..5ef22e426 100644 --- a/test/async_reset_test.dart +++ b/test/async_reset_test.dart @@ -65,6 +65,21 @@ class MultipleTriggerSeq extends Module { } } +class AsyncResetMod extends Module { + Logic get val => output('val'); + AsyncResetMod({ + required Logic clk, + required Logic reset, + required void Function(Logic clk, Logic reset, Logic val) seqBuilder, + }) { + clk = addInput('clk', clk); + reset = addInput('reset', reset); + addOutput('val'); + + seqBuilder(clk, reset, val); + } +} + void main() { tearDown(() async { await Simulator.reset(); @@ -94,7 +109,7 @@ void main() { FlipFlop( clk, reset: reset, - val, + Const(1), asyncReset: true, ).q; }, @@ -103,38 +118,63 @@ void main() { flop( clk, reset: reset, - val, + Const(1), asyncReset: true, ); }, }; - //TODO: doc clearly the behavior of sampling async triggersl - for (final mechanism in seqMechanism.entries) { - test('using ${mechanism.key}', () async { - final clk = Logic(name: 'clk'); - final reset = Logic(name: 'reset'); - final val = Logic(name: 'val'); + group('using ${mechanism.key}', () { + test('functional', () async { + final clk = Logic(name: 'clk'); + final reset = Logic(name: 'reset'); + final val = Logic(name: 'val'); - reset.inject(0); - clk.inject(0); + reset.inject(0); + clk.inject(0); - mechanism.value(clk, reset, val); + mechanism.value(clk, reset, val); - Simulator.registerAction(10, () { - clk.put(1); - }); + Simulator.registerAction(10, () { + clk.put(1); + }); - Simulator.registerAction(14, () { - reset.put(1); - }); + Simulator.registerAction(11, () { + expect(val.value.toInt(), 1); + }); - Simulator.registerAction(15, () { - expect(val.value.toInt(), 0); + Simulator.registerAction(14, () { + reset.put(1); + }); + + Simulator.registerAction(15, () { + expect(val.value.toInt(), 0); + }); + + await Simulator.run(); }); - await Simulator.run(); + test('simcompare', () async { + final mod = AsyncResetMod( + clk: Logic(name: 'clk'), + reset: Logic(name: 'reset'), + seqBuilder: mechanism.value, + ); + + await mod.build(); + + final vectors = [ + Vector({'clk': 0, 'reset': 0}, {}), + Vector({'clk': 1, 'reset': 0}, {'val': 1}), + Vector({'clk': 1, 'reset': 0}, {'val': 1}), + Vector({'clk': 1, 'reset': 1}, {'val': 0}), + Vector({'clk': 1, 'reset': 1}, {'val': 0}), + ]; + + await SimCompare.checkFunctionalVector(mod, vectors); + SimCompare.checkIverilogVector(mod, vectors); + }); }); } }); @@ -154,14 +194,9 @@ void main() { await clk.nextPosedge; await clk.nextPosedge; - // reset.inject(1); //TODO - Simulator.injectAction(() { - // print('asdf1'); - reset.put(1); - }); + reset.inject(1); Simulator.registerAction(Simulator.time + 1, () { - // print('asdf'); expect(val.value.toInt(), 0); }); @@ -249,7 +284,7 @@ void main() { Vector({'trigger': 1}, {'result': 'x'}), ]; - await SimCompare.checkFunctionalVector(mod, vectorsRohd); //TODO fix + await SimCompare.checkFunctionalVector(mod, vectorsRohd); final vectorsSv = [ Vector({'trigger': 0}, {}), @@ -340,7 +375,9 @@ void main() { }); }); - test('put before trigger changed does not cause race x generation', () async { + test( + 'put at simulation start before trigger changed ' + 'does not cause race x generation', () async { final d = Logic(); final clk = SimpleClockGenerator(10).clk; From b39e183ed716b5f13cf454cefd25b3e7abf88445 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Fri, 13 Dec 2024 10:44:40 -0800 Subject: [PATCH 20/28] async reset vcd test --- lib/src/utilities/vcd_parser.dart | 5 +++ test/wave_dumper_test.dart | 52 +++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/lib/src/utilities/vcd_parser.dart b/lib/src/utilities/vcd_parser.dart index c8adc75df..807b04f49 100644 --- a/lib/src/utilities/vcd_parser.dart +++ b/lib/src/utilities/vcd_parser.dart @@ -69,6 +69,11 @@ abstract class VcdParser { } } } + + if (state == _VcdParseState.findSig) { + throw Exception('Signal $signalName not found in VCD file'); + } + return currentValue == value; } } diff --git a/test/wave_dumper_test.dart b/test/wave_dumper_test.dart index eae660cee..fc3109d4b 100644 --- a/test/wave_dumper_test.dart +++ b/test/wave_dumper_test.dart @@ -24,6 +24,17 @@ class SimpleModule extends Module { } } +class SimpleModWithSeq extends Module { + Logic get val => output('val'); + SimpleModWithSeq(Logic asyncReset, Logic clk) { + clk = addInput('clk', clk); + asyncReset = addInput('asyncReset', asyncReset); + addOutput('val'); + + val <= flop(clk, Const(1), reset: asyncReset, asyncReset: true); + } +} + const tempDumpDir = 'tmp_test'; /// Gets the path of the VCD file based on a name. @@ -238,4 +249,45 @@ void main() { File(dir1Path).deleteSync(recursive: true); } }); + + test('async reset shown in waves correctly', () async { + final reset = Logic(); + final clk = SimpleClockGenerator(10).clk; + final mod = SimpleModWithSeq(reset, clk); + + await mod.build(); + + const dumpName = 'asyncReset'; + + Simulator.setMaxSimTime(100); + Simulator.registerAction(13, () => reset.put(1)); + reset.put(0); + + // add wave dumper *after* the put to reset + createTemporaryDump(mod, dumpName); + + // check functional matches + Simulator.registerAction(0, () => expect(reset.value.toInt(), 0)); + Simulator.registerAction(6, () => expect(mod.val.value.toInt(), 1)); + Simulator.registerAction(14, () => expect(mod.val.value.toInt(), 0)); + + await Simulator.run(); + + final vcdContents = File(temporaryDumpPath(dumpName)).readAsStringSync(); + + // reset is 0 initially + expect( + VcdParser.confirmValue(vcdContents, 'asyncReset', 1, LogicValue.zero), + equals(true)); + + // 1 after first clock edge + expect(VcdParser.confirmValue(vcdContents, 'val', 6, LogicValue.one), + equals(true)); + + // 0 after async reset + expect(VcdParser.confirmValue(vcdContents, 'val', 14, LogicValue.zero), + equals(true)); + + deleteTemporaryDump(dumpName); + }); } From 9a9894807b5ee47d14dcf08dd27fe5a128441384 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Fri, 13 Dec 2024 10:53:05 -0800 Subject: [PATCH 21/28] improve invalid reserved name exception message --- .../name/invalid_reserved_name_exception.dart | 12 ++++++------ lib/src/utilities/naming.dart | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/src/exceptions/name/invalid_reserved_name_exception.dart b/lib/src/exceptions/name/invalid_reserved_name_exception.dart index f908468d7..0f5f9c1d3 100644 --- a/lib/src/exceptions/name/invalid_reserved_name_exception.dart +++ b/lib/src/exceptions/name/invalid_reserved_name_exception.dart @@ -11,12 +11,12 @@ import 'package:rohd/src/exceptions/rohd_exception.dart'; /// An exception that thrown when a reserved name is invalid. class InvalidReservedNameException extends RohdException { - /// Display error [message] on invalid reserved name. + /// An exception with an error [message] for an invalid reserved name. /// /// Creates a [InvalidReservedNameException] with an optional error [message]. - InvalidReservedNameException( - [super.message = 'Reserved Name need to follow proper naming ' - 'convention if reserved' - ' name set to true']); - //TODO: make this error message better (what's the name, fix grammar) + InvalidReservedNameException(String name) + : super('The name "$name" was reserved but does not follow' + ' safe naming conventions. ' + 'Generally, reserved names should be valid variable identifiers' + ' in languages such as Dart and SystemVerilog.'); } diff --git a/lib/src/utilities/naming.dart b/lib/src/utilities/naming.dart index d9bb4c956..260947661 100644 --- a/lib/src/utilities/naming.dart +++ b/lib/src/utilities/naming.dart @@ -41,7 +41,7 @@ enum Naming { } else if (name.isEmpty) { throw EmptyReservedNameException(); } else if (!Sanitizer.isSanitary(name)) { - throw InvalidReservedNameException(); + throw InvalidReservedNameException(name); } } From 1fe0b8e7a568a91df2a35f2350fdd107a984259e Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Fri, 13 Dec 2024 11:09:02 -0800 Subject: [PATCH 22/28] cleanup and doc --- CHANGELOG.md | 6 ++++++ lib/src/modules/conditional.dart | 31 ++++++++++++++++++++----------- lib/src/modules/pipeline.dart | 17 ----------------- lib/src/simulator.dart | 1 - lib/src/utilities/simcompare.dart | 1 + test/pipeline_test.dart | 1 + 6 files changed, 28 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 853bee3dc..d458f7e88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,12 @@ - Breaking: Updated APIs for `Synthesizer.synthesize` and down the stack to use a `Function` to calculate the instance type of a module instead of a `Map` look-up table. - Added `srcConnections` API to `Logic` to make it easier to trace drivers of subtypes of `Logic` which contain multiple drivers. - Breaking: `Const` constructor updated so that specified `width` takes precedence over the inherent width of a provided `LogicValue` `val`. +- Added flags to support an `asyncReset` option in places where sequential reset automation was already present. +- Breaking: `Sequential` has new added strictness checking when triggers and non-triggers change simultaneously (in the same `Simulator` tick) when it may be unpredictable how the hardware would synthesize, driving `X`s on outputs instead of just picking an order. Descriptions that imply asynchronous resets are predictable and therefore unaffected. +- Breaking: injected actions in the `Simulator` can now occur in either the `mainTick` or `clkStable` phases. This API will generally continue to work as expected and as it always has, but in some scenarios could slightly change the behavior of existing testbenches. +- Breaking: `Simulator.run` now yields execution of the Dart event loop prior to beginning the simulation. This makes actions taken before starting the simulation more predictable, but may slightly change behavior in existing testbenches that relied on a potential delay. +- Improved error and exception messages. +- Fixed a bug where asynchronous events could sometimes show up late in generated waveforms from `WaveDumper`. ## 0.5.3 diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart index 0e1f62d2e..807009b12 100644 --- a/lib/src/modules/conditional.dart +++ b/lib/src/modules/conditional.dart @@ -458,24 +458,39 @@ class _SequentialTrigger { String toString() => '@$verilogTriggerKeyword ${signal.name}'; } -//TODO: doc stuff +/// A tracking construct for potential race conditions between triggers and +/// non-triggers in [Sequential]s. +/// +/// In general, if a trigger and non-trigger toggle "simulatneously" during the +/// same time step, then the outputs of the [Sequential] should be driven to +/// [LogicValue.x], since it is unpredictable how it will be synthesized. class _SequentialTriggerRaceTracker { + /// Tracks whether a trigger has occurred in this timestep. bool _triggerOccurred = false; + + /// Tracks whether a non-trigger has occurred in this timestep. bool _nonTriggerOccurred = false; - bool _registeredPostTick = false; + /// Indicates whether the current timestep has violated the rules for the race + /// condition. + bool get isInViolation => _triggerOccurred && _nonTriggerOccurred; + + /// Should be called when a trigger has occurred. void triggered() { _triggerOccurred = true; _registerPostTick(); } + /// Should be called when a non-trigger has occurred. void nonTriggered() { _nonTriggerOccurred = true; _registerPostTick(); } - bool get isInViolation => _triggerOccurred && _nonTriggerOccurred; + /// Whether a post-tick has been registered alreayd for this timestep. + bool _registeredPostTick = false; + /// Registers a post-tick event to clear the flags. void _registerPostTick() { if (!_registeredPostTick) { unawaited(Simulator.postTick.first.then((value) { @@ -641,7 +656,8 @@ class Sequential extends _Always { return true; } - //TODO: doc, and is this the right line to put this? + /// A tracking construct for potential race conditions between triggers and + /// non-triggers. final _SequentialTriggerRaceTracker _raceTracker = _SequentialTriggerRaceTracker(); @@ -665,9 +681,6 @@ class Sequential extends _Always { final didUpdate = _updateInputToPreTickInputValue(driverInput); if (didUpdate && Simulator.phase != SimulatorPhase.outOfTick) { - //TODO - // print( - // '@${Simulator.time} input glitch: ${driverInput.name} ${Simulator.phase} $event'); _raceTracker.nonTriggered(); } } else { @@ -706,9 +719,6 @@ class Sequential extends _Always { trigger.preTickValue ??= event.previousValue; if (Simulator.phase != SimulatorPhase.outOfTick) { - //TODO - // print( - // '@${Simulator.time} trigger glitch ${trigger.signal.name} ${Simulator.phase} $event'); _raceTracker.triggered(); } @@ -754,7 +764,6 @@ class Sequential extends _Always { _driveX(); } else if (anyTriggered) { if (_raceTracker.isInViolation) { - // print('@${Simulator.time} violation!'); //TODO _driveX(); } else { if (allowMultipleAssignments) { diff --git a/lib/src/modules/pipeline.dart b/lib/src/modules/pipeline.dart index d045eb7bc..a236718a2 100644 --- a/lib/src/modules/pipeline.dart +++ b/lib/src/modules/pipeline.dart @@ -159,7 +159,6 @@ class Pipeline { reset: reset, asyncReset: asyncReset); - // TODO: allow Pipeline to support async reset /// Constructs a [Pipeline] with multiple triggers on any of [_clks]. Pipeline.multi(this._clks, {List Function(PipelineStageInfo p)> stages = const [], @@ -262,22 +261,6 @@ class Pipeline { return ffAssign.receiver < driver; }); - //TODO: can we just delete this?? - // if (reset != null) { - // ffAssignsWithStall = [ - // If.block([ - // Iff( - // reset!, - // ffAssigns.map((conditional) { - // conditional as ConditionalAssign; - // return conditional.receiver < (resetValue ?? 0); - // }).toList(growable: false), - // ), - // Else(ffAssignsWithStall) - // ]) - // ]; - // } - Sequential.multi( _clks, reset: reset, diff --git a/lib/src/simulator.dart b/lib/src/simulator.dart index aac67d104..89a4aebbd 100644 --- a/lib/src/simulator.dart +++ b/lib/src/simulator.dart @@ -328,7 +328,6 @@ abstract class Simulator { await _pendingList.removeFirst()(); } - //TODO: is this appropriate to do? await _executeInjectedActions(); } diff --git a/lib/src/utilities/simcompare.dart b/lib/src/utilities/simcompare.dart index 2dc6cb3be..594fe0950 100644 --- a/lib/src/utilities/simcompare.dart +++ b/lib/src/utilities/simcompare.dart @@ -293,6 +293,7 @@ abstract class SimCompare { // ignore: parameter_assignments, prefer_interpolation_to_compose_strings return signalType + ' ' + + // ignore: prefer_interpolation_to_compose_strings packedDims.map((d) => '[${d - 1}:0]').join() + ' [${signal.elementWidth - 1}:0] $signalName' + unpackedDims.map((d) => '[${d - 1}:0]').join(); diff --git a/test/pipeline_test.dart b/test/pipeline_test.dart index a70d7815b..1dd4d6563 100644 --- a/test/pipeline_test.dart +++ b/test/pipeline_test.dart @@ -203,6 +203,7 @@ void main() { await pipem.build(); //TODO: test that reset works! + //TODO: test that async reset works! //TODO: test that reset value works! final vectors = [ Vector({'a': 1}, {}), From 7a8bfbe51b680b6df31078377c524a76b0707cea Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Fri, 13 Dec 2024 11:22:47 -0800 Subject: [PATCH 23/28] test fsm with async reset --- CHANGELOG.md | 1 + lib/src/finite_state_machine.dart | 2 -- lib/src/modules/pipeline.dart | 7 +------ test/fsm_test.dart | 27 ++++++++++++++++++++------- 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d458f7e88..1a6d4bdd7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - Breaking: `Simulator.run` now yields execution of the Dart event loop prior to beginning the simulation. This makes actions taken before starting the simulation more predictable, but may slightly change behavior in existing testbenches that relied on a potential delay. - Improved error and exception messages. - Fixed a bug where asynchronous events could sometimes show up late in generated waveforms from `WaveDumper`. +- Added support for negative edge triggers to `Sequential.multi` for cases where synthesis may interpret an inverted `posedge` as different from a `negedge`. ## 0.5.3 diff --git a/lib/src/finite_state_machine.dart b/lib/src/finite_state_machine.dart index 6e169e04f..4bbd8baa1 100644 --- a/lib/src/finite_state_machine.dart +++ b/lib/src/finite_state_machine.dart @@ -94,8 +94,6 @@ class FiniteStateMachine { bool asyncReset = false, }) : this.multi([clk], reset, resetState, states, asyncReset: asyncReset); - //TODO: test FSMs with async reset - /// Creates an finite state machine for the specified list of [_states], with /// an initial state of [resetState] (when [reset] is high) and transitions on /// positive edges of any of [_clks]. diff --git a/lib/src/modules/pipeline.dart b/lib/src/modules/pipeline.dart index a236718a2..33bd2fa88 100644 --- a/lib/src/modules/pipeline.dart +++ b/lib/src/modules/pipeline.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2023 Intel Corporation +// Copyright (C) 2021-2024 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // pipeline.dart @@ -236,11 +236,6 @@ class Pipeline { /// Adds a new signal to be pipelined across all stages. void _add(Logic newLogic) { - // dynamic resetValue; - // if (_resetValues.containsKey(newLogic)) { - // resetValue = _resetValues[newLogic]; - // } - for (var i = 0; i < _stages.length; i++) { _stages[i]._addLogic(newLogic, i); } diff --git a/test/fsm_test.dart b/test/fsm_test.dart index ca173c564..397b59803 100644 --- a/test/fsm_test.dart +++ b/test/fsm_test.dart @@ -21,11 +21,11 @@ const _simpleFSMPath = '$_tmpDir/simple_fsm.md'; const _trafficFSMPath = '$_tmpDir/traffic_light_fsm.md'; class TestModule extends Module { - TestModule(Logic a, Logic c, Logic reset) { + TestModule(Logic a, Logic c, Logic reset, {bool testingAsyncReset = false}) { a = addInput('a', a); c = addInput('c', c, width: c.width); final b = addOutput('b', width: c.width); - final clk = SimpleClockGenerator(10).clk; + final clk = testingAsyncReset ? Const(0) : SimpleClockGenerator(10).clk; reset = addInput('reset', reset); final states = [ State(MyStates.state1, events: { @@ -45,8 +45,9 @@ class TestModule extends Module { ]), ]; - final fsm = - FiniteStateMachine(clk, reset, MyStates.state1, states); + final fsm = FiniteStateMachine( + clk, reset, MyStates.state1, states, + asyncReset: testingAsyncReset); if (!kIsWeb) { fsm.generateDiagram(outputPath: _simpleFSMPath); @@ -241,13 +242,25 @@ void main() { Vector({'c': 1}, {'b': 0}), ]; await SimCompare.checkFunctionalVector(pipem, vectors); - final simResult = SimCompare.iverilogVector(pipem, vectors); - - expect(simResult, equals(true)); + SimCompare.checkIverilogVector(pipem, vectors); verifyMermaidStateDiagram(_simpleFSMPath); }); + test('simple fsm async reset', () async { + final pipem = + TestModule(Logic(), Logic(), Logic(), testingAsyncReset: true); + + await pipem.build(); + + final vectors = [ + Vector({'reset': 0, 'a': 0, 'c': 0}, {}), + Vector({'reset': 1}, {'b': 0}), + ]; + await SimCompare.checkFunctionalVector(pipem, vectors); + SimCompare.checkIverilogVector(pipem, vectors); + }); + test('default next state fsm', () async { final pipem = DefaultStateFsmMod(Logic()); From 06f8422e1de0d4321900571849fb33316b2c5ba5 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Fri, 13 Dec 2024 11:32:15 -0800 Subject: [PATCH 24/28] test async reset in pipeline --- lib/src/modules/pipeline.dart | 3 +++ test/pipeline_test.dart | 34 ++++++++++++++++++++++++---------- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/lib/src/modules/pipeline.dart b/lib/src/modules/pipeline.dart index 33bd2fa88..1c2f3e3c4 100644 --- a/lib/src/modules/pipeline.dart +++ b/lib/src/modules/pipeline.dart @@ -356,6 +356,7 @@ class ReadyValidPipeline extends Pipeline { Map resetValues = const {}, List signals = const [], Logic? reset, + bool asyncReset = false, }) : this.multi( [clk], validPipeIn, @@ -364,6 +365,7 @@ class ReadyValidPipeline extends Pipeline { resetValues: resetValues, signals: signals, reset: reset, + asyncReset: asyncReset, ); /// Creates a [ReadyValidPipeline] with multiple triggers. @@ -375,6 +377,7 @@ class ReadyValidPipeline extends Pipeline { super.resetValues, List signals = const [], super.reset, + super.asyncReset, }) : super.multi( stages: stages, signals: [validPipeIn, ...signals], diff --git a/test/pipeline_test.dart b/test/pipeline_test.dart index 1dd4d6563..dab7c9ba8 100644 --- a/test/pipeline_test.dart +++ b/test/pipeline_test.dart @@ -132,21 +132,24 @@ class PipelineInitWithGet extends Module { } class RVPipelineModule extends Module { - RVPipelineModule(Logic a, Logic reset, Logic validIn, Logic readyForOut) + RVPipelineModule(Logic a, Logic reset, Logic validIn, Logic readyForOut, + {bool testingAsyncReset = false}) : super(name: 'rv_pipeline_module') { - final clk = SimpleClockGenerator(10).clk; + final clk = testingAsyncReset ? Const(0) : SimpleClockGenerator(10).clk; a = addInput('a', a, width: a.width); validIn = addInput('validIn', validIn); readyForOut = addInput('readyForOut', readyForOut); reset = addInput('reset', reset); final b = addOutput('b', width: a.width); - final pipeline = - ReadyValidPipeline(clk, validIn, readyForOut, reset: reset, stages: [ - (p) => [p.get(a) < p.get(a) + 1], - (p) => [p.get(a) < p.get(a) + 1], - (p) => [p.get(a) < p.get(a) + 1], - ]); + final pipeline = ReadyValidPipeline(clk, validIn, readyForOut, + reset: reset, + stages: [ + (p) => [p.get(a) < p.get(a) + 1], + (p) => [p.get(a) < p.get(a) + 1], + (p) => [p.get(a) < p.get(a) + 1], + ], + asyncReset: testingAsyncReset); b <= pipeline.get(a); addOutput('validOut') <= pipeline.validPipeOut; @@ -202,8 +205,6 @@ void main() { final pipem = SimplePipelineModule(Logic(width: 8)); await pipem.build(); - //TODO: test that reset works! - //TODO: test that async reset works! //TODO: test that reset value works! final vectors = [ Vector({'a': 1}, {}), @@ -381,6 +382,19 @@ void main() { SimCompare.checkIverilogVector(pipem, vectors); }); + test('rv pipeline simple async reset', () async { + final pipem = RVPipelineModule(Logic(width: 8), Logic(), Logic(), Logic(), + testingAsyncReset: true); + await pipem.build(); + + final vectors = [ + Vector({'reset': 0, 'a': 1, 'validIn': 0, 'readyForOut': 1}, {}), + Vector({'reset': 1}, {'validOut': 0}), + ]; + await SimCompare.checkFunctionalVector(pipem, vectors); + SimCompare.checkIverilogVector(pipem, vectors); + }); + test('rv pipeline notready', () async { final pipem = RVPipelineModule(Logic(width: 8), Logic(), Logic(), Logic()); From 17c41f4d7557da2bc4110daa8d9878d359bd3bb6 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Fri, 13 Dec 2024 11:56:17 -0800 Subject: [PATCH 25/28] fix resetvalues in pipelines --- CHANGELOG.md | 1 + lib/src/modules/pipeline.dart | 94 ++++++++++++++++++++--------------- test/pipeline_test.dart | 38 ++++++++++++-- 3 files changed, 89 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a6d4bdd7..b20b06048 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ - Improved error and exception messages. - Fixed a bug where asynchronous events could sometimes show up late in generated waveforms from `WaveDumper`. - Added support for negative edge triggers to `Sequential.multi` for cases where synthesis may interpret an inverted `posedge` as different from a `negedge`. +- Fixed a bug where `resetValues` would not take effect in `Pipeline`s. ## 0.5.3 diff --git a/lib/src/modules/pipeline.dart b/lib/src/modules/pipeline.dart index 1c2f3e3c4..74226a1c9 100644 --- a/lib/src/modules/pipeline.dart +++ b/lib/src/modules/pipeline.dart @@ -109,7 +109,7 @@ class Pipeline { int get stageCount => _stages.length; /// A map of reset values for every signal. - late final Map _resetValues; + final Map? _resetValues; /// Tracks whether this [Pipeline] is done being constructed to conditionally /// run safety checks on API calls. @@ -121,34 +121,35 @@ class Pipeline { /// Each stage in the list [stages] is a function whose sole parameter is a /// [PipelineStageInfo] object and which returns a [List] of [Conditional] /// objects. Each stage can be thought of as being the contents of a - /// [Combinational] block. Use the [PipelineStageInfo] object to grab - /// signals for a given pipe stage. Flops are positive edge triggered - /// based on [clk]. + /// [Combinational] block. Use the [PipelineStageInfo] object to grab signals + /// for a given pipe stage. Flops are positive edge triggered based on [clk]. /// /// Then `i`th element of [stages] defines the combinational logic driving a - /// flop of index `i`. That is, the first entry in [stages] drives the - /// first set of flops, so logic defined in the first stage combinationally - /// consumes inputs to the [Pipeline]. The output of the [Pipeline] is - /// driven by flops driven by the last entry of [stages]. + /// flop of index `i`. That is, the first entry in [stages] drives the first + /// set of flops, so logic defined in the first stage combinationally consumes + /// inputs to the [Pipeline]. The output of the [Pipeline] is driven by flops + /// driven by the last entry of [stages]. /// - /// Signals to be pipelined can optionally be specified in the [signals] - /// list. Any signal referenced in a stage via the [PipelineStageInfo] - /// will automatically be included in the entire pipeline. + /// Signals to be pipelined can optionally be specified in the [signals] list. + /// Any signal referenced in a stage via the [PipelineStageInfo] will + /// automatically be included in the entire pipeline. /// /// If a [reset] signal is provided, then it will be consumed as an - /// active-high reset for every signal through the pipeline. The default - /// reset value is 0 for all signals, but that can be overridden by - /// setting [resetValues] to the desired value. The values specified - /// in [resetValues] should be a type acceptable to [Logic]'s `put` function. + /// active-high reset for every signal through the pipeline. The default reset + /// value is 0 for all signals, but that can be overridden by setting + /// [resetValues] to the desired value. Every stage's flops for the keys of + /// [resetValues] will be set to the same corresponding value. The values + /// specified in [resetValues] should be a type acceptable to [Logic]'s `put` + /// function. /// /// Each stage can be stalled independently using [stalls], where every index - /// of [stalls] corresponds to the index of the stage to be stalled. When - /// a stage's stall is asserted, the output of that stage will not change. + /// of [stalls] corresponds to the index of the stage to be stalled. When a + /// stage's stall is asserted, the output of that stage will not change. Pipeline(Logic clk, {List Function(PipelineStageInfo p)> stages = const [], List? stalls, List signals = const [], - Map resetValues = const {}, + Map resetValues = const {}, Logic? reset, bool asyncReset = false}) : this.multi([clk], @@ -164,14 +165,13 @@ class Pipeline { {List Function(PipelineStageInfo p)> stages = const [], List? stalls, List signals = const [], - Map resetValues = const {}, + Map? resetValues, this.reset, - this.asyncReset = false}) { + this.asyncReset = false}) + : _resetValues = resetValues == null ? null : Map.from(resetValues) { _stages = stages.map(_PipeStage.new).toList(); _stages.add(_PipeStage((p) => [])); // output stage - _resetValues = Map.from(resetValues); - _setStalls(stalls); signals.forEach(_add); @@ -241,25 +241,33 @@ class Pipeline { } _stages[0].input[newLogic]! <= newLogic; - final ffAssigns = []; + final ffAssigns = []; for (var i = 1; i < _stages.length; i++) { - ffAssigns.add(_i(newLogic, i) < _o(newLogic, i - 1)); + ffAssigns.add(_i(newLogic, i) < _o(newLogic, i - 1) as ConditionalAssign); } final ffAssignsWithStall = - List.generate(stageCount - 1, (index) { + List.generate(stageCount - 1, (index) { final stall = _stages[index].stall; - final ffAssign = ffAssigns[index] as ConditionalAssign; + final ffAssign = ffAssigns[index]; final driver = stall != null ? mux(stall, ffAssign.receiver, ffAssign.driver) : ffAssign.driver; - return ffAssign.receiver < driver; + return ffAssign.receiver < driver as ConditionalAssign; }); + final stageResetVal = _resetValues?[newLogic]; + final resetValuesForNewLogic = {}; + if (stageResetVal != null) { + for (final ffAssign in ffAssignsWithStall) { + resetValuesForNewLogic[ffAssign.receiver] = stageResetVal; + } + } + Sequential.multi( _clks, reset: reset, - resetValues: _resetValues, + resetValues: resetValuesForNewLogic, asyncReset: asyncReset, ffAssignsWithStall, name: 'ff_${newLogic.name}'); @@ -333,27 +341,31 @@ class ReadyValidPipeline extends Pipeline { /// Constructs a pipeline with Ready/Valid protocol at each stage. /// - /// The [validPipeIn] signal indicates that the input to the pipeline - /// is valid. The [readyPipeOut] signal indicates that the receiver - /// of the output of the pipeline is ready to pull out of the pipeline. + /// The [validPipeIn] signal indicates that the input to the pipeline is + /// valid. The [readyPipeOut] signal indicates that the receiver of the + /// output of the pipeline is ready to pull out of the pipeline. + /// + /// The [validPipeOut] signal indicates that valid contents are ready to be + /// received at the output of the pipeline. The [readyPipeIn] signal + /// indicates that the pipeline is ready to accept new content. /// - /// The [validPipeOut] signal indicates that valid contents are ready - /// to be received at the output of the pipeline. The [readyPipeIn] - /// signal indicates that the pipeline is ready to accept new content. + /// The pipeline will only progress through any stage, including the output, + /// if both valid and ready are asserted at the same time. This pipeline is + /// capable of having bubbles, but they will collapse if downstream stages are + /// backpressured. /// - /// The pipeline will only progress through any stage, including the - /// output, if both valid and ready are asserted at the same time. This - /// pipeline is capable of having bubbles, but they will collapse if - /// downstream stages are backpressured. + /// If contents are pushed in when the pipeline is not ready, they will be + /// dropped. /// - /// If contents are pushed in when the pipeline is not ready, they - /// will be dropped. + /// Note that the [resetValues] will take effect the same way as a normal + /// [Pipeline], but the valid indication on the output will remain at 0 until + /// a valid input has made its way from the input to the output. ReadyValidPipeline( Logic clk, Logic validPipeIn, Logic readyPipeOut, { List Function(PipelineStageInfo p)> stages = const [], - Map resetValues = const {}, + Map? resetValues, List signals = const [], Logic? reset, bool asyncReset = false, diff --git a/test/pipeline_test.dart b/test/pipeline_test.dart index dab7c9ba8..f83ae3c62 100644 --- a/test/pipeline_test.dart +++ b/test/pipeline_test.dart @@ -133,7 +133,7 @@ class PipelineInitWithGet extends Module { class RVPipelineModule extends Module { RVPipelineModule(Logic a, Logic reset, Logic validIn, Logic readyForOut, - {bool testingAsyncReset = false}) + {bool testingAsyncReset = false, dynamic aResetVal}) : super(name: 'rv_pipeline_module') { final clk = testingAsyncReset ? Const(0) : SimpleClockGenerator(10).clk; a = addInput('a', a, width: a.width); @@ -149,7 +149,8 @@ class RVPipelineModule extends Module { (p) => [p.get(a) < p.get(a) + 1], (p) => [p.get(a) < p.get(a) + 1], ], - asyncReset: testingAsyncReset); + asyncReset: testingAsyncReset, + resetValues: aResetVal != null ? {a: aResetVal} : null); b <= pipeline.get(a); addOutput('validOut') <= pipeline.validPipeOut; @@ -205,7 +206,6 @@ void main() { final pipem = SimplePipelineModule(Logic(width: 8)); await pipem.build(); - //TODO: test that reset value works! final vectors = [ Vector({'a': 1}, {}), Vector({'a': 2}, {}), @@ -395,6 +395,38 @@ void main() { SimCompare.checkIverilogVector(pipem, vectors); }); + test('rv pipeline simple reset vals', () async { + final pipem = RVPipelineModule(Logic(width: 8), Logic(), Logic(), Logic(), + aResetVal: 5); + await pipem.build(); + + final vectors = [ + Vector({'reset': 1, 'a': 1, 'validIn': 0, 'readyForOut': 1}, {}), + Vector({'reset': 1, 'a': 1, 'validIn': 0, 'readyForOut': 1}, {}), + Vector({'reset': 1, 'a': 1, 'validIn': 0, 'readyForOut': 1}, {}), + Vector({'reset': 0, 'a': 1, 'validIn': 1, 'readyForOut': 1}, + {'validOut': 0, 'b': 5}), + Vector({'reset': 0, 'a': 2, 'validIn': 1, 'readyForOut': 1}, + {'validOut': 0, 'b': 6}), + Vector({'reset': 0, 'a': 3, 'validIn': 1, 'readyForOut': 1}, + {'validOut': 0, 'b': 7}), + Vector({'reset': 0, 'a': 4, 'validIn': 1, 'readyForOut': 1}, + {'validOut': 1, 'b': 4}), + Vector({'reset': 0, 'a': 0, 'validIn': 0, 'readyForOut': 1}, + {'validOut': 1, 'b': 5}), + Vector({'reset': 0, 'a': 0, 'validIn': 0, 'readyForOut': 1}, + {'validOut': 1, 'b': 6}), + Vector({'reset': 0, 'a': 0, 'validIn': 0, 'readyForOut': 1}, + {'validOut': 1, 'b': 7}), + Vector({'reset': 0, 'a': 0, 'validIn': 0, 'readyForOut': 1}, + {'validOut': 0}), + Vector({'reset': 0, 'a': 0, 'validIn': 0, 'readyForOut': 1}, + {'validOut': 0}), + ]; + await SimCompare.checkFunctionalVector(pipem, vectors); + SimCompare.checkIverilogVector(pipem, vectors); + }); + test('rv pipeline notready', () async { final pipem = RVPipelineModule(Logic(width: 8), Logic(), Logic(), Logic()); From 76a28f31db4416466303894700eda1560602083b Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Fri, 13 Dec 2024 14:34:33 -0800 Subject: [PATCH 26/28] tweaks --- CHANGELOG.md | 1 + lib/src/exceptions/name/invalid_reserved_name_exception.dart | 2 +- test/fsm_test.dart | 2 ++ 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b20b06048..90ece2f5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ - Fixed a bug where asynchronous events could sometimes show up late in generated waveforms from `WaveDumper`. - Added support for negative edge triggers to `Sequential.multi` for cases where synthesis may interpret an inverted `posedge` as different from a `negedge`. - Fixed a bug where `resetValues` would not take effect in `Pipeline`s. +- Fixed a bug where a multi-triggered `Sequential` may not generate X's if one trigger is valid and another trigger is invalid. ## 0.5.3 diff --git a/lib/src/exceptions/name/invalid_reserved_name_exception.dart b/lib/src/exceptions/name/invalid_reserved_name_exception.dart index 0f5f9c1d3..f24089d8c 100644 --- a/lib/src/exceptions/name/invalid_reserved_name_exception.dart +++ b/lib/src/exceptions/name/invalid_reserved_name_exception.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2022-2023 Intel Corporation +// Copyright (C) 2022-2024 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // invalid_reserved_name_exception.dart diff --git a/test/fsm_test.dart b/test/fsm_test.dart index 397b59803..0d84ed270 100644 --- a/test/fsm_test.dart +++ b/test/fsm_test.dart @@ -259,6 +259,8 @@ void main() { ]; await SimCompare.checkFunctionalVector(pipem, vectors); SimCompare.checkIverilogVector(pipem, vectors); + + verifyMermaidStateDiagram(_simpleFSMPath); }); test('default next state fsm', () async { From 0c9def66fc5a515da271299640312c55b0db9346 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Mon, 16 Dec 2024 13:17:42 -0800 Subject: [PATCH 27/28] cleanup code review --- lib/src/modules/conditional.dart | 13 +++++++------ test/async_reset_test.dart | 9 +++++++++ test/changed_test.dart | 2 +- test/example_test.dart | 2 +- test/pipeline_test.dart | 2 +- test/wave_dumper_test.dart | 2 +- 6 files changed, 20 insertions(+), 10 deletions(-) diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart index 807009b12..6a7f0c15c 100644 --- a/lib/src/modules/conditional.dart +++ b/lib/src/modules/conditional.dart @@ -567,12 +567,13 @@ class Sequential extends _Always { /// as an async reset. If [asyncReset] is false, the reset signal will be /// treated as synchronous. /// - /// If a trigger is sampled within the `conditionals`, the value will be the - /// "new" value of that trigger. This is meant to help model how an - /// asynchronous trigger (e.g. async reset) could affect the behavior of the - /// sequential elements implied. One must be careful to describe logic which - /// is synthesizable. The [Sequential] will attempt to drive `X` in scenarios - /// it can detect may not simulate and synthesize the same way, but it cannot + /// If a trigger signal is sampled within the `conditionals`, the value will + /// be the "new" value of that trigger, as opposed to the "old" value as with + /// other non-trigger signals. This is meant to help model how an asynchronous + /// trigger (e.g. async reset) could affect the behavior of the sequential + /// elements implied. One must be careful to describe logic which is + /// synthesizable. The [Sequential] will attempt to drive `X` in scenarios it + /// can detect may not simulate and synthesize the same way, but it cannot /// guarantee it. If both a trigger and an input that is not a trigger glitch /// simultaneously during the phases of the [Simulator], then all outputs of /// this [Sequential] will be driven to [LogicValue.x]. diff --git a/test/async_reset_test.dart b/test/async_reset_test.dart index 5ef22e426..83a037bc3 100644 --- a/test/async_reset_test.dart +++ b/test/async_reset_test.dart @@ -1,3 +1,12 @@ +// Copyright (C) 2024 Intel Corporation +// SPDX-License-Identifier: BSD-3-Clause +// +// async_reset_test.dart +// Tests for asynchronous resets and triggers +// +// 2024 December 16 +// Author: Max Korbel + import 'dart:async'; import 'package:rohd/rohd.dart'; diff --git a/test/changed_test.dart b/test/changed_test.dart index 89e11de57..cdafa1968 100644 --- a/test/changed_test.dart +++ b/test/changed_test.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2023 Intel Corporation +// Copyright (C) 2021-2024 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // changed_test.dart diff --git a/test/example_test.dart b/test/example_test.dart index 14e11685e..b34d251fc 100644 --- a/test/example_test.dart +++ b/test/example_test.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2023 Intel Corporation +// Copyright (C) 2021-2024 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // example_test.dart diff --git a/test/pipeline_test.dart b/test/pipeline_test.dart index f83ae3c62..fbb84d86a 100644 --- a/test/pipeline_test.dart +++ b/test/pipeline_test.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2023 Intel Corporation +// Copyright (C) 2021-2024 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // pipeline_test.dart diff --git a/test/wave_dumper_test.dart b/test/wave_dumper_test.dart index fc3109d4b..07aafc8c8 100644 --- a/test/wave_dumper_test.dart +++ b/test/wave_dumper_test.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2023 Intel Corporation +// Copyright (C) 2021-2024 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // wave_dumper_test.dart From e64f4dcedfb66c8d0a790ba6716c74cb856ad7b3 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Mon, 16 Dec 2024 13:31:43 -0800 Subject: [PATCH 28/28] fix dart.dev link in docs --- doc/user_guide/_get-started/01-overview.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user_guide/_get-started/01-overview.md b/doc/user_guide/_get-started/01-overview.md index 7b84a1fbb..c1a98cdc1 100644 --- a/doc/user_guide/_get-started/01-overview.md +++ b/doc/user_guide/_get-started/01-overview.md @@ -60,6 +60,6 @@ If you're thinking "SystemVerilog is just fine, I don't need something new", it Try out Dart instantly from your browser here (it supports ROHD too!): -See some Dart language samples here: +See some Dart language samples here: For more information on Dart and tutorials, see and