From f26de2614b9870698dc25ab724b8827a894a313c Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Tue, 21 Jan 2025 10:37:09 -0800 Subject: [PATCH] Add `Logic.named` and broaden `clone` (#550) --- lib/src/signals/logic.dart | 29 ++++ lib/src/signals/logic_array.dart | 36 ++++- lib/src/signals/logic_structure.dart | 29 +++- lib/src/utilities/naming.dart | 23 +++ test/logic_array_test.dart | 36 ++++- test/logic_name_test.dart | 202 +++++++++++++++++++++++++++ test/logic_structure_test.dart | 14 ++ 7 files changed, 353 insertions(+), 16 deletions(-) diff --git a/lib/src/signals/logic.dart b/lib/src/signals/logic.dart index ba07d68e3..004b5ba22 100644 --- a/lib/src/signals/logic.dart +++ b/lib/src/signals/logic.dart @@ -255,6 +255,35 @@ class Logic { naming: naming, ); + /// A cloning utility for [clone] and [named]. + Logic _clone({String? name, Naming? naming}) => + (isNet ? LogicNet.new : Logic.new)( + name: name ?? this.name, + naming: Naming.chooseCloneNaming( + originalName: this.name, + newName: name, + originalNaming: this.naming, + newNaming: naming), + width: width); + + /// Makes a copy of `this`, optionally with the specified [name], but the same + /// [width]. + Logic clone({String? name}) => _clone(name: name); + + /// Makes a [clone] with the provided [name] and optionally [naming], then + /// assigns it to be driven by `this`. + /// + /// This is a useful utility for naming the result of some hardware + /// construction without separately declaring a new named signal and then + /// assigning. For example: + /// + /// ```dart + /// // named "myImportantNode" instead of a generated name like "a_xor_b" + /// final myImportantNode = (a ^ b).named('myImportantNode'); + /// ``` + Logic named(String name, {Naming? naming}) => + _clone(name: name, naming: naming)..gets(this); + /// An internal constructor for [Logic] which additional provides access to /// setting the [wire]. Logic._({ diff --git a/lib/src/signals/logic_array.dart b/lib/src/signals/logic_array.dart index 897c73aae..752350abf 100644 --- a/lib/src/signals/logic_array.dart +++ b/lib/src/signals/logic_array.dart @@ -137,8 +137,7 @@ class LogicArray extends LogicStructure { // calculate the next layer's dimensions final nextDimensions = dimensions.length == 1 ? null - : UnmodifiableListView( - dimensions.getRange(1, dimensions.length).toList(growable: false)); + : List.unmodifiable(dimensions.getRange(1, dimensions.length)); // if the total width will eventually be 0, then force element width to 0 if (elementWidth != 0 && dimensions.reduce((a, b) => a * b) == 0) { @@ -168,7 +167,7 @@ class LogicArray extends LogicStructure { )) .._arrayIndex = index, growable: false), - dimensions: UnmodifiableListView(dimensions), + dimensions: List.unmodifiable(dimensions), elementWidth: elementWidth, numUnpackedDimensions: numUnpackedDimensions, name: name, @@ -177,13 +176,38 @@ class LogicArray extends LogicStructure { ); } + @override + LogicArray _clone({String? name, Naming? naming}) => LogicArray._factory( + dimensions, + elementWidth, + name: name ?? this.name, + numUnpackedDimensions: numUnpackedDimensions, + naming: Naming.chooseCloneNaming( + originalName: this.name, + newName: name, + originalNaming: this.naming, + newNaming: naming), + logicBuilder: isNet ? LogicNet.new : Logic.new, + logicArrayBuilder: isNet ? LogicArray.net : LogicArray.new, + isNet: isNet, + ); + /// Creates a new [LogicArray] which has the same [dimensions], - /// [elementWidth], [numUnpackedDimensions] as `this`. + /// [elementWidth], [numUnpackedDimensions], and [isNet] as `this`. /// /// If no new [name] is specified, then it will also have the same name. @override - LogicArray clone({String? name}) => LogicArray(dimensions, elementWidth, - numUnpackedDimensions: numUnpackedDimensions, name: name ?? this.name); + LogicArray clone({String? name}) => _clone(name: name); + + /// Makes a [clone] with the provided [name] and optionally [naming], then + /// assigns it to be driven by `this`. + /// + /// This is a useful utility for naming the result of some hardware + /// construction without separately declaring a new named signal and then + /// assigning. + @override + LogicArray named(String name, {Naming? naming}) => + _clone(name: name, naming: naming)..gets(this); /// Private constructor for the factory [LogicArray] constructor. /// diff --git a/lib/src/signals/logic_structure.dart b/lib/src/signals/logic_structure.dart index e2b64aee6..10a0275a0 100644 --- a/lib/src/signals/logic_structure.dart +++ b/lib/src/signals/logic_structure.dart @@ -53,12 +53,29 @@ class LogicStructure implements Logic { }); } - /// Creates a new [LogicStructure] with the same structure as `this`. - LogicStructure clone({String? name}) => LogicStructure( - elements.map((e) => e is LogicStructure - ? e.clone() - : Logic(name: e.name, width: e.width, naming: e.naming)), - name: name ?? this.name); + @override + LogicStructure _clone({String? name, Naming? naming}) => + // naming is not used for LogicStructure + LogicStructure(elements.map((e) => e.clone(name: e.name)), + name: name ?? this.name); + + /// Creates a new [LogicStructure] with the same structure as `this` and + /// [clone]d [elements], optionally with the provided [name]. + @override + LogicStructure clone({String? name}) => _clone(name: name); + + /// Makes a [clone], optionally with the specified [name], then assigns it to + /// be driven by `this`. + /// + /// The [naming] argument will not have any effect on a generic + /// [LogicStructure], but behavior may be overridden by implementers. + /// + /// This is a useful utility for naming the result of some hardware + /// construction without separately declaring a new named signal and then + /// assigning. + @override + LogicStructure named(String name, {Naming? naming}) => + clone(name: name)..gets(this); @override String get structureName { diff --git a/lib/src/utilities/naming.dart b/lib/src/utilities/naming.dart index 260947661..e2b6e74cc 100644 --- a/lib/src/utilities/naming.dart +++ b/lib/src/utilities/naming.dart @@ -77,6 +77,29 @@ enum Naming { : Naming.renameable : Naming.unnamed); + /// Picks a [Naming] for a clone based on its original conditions and + /// optionally provided new conditions. + static Naming chooseCloneNaming({ + required String originalName, + required String? newName, + required Naming originalNaming, + required Naming? newNaming, + }) { + if (newNaming != null) { + // if provided, then use that + return newNaming; + } + + if (newName == null && newNaming == null) { + // if not provided, we can default to mergeable, since we clone the old + // name and don't necessarily need the duplicate around + return Naming.mergeable; + } + + // otherwise, use default + return Naming.chooseNaming(newName, newNaming); + } + /// Picks a [String] name based on an initial [name] and [naming]. /// /// If [name] is null, the name will be based on [nullStarter]. diff --git a/test/logic_array_test.dart b/test/logic_array_test.dart index 9d2689c65..aacfad8e9 100644 --- a/test/logic_array_test.dart +++ b/test/logic_array_test.dart @@ -957,12 +957,11 @@ void main() { final mod = WithSetArrayOffsetModule(LogicArray([2, 2], 8)); await testArrayPassthrough(mod, checkNoSwizzle: false); + final sv = mod.generateSynth(); + // make sure we're reassigning both times it overlaps! expect( - RegExp('assign laIn.*=.*swizzled') - .allMatches(mod.generateSynth()) - .length, - 2); + RegExp(r'assign laOut\[1\].*=.*swizzled').allMatches(sv).length, 2); }); }); @@ -1078,4 +1077,33 @@ void main() { SimCompare.checkIverilogVector(mod, vectors); }); }); + + group('array clone', () { + for (final isNet in [true, false]) { + test('isNet = $isNet', () { + final la = (isNet ? LogicArray.net : LogicArray.new)( + [3, 2, 4], + 8, + numUnpackedDimensions: 1, + name: 'myarray', + naming: Naming.reserved, + ); + final clone = la.clone(); + expect(la.dimensions, clone.dimensions); + expect(la.elementWidth, clone.elementWidth); + expect(la.numUnpackedDimensions, clone.numUnpackedDimensions); + expect(la.width, clone.width); + expect(la.elements.length, clone.elements.length); + for (var i = 0; i < la.elements.length; i++) { + expect(la.elements[i].width, clone.elements[i].width); + } + expect(la.name, clone.name); + expect(la.isNet, clone.isNet); + expect(clone.elements[0].elements[1].isNet, isNet); + expect( + clone.elements[1].elements[1].elements[1] is LogicArray, isFalse); + expect(clone.elements[1].elements[1].elements[1].isNet, isNet); + }); + } + }); } diff --git a/test/logic_name_test.dart b/test/logic_name_test.dart index 969614f09..624a6b4d2 100644 --- a/test/logic_name_test.dart +++ b/test/logic_name_test.dart @@ -7,9 +7,23 @@ // 2022 October 26 // Author: Yao Jing Quek +import 'package:collection/collection.dart'; import 'package:rohd/rohd.dart'; import 'package:rohd/src/utilities/sanitizer.dart'; import 'package:test/test.dart'; +import 'logic_structure_test.dart' as logic_structure_test; + +class MyStruct extends LogicStructure { + final Logic ready; + final Logic valid; + + factory MyStruct() => MyStruct._( + Logic(name: 'ready'), + Logic(name: 'valid'), + ); + + MyStruct._(this.ready, this.valid) : super([ready, valid], name: 'myStruct'); +} class LogicTestModule extends Module { LogicTestModule(String logicName) { @@ -211,4 +225,192 @@ void main() { '.o(o),' '.portB_1(portB_1),.portB(portB))')); }); + + group('clone', () { + test('name selection', () { + const originalName = 'original'; + for (final newName in ['new', null]) { + for (final originalNaming in Naming.values) { + for (final newNaming in [...Naming.values, null]) { + final selectedName = newName ?? originalName; + final selectedNaming = Naming.chooseCloneNaming( + originalName: originalName, + newName: newName, + originalNaming: originalNaming, + newNaming: newNaming, + ); + + final reason = 'original: ($originalName, ${originalNaming.name}),' + ' new: ($newName, ${newNaming?.name})' + ' => ($selectedName, ${selectedNaming.name})'; + + if (newNaming != null) { + expect(selectedNaming, newNaming, reason: reason); + } else if (newName == null && newNaming == null) { + expect(selectedNaming, Naming.mergeable, reason: reason); + } else if (newName != null && newNaming == null) { + expect(selectedNaming, Naming.renameable, reason: reason); + } else { + fail('Undefined scenario: $reason'); + } + } + } + } + }); + + group('logic', () { + test('name null', () { + final c = Logic(name: 'a').clone(); + expect(c.name, 'a'); + expect(c.naming, Naming.mergeable); + }); + + test('name provided', () { + final c = Logic(name: 'a').clone(name: 'b'); + expect(c.name, 'b'); + expect(c.naming, Naming.renameable); + }); + + test('net', () { + final c = LogicNet(name: 'a').clone(); + expect(c.name, 'a'); + expect(c.naming, Naming.mergeable); + expect(c, isA()); + }); + }); + + group('logic structure', () { + test('name null', () { + final c = MyStruct().clone(); + expect(c.name, 'myStruct'); + expect(c.naming, Naming.unnamed); + }); + + test('name provided', () { + final c = MyStruct().clone(name: 'newName'); + expect(c.name, 'newName'); + expect(c.naming, Naming.unnamed); + }); + }); + + group('logic array', () { + test('name null', () { + final c = LogicArray([1, 2], 3, name: 'a').clone(); + expect(c.name, 'a'); + expect(c.naming, Naming.mergeable); + }); + + test('name provided', () { + final c = LogicArray([1, 2], 3, name: 'a').clone(name: 'b'); + expect(c.name, 'b'); + expect(c.naming, Naming.renameable); + }); + + test('net', () { + final c = LogicArray.net([1, 2], 3, name: 'a').clone(); + expect(c.name, 'a'); + expect(c.naming, Naming.mergeable); + expect(c.isNet, true); + }); + }); + }); + + group('named', () { + test('logic', () { + final a = Logic(name: 'a'); + final b = a.named('b'); + + a.put(1); + + expect(b.value.toInt(), 1); + expect(b.name, 'b'); + expect(b.naming, Naming.renameable); + }); + + test('logic with naming', () { + final a = Logic(name: 'a'); + final b = a.named('b', naming: Naming.reserved); + + a.put(1); + + expect(b.value.toInt(), 1); + expect(b.name, 'b'); + expect(b.naming, Naming.reserved); + }); + + test('net', () { + final a = LogicNet(name: 'a'); + final b = a.named('b'); + + a.put(1); + + expect(b.value.toInt(), 1); + expect(b.name, 'b'); + expect(b.naming, Naming.renameable); + expect(b.isNet, true); + expect(b, isA()); + }); + + test('array', () { + final a = LogicArray([1, 2], 3, name: 'a', numUnpackedDimensions: 1); + final b = a.named('b'); + + a.elements[0].elements[0].put(1); + + final listEq = const ListEquality().equals; + + expect(b.elements[0].elements[0].value.toInt(), 1); + expect(listEq(b.dimensions, a.dimensions), true); + expect(b.numUnpackedDimensions, a.numUnpackedDimensions); + expect(b.name, 'b'); + expect(b.naming, Naming.renameable); + }); + + test('array net with naming', () { + final a = LogicArray.net([1, 2], 3, name: 'a', numUnpackedDimensions: 1); + final b = a.named('b', naming: Naming.reserved); + + a.elements[0].elements[0].put(1); + + final listEq = const ListEquality().equals; + + expect(b.elements[0].elements[0].value.toInt(), 1); + expect(listEq(b.dimensions, a.dimensions), true); + expect(b.numUnpackedDimensions, a.numUnpackedDimensions); + expect(b.name, 'b'); + expect(b.naming, Naming.reserved); + expect(b.isNet, true); + expect(b, isA()); + }); + + test('structure', () { + final a = logic_structure_test.MyFancyStruct(); + final b = a.named( + 'b', + + // naming should have no effect + naming: Naming.reserved, + ); + + expect(b.name, 'b'); + + expect(b.width, a.width); + expect(b.elements[0], isA()); + expect(b.elements[0].name, a.elements[0].name); + expect(b.elements[0].naming, Naming.renameable); + + expect(b.elements[1], isA()); + expect(b.elements[1].name, a.elements[1].name); + expect(b.elements[1].naming, Naming.renameable); + + expect(b.elements[2], isA()); + expect(b.elements[2].name, a.elements[2].name); + expect(b.elements[2].naming, a.elements[2].naming); + expect(b.elements[2].elements[0].name, a.elements[2].elements[0].name); + + a.arr.elements[0].put(1); + + expect(b.elements[0].elements[0].value.toInt(), 1); + }); + }); } diff --git a/test/logic_structure_test.dart b/test/logic_structure_test.dart index aa8724e27..35ee34ecd 100644 --- a/test/logic_structure_test.dart +++ b/test/logic_structure_test.dart @@ -182,9 +182,23 @@ void main() { final copy = orig.clone(); expect(copy.name, orig.name); + expect(copy.width, orig.width); expect(copy.elements[0], isA()); + expect(copy.elements[0].name, orig.elements[0].name); + expect(copy.elements[0].naming, Naming.renameable); + + expect(copy.elements[1], isA()); + expect(copy.elements[1].name, orig.elements[1].name); + expect(copy.elements[1].naming, Naming.renameable); + expect(copy.elements[2], isA()); + expect(copy.elements[2].name, orig.elements[2].name); + expect(copy.elements[2].naming, orig.elements[2].naming); + expect( + copy.elements[2].elements[0].name, orig.elements[2].elements[0].name); + + expect(orig.clone(name: 'newName').name, 'newName'); }); });