Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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 (<https://github.com/intel/rohd/pull/655>).
Expand Down
121 changes: 109 additions & 12 deletions lib/src/modules/bus.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (C) 2021-2025 Intel Corporation
// Copyright (C) 2021-2026 Intel Corporation
// SPDX-License-Identifier: BSD-3-Clause
//
// bus.dart
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand All @@ -299,8 +304,7 @@ class Swizzle extends Module with InlineSystemVerilog {
final inputLines = <String>[];
var descIndex = 0;

for (final e in validInputs) {
final inName = inputs[e.name]!;
for (final operand in operands) {
final desc = widthDescriptions[descIndex++];

String alignedDesc;
Expand All @@ -315,15 +319,108 @@ 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 '''
{
${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(
Comment thread
mkorbel1 marked this conversation as resolved.
List<Logic> validInputs,
Map<String, String> 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]',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IT would be kinda fun to add the SV shorthands for when you are selecting up to the end of the array, or reverse-indexing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are those safe with all tools? generally have been trying to keep things minimal fanciness to maximize tool compatibility

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;
}
}
14 changes: 7 additions & 7 deletions test/net_bus_test.dart
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 = [
Expand Down Expand Up @@ -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 = [
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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]}));'));
}
}

Expand Down
Loading
Loading