diff --git a/CHANGELOG.md b/CHANGELOG.md index 74c6b4765..8d73834c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## Next release + +- Improved generated SystemVerilog for swizzles to compact adjacent bit selections into legal slice expressions. + ## 0.6.9 - Fixed a bug where unnamed or mergeable inOut loop-back connections on a single module could be incorrectly omitted from the generated SystemVerilog (). diff --git a/lib/src/modules/bus.dart b/lib/src/modules/bus.dart index f3dfd8693..630d69ff9 100644 --- a/lib/src/modules/bus.dart +++ b/lib/src/modules/bus.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2025 Intel Corporation +// Copyright (C) 2021-2026 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // bus.dart @@ -183,6 +183,11 @@ class BusSubset extends Module with InlineSystemVerilog { class Swizzle extends Module with InlineSystemVerilog { final String _out = Naming.unpreferredName('swizzled'); + /// A regular expression that will have matches if an expression is a single + /// bit select of a signal or packed array element. + static final RegExp _singleBitSelectRegex = + RegExp(r'^\(?([A-Za-z_][A-Za-z0-9_$]*(?:\[\d+\])*)\[(\d+)\]\)?$'); + /// The output port containing concatenated signals. late final Logic out; @@ -259,25 +264,25 @@ class Swizzle extends Module with InlineSystemVerilog { // Calculate all width descriptions upfront to determine alignment final validInputs = _swizzleInputs.reversed.where((e) => e.width > 0).toList(); + final operands = _collapseContiguousBitSelects(validInputs, inputs); // If there's only one element, no need for width descriptions - if (validInputs.length == 1) { - final inName = inputs[validInputs.first.name]!; - return inName; + if (operands.length == 1) { + return operands.first.expression; } final widthDescriptions = <({int upper, int? lower})>[]; var upperIndex = out.width - 1; // First pass: calculate all width descriptions - for (final e in validInputs) { - if (e.width > 1) { - final lowerIndex = upperIndex - e.width + 1; + for (final operand in operands) { + if (operand.width > 1) { + final lowerIndex = upperIndex - operand.width + 1; widthDescriptions.add((upper: upperIndex, lower: lowerIndex)); } else { widthDescriptions.add((upper: upperIndex, lower: null)); } - upperIndex -= e.width; + upperIndex -= operand.width; } // Find maximum width for alignment @@ -299,8 +304,7 @@ class Swizzle extends Module with InlineSystemVerilog { final inputLines = []; var descIndex = 0; - for (final e in validInputs) { - final inName = inputs[e.name]!; + for (final operand in operands) { final desc = widthDescriptions[descIndex++]; String alignedDesc; @@ -315,10 +319,10 @@ class Swizzle extends Module with InlineSystemVerilog { alignedDesc = desc.upper.toString().padLeft(totalWidth); } - upperIndex -= e.width; + upperIndex -= operand.width; final maybeComma = upperIndex >= 0 ? ',' : ' '; // space at end for alignment - inputLines.add('$inName$maybeComma /* $alignedDesc */'); + inputLines.add('${operand.expression}$maybeComma /* $alignedDesc */'); } return ''' @@ -326,4 +330,97 @@ class Swizzle extends Module with InlineSystemVerilog { ${inputLines.join('\n')} }'''; } + + /// Rewrites runs of adjacent descending single-bit selects from the same + /// packed signal into wider SystemVerilog slices. + /// + /// For example, `a[7], a[6], a[5]` becomes `a[7:5]`, and + /// `a[0][1], a[0][0]` becomes `a[0][1:0]`. Ascending runs are intentionally + /// left expanded because SystemVerilog slices cannot reverse bit order with + /// `lower:upper` syntax. + List<({String expression, int width})> _collapseContiguousBitSelects( + List validInputs, + Map inputs, + ) { + final operands = <({String expression, int width})>[]; + + var index = 0; + while (index < validInputs.length) { + final input = validInputs[index]; + final expression = inputs[input.name]!; + final selectedBit = _singleBitSelect(input, expression); + if (selectedBit == null) { + operands.add((expression: expression, width: input.width)); + index++; + continue; + } + + var lowerIndex = selectedBit.index; + var endIndex = index + 1; + while (endIndex < validInputs.length) { + final nextInput = validInputs[endIndex]; + final nextExpression = inputs[nextInput.name]!; + final nextSelectedBit = _singleBitSelect(nextInput, nextExpression); + if (nextSelectedBit == null || + nextSelectedBit.source != selectedBit.source || + nextSelectedBit.index != lowerIndex - 1) { + break; + } + + lowerIndex = nextSelectedBit.index; + endIndex++; + } + + if (endIndex == index + 1) { + operands.add((expression: expression, width: input.width)); + } else { + operands.add(( + expression: '${selectedBit.source}[${selectedBit.index}:$lowerIndex]', + width: endIndex - index, + )); + } + index = endIndex; + } + + return operands; + } + + /// Parses [expression] as a single-bit select of a packed signal when it is + /// safe to participate in slice collapsing. + /// + /// Returns `null` for multi-bit inputs, non-select expressions, or selects + /// sourced from unpacked arrays. + ({String source, int index})? _singleBitSelect( + Logic input, + String expression, + ) { + if (input.width != 1 || _hasUnpackedArraySource(input.srcConnection)) { + return null; + } + + final match = _singleBitSelectRegex.firstMatch(expression); + if (match == null) { + return null; + } + + return (source: match.group(1)!, index: int.parse(match.group(2)!)); + } + + /// Walks up [logic]'s containing structures to detect unpacked arrays. + /// + /// SystemVerilog packed slices are not interchangeable with unpacked array + /// indexing, so any unpacked array source disables bit-select collapsing. + bool _hasUnpackedArraySource(Logic? logic) { + var current = logic; + while (current?.parentStructure != null) { + final parentStructure = current!.parentStructure!; + if (parentStructure is LogicArray && + parentStructure.numUnpackedDimensions > 0) { + return true; + } + current = parentStructure; + } + + return false; + } } diff --git a/test/net_bus_test.dart b/test/net_bus_test.dart index 2d5fccdc0..73e8c451e 100644 --- a/test/net_bus_test.dart +++ b/test/net_bus_test.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2024-2025 Intel Corporation +// Copyright (C) 2024-2026 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // net_bus_test.dart @@ -601,7 +601,7 @@ void main() { sv, contains('net_connect_0' ' (_original__swizzled, ' - '({bus[1][1],bus[1][0],bus[0][3],bus[0][2]}));')); + '({bus[1][1:0],bus[0][3:2]}));')); } final vectors = [ @@ -631,7 +631,7 @@ void main() { sv, contains('net_connect_0' ' (_original__swizzled, ' - '({bus[1][1],bus[1][0],bus[0][3],bus[0][2]}));')); + '({bus[1][1:0],bus[0][3:2]}));')); } final vectors = [ @@ -788,7 +788,7 @@ void main() { sv, contains('assign _swizzled = ' '{({({in0[1][1],in0[1][0]}),({in0[0][1],in0[0][0]})}),' - '({in1[3],in1[2],in1[1],in1[0]})};')); + '(in1[3:0])};')); }); test('net array 2', () async { @@ -805,7 +805,7 @@ void main() { expect( sv, contains('net_connect (swizzled,' - ' ({({in0[3],in0[2],in0[1],in0[0]}),in1[0]}));')); + ' ({(in0[3:0]),in1[0]}));')); }); test('net array 3', () async { @@ -825,7 +825,7 @@ void main() { contains('net_connect (swizzled, ' '({({({in0[1][1],in0[1][0]}),' '({in0[0][1],in0[0][0]})}),' - '({in1[3],in1[2],in1[1],in1[0]}),in2[0]}));')); + '(in1[3:0]),in2[0]}));')); }); test('net and non-net', () async { @@ -931,7 +931,7 @@ void main() { 'net_connect #(.WIDTH(16)) net_connect (swizzled, ' '({({({in0[1][1],in0[1][0]}),' '({in0[0][1],in0[0][0]})}),' - '({in1[3],in1[2],in1[1],in1[0]}),in2[0]}));')); + '(in1[3:0]),in2[0]}));')); } } diff --git a/test/swizzle_test.dart b/test/swizzle_test.dart index 6e1fc0949..d6ccf9d4f 100644 --- a/test/swizzle_test.dart +++ b/test/swizzle_test.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2022-2025 Intel Corporation +// Copyright (C) 2022-2026 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // swizzle_test.dart @@ -9,6 +9,7 @@ import 'package:rohd/rohd.dart'; import 'package:rohd/src/utilities/simcompare.dart'; +import 'package:rohd/src/utilities/sv_cleaner.dart'; import 'package:test/test.dart'; class SwizzlyModule extends Module { @@ -113,6 +114,70 @@ class LargeWidthSwizzle extends Module { } } +class SwizzleAdjacentBitSlices extends Module { + SwizzleAdjacentBitSlices() { + final a = addInput('a', Logic(width: 8), width: 8); + addOutput('out', width: 6) <= + [a[7], a[6], a[5], a[2], a[1], a[0]].swizzle(); + } +} + +class SwizzleAllAdjacentBitSlices extends Module { + SwizzleAllAdjacentBitSlices() { + final a = addInput('a', Logic(width: 8), width: 8); + addOutput('out', width: 3) <= [a[7], a[6], a[5]].swizzle(); + } +} + +class SwizzleAscendingBitSlices extends Module { + SwizzleAscendingBitSlices() { + final a = addInput('a', Logic(width: 8), width: 8); + addOutput('out', width: 3) <= [a[0], a[1], a[2]].swizzle(); + } +} + +class SwizzleNestedAdjacentBitSlices extends Module { + SwizzleNestedAdjacentBitSlices() { + final a = addInput('a', Logic(width: 8), width: 8); + final inner = Swizzle([a[7], a[6], a[5]]).out; + addOutput('out', width: 5) <= [inner, a[4], a[3]].swizzle(); + } +} + +class SwizzleAdjacentRanges extends Module { + SwizzleAdjacentRanges() { + final a = addInput('a', Logic(width: 8), width: 8); + addOutput('out', width: 6) <= [a.slice(5, 2), a.slice(1, 0)].swizzle(); + } +} + +class SwizzlePackedArrayElementBits extends Module { + SwizzlePackedArrayElementBits(LogicArray arr) { + final inArr = addInputArray('arr', arr, dimensions: [2, 2]); + final upper = inArr.elements[1] as LogicArray; + final lower = inArr.elements[0] as LogicArray; + addOutput('out', width: 4) <= + [ + upper.elements[1], + upper.elements[0], + lower.elements[1], + lower.elements[0], + ].swizzle(); + } +} + +class SwizzleUnpackedArrayElements extends Module { + SwizzleUnpackedArrayElements(LogicArray arr) { + final inArr = addInputArray( + 'arr', + arr, + dimensions: [4], + numUnpackedDimensions: 1, + ); + addOutput('out', width: 4) <= inArr.elements.reversed.toList().swizzle(); + } +} + void main() { tearDown(() async { await Simulator.reset(); @@ -319,6 +384,146 @@ void main() { }); }); + group('SystemVerilog slice collapsing', () { + test('collapses descending contiguous bit selects', () async { + final mod = SwizzleAdjacentBitSlices(); + await mod.build(); + + final sv = SvCleaner.removeSwizzleAnnotationComments(mod.generateSynth()); + + expect(sv, contains('assign out = {a[7:5],a[2:0]};')); + expect(sv, isNot(contains('a[7],a[6]'))); + + final vectors = [ + for (final value in [0x00, 0xe5, 0x3c, 0xff]) + Vector({ + 'a': value + }, { + 'out': (((value >> 5) & 0x7) << 3) | (value & 0x7), + }), + ]; + await SimCompare.checkFunctionalVector(mod, vectors); + final simResult = SimCompare.iverilogVector(mod, vectors); + expect(simResult, equals(true)); + }); + + test('omits braces when the whole swizzle collapses to one slice', + () async { + final mod = SwizzleAllAdjacentBitSlices(); + await mod.build(); + + final sv = SvCleaner.removeSwizzleAnnotationComments(mod.generateSynth()); + + expect(sv, contains('assign out = a[7:5];')); + expect(sv, isNot(contains('assign out = {a[7:5]};'))); + + final vectors = [ + for (final value in [0x00, 0xe0, 0xa0, 0xff]) + Vector({'a': value}, {'out': (value >> 5) & 0x7}), + ]; + await SimCompare.checkFunctionalVector(mod, vectors); + final simResult = SimCompare.iverilogVector(mod, vectors); + expect(simResult, equals(true)); + }); + + test('does not collapse ascending bit selects', () async { + final mod = SwizzleAscendingBitSlices(); + await mod.build(); + + final sv = SvCleaner.removeSwizzleAnnotationComments(mod.generateSynth()); + + expect(sv, isNot(contains('a[2:0]'))); + expect(sv, contains('a[0]')); + expect(sv, contains('a[1]')); + expect(sv, contains('a[2]')); + + final vectors = [ + for (final value in [0x00, 0x05, 0x06, 0xff]) + Vector({ + 'a': value + }, { + 'out': ((value & 0x1) << 2) | + (((value >> 1) & 0x1) << 1) | + ((value >> 2) & 0x1), + }), + ]; + await SimCompare.checkFunctionalVector(mod, vectors); + final simResult = SimCompare.iverilogVector(mod, vectors); + expect(simResult, equals(true)); + }); + + test('collapses nested swizzle internals but not across the nested range', + () async { + final mod = SwizzleNestedAdjacentBitSlices(); + await mod.build(); + + final sv = SvCleaner.removeSwizzleAnnotationComments(mod.generateSynth()); + + expect(sv, contains('assign out = {(a[7:5]),a[4:3]};')); + expect(sv, isNot(contains('a[7:3]'))); + + final vectors = [ + for (final value in [0x00, 0xf8, 0xa8, 0xff]) + Vector({'a': value}, {'out': (value >> 3) & 0x1f}), + ]; + await SimCompare.checkFunctionalVector(mod, vectors); + final simResult = SimCompare.iverilogVector(mod, vectors); + expect(simResult, equals(true)); + }); + + test('does not re-collapse adjacent range operands', () async { + final mod = SwizzleAdjacentRanges(); + await mod.build(); + + final sv = SvCleaner.removeSwizzleAnnotationComments(mod.generateSynth()); + + expect(sv, contains('assign out = {(a[5:2]),(a[1:0])};')); + expect(sv, isNot(contains('a[5:0]'))); + + final vectors = [ + for (final value in [0x00, 0x15, 0x2a, 0xff]) + Vector({'a': value}, {'out': value & 0x3f}), + ]; + await SimCompare.checkFunctionalVector(mod, vectors); + final simResult = SimCompare.iverilogVector(mod, vectors); + expect(simResult, equals(true)); + }); + + test('collapses packed array element bit selects within each element', + () async { + final mod = SwizzlePackedArrayElementBits(LogicArray([2, 2], 1)); + await mod.build(); + + final sv = SvCleaner.removeSwizzleAnnotationComments(mod.generateSynth()); + + expect(sv, contains('assign out = {arr[1][1:0],arr[0][1:0]};')); + expect(sv, isNot(contains('arr[1][1],arr[1][0]'))); + + final vectors = [ + for (final value in [0x0, 0x5, 0xa, 0xf]) + Vector({'arr': value}, {'out': value}), + ]; + await SimCompare.checkFunctionalVector(mod, vectors); + final simResult = SimCompare.iverilogVector(mod, vectors); + expect(simResult, equals(true)); + }); + + test('does not collapse unpacked array element selects', () async { + final mod = SwizzleUnpackedArrayElements( + LogicArray([4], 1, numUnpackedDimensions: 1), + ); + await mod.build(); + + final sv = SvCleaner.removeSwizzleAnnotationComments(mod.generateSynth()); + + expect(sv, isNot(contains('arr[3:0]'))); + expect(sv, contains('arr[3]')); + expect(sv, contains('arr[2]')); + expect(sv, contains('arr[1]')); + expect(sv, contains('arr[0]')); + }); + }); + test('annotated elements of swizzle in generated sv', () async { final mod = SwizzleVariety(Logic(width: 8)); await mod.build();