diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d1d25bae..d7681db89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## Next release +- Improved simulation performance and generated outputs by avoiding module creation for four-state-safe operations involving `Const`s, zero shifts, and muxes with constant controls (). +- Enforced that `Const` values cannot be changed through `put` or `inject`, including through another `Logic` driven by a `Const` (). - Added per-`Const` preferred radix control for generated literals, normalized `Const` names to reflect their displayed radix, and enhanced `LogicValue.toRadixString` to omit its width and radix decorator or digit separators (). - Improved generated SystemVerilog to collapse contiguous partial array and range assignments into packed slice assignments when safe (). - Added configurable explicit or implicit object and data types for generated SystemVerilog ports, defaulting to `input wire logic`, `output var logic`, and `inout wire logic` (). diff --git a/lib/src/modules/gates.dart b/lib/src/modules/gates.dart index 2780cde9d..9bdc858eb 100644 --- a/lib/src/modules/gates.dart +++ b/lib/src/modules/gates.dart @@ -815,7 +815,22 @@ class LShift extends _ShiftGate { /// ```SystemVerilog /// control ? d1 : d0 /// ``` -Logic mux(Logic control, Logic d1, Logic d0) => Mux(control, d1, d0).out; +/// +/// If [control] is a valid [Const], returns the selected input directly. +Logic mux(Logic control, Logic d1, Logic d0) { + if (control.width != 1) { + throw PortWidthMismatchException(control, 1); + } + if (d0.width != d1.width) { + throw PortWidthMismatchException.equalWidth(d0, d1); + } + + if (control is Const && control.value.isValid) { + return control.value == LogicValue.one ? d1 : d0; + } + + return Mux(control, d1, d0).out; +} /// A mux (multiplexer) module. /// diff --git a/lib/src/signals/const.dart b/lib/src/signals/const.dart index 10787e10b..3e6989145 100644 --- a/lib/src/signals/const.dart +++ b/lib/src/signals/const.dart @@ -45,7 +45,16 @@ String _constName(LogicValue value, int? preferredRadix) { } /// Represents a [Logic] that never changes value. +/// +/// Attempts to assign, [put], or [inject] a new value throw an +/// [UnassignableException], including through another [Logic] driven by the +/// [Const]. class Const extends Logic { + /// The explanation included when an attempt is made to modify a [Const]. + static const _unassignableMessage = + 'A `Const` value cannot be modified, including through another `Logic` ' + 'driven by it.'; + /// The preferred radix for displaying this constant in generated outputs. /// /// Supported radices are binary (2), octal (8), decimal (10), and @@ -88,12 +97,136 @@ class Const extends Logic { // we don't care about maintaining this node unless necessary naming: Naming.unnamed, ) { - _wire.put(value, signalName: name); + _wire + ..put(value, signalName: name) + ..makeImmutable(this, reason: _unassignableMessage); - makeUnassignable(reason: '`Const` signals are unassignable.'); + makeUnassignable(reason: _unassignableMessage); } @override Const clone({String? name}) => Const(value, width: width, preferredRadix: preferredRadix); + + @override + void put(dynamic val, {bool fill = false}) => + throw UnassignableException(this, reason: _unassignableMessage); + + @override + void inject(dynamic val, {bool fill = false}) => + throw UnassignableException(this, reason: _unassignableMessage); + + /// Verifies that [other] has the same width as this [Const]. + void _checkMatchingWidth(Logic other) { + if (width != other.width) { + throw PortWidthMismatchException.equalWidth(this, other); + } + } + + @override + Logic operator ~() => Const(~value); + + @override + Logic operator &(Logic other) { + _checkMatchingWidth(other); + + if (other is Const) { + return Const(value & other.value); + } else if (value.isValid && value.isZero) { + return this; + } + + return And2Gate(this, other).out; + } + + @override + Logic operator |(Logic other) { + _checkMatchingWidth(other); + + if (other is Const) { + return Const(value | other.value); + } else if (value.isValid && + value == LogicValue.filled(width, LogicValue.one)) { + return this; + } + + return Or2Gate(this, other).out; + } + + @override + Logic operator ^(Logic other) { + _checkMatchingWidth(other); + + if (other is Const) { + return Const(value ^ other.value); + } + + return Xor2Gate(this, other).out; + } + + @override + Logic operator >>(dynamic other) { + if (Logic._isZeroShiftAmount(other)) { + return this; + } else if (other is Logic && other is! Const) { + return ARShift(this, other).out; + } + + return Const(value >> (other is Const ? other.value : other)); + } + + @override + Logic operator <<(dynamic other) { + if (Logic._isZeroShiftAmount(other)) { + return this; + } else if (other is Logic && other is! Const) { + return LShift(this, other).out; + } + + return Const(value << (other is Const ? other.value : other)); + } + + @override + Logic operator >>>(dynamic other) { + if (Logic._isZeroShiftAmount(other)) { + return this; + } else if (other is Logic && other is! Const) { + return RShift(this, other).out; + } + + return Const(value >>> (other is Const ? other.value : other)); + } + + @override + Logic and() => Const(value.and()); + + @override + Logic or() => Const(value.or()); + + @override + Logic xor() => Const(value.xor()); + + @override + Logic eq(dynamic other) { + if (other is Logic) { + _checkMatchingWidth(other); + return other is Const + ? Const(value.eq(other.value)) + : Equals(this, other).out; + } + + return Const(value.eq(LogicValue.of(other, width: width))); + } + + @override + Logic neq(dynamic other) { + if (other is Logic) { + _checkMatchingWidth(other); + return other is Const + ? Const(value.neq(other.value)) + : NotEquals(this, other).out; + } + + return Const(value.neq(LogicValue.of(other, width: width))); + } } diff --git a/lib/src/signals/logic.dart b/lib/src/signals/logic.dart index a412f0943..aca0e5b7c 100644 --- a/lib/src/signals/logic.dart +++ b/lib/src/signals/logic.dart @@ -445,10 +445,22 @@ class Logic { Logic operator ~() => NotGate(this).out; /// Logical bitwise AND. - Logic operator &(Logic other) => And2Gate(this, other).out; + Logic operator &(Logic other) { + if (other is Const) { + return other & this; + } + + return And2Gate(this, other).out; + } /// Logical bitwise OR. - Logic operator |(Logic other) => Or2Gate(this, other).out; + Logic operator |(Logic other) { + if (other is Const) { + return other | this; + } + + return Or2Gate(this, other).out; + } /// Logical bitwise XOR. Logic operator ^(Logic other) => Xor2Gate(this, other).out; @@ -478,6 +490,10 @@ class Logic { /// /// If [isNet] and [other] is constant, then the result will also be a net. Logic operator >>(dynamic other) { + if (_isZeroShiftAmount(other)) { + return this; + } + if (isNet) { // many SV simulators don't support shifting of nets, so default this final shamt = _constShiftAmount(other); @@ -498,6 +514,10 @@ class Logic { /// /// If [isNet] and [other] is constant, then the result will also be a net. Logic operator <<(dynamic other) { + if (_isZeroShiftAmount(other)) { + return this; + } + if (isNet) { // many SV simulators don't support shifting of nets, so default this final shamt = _constShiftAmount(other); @@ -518,6 +538,10 @@ class Logic { /// /// If [isNet] and [other] is constant, then the result will also be a net. Logic operator >>>(dynamic other) { + if (_isZeroShiftAmount(other)) { + return this; + } + if (isNet) { // many SV simulators don't support shifting of nets, so default this final shamt = _constShiftAmount(other); @@ -543,6 +567,17 @@ class Logic { } } + /// Indicates whether [other] represents a known constant shift of zero. + static bool _isZeroShiftAmount(dynamic other) { + if (other is Const) { + return other.value.isValid && other.value.isZero; + } else if (other is Logic) { + return false; + } else { + return LogicValue.ofInferWidth(other).isZero; + } + } + /// Unary AND. Logic and() => AndUnary(this).out; diff --git a/lib/src/signals/wire.dart b/lib/src/signals/wire.dart index 883e3eaf0..812b09866 100644 --- a/lib/src/signals/wire.dart +++ b/lib/src/signals/wire.dart @@ -26,6 +26,39 @@ class _Wire { /// The current active value of this signal. LogicValue _currentValue; + /// The [Logic] whose immutability makes this wire immutable. + Logic? _immutableOwner; + + /// Additional context explaining why this wire is immutable. + String? _immutableReason; + + /// Makes this wire immutable on behalf of [owner]. + /// + /// If this wire is already immutable, the original owner and reason are + /// preserved. + void makeImmutable(Logic owner, {String? reason}) { + _immutableOwner ??= owner; + _immutableReason ??= reason; + } + + /// Throws an [UnassignableException] if this wire is immutable, identifying + /// [signalName] as the signal through which the update was attempted. + void _assertMutable({required String signalName}) { + final immutableOwner = _immutableOwner; + if (immutableOwner != null) { + final attemptedUpdateReason = + 'Signal "$signalName" cannot be updated because it is driven by ' + 'immutable signal "$immutableOwner".'; + throw UnassignableException( + immutableOwner, + reason: [ + attemptedUpdateReason, + if (_immutableReason != null) _immutableReason, + ].join(' '), + ); + } + } + /// The last value of this signal before the [Simulator] tick. /// /// This is useful for detecting when to trigger an edge. @@ -143,6 +176,11 @@ class _Wire { /// Tells this [_Wire] to adopt all the behavior of [other] so that /// it can replace [other]. Returns the [_Wire] that has adopted everything. _Wire _adopt(_Wire other) { + if (_immutableOwner == null && other._immutableOwner != null) { + _immutableOwner = other._immutableOwner; + _immutableReason = other._immutableReason; + } + _glitchController.emitter.adopt(other._glitchController.emitter); other._migrateChangedTriggers(this); @@ -218,6 +256,7 @@ class _Wire { /// /// This function calls [put()] in [Simulator.injectAction()]. void inject(dynamic val, {required String signalName, bool fill = false}) { + _assertMutable(signalName: signalName); Simulator.injectAction(() => put(val, signalName: signalName, fill: fill)); } @@ -233,6 +272,8 @@ class _Wire { /// This function is used for propagating glitches through connected signals. /// Use this function for custom definitions of [Module] behavior. void put(dynamic val, {required String signalName, bool fill = false}) { + _assertMutable(signalName: signalName); + var newValue = LogicValue.of(val, fill: fill, width: width); if (newValue.width != width) { diff --git a/test/gate_test.dart b/test/gate_test.dart index 905c912d9..b2c47c21e 100644 --- a/test/gate_test.dart +++ b/test/gate_test.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2024 Intel Corporation +// Copyright (C) 2021-2026 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // gate_test.dart @@ -68,7 +68,8 @@ class Absolute extends Module { class ShiftTestModule extends Module { dynamic constant; // int or BigInt - ShiftTestModule(Logic a, Logic b, {this.constant = 3}) + ShiftTestModule(Logic a, Logic b, + {this.constant = 3, bool useExplicitConstShiftModules = false}) : super(name: 'shifttestmodule') { a = addInput('a', a, width: a.width); b = addInput('b', b, width: b.width); @@ -84,9 +85,15 @@ class ShiftTestModule extends Module { aRshiftB <= a >>> b; aLshiftB <= a << b; aArshiftB <= a >> b; - aRshiftConst <= a >>> constant; - aLshiftConst <= a << constant; - aArshiftConst <= a >> constant; + if (useExplicitConstShiftModules) { + aRshiftConst <= RShift(a, constant).out; + aLshiftConst <= LShift(a, constant).out; + aArshiftConst <= ARShift(a, constant).out; + } else { + aRshiftConst <= a >>> constant; + aLshiftConst <= a << constant; + aArshiftConst <= a >> constant; + } } } @@ -112,6 +119,20 @@ class IndexGateTestModule extends Module { } } +/// A module with a signal driven by a [Const]. +class ConstAliasModule extends Module { + /// The constant source protected from mutation. + late final Const constant = Const(0); + + /// A named signal driven by [constant]. + late final Logic alias = constant.named('alias'); + + /// Creates a module whose output is driven by [alias]. + ConstAliasModule() : super(name: 'constaliasmodule') { + addOutput('out') <= alias; + } +} + void main() { tearDown(() async { await Simulator.reset(); @@ -352,6 +373,197 @@ void main() { }); }); + group('constant gate optimizations', () { + test('Const cannot be mutated', () async { + final module = ConstAliasModule(); + final constant = module.constant; + final alias = module.alias; + + expect(() => constant.put(1), throwsA(isA())); + expect(() => constant.inject(1), throwsA(isA())); + expect( + () => alias.put(1), + throwsA(isA().having( + (exception) => exception.message, + 'message', + allOf( + contains('"${alias.name}" cannot be updated'), + contains('is driven by immutable signal'), + contains('A `Const` value cannot be modified'), + )))); + expect(() => alias.inject(1), throwsA(isA())); + expect(constant.value, LogicValue.zero); + expect(alias.value, LogicValue.zero); + + await module.build(); + expect(module.generateSynth(), contains("1'h0")); + }); + + test('bitwise NOT folds Const inputs', () { + final result = ~Const(LogicValue.ofString('01xz')); + + expect(result, isA()); + expect(result.value, LogicValue.ofString('10xx')); + + final normalResult = ~(Logic()..put(1)); + expect(normalResult, isNot(isA())); + expect(normalResult.value, LogicValue.zero); + }); + + test('bitwise AND folds only four-state-safe cases', () { + final signal = Logic(width: 4)..put(LogicValue.ofString('10z1')); + final zeros = Const(0, width: 4); + final ones = Const(0xf, width: 4); + + expect(signal & zeros, same(zeros)); + expect(zeros & signal, same(zeros)); + + final identityResult = signal & ones; + expect(identityResult, isNot(same(signal))); + expect(identityResult.value, LogicValue.ofString('10x1')); + + final folded = Const(LogicValue.ofString('11x1')) & + Const(LogicValue.ofString('1011')); + expect(folded, isA()); + expect(folded.value, LogicValue.ofString('10x1')); + + expect(() => Const(0, width: 2) & Logic(), + throwsA(isA())); + }); + + test('bitwise OR folds only four-state-safe cases', () { + final signal = Logic(width: 4)..put(LogicValue.ofString('01z0')); + final zeros = Const(0, width: 4); + final ones = Const(0xf, width: 4); + + expect(signal | ones, same(ones)); + expect(ones | signal, same(ones)); + + final identityResult = signal | zeros; + expect(identityResult, isNot(same(signal))); + expect(identityResult.value, LogicValue.ofString('01x0')); + + final folded = Const(LogicValue.ofString('10x0')) | + Const(LogicValue.ofString('0101')); + expect(folded, isA()); + expect(folded.value, LogicValue.ofString('11x1')); + + expect(() => Const(0, width: 2) | Logic(), + throwsA(isA())); + }); + + test('bitwise XOR folds only when both inputs are Const', () { + final folded = Const(LogicValue.ofString('10x0')) ^ + Const(LogicValue.ofString('0101')); + expect(folded, isA()); + expect(folded.value, LogicValue.ofString('11x1')); + + final signal = Logic(width: 4)..put(LogicValue.ofString('01z0')); + final identityResult = Const(0, width: 4) ^ signal; + expect(identityResult, isNot(same(signal))); + expect(identityResult.value, LogicValue.ofString('01x0')); + }); + + test('reductions and comparisons fold Const inputs', () { + final value = Const(0xa, width: 4); + + expect(value.and(), isA()); + expect(value.and().value, LogicValue.zero); + expect(value.or().value, LogicValue.one); + expect(value.xor().value, LogicValue.zero); + + final equal = value.eq(Const(0xa, width: 4)); + final notEqual = value.neq(0xb); + final notEqualConst = value.neq(Const(0xb, width: 4)); + expect(equal, isA()); + expect(equal.value, LogicValue.one); + expect(notEqual, isA()); + expect(notEqual.value, LogicValue.one); + expect(notEqualConst, isA()); + expect(notEqualConst.value, LogicValue.one); + + final unknown = Const(LogicValue.z).eq(Const(LogicValue.z)); + expect(unknown, isA()); + expect(unknown.value, LogicValue.x); + }); + + test('comparisons with mutable Logic retain comparison modules', () { + final value = Const(0xa, width: 4); + final mutable = Logic(width: 4)..put(0xa); + + final equal = value.eq(mutable); + final notEqual = value.neq(mutable); + + expect(equal.parentModule, isA()); + expect(notEqual.parentModule, isA()); + expect(equal.value, LogicValue.one); + expect(notEqual.value, LogicValue.zero); + + mutable.put(0xb); + expect(equal.value, LogicValue.zero); + expect(notEqual.value, LogicValue.one); + }); + + test('shifts by constant zero return the original signal', () { + final signal = Logic(width: 4)..put(LogicValue.ofString('10z1')); + + expect(signal << 0, same(signal)); + expect(signal >> Const(0), same(signal)); + expect(signal >>> BigInt.zero, same(signal)); + + final constant = Const(bin('1010'), width: 4); + expect(constant << 0, same(constant)); + expect(constant >> BigInt.zero, same(constant)); + expect(constant >>> Const(0), same(constant)); + + final left = constant << 1; + final right = constant >>> 1; + final arithmeticRight = constant >> 1; + expect(left, isA()); + expect(left.value, LogicValue.ofString('0100')); + expect(right.value, LogicValue.ofString('0101')); + expect(arithmeticRight.value, LogicValue.ofString('1101')); + }); + + test('Const shifts by mutable Logic retain shift modules', () { + final constant = Const(bin('1010'), width: 4); + final amount = Logic(width: 2)..put(1); + + final left = constant << amount; + final right = constant >>> amount; + final arithmeticRight = constant >> amount; + + expect(left.parentModule, isA()); + expect(right.parentModule, isA()); + expect(arithmeticRight.parentModule, isA()); + expect(left.value, LogicValue.ofString('0100')); + expect(right.value, LogicValue.ofString('0101')); + expect(arithmeticRight.value, LogicValue.ofString('1101')); + + amount.put(2); + expect(left.value, LogicValue.ofString('1000')); + expect(right.value, LogicValue.ofString('0010')); + expect(arithmeticRight.value, LogicValue.ofString('1110')); + }); + + test('mux bypasses only valid constant controls', () { + final d0 = Logic(width: 4); + final d1 = Logic(width: 4); + + expect(mux(Const(0), d1, d0), same(d0)); + expect(mux(Const(1), d1, d0), same(d1)); + + final invalidResult = mux(Const(LogicValue.z), d1, d0); + expect(invalidResult, isNot(anyOf(same(d0), same(d1)))); + expect(invalidResult.value, LogicValue.filled(4, LogicValue.x)); + + expect(() => mux(Const(0, width: 2), d1, d0), + throwsA(isA())); + expect(() => mux(Const(0), Logic(width: 2), d0), + throwsA(isA())); + }); + }); + group('simcompare', () { test('NotGate single bit', () async { final gtm = GateTestModule(Logic(), Logic()); @@ -534,8 +746,12 @@ void main() { }); test('shift by const zero', () async { - final gtm = - ShiftTestModule(Logic(width: 3), Logic(width: 8), constant: 0); + final gtm = ShiftTestModule( + Logic(width: 3), + Logic(width: 8), + constant: 0, + useExplicitConstShiftModules: true, + ); await gtm.build(); final sv = gtm.generateSynth(); diff --git a/test/sv_gen_test.dart b/test/sv_gen_test.dart index 13fe8032f..a9875aa6d 100644 --- a/test/sv_gen_test.dart +++ b/test/sv_gen_test.dart @@ -496,7 +496,7 @@ class ModWithPartialArrayAssignment extends Module { class ModWithConstInlineUnaryOp extends Module { ModWithConstInlineUnaryOp() { - addOutput('b', width: 8) <= ~Const(0, width: 8); + addOutput('b', width: 8) <= NotGate(Const(0, width: 8)).out; } } @@ -682,6 +682,9 @@ void main() { test('const unary inline op', () async { final mod = ModWithConstInlineUnaryOp(); await mod.build(); + final sv = mod.generateSynth(); + + expect(sv, contains("~8'h0"), reason: sv); final vectors = [ Vector({}, {'b': 0xff}), diff --git a/tool/generate_coverage.sh b/tool/generate_coverage.sh index 7717c71ae..ddaebabb4 100755 --- a/tool/generate_coverage.sh +++ b/tool/generate_coverage.sh @@ -34,8 +34,8 @@ done # Remove old coverage data rm -rf coverage -# Run tests with coverage -dart test --coverage=coverage || true +# Run tests with line and branch coverage +dart test --coverage=coverage --branch-coverage || true # Check if coverage was generated if [ ! -d "coverage" ]; then @@ -51,6 +51,25 @@ dart run coverage:format_coverage \ --packages=.dart_tool/package_config.json \ --report-on=lib +# package:coverage emits branch details as BRDA records without BRF/BRH totals, +# which genhtml 2.x does not summarize. Calculate the branch rate directly. +BRANCH_SUMMARY=$(awk -F'[:,]' ' + /^BRDA:/ { + found++ + if ($5 ~ /^[1-9][0-9]*$/) { + hit++ + } + } + END { + if (found == 0) { + print "Error: no branch coverage data found" > "/dev/stderr" + exit 1 + } + printf "%.1f%% (%d of %d branches)", 100 * hit / found, hit, found + } +' coverage/lcov.info) +echo "Branch coverage: $BRANCH_SUMMARY" + # Install lcov if needed if ! command -v lcov &> /dev/null; then if [ -n "${CI:-}" ] || [ -n "${GITHUB_ACTIONS:-}" ]; then @@ -59,8 +78,8 @@ if ! command -v lcov &> /dev/null; then fi fi -# Generate HTML report -genhtml -o coverage/html coverage/lcov.info --branch-coverage +# Generate HTML line coverage report +genhtml -o coverage/html coverage/lcov.info printf '\n%s\n\n' "Open coverage/html/index.html to review code coverage results." # Extract coverage percentage