Skip to content

Fix aggregate netlist validation for disjoint field drivers, Issue #724 #725

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
desmonddak wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix aggregate netlist validation for disjoint field drivers, Issue #724 #725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion lib/src/synthesizers/netlist/netlist_synthesizer.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ class NetlistSynthesizer extends Synthesizer {
final netnames = translation.netnames;

// -- Structural validation -------------------------------------------
NetlistValidation.validate(ports, cells, module.name, netnames: netnames);
NetlistValidation.validate(ports, cells, module.name);

return NetlistSynthesisResult(module, getInstanceTypeOfModule,
ports: ports, cells: cells, netnames: netnames, attributes: attr);
Expand Down
32 changes: 2 additions & 30 deletions lib/src/synthesizers/netlist/netlist_validation.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,8 @@ class NetlistValidation {
static void validate(
Map<String, Map<String, Object?>> ports,
Map<String, Map<String, Object?>> cells,
String moduleName, {
Map<String, Object?>? netnames,
}) {
String moduleName,
) {
final issues = <NetlistValidationIssue>[];

final driversByBit = _driversByBit(ports, cells);
Expand All @@ -70,33 +69,6 @@ class NetlistValidation {
));
}

if (netnames != null) {
for (final entry in netnames.entries) {
final netname = entry.value;
if (netname is! Map<String, Object?>) {
continue;
}
final logicType = netname['logic_type'];
if (logicType is! Map ||
(logicType['arrayDims'] is! List && logicType['fields'] is! List)) {
continue;
}
final bits = (netname['bits'] as List?)?.whereType<int>() ?? const [];
final aggregateDrivers = <String>{
for (final bit in bits) ...driversByBit[bit] ?? const <String>[],
};
if (aggregateDrivers.length <= 1) {
continue;
}
issues.add(NetlistValidationIssue(
'aggregate net "${entry.key}" is reached from multiple drivers: '
'${aggregateDrivers.join(', ')}',
netname: entry.key,
drivers: aggregateDrivers.toList(),
));
}
}

if (issues.isNotEmpty) {
throw NetlistValidationException(moduleName, issues);
}
Expand Down
50 changes: 31 additions & 19 deletions test/netlist_synthesizer_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2119,7 +2119,7 @@ void main() {
);
});

test('struct aggregate netnames cannot span multiple drivers', () {
test('struct aggregate netnames allow disjoint field drivers', () {
final ports = <String, Map<String, Object?>>{};
final cells = <String, Map<String, Object?>>{
'first_driver': {
Expand All @@ -2145,30 +2145,42 @@ void main() {
},
},
};
final netnames = <String, Object?>{
'values': {
'bits': [
...List<Object>.generate(8, (index) => 200 + index),
...List<Object>.generate(8, (index) => 400 + index),
],
'logic_type': {
'typeName': 'PairStructure',
'fields': [
{'name': 'first', 'width': 8},
{'name': 'second', 'width': 8},
],
expect(
() => NetlistValidation.validate(ports, cells, 'struct_module'),

@mkorbel1 mkorbel1 Sep 24, 2026

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.

Non-blocking test suggestion: could we add a synthesis-level regression reduced from the failing design in #724? This test now omits netnames, so it also passes with the old validator, where the aggregate check was conditional on that argument. I ran the entire revised test file against the pre-fix production sources and all 87 tests passed. A case that retains an aggregate netname spanning independent mux drivers and fails before this fix would protect the behavior being corrected; the shared-bit rejection test can remain as complementary coverage. The implementation itself looks correct; this is a coverage improvement, not a functional defect or merge blocker.

returnsNormally,
);
});

test('structured netnames reject overlapping field drivers', () {
final ports = <String, Map<String, Object?>>{};
final cells = <String, Map<String, Object?>>{
'first_driver': {
'type': r'$buf',
'port_directions': {'A': 'input', 'Y': 'output'},
'connections': {
'A': [100, 101],
'Y': [200, 201],
},
},
'second_driver': {
'type': r'$buf',
'port_directions': {'A': 'input', 'Y': 'output'},
'connections': {
'A': [102, 103],
'Y': [201, 202],
},
},
};

expect(
() => NetlistValidation.validate(
ports,
cells,
'struct_module',
netnames: netnames,
() => NetlistValidation.validate(ports, cells, 'struct_module'),
throwsA(
isA<NetlistValidationException>().having(
(error) => error.issues.map((issue) => issue.wireBit),
'overlapping wire bit',
contains(201),
),
),
throwsA(isA<NetlistValidationException>()),
);
});

Expand Down
Loading