diff --git a/.github/workflows/general.yml b/.github/workflows/general.yml index 48215e8e1..a4121f0b1 100644 --- a/.github/workflows/general.yml +++ b/.github/workflows/general.yml @@ -16,7 +16,7 @@ jobs: name: Run Checks permissions: {} timeout-minutes: 60 - runs-on: ${{ github.repository_owner == 'intel' && 'intel-ubuntu-latest' || 'ubuntu-latest' }} + runs-on: 'jf5-ubuntu-latest' #${{ github.repository_owner == 'intel' && 'intel-ubuntu-latest' || 'ubuntu-latest' }} steps: - name: Checkout uses: actions/checkout@v4 diff --git a/lib/src/finite_state_machine.dart b/lib/src/finite_state_machine.dart index 689f7fabc..56a8bdd71 100644 --- a/lib/src/finite_state_machine.dart +++ b/lib/src/finite_state_machine.dart @@ -15,13 +15,13 @@ import 'package:rohd/rohd.dart'; /// Deprecated: use [FiniteStateMachine] instead. @Deprecated('Use FiniteStateMachine instead') -typedef StateMachine = FiniteStateMachine; +typedef StateMachine = FiniteStateMachine; /// Simple class for FSM [FiniteStateMachine]. /// /// Abstraction for representing Finite state machines (FSM). /// Contains the logic for performing the state transitions. -class FiniteStateMachine { +class FiniteStateMachine { /// List of all the [State]s in this machine. List> get states => UnmodifiableListView(_states); final List> _states; @@ -71,7 +71,8 @@ class FiniteStateMachine { /// /// Use [getStateIndex] to map from a [StateIdentifier] to the value on this /// bus. - final Logic currentState; + late final LogicEnum currentState = + stateEnum(name: 'currentState'); /// A [List] of [Conditional] actions to perform at the beginning of the /// evaluation of actions for the [FiniteStateMachine]. This is useful for @@ -82,7 +83,8 @@ class FiniteStateMachine { /// /// Use [getStateIndex] to map from a [StateIdentifier] to the value on this /// bus. - final Logic nextState; + late final LogicEnum nextState = + stateEnum(name: 'nextState'); /// Returns a ceiling on the log of [x] base [base]. static int _logBase(num x, num base) => (log(x) / log(base)).ceil(); @@ -93,6 +95,9 @@ class FiniteStateMachine { /// If `true`, the [reset] signal is asynchronous. final bool asyncReset; + LogicEnum stateEnum({String? name}) => + LogicEnum.withMapping(stateIndexLookup, name: name); + /// 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. @@ -119,11 +124,7 @@ class FiniteStateMachine { this.asyncReset = false, List setupActions = const [], }) : setupActions = List.unmodifiable(setupActions), - stateWidth = _logBase(_states.length, 2), - currentState = - Logic(name: 'currentState', width: _logBase(_states.length, 2)), - nextState = - Logic(name: 'nextState', width: _logBase(_states.length, 2)) { + stateWidth = _logBase(_states.length, 2) { _validate(); var stateCounter = 0; @@ -138,8 +139,9 @@ class FiniteStateMachine { currentState, _states .map((state) => CaseItem( - Const(_stateValueLookup[state], width: stateWidth) - .named(state.identifier.toString()), + stateEnum()..getsEnum(state.identifier), + // Const(_stateValueLookup[state], width: stateWidth) + // .named(state.identifier.toString()), [ ...state.actions, Case( @@ -226,7 +228,7 @@ class FiniteStateMachine { } /// Simple class to initialize each state of the FSM. -class State { +class State { /// Identifier or name of the state. final StateIdentifier identifier; diff --git a/lib/src/module.dart b/lib/src/module.dart index 73cead4b4..4bba298f3 100644 --- a/lib/src/module.dart +++ b/lib/src/module.dart @@ -55,6 +55,17 @@ abstract class Module { /// An internal mapping of inOut names to their sources to this [Module]. late final Map _inOutSources = {}; + /// A mapping between [inputs], [outputs], and/or [inOuts] which must have the + /// same type as each other. The keys of the map will be updated to match the + /// type of the values. + /// + /// This is used for type checking for [LogicEnum]s through [Conditional]s. + /// + /// NOTE: This is for internal usage only, and the API will not be guaranteed + /// to be stable. + @internal + final Map portTypePairs = {}; + /// The parent [Module] of this [Module]. /// /// This only gets populated after its parent [Module], if it exists, has @@ -307,6 +318,7 @@ abstract class Module { // set unique module instance names for submodules final uniquifier = Uniquifier(); + //TODO: BUG! we must guarantee this unique name is the same one used for generation!? for (final module in _subModules) { module._uniqueInstanceName = uniquifier.getUniqueName( initialName: Sanitizer.sanitizeSV(module.name), diff --git a/lib/src/modules/conditionals/always.dart b/lib/src/modules/conditionals/always.dart index 0fe6b3f99..69ad9df16 100644 --- a/lib/src/modules/conditionals/always.dart +++ b/lib/src/modules/conditionals/always.dart @@ -137,6 +137,11 @@ abstract class Always extends Module with SystemVerilog { // share the registration information down conditional.updateAssignmentMaps( assignedReceiverToOutputMap, assignedDriverToInputMap); + + portTypePairs.addAll(conditional.portTypePairs.map((k, v) => MapEntry( + conditional.driverOrReceiverPort(k), + conditional.driverOrReceiverPort(v), + ))); } } @@ -177,6 +182,9 @@ abstract class Always extends Module with SystemVerilog { final outputs = Map.fromEntries(ports.entries .where((element) => this.outputs.containsKey(element.key))); + assert(ports.length == inputs.length + outputs.length, + 'All ports of an always should be inputs or outputs'); + var verilog = ''; verilog += '// $instanceName\n'; verilog += '${alwaysVerilogStatement(inputs)} begin\n'; diff --git a/lib/src/modules/conditionals/case.dart b/lib/src/modules/conditionals/case.dart index 590437391..54f237b02 100644 --- a/lib/src/modules/conditionals/case.dart +++ b/lib/src/modules/conditionals/case.dart @@ -88,9 +88,11 @@ Logic cases(Logic expression, Map conditions, [ for (final condition in conditions.entries) CaseItem( - condition.key is Logic - ? condition.key as Logic - : Const(condition.key, width: expression.width), + (expression is LogicEnum && condition.key is Enum) + ? (expression.clone()..getsEnum(condition.key as Enum)) + : condition.key is Logic + ? condition.key as Logic + : Const(condition.key, width: expression.width), [result < condition.value]) ], conditionalType: conditionalType, @@ -125,6 +127,15 @@ class Case extends Conditional { /// See [ConditionalType] for more details. final ConditionalType conditionalType; + @override + Map get portTypePairs => { + ...super.portTypePairs, + ..._itemTypePortPairs, + }; + + //TODO doc + final Map _itemTypePortPairs = {}; + /// Whenever an item in [items] matches [expression], it will be executed. /// /// If none of [items] match, then [defaultItem] is executed. @@ -136,6 +147,8 @@ class Case extends Conditional { if (item.value.width != expression.width) { throw PortWidthMismatchException.equalWidth(expression, item.value); } + + _itemTypePortPairs[item.value] = expression; } } diff --git a/lib/src/modules/conditionals/conditional.dart b/lib/src/modules/conditionals/conditional.dart index c26a60b3a..8b25fb477 100644 --- a/lib/src/modules/conditionals/conditional.dart +++ b/lib/src/modules/conditionals/conditional.dart @@ -71,6 +71,14 @@ abstract class Conditional { Logic receiverOutput(Logic receiver) => _assignedReceiverToOutputMap[receiver]!; + //TODO: do we like this API? + @protected + Logic driverOrReceiverPort(Logic driverOrReceiver) => + _assignedDriverToInputMap[driverOrReceiver] ?? + _assignedReceiverToOutputMap[driverOrReceiver] ?? + (throw Exception(//TODO: exception + 'Logic $driverOrReceiver is not a driver or receiver in this Conditional.')); + /// Executes the functionality of this [Conditional] and /// populates [drivenSignals] with all [Logic]s that were driven /// during execution. @@ -120,6 +128,15 @@ abstract class Conditional { /// Does *not* recursively call down through sub-[Conditional]s. List get conditionals; + /// A mapping between [receivers] and [drivers] to be fed up to the enclosing + /// [Combinational] or [Sequential]'s [Module.portTypePairs]. + /// + /// NOTE: This is for internal usage only, and the API will not be guaranteed + /// to be stable. + @internal + Map get portTypePairs => + {for (final cond in conditionals) ...cond.portTypePairs}; + /// Returns a [String] of SystemVerilog to be used in generated output. /// /// The [indent] is used for pretty-printing, and should generally be diff --git a/lib/src/modules/conditionals/conditional_assign.dart b/lib/src/modules/conditionals/conditional_assign.dart index 6d5d6904e..f62a28d8e 100644 --- a/lib/src/modules/conditionals/conditional_assign.dart +++ b/lib/src/modules/conditionals/conditional_assign.dart @@ -13,15 +13,18 @@ import 'package:rohd/src/modules/conditionals/ssa.dart'; /// An assignment that only happens under certain conditions. /// -/// [Logic] has a short-hand for creating [ConditionalAssign] via the -/// `<` operator. +/// [Logic] has a short-hand for creating [ConditionalAssign] via the `<` +/// operator. class ConditionalAssign extends Conditional { - /// The input to this assignment. + /// The receiver for this assignment. final Logic receiver; - /// The output of this assignment. + /// The driver for this assignment. final Logic driver; + @override + Map get portTypePairs => {driver: receiver}; + /// Conditionally assigns [receiver] to the value of [driver]. ConditionalAssign(this.receiver, this.driver) { if (driver.width != receiver.width) { diff --git a/lib/src/signals/logic.dart b/lib/src/signals/logic.dart index 981ad75f3..cb1cff37d 100644 --- a/lib/src/signals/logic.dart +++ b/lib/src/signals/logic.dart @@ -386,7 +386,8 @@ class Logic { /// Handles the actual connection of this [Logic] to be driven by [other]. void _connect(Logic other) { - _unassignable = true; + makeUnassignable(reason: '$this is connected to $other.'); + if (other is LogicNet) { put(other.value); other.glitch.listen((args) { @@ -670,11 +671,11 @@ class Logic { /// [Conditional]. Conditional operator <(dynamic other) { if (_unassignable) { - throw Exception('This signal "$this" has been marked as unassignable. ' - 'It may be a constant expression or otherwise' - ' should not be assigned.'); + throw UnassignableException(this, reason: _unassignableReason); } + //TODO: add support for enum types + if (other is Logic) { return ConditionalAssign(this, other); } else { diff --git a/lib/src/signals/logic_def.dart b/lib/src/signals/logic_def.dart new file mode 100644 index 000000000..e992c0612 --- /dev/null +++ b/lib/src/signals/logic_def.dart @@ -0,0 +1,20 @@ +part of 'signals.dart'; + +@internal +sealed class LogicDef extends Logic { + final bool reserveDefinitionName; + + // TODO: test naming conflicts in generated RTL + String get definitionName => + Sanitizer.sanitizeSV(_definitionName ?? runtimeType.toString()); + final String? _definitionName; + + LogicDef({ + super.width, + super.name, + super.naming, + String? definitionName, + this.reserveDefinitionName = false, + }) : _definitionName = Naming.validatedName(definitionName, + reserveName: reserveDefinitionName); +} diff --git a/lib/src/signals/logic_enum.dart b/lib/src/signals/logic_enum.dart new file mode 100644 index 000000000..a00868de9 --- /dev/null +++ b/lib/src/signals/logic_enum.dart @@ -0,0 +1,236 @@ +part of 'signals.dart'; + +// TODO: what if you assign between LogicEnums of the same T, but different mappings? +// - by default, throw an exception if assignment between enums of different mappings? +// TODO: should we support enum ports? then where does the typedef live? + +//TODO: do we need to support arrays of enums? + +//TODO: how do you define a constant version of an enum? + +class LogicEnum extends LogicDef { + //TODO: if a `put` has an illegal value prop X + + late final Map mapping; + // late final List values; + + //TODO better exceptions throughout + + T get valueEnum => mapping.entries + .firstWhere((entry) => entry.value == value, + orElse: () => throw StateError( + 'Value $value does not correspond to any enum in $mapping')) + .key; + + static Map _computeMapping( + {required Map mapping, required int width}) { + final computedMapping = mapping + .map((key, value) => MapEntry(key, LogicValue.of(value, width: width))); + + if (computedMapping.values.any((v) => !v.isValid)) { + throw ArgumentError('Mapping values must be valid LogicValues,' + ' but found: $computedMapping'); + } + + // check that any `int` or `BigInt` mappings actually ended up matching + for (final MapEntry(key: key, value: computedValue) + in computedMapping.entries) { + if (mapping[key] is int) { + if (computedValue.toInt() != mapping[key]) { + throw ArgumentError( + 'Mapping value for $key is not equal to the original int value.' + ' Computed: $computedValue, Original: ${mapping[key]}'); + } + } + + if (mapping[key] is BigInt) { + if (computedValue.toBigInt() != mapping[key]) { + throw ArgumentError( + 'Mapping value for $key is not equal to the original BigInt value.' + ' Computed: $computedValue, Original: ${mapping[key]}'); + } + } + } + + if (computedMapping.values.toSet().length != + computedMapping.values.length) { + throw ArgumentError('Mapping values must be unique,' + ' but found duplicates: $computedMapping'); + } + + return computedMapping; + } + + static int _computeWidth( + {int? requestedWidth, Map? mapping}) { + var width = 1; + + if (mapping != null) { + width = LogicValue.ofInt(mapping.length, 32).clog2().toInt(); + + if (mapping.values.toSet().length != mapping.values.length) { + throw ArgumentError( + 'Mapping values must be unique, but found duplicates: $mapping'); + } + + for (final value in [ + ...mapping.values.whereType(), + ...mapping.values.whereType().map(LogicValue.ofString), + ...mapping.values + .whereType>() + .map(LogicValue.ofIterable) + ]) { + if (value.width > width) { + width = value.width; + } + } + } + + if (requestedWidth != null) { + if (requestedWidth < width) { + throw ArgumentError( + 'Requested width $requestedWidth is less than the minimum' + ' required width $width.'); + } + width = requestedWidth; + } + + return width; + } + + /// TODO + /// + /// If [reserveDefinitionName] is true, then the enum names will be reserved + /// as well. + LogicEnum.withMapping( + Map mapping, { + int? width, + super.name, + super.naming, + String? definitionName, + super.reserveDefinitionName, + }) : super( + width: _computeWidth(requestedWidth: width, mapping: mapping), + definitionName: definitionName ?? + Naming.validatedName(T.toString(), + reserveName: reserveDefinitionName)) { + this.mapping = + Map.unmodifiable(_computeMapping(mapping: mapping, width: this.width)); + + _wire._constrainValue((value) { + if (value.isFloating) { + return LogicValue.filled(this.width, LogicValue.z); + } + if (!value.isValid) { + return LogicValue.filled(this.width, LogicValue.x); + } + if (!this.mapping.containsValue(value)) { + return LogicValue.filled(this.width, LogicValue.x); + } + return value; + }); + } + + LogicEnum(List values, + {int? width, + String? name, + Naming? naming, + String? definitionName, + bool reserveDefinitionName = false}) + //TODO: add an optional function arg to remap values + : this.withMapping( + Map.fromEntries( + values.mapIndexed((index, value) => MapEntry(value, index))), + width: width, + name: name, + naming: naming, + definitionName: definitionName, + reserveDefinitionName: reserveDefinitionName); + + // TODO: do we really need to track this isConst? + bool _isConst = false; + bool get isConst => _isConst; + + /// Drives this [LogicEnum] with a constant value matching the enum [value]. + void getsEnum(T value) { + if (!mapping.containsKey(value)) { + //TODO exception + throw Exception('Value $value is not mapped in $mapping for enum $T.'); + } + gets(Const(mapping[value])); + } + + @override + void gets(Logic other) { + if (other is Const) { + if (!mapping.containsValue(other.value)) { + throw Exception( + 'Value ${other.value} is not mapped in $mapping for enum $T.'); + } + + _isConst = true; + } + + super.gets(other); + } + + /// Conditional assignment operator, with added support for [T] enums. + @override + Conditional operator <(dynamic other) { + if (_unassignable) { + throw UnassignableException(this, reason: _unassignableReason); + } + + //TODO: test this! + + if (other is T) { + return super < (clone()..getsEnum(other)); + } else { + return super < other; + } + } + + @override + void put(dynamic val, {bool fill = false}) { + if (val is T) { + if (fill) { + throw Exception(); //TODO + } + + if (!mapping.containsKey(val)) { + throw Exception('Value $val is not mapped in $mapping.'); + } + + // ignore: unnecessary_null_checks + super.put(mapping[val]!); + } else { + super.put(val, fill: fill); + } + } + + bool isEquivalentTypeTo(Logic other) { + if (other is! LogicEnum) { + return false; + } + + final mappingsEqual = const MapEquality().equals( + mapping, + other.mapping, + ); + + if (!mappingsEqual) { + return false; + } + + return true; + } + + @override + LogicEnum clone({String? name}) => LogicEnum.withMapping(mapping, + width: width, + name: name ?? this.name, + naming: + naming, //TODO: use same mechanism as Logic for naming determination + definitionName: definitionName, + reserveDefinitionName: reserveDefinitionName); +} diff --git a/lib/src/signals/signals.dart b/lib/src/signals/signals.dart index 348487a72..8971c04c0 100644 --- a/lib/src/signals/signals.dart +++ b/lib/src/signals/signals.dart @@ -23,3 +23,5 @@ part 'wire_net.dart'; part 'logic_structure.dart'; part 'logic_array.dart'; part 'logic_net.dart'; +part 'logic_enum.dart'; +part 'logic_def.dart'; diff --git a/lib/src/signals/wire.dart b/lib/src/signals/wire.dart index 32241f169..d78a00100 100644 --- a/lib/src/signals/wire.dart +++ b/lib/src/signals/wire.dart @@ -145,6 +145,7 @@ class _Wire { _Wire _adopt(_Wire other) { _glitchController.emitter.adopt(other._glitchController.emitter); other._migrateChangedTriggers(this); + _valueConstraints.addAll(other._valueConstraints); // ignore: avoid_returning_this return this; @@ -245,9 +246,19 @@ class _Wire { newValue = LogicValue.filled(width, LogicValue.x); } + for (final constraint in _valueConstraints) { + newValue = constraint(newValue); + } + _updateValue(newValue, signalName: signalName); } + //TODO docs + final List<_LogicValueConstraint> _valueConstraints = []; + void _constrainValue(_LogicValueConstraint constraint) { + _valueConstraints.add(constraint); + } + /// Updates the value of this signal to [newValue]. void _updateValue(LogicValue newValue, {required String signalName}) { final prevValue = value; @@ -265,3 +276,6 @@ class _Wire { @override String toString() => 'wire $hashCode'; } + +//TODO docs +typedef _LogicValueConstraint = LogicValue Function(LogicValue origValue); diff --git a/lib/src/synthesizers/systemverilog/systemverilog_synth_module_definition.dart b/lib/src/synthesizers/systemverilog/systemverilog_synth_module_definition.dart index b2758c742..45bd6c808 100644 --- a/lib/src/synthesizers/systemverilog/systemverilog_synth_module_definition.dart +++ b/lib/src/synthesizers/systemverilog/systemverilog_synth_module_definition.dart @@ -14,7 +14,13 @@ import 'package:rohd/src/synthesizers/utilities/utilities.dart'; /// A special [SynthModuleDefinition] for SystemVerilog modules. class SystemVerilogSynthModuleDefinition extends SynthModuleDefinition { /// Creates a new [SystemVerilogSynthModuleDefinition] for the given [module]. - SystemVerilogSynthModuleDefinition(super.module); + SystemVerilogSynthModuleDefinition(super.module) + : assert( + !(module is SystemVerilog && + module.generatedDefinitionType == + DefinitionGenerationType.none), + 'Do not build a definition for a module' + ' which generates no definition!'); @override void process() { diff --git a/lib/src/synthesizers/systemverilog/systemverilog_synthesis_result.dart b/lib/src/synthesizers/systemverilog/systemverilog_synthesis_result.dart index 069f3e31a..cdb4b11bb 100644 --- a/lib/src/synthesizers/systemverilog/systemverilog_synthesis_result.dart +++ b/lib/src/synthesizers/systemverilog/systemverilog_synthesis_result.dart @@ -11,12 +11,28 @@ import 'package:collection/collection.dart'; import 'package:rohd/rohd.dart'; import 'package:rohd/src/synthesizers/systemverilog/systemverilog_synth_module_definition.dart'; import 'package:rohd/src/synthesizers/systemverilog/systemverilog_synth_sub_module_instantiation.dart'; +import 'package:rohd/src/synthesizers/utilities/synth_enum_definition.dart'; import 'package:rohd/src/synthesizers/utilities/utilities.dart'; /// Extra utilities on [SynthLogic] to help with SystemVerilog synthesis. extension on SynthLogic { /// Gets the SystemVerilog type for this signal. - String definitionType() => isNet ? 'wire' : 'logic'; + String definitionType() => isEnum + ? enumDefinition!.definitionName + : isNet + ? 'wire' + : 'logic'; +} + +extension on SynthEnumDefinition { + String toSystemVerilogTypedef() { + final enumName = definitionName; + final enumType = 'logic [${characteristicEnum.width - 1}:0]'; + final enumValues = enumToNameMapping.entries + .map((e) => '${e.value} = ${characteristicEnum.mapping[e.key]}') + .join(', '); + return 'typedef enum $enumType { $enumValues } $enumName;'; + } } /// A [SynthesisResult] representing a [Module] that provides a custom @@ -152,6 +168,7 @@ class SystemVerilogSynthesisResult extends SynthesisResult { 'Net connections should have been implemented as' ' bidirectional net connections.'); + // TODO: if we have an enum assigned to a constant, then use enum! assignmentLines .add('assign ${assignment.dst.name} = ${assignment.src.name};'); } @@ -178,11 +195,19 @@ class SystemVerilogSynthesisResult extends SynthesisResult { return subModuleLines.join('\n'); } + /// Internal `typedef` definitions for this module. + String _verilogTypedefs() => _enumTypeDefs(); + + String _enumTypeDefs() => _synthModuleDefinition.enumDefinitions + .map((e) => e.toSystemVerilogTypedef()) + .join('\n'); + /// The contents of this module converted to SystemVerilog without module /// declaration, ports, etc. String _verilogModuleContents( String Function(Module module) getInstanceTypeOfModule) => [ + _verilogTypedefs(), _verilogInternalSignals(), _verilogAssignments(), // order matters! _verilogSubModuleInstantiations(getInstanceTypeOfModule), @@ -202,6 +227,9 @@ class SystemVerilogSynthesisResult extends SynthesisResult { return null; } + //TODO: throw error if there are multiple definitionParameters with the + // same name + return [ '#(', defParams diff --git a/lib/src/synthesizers/systemverilog/systemverilog_synthesizer.dart b/lib/src/synthesizers/systemverilog/systemverilog_synthesizer.dart index d8b5bae36..84a89559b 100644 --- a/lib/src/synthesizers/systemverilog/systemverilog_synthesizer.dart +++ b/lib/src/synthesizers/systemverilog/systemverilog_synthesizer.dart @@ -15,6 +15,8 @@ import 'package:rohd/src/synthesizers/systemverilog/systemverilog_synthesis_resu /// /// Attempts to maintain signal naming and structure as much as possible. class SystemVerilogSynthesizer extends Synthesizer { + //TODO: add configuration for whether to emit enums or not + @override bool generatesDefinition(Module module) => // ignore: deprecated_member_use_from_same_package diff --git a/lib/src/synthesizers/utilities/synth_enum_definition.dart b/lib/src/synthesizers/utilities/synth_enum_definition.dart new file mode 100644 index 000000000..24f69d30b --- /dev/null +++ b/lib/src/synthesizers/utilities/synth_enum_definition.dart @@ -0,0 +1,58 @@ +import 'package:collection/collection.dart'; +import 'package:meta/meta.dart'; +import 'package:rohd/rohd.dart'; +import 'package:rohd/src/utilities/uniquifier.dart'; + +@immutable +class SynthEnumDefinition { + final LogicEnum characteristicEnum; + + final String definitionName; + final Map enumToNameMapping; + + SynthEnumDefinition(this.characteristicEnum, Uniquifier identifierUniquifier) + //TODO: sanitization! + : definitionName = identifierUniquifier.getUniqueName( + initialName: characteristicEnum.definitionName, + reserved: characteristicEnum.reserveDefinitionName), + enumToNameMapping = Map.unmodifiable(characteristicEnum.mapping.map( + (key, value) => MapEntry( + key, + identifierUniquifier.getUniqueName( + initialName: key.name, + reserved: characteristicEnum.reserveDefinitionName), + ), + )); +} + +@immutable +class SynthEnumDefinitionKey { + //TODO: finish up this key as a lookup key for SynthEnumDefinition + final Map enumMapping; + final String? reservedName; + SynthEnumDefinitionKey(LogicEnum characteristicEnum) + : enumMapping = characteristicEnum.mapping, + reservedName = characteristicEnum.reserveDefinitionName + ? characteristicEnum.definitionName + : null; + + @override + bool operator ==(Object other) { + if (identical(this, other)) { + return true; + } + if (other.runtimeType != runtimeType) { + return false; + } + + return other is SynthEnumDefinitionKey && + const MapEquality() + .equals(other.enumMapping, enumMapping) && + other.reservedName == reservedName; + } + + @override + int get hashCode => + const MapEquality().hash(enumMapping) ^ + reservedName.hashCode; +} diff --git a/lib/src/synthesizers/utilities/synth_logic.dart b/lib/src/synthesizers/utilities/synth_logic.dart index 182774cf8..98caa92bb 100644 --- a/lib/src/synthesizers/utilities/synth_logic.dart +++ b/lib/src/synthesizers/utilities/synth_logic.dart @@ -9,6 +9,7 @@ import 'package:collection/collection.dart'; import 'package:rohd/rohd.dart'; +import 'package:rohd/src/synthesizers/utilities/synth_enum_definition.dart'; import 'package:rohd/src/utilities/sanitizer.dart'; import 'package:rohd/src/utilities/uniquifier.dart'; @@ -46,6 +47,22 @@ class SynthLogic { /// The [Logic] whose name is renameable, if there is one. Logic? _renameableLogic; + /// A [LogicEnum] that is characteristic of any merged [LogicEnum]s into this. + LogicEnum? get characteristicEnum => _characteristicEnum; + + /// The first [LogicEnum] merged into this [SynthLogic], if there is one. + LogicEnum? _characteristicEnum; + + SynthEnumDefinition? get enumDefinition => _enumDefinition; + set enumDefinition(SynthEnumDefinition? definition) { + assert(definition != null, 'Cannot set enum definition to null.'); + assert( + _enumDefinition == null, 'Cannot set enum definition more than once.'); + _enumDefinition = definition; + } + + SynthEnumDefinition? _enumDefinition; + /// [Logic]s that are marked mergeable. final Set _mergeableLogics = {}; @@ -68,6 +85,9 @@ class SynthLogic { // can just look at the first since nets and non-nets cannot be merged logics.first.isNet || (isArray && (logics.first as LogicArray).isNet); + /// Whether this represents an enum. + bool get isEnum => characteristicEnum != null; + /// If set, then this should never pick the constant as the name. bool get constNameDisallowed => _constNameDisallowed; bool _constNameDisallowed; @@ -106,14 +126,22 @@ class SynthLogic { assert(_name == null, 'Should only pick a name once.'); _name = _findName(uniquifier); + //TODO: dont allow merge after name picked? } /// Finds the best name from the collection of [Logic]s. String _findName(Uniquifier uniquifier) { // check for const - if (_constLogic != null) { + if (isConstant) { if (!_constNameDisallowed) { - return _constLogic!.value.toString(); + if (isEnum) { + return enumDefinition!.enumToNameMapping[characteristicEnum! + .mapping.entries + .firstWhere((e) => e.value == _constLogic!.value) + .key]!; + } else { + return _constLogic!.value.toString(); + } } else { assert( logics.length > 1, @@ -122,6 +150,8 @@ class SynthLogic { } } + //TODO: for enums, all the value names must be unique in the scope as well! + // check for reserved if (_reservedLogic != null) { return uniquifier.getUniqueName( @@ -184,18 +214,32 @@ class SynthLogic { /// Returns the [SynthLogic] that should be *removed*. static SynthLogic? tryMerge(SynthLogic a, SynthLogic b) { + assert(a != b, 'Cannot merge a SynthLogic with itself.'); + if (_constantsMergeable(a, b)) { // case to avoid things like a constant assigned to another constant a.adopt(b); return b; } - if (!a.mergeable && !b.mergeable) { + if (a.isNet != b.isNet) { + // do not merge nets with non-nets return null; } - if (a.isNet != b.isNet) { - // do not merge nets with non-nets + if (_enumAndConstMergeable(a, b)) { + a.adopt(b); + return b; + } else if (a.isEnum && b.isEnum) { + //TODO: test this scenario + + // don't merge enums if they are not mergeable + return null; + } + + //TODO: should enum and non-enum be mergeable? + + if (!a.mergeable && !b.mergeable) { return null; } @@ -216,12 +260,67 @@ class SynthLogic { !a._constNameDisallowed && !b._constNameDisallowed; + /// Indicates whether [a] and [b] represent enum(s) and constant(s) that + /// can be merged. + static bool _enumAndConstMergeable(SynthLogic a, SynthLogic b) { + final enums = [a, b].where((e) => e.isEnum).toList(growable: false); + final constants = [a, b].where((e) => e.isConstant).toList(growable: false); + + // if no enums, then this is not a reason to merge + if (enums.isEmpty) { + return false; + } + + // if we have two enums, make sure they are compatible + if (enums.length == 2) { + if (!enums[0] + .characteristicEnum! + .isEquivalentTypeTo(enums[1].characteristicEnum!)) { + return false; + } + + if (enums[0].characteristicEnum!.reserveDefinitionName && + enums[1].characteristicEnum!.reserveDefinitionName && + enums[0].characteristicEnum!.definitionName != + enums[1].characteristicEnum!.definitionName) { + return false; + } + + for (final e in enums) { + if (!e.mergeable) { + return false; + } + } + } + + // if one is constant, then ensure the constant is legal + if (constants.isNotEmpty) { + for (final c in constants) { + for (final e in enums) { + final enumMapping = e.characteristicEnum!.mapping; + if (!enumMapping.values.contains(c._constLogic!.value)) { + return false; + } + } + } + } + + return true; + } + /// Merges [other] to be represented by `this` instead, and updates the /// [other] that it has been replaced. void adopt(SynthLogic other) { - assert(other.mergeable || _constantsMergeable(this, other), + assert( + other.mergeable || + _constantsMergeable(this, other) || + _enumAndConstMergeable(this, other), 'Cannot merge a non-mergeable into this.'); assert(other.isArray == isArray, 'Cannot merge arrays and non-arrays'); + assert( + _name == null, 'Cannot merge into this after a name has been picked.'); + assert(other._name == null, + 'Cannot merge into other after a name has been picked.'); _constNameDisallowed |= other._constNameDisallowed; @@ -229,6 +328,7 @@ class SynthLogic { _constLogic ??= other._constLogic; _reservedLogic ??= other._reservedLogic; _renameableLogic ??= other._renameableLogic; + _characteristicEnum ??= other._characteristicEnum; // the rest, take them all _mergeableLogics.addAll(other._mergeableLogics); @@ -255,6 +355,26 @@ class SynthLogic { _unnamedLogics.add(logic); } } + + if (logic is LogicEnum) { + assert(characteristicEnum?.isEquivalentTypeTo(logic) ?? true, + 'Cannot add a LogicEnum that is not equivalent to the existing one.'); + + if (logic.reserveDefinitionName) { + // if the added `logic` reserves its definition name, then we + // should use it as the characteristic enum + assert( + _characteristicEnum == null || + !_characteristicEnum!.reserveDefinitionName || + logic.definitionName == _characteristicEnum!.definitionName, + 'Cannot add a LogicEnum that reserves its definition name, but has a ' + 'different definition name than the existing characteristic enum.', + ); + _characteristicEnum = logic; + } + + _characteristicEnum ??= logic; + } } @override @@ -273,6 +393,10 @@ class SynthLogic { /// Computes the name of the signal at declaration time with appropriate /// dimensions included. String definitionName() { + if (isEnum) { + return name; + } + String packedDims; String unpackedDims; diff --git a/lib/src/synthesizers/utilities/synth_module_definition.dart b/lib/src/synthesizers/utilities/synth_module_definition.dart index d92df06cf..baf219860 100644 --- a/lib/src/synthesizers/utilities/synth_module_definition.dart +++ b/lib/src/synthesizers/utilities/synth_module_definition.dart @@ -12,6 +12,7 @@ import 'dart:collection'; import 'package:meta/meta.dart'; import 'package:rohd/rohd.dart'; import 'package:rohd/src/collections/traverseable_collection.dart'; +import 'package:rohd/src/synthesizers/utilities/synth_enum_definition.dart'; import 'package:rohd/src/synthesizers/utilities/utilities.dart'; import 'package:rohd/src/utilities/uniquifier.dart'; @@ -79,7 +80,7 @@ class SynthModuleDefinition { /// Used to uniquify any identifiers, including signal names /// and module instances. - final Uniquifier _synthInstantiationNameUniquifier; + final Uniquifier _synthIdentifierUniquifier; /// Either accesses a previously created [SynthLogic] corresponding to /// [logic], or else creates a new one and adds it to the [logicToSynthMap]. @@ -132,19 +133,13 @@ class SynthModuleDefinition { /// Creates a new definition representation for this [module]. SynthModuleDefinition(this.module) - : _synthInstantiationNameUniquifier = Uniquifier( + : _synthIdentifierUniquifier = Uniquifier( reservedNames: { ...module.inputs.keys, ...module.outputs.keys, ...module.inOuts.keys, }, - ), - assert( - !(module is SystemVerilog && - module.generatedDefinitionType == - DefinitionGenerationType.none), - 'Do not build a definition for a module' - ' which generates no definition!') { + ) { // start by traversing output signals final logicsToTraverse = TraverseableCollection() ..addAll(module.outputs.values) @@ -311,6 +306,7 @@ class SynthModuleDefinition { _collapseArrays(); _collapseAssignments(); _assignSubmodulePortMapping(); + _adjustTypePairs(); process(); _pickNames(); } @@ -351,24 +347,65 @@ class SynthModuleDefinition { } } + void _adjustTypePairs() { + for (final submoduleInstantiation + in moduleToSubModuleInstantiationMap.values) { + submoduleInstantiation.adjustTypePairs(); + } + } + + final Map _enumDefinitions = + {}; + + List get enumDefinitions => + _enumDefinitions.values.toList(growable: false); + + void _pickDefinitionEnumName(SynthLogic synthEnum) { + assert(synthEnum.isEnum, 'Only call this on SynthLogic that is an enum.'); + final key = SynthEnumDefinitionKey(synthEnum.characteristicEnum!); + if (_enumDefinitions.containsKey(key)) { + // already have a definition for this enum + synthEnum.enumDefinition = _enumDefinitions[key]; + } else { + // create a new definition for this enum + final newDefinition = SynthEnumDefinition( + synthEnum.characteristicEnum!, + _synthIdentifierUniquifier, + ); + _enumDefinitions[key] = newDefinition; + synthEnum.enumDefinition = newDefinition; + } + } + /// Picks names of signals and sub-modules. void _pickNames() { // first ports get priority for (final input in inputs) { - input.pickName(_synthInstantiationNameUniquifier); + input.pickName(_synthIdentifierUniquifier); } for (final output in outputs) { - output.pickName(_synthInstantiationNameUniquifier); + output.pickName(_synthIdentifierUniquifier); } for (final inOut in inOuts) { - inOut.pickName(_synthInstantiationNameUniquifier); + inOut.pickName(_synthIdentifierUniquifier); + } + + // pick names of *reserved* definition-type enums + final nonReservedEnumDefs = []; + for (final signal in internalSignals.where((e) => e.isEnum)) { + if (signal.characteristicEnum!.reserveDefinitionName) { + _pickDefinitionEnumName(signal); + } else { + nonReservedEnumDefs.add(signal); + } } // pick names of *reserved* submodule instances final nonReservedSubmodules = []; for (final submodule in moduleToSubModuleInstantiationMap.values) { + //TODO: should ensure that uniqueInstanceName is usable! if (submodule.module.reserveName) { - submodule.pickName(_synthInstantiationNameUniquifier); + submodule.pickName(_synthIdentifierUniquifier); assert(submodule.module.name == submodule.name, 'Expect reserved names to retain their name.'); } else { @@ -380,21 +417,24 @@ class SynthModuleDefinition { final nonReservedSignals = []; for (final signal in internalSignals) { if (signal.isReserved) { - signal.pickName(_synthInstantiationNameUniquifier); + signal.pickName(_synthIdentifierUniquifier); } else { nonReservedSignals.add(signal); } } + // then enum definitions that are not reserved + nonReservedEnumDefs.forEach(_pickDefinitionEnumName); + // then submodule instances for (final submodule in nonReservedSubmodules.where((element) => element.needsDeclaration)) { - submodule.pickName(_synthInstantiationNameUniquifier); + submodule.pickName(_synthIdentifierUniquifier); } // then the rest of the internal signals for (final signal in nonReservedSignals) { - signal.pickName(_synthInstantiationNameUniquifier); + signal.pickName(_synthIdentifierUniquifier); } } diff --git a/lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart b/lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart index dd3efae00..1954185d9 100644 --- a/lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart +++ b/lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart @@ -9,7 +9,9 @@ import 'dart:collection'; +import 'package:meta/meta.dart'; import 'package:rohd/rohd.dart'; +import 'package:rohd/src/synthesizers/utilities/synth_enum_definition.dart'; import 'package:rohd/src/synthesizers/utilities/utilities.dart'; import 'package:rohd/src/utilities/uniquifier.dart'; @@ -89,6 +91,41 @@ class SynthSubModuleInstantiation { _inOutMapping[name] = synthLogic; } + @internal + void adjustTypePairs() { + for (final MapEntry(key: toUpdate, value: reference) + in module.portTypePairs.entries) { + final toUpdateSynth = inputMapping[toUpdate.name] ?? + outputMapping[toUpdate.name] ?? + inOutMapping[toUpdate.name]!; + final referenceSynth = inputMapping[reference.name] ?? + outputMapping[reference.name] ?? + inOutMapping[reference.name]!; + + if (referenceSynth.isEnum) { + if (toUpdateSynth.isEnum && + SynthEnumDefinitionKey(toUpdateSynth.characteristicEnum!) == + SynthEnumDefinitionKey(referenceSynth.characteristicEnum!)) { + // If the types are equivalent, we can just use the original, no need + // to do any additional merging. + continue; + } + + final mergeResult = SynthLogic.tryMerge( + toUpdateSynth, + SynthLogic( + referenceSynth.characteristicEnum!.clone(name: 'reference')), + ); + if (mergeResult == null) { + //TODO + throw Exception('Unmergeable types'); + } + assert(mergeResult != toUpdateSynth, + 'We should not be replacing the original one.'); + } + } + } + /// Indicates whether this module should be declared. bool get needsDeclaration => _needsDeclaration; bool _needsDeclaration = true; diff --git a/test/fsm_test.dart b/test/fsm_test.dart index b5f010a56..da5e4ec0d 100644 --- a/test/fsm_test.dart +++ b/test/fsm_test.dart @@ -115,6 +115,9 @@ class TrafficTestModule extends Module { final northLight = addOutput('northLight', width: traffic.width); final eastLight = addOutput('eastLight', width: traffic.width); + //TODO: can we use lightcolor as an enum also in here? + // TODO: make sure ports and enums don't merge badly! + final clk = SimpleClockGenerator(10).clk; final states = >[ @@ -343,7 +346,7 @@ void main() { }) ]; await SimCompare.checkFunctionalVector(mod, vectors); - SimCompare.checkIverilogVector(mod, vectors); + SimCompare.checkIverilogVector(mod, vectors, dontDeleteTmpFiles: true); verifyMermaidStateDiagram(_trafficFSMPath); }); diff --git a/test/logic_enum_test.dart b/test/logic_enum_test.dart new file mode 100644 index 000000000..1b9527247 --- /dev/null +++ b/test/logic_enum_test.dart @@ -0,0 +1,165 @@ +import 'package:rohd/rohd.dart'; +import 'package:rohd/src/signals/signals.dart'; +import 'package:test/test.dart'; + +enum TestEnum { a, b, c } + +class MyListLogicEnum extends LogicEnum { + MyListLogicEnum({super.name}) : super(TestEnum.values); +} + +class MyMapLogicEnum extends LogicEnum { + MyMapLogicEnum({super.name}) + : super.withMapping({ + TestEnum.a: 1, + // TestEnum.b: 5, // `b` is not mapped! + TestEnum.c: 7, + }, width: 3); +} + +class SimpleModWithEnum extends Module { + SimpleModWithEnum(Logic carrot) { + carrot = addInput('carrot', carrot, width: 3); + final e = MyMapLogicEnum(name: 'elephant'); + addOutput('banana', width: 3) <= carrot & e; + } +} + +class ConflictingEnumMod extends Module { + ConflictingEnumMod(Logic carrot) { + carrot = addInput('carrot', carrot, width: 3); + final e1 = MyListLogicEnum(name: 'elephantList'); + final e2 = MyMapLogicEnum(name: 'elephantMap'); + + addOutput('banana', width: 3) <= carrot & (e1.zeroExtend(3) ^ e2); + } +} + +class ModWithEnumConstAssignment extends Module { + ModWithEnumConstAssignment(Logic carrot) { + carrot = addInput('carrot', carrot, width: 2); + final e = MyListLogicEnum(name: 'elephant')..getsEnum(TestEnum.b); + addOutput('banana', width: 2) <= carrot & e; + } +} + +class ModWithCaseAndEnumCondAssign extends Module { + ModWithCaseAndEnumCondAssign(Logic durian) { + durian = addInput('durian', durian); + + final currState = MyListLogicEnum(name: 'currState'); + final nextState = MyListLogicEnum(name: 'nextState'); + + nextState <= + cases( + currState, + { + 0: 0, + MyListLogicEnum()..getsEnum(TestEnum.b): MyListLogicEnum() + ..getsEnum(TestEnum.b), + // TestEnum.c: TestEnum.c, + TestEnum.c: 2, + //TODO: get all these working nicely + }, + width: 2); + + addOutput('pineapple') <= durian & nextState.xor(); + } +} + +void main() { + test('enum populates based on list of values', () { + final e = MyListLogicEnum(); + + expect(e.mapping.length, TestEnum.values.length); + expect(e.width, 2); + + var idx = 0; + for (final val in TestEnum.values) { + expect(e.mapping.containsKey(val), isTrue); + expect(e.mapping[val]!.width, e.width); + expect(e.mapping[val]!.toInt(), idx++); + } + }); + + test('enum only allows legal values', () { + final e = MyListLogicEnum(); + expect(e.value.isFloating, isTrue); + e.put(0); + expect(e.value.toInt(), 0); + expect(e.valueEnum, TestEnum.a); + e.put(1); + expect(e.value.toInt(), 1); + expect(e.valueEnum, TestEnum.b); + e.put(2); + expect(e.value.toInt(), 2); + expect(e.valueEnum, TestEnum.c); + e.put(3); + expect(e.value, LogicValue.filled(e.width, LogicValue.x)); + // expect(() => e.valueEnum, throwsA(isA())); //TODO + }); + + test('enum puts with enums', () { + final e = MyListLogicEnum()..put(TestEnum.b); + expect(e.value.toInt(), TestEnum.b.index); + expect(e.valueEnum, TestEnum.b); + }); + + group('enum sv gen', () { + test('simple mod with enum gen good sv', () async { + final mod = SimpleModWithEnum(Logic(width: 3)); + await mod.build(); + + final sv = mod.generateSynth(); + + print(sv); + + expect( + sv, + contains( + "typedef enum logic [2:0] { a = 3'h1, c = 3'h7 } TestEnum;")); + expect(sv, contains('TestEnum elephant;')); + }); + + test('conflicting enum mod gen good sv', () async { + final mod = ConflictingEnumMod(Logic(width: 3)); + await mod.build(); + + final sv = mod.generateSynth(); + + // don't care which one has _0, but one of them does! + expect( + sv, + contains( + 'typedef enum logic [2:0]' " { a = 3'h1, c = 3'h7 } TestEnum;")); + expect( + sv, + contains('typedef enum logic [1:0]' + " { a_0 = 2'h0, b = 2'h1, c_0 = 2'h2 } TestEnum_0;")); + }); + + test('enum constant assignment uses enum name', () async { + final mod = ModWithEnumConstAssignment(Logic(width: 2)); + await mod.build(); + + final sv = mod.generateSynth(); + + expect( + sv, + contains('typedef enum logic [1:0]' + " { a = 2'h0, b = 2'h1, c = 2'h2 } TestEnum;")); + expect(sv, contains('assign banana = carrot & b;')); + }); + + test('enum with case and cond assignments', () async { + final mod = ModWithCaseAndEnumCondAssign(Logic()); + await mod.build(); + + final sv = mod.generateSynth(); + print(sv); + + expect(sv, contains(' a : begin')); + expect(sv, contains('nextState = a;')); + }); + }); +}