Netlist - #675
Conversation
…al/instance naming routine names
Clarify comment Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This aligns central_naming with the simplified naming approach already adopted by all downstream branches (module_services, netlist, source_debug, systemc_trace, fst-writer). Changes: - Remove Namer._instanceNames cache field - Remove Namer.instanceNameOf(Module) method - Update synthesizers to use Namer.allocateName(String) directly - Remove destination tracking from _BusSubsetForStructSlice Benefit: Eliminates duplication across 5+ branches, making each branch truly orthogonal and mergeable without conflicts. Trade-off: Instance names no longer cached across synthesis passes, but all downstreams already use this simpler approach.
# Conflicts: # tool/gh_codespaces/install_dart.sh
instanceNameOf(Module) allocates a collision-free instance name on the first call and returns the cached result thereafter. The _instanceNames Map is keyed by Module.instanceNameKey so repeated synthesis passes over the same hierarchy always produce stable names. This method belongs in central_naming because it is pure naming infrastructure with no dependency on any feature branch.
- Update comment: 'allocateName' → 'instanceNameOf' - Add 'submodule instance names are stable across repeated definitions' test (the canonical 'run synthesis twice, same names' regression test) Both belong here since they directly exercise Namer.instanceNameOf, which is now defined in central_naming.
mkorbel1
left a comment
There was a problem hiding this comment.
can you pull main also again just so it's up to date?
| /// | ||
| /// This is backward-compatible: Yosys-format arrays already mix | ||
| /// integers with constant strings `"0"` and `"1"`. Parsers can | ||
| /// detect range strings by the presence of `:`. |
There was a problem hiding this comment.
why do you want this to be exposed as a configurability option? any config you have is something you have to support or else deprecate in the future. this applies to the other ones in here too (e.g. enableDCE)
There was a problem hiding this comment.
I made these internal only for now in tests. The reason is that we really should study migrating these optimizations into the core synthesis traversal.
There was a problem hiding this comment.
I still see compressBitRanges as non-internal, is that intentional?
|
|
||
| import 'package:rohd/rohd.dart'; | ||
|
|
||
| /// Provides bit ranges and field names for a packed [LogicStructure]. |
There was a problem hiding this comment.
question: this applies to the leaves of a structure if there is hierarchy?
There was a problem hiding this comment.
It recurses: _addStructure line 46
if (element is LogicStructure && element is! LogicArray) {
_addStructure(element, offset, path);
}There was a problem hiding this comment.
but i mean if you ask for a.b.c vs a.b it can give you either one?
There was a problem hiding this comment.
Yes. It gives you the range of bits for whatever field you choose regardless of what level of hierarchy.
I added a method that will export this range.
test('returns bit ranges for nested field paths', () {
final nested = LogicStructure([
Logic(name: 'b', width: 2),
LogicStructure([
Logic(name: 'd', width: 3),
], name: 'c'),
], name: 'a');
final structure = LogicStructure([
Logic(name: 'prefix'),
nested,
]);
final layout = SynthStructureLayout(structure);
expect(layout.bitRangeForPath('a'), (start: 1, end: 6));
expect(layout.bitRangeForPath('a.b'), (start: 1, end: 3));
expect(layout.bitRangeForPath('a.c'), (start: 3, end: 6));
expect(layout.bitRangeForPath('a.c.d'), (start: 3, end: 6));
expect(layout.bitRangeForPath('a.missing'), isNull);
});| String get instanceTypeName => getInstanceTypeOfModule(module); | ||
|
|
||
| /// Non-fatal warnings reported while producing this synthesis result. | ||
| final List<String> warnings; |
There was a problem hiding this comment.
In general, been avoiding adding "warnings" to ROHD -- either things are bad and fatal or good and allowed. Trying to avoid warning soup that happens in a lot of EDA flows, where they grow forever to the point where it's just slowing down and bloating logs. Do we really need warnings?
There was a problem hiding this comment.
I removed the warnings, and use a throw instead.
There was a problem hiding this comment.
I still see warnings APIs added to synthesis result etc.
There was a problem hiding this comment.
I removed the dead plumbing for collecting ...
Moved from Options to Configuration nomenclature. Dependency on rohd_hierarchy so we can use Occurrence classes Leverage Namer and eliminate residual naming functionality. Remove warnings and move to just throw on error.
There was a problem hiding this comment.
Pull request overview
Adds a Yosys-style JSON netlist synthesizer, shared synthesis utilities, hierarchy support, and a filter-bank example.
Changes:
- Introduces configurable netlist generation, validation, cell mapping, and optimization passes.
- Adds shared array/structure synthesis utilities and warning propagation.
- Adds extensive netlist tests and hierarchical filter-bank examples.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
test/synth_structure_layout_test.dart |
Tests packed structure layout. |
test/synth_name_parity_test.dart |
Tests naming parity across synthesizers. |
test/struct_port_pruning_test.dart |
Tests structured-port preservation. |
test/netlist_test.dart |
Tests netlist APIs and JSON output. |
test/netlist_example_test.dart |
Tests example netlist generation. |
pubspec.yaml |
Adds hierarchy dependency and publication settings. |
lib/src/synthesizers/utilities/utilities.dart |
Exports new synthesis utilities. |
lib/src/synthesizers/utilities/synth_structure_slice.dart |
Adds structure-slice helper. |
lib/src/synthesizers/utilities/synth_structure_layout.dart |
Adds structure layout mapping. |
lib/src/synthesizers/utilities/synth_structure_concat.dart |
Adds structure concatenation helper. |
lib/src/synthesizers/utilities/synth_module_stop_policy.dart |
Adds hierarchy stopping policies. |
lib/src/synthesizers/utilities/synth_logic.dart |
Adds nullable synthesized-name access. |
lib/src/synthesizers/utilities/synth_array_slice.dart |
Adds array-slice helper. |
lib/src/synthesizers/utilities/synth_array_concat.dart |
Adds array concatenation helper. |
lib/src/synthesizers/synthesizers.dart |
Exports netlist APIs. |
lib/src/synthesizers/synthesis_result.dart |
Adds synthesis warnings. |
lib/src/synthesizers/synth_builder.dart |
Aggregates result warnings. |
lib/src/synthesizers/netlist/netlist.dart |
Adds netlist barrel exports. |
lib/src/synthesizers/netlist/netlist_validation.dart |
Validates generated connectivity. |
lib/src/synthesizers/netlist/netlist_utils.dart |
Adds netlist translation utilities. |
lib/src/synthesizers/netlist/netlist_synthesizer.dart |
Implements JSON netlist synthesis. |
lib/src/synthesizers/netlist/netlist_synthesizer_configuration.dart |
Defines netlist options. |
lib/src/synthesizers/netlist/netlist_synthesis_result.dart |
Stores per-module netlists. |
lib/src/synthesizers/netlist/netlist_synth_module_definition.dart |
Preserves structural netlist cells. |
lib/src/synthesizers/netlist/netlist_passes.dart |
Adds optimization passes. |
lib/src/synthesizers/netlist/netlist_module_translation.dart |
Translates modules into netlist data. |
lib/src/synthesizers/netlist/netlist_cell_mapper.dart |
Maps ROHD modules to cells. |
lib/src/signals/const.dart |
Reformats constant construction. |
lib/src/module.dart |
Refactors constant detection. |
example/filter_bank/shared_data_bus.dart |
Adds bidirectional-bus example. |
example/filter_bank/mac_unit.dart |
Adds pipelined MAC example. |
example/filter_bank/filter_sample.dart |
Adds structured sample type. |
example/filter_bank/filter_data_interface.dart |
Adds filter interface. |
example/filter_bank/filter_controller.dart |
Adds filter FSM controller. |
example/filter_bank/filter_channel.dart |
Adds FIR channel implementation. |
example/filter_bank/filter_bank.dart |
Adds filter-bank top module. |
example/filter_bank/filter_bank_modules.dart |
Exports filter modules. |
example/filter_bank/coeff_bank.dart |
Adds coefficient storage. |
example/filter_bank.dart |
Adds simulation entry point. |
dart_test.yaml |
Configures benchmark timeout. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| return ( | ||
| cellType: r'$dff', | ||
| portDirs: pd, | ||
| connections: cn, | ||
| parameters: <String, Object?>{ | ||
| 'WIDTH': ctx.width(d), | ||
| 'CLK_POLARITY': 1, | ||
| }, |
| const <Type, String>{ | ||
| LShift: r'$shl', | ||
| RShift: r'$shr', | ||
| ARShift: r'$shiftx', |
| final inName = tsb.inputs.keys.first; // data input | ||
| final enName = tsb.inputs.keys.last; // enable | ||
| final outName = tsb.inOuts.keys.first; // inout output | ||
| final r = ctx.remap({inName: 'A', enName: 'EN', outName: 'Y'}); |
| extension _NetlistTestModule on Module { | ||
| String generateNetlist( |
There was a problem hiding this comment.
This is correct. We will not be adding to the Module API for new services, we will use the service model. Only for SV and Waves do we have these generate routines. This is as agreed, right -- the helper routines are for simplifying the most common two cases.
| final parts = [ | ||
| ..._modulePathIndices(module).path, | ||
| ...rootLocation.path, | ||
| ...elementPath, |
| final pipe = Pipeline( | ||
| clk, | ||
| reset: reset, | ||
| stages: [ |
| repository: https://github.com/intel/rohd | ||
| issue_tracker: https://github.com/intel/rohd/issues | ||
| documentation: https://intel.github.io/rohd-website/docs/sample-example/ | ||
| publish_to: none |
| if (coefficients.length != numChannels) { | ||
| throw Exception( | ||
| 'coefficients must have $numChannels entries (one per channel).'); | ||
| } |
|
|
||
| /// A structured signal bundling a data sample with metadata. | ||
| /// | ||
| /// Packs three fields — [data], and [valid] — into a single |
| if (carryName.isNotEmpty) { | ||
| pd['CO'] = 'output'; | ||
| cn['CO'] = ctx.rawConns[carryName] ?? []; | ||
| } |
| /// | ||
| /// This is backward-compatible: Yosys-format arrays already mix | ||
| /// integers with constant strings `"0"` and `"1"`. Parsers can | ||
| /// detect range strings by the presence of `:`. |
There was a problem hiding this comment.
I still see compressBitRanges as non-internal, is that intentional?
|
|
||
| for (final cellEntry in cells.entries) { | ||
| if (!cellEntry.key.startsWith( | ||
| SynthArraySlice.operationName, |
There was a problem hiding this comment.
is a string "starts with" really a robust method for identifying the type of a module?
| String get instanceTypeName => getInstanceTypeOfModule(module); | ||
|
|
||
| /// Non-fatal warnings reported while producing this synthesis result. | ||
| final List<String> warnings; |
There was a problem hiding this comment.
I still see warnings APIs added to synthesis result etc.
| int nextId = 2; | ||
|
|
||
| /// Wire identifiers allocated for each synthesis logic. | ||
| final Map<SynthLogic, List<int>> synthLogicIds = {}; |
There was a problem hiding this comment.
i notice this synthLogicIds, blockedConstSynthLogics, are not referenced outside of this class. even though it's an internal class, might be worth considering which pieces should be private to prevent leaky abstractions?
| concatConnections['Y'] = outputIds.cast<Object>(); | ||
| concatDirections['Y'] = 'output'; | ||
|
|
||
| cells['array_concat_output_$concatName'] = { |
There was a problem hiding this comment.
is this guaranteed unique? does it have to be?
| /// and be emitted as cells in their parent. Defaults to [FlipFlop], which | ||
| /// contains internal sequential submodules but should be emitted as a `$dff` | ||
| /// netlist cell. | ||
| final List<Type> leafModuleTypes; |
There was a problem hiding this comment.
should this be private with an UnmodifiableListView accessor (if needed)?
| /// [leafModuleTypes] is ignored. | ||
| final SynthModuleStopPolicy? moduleStopPolicy; | ||
|
|
||
| /// Exact [Module.runtimeType]s that should stop netlist hierarchy traversal |
There was a problem hiding this comment.
runtimeType is maybe a little too delicate? First, it's sort of weird when you compile to javascript with optimization. Second, it means it doesn't handle inheritance gracefully (e.g. if someone makes a custom version that extends FlipFlop they need to remember to update this policy too). Would a Function be better with a default that checks is FlipFlop be better?
| return outPort; | ||
| } | ||
|
|
||
| bool _hasConsts(LogicStructure structure) => structure.elements.any( |
There was a problem hiding this comment.
why did you change this from the private extension on LogicStructure to a method on Module?
| // SPDX-License-Identifier: BSD-3-Clause | ||
| // | ||
| // synth_name_parity_test.dart | ||
| // Tests that verify canonicalNameOf works consistently across |
There was a problem hiding this comment.
there's no more canonicalNameOf?
| repository: https://github.com/intel/rohd | ||
| issue_tracker: https://github.com/intel/rohd/issues | ||
| documentation: https://intel.github.io/rohd-website/docs/sample-example/ | ||
| publish_to: none |
There was a problem hiding this comment.
no :) we do want to publish rohd
is it complaining that the path dependency is not allowed if we publish? what's the right way to handle this then? do we need to release rohd_hierarchy also?
There was a problem hiding this comment.
We want to be able to use the local version when we are in this repo. I will try to figure this out, but it is the monorepo problem with multiple packages. You don't want to be in a loop where you have to publish to test a change that crosses between the package and the rest of the repo. But it complained about using a path for an internal package.
WE do need to release rohd_hierarchy in pub.dev. Perhaps this is a dependency override situation.
There was a problem hiding this comment.

Description & Motivation
This is a netlist synthesizer that produces a netlist for the generated design in an extension of the Yosys output netlist format.
It provides routines for emitting just the hierarchy and ports ("slim" mode) as well as fully expanded and has hooks for even more incremental expansion modes.
Related Issue(s)
None.
Testing
There is a suite of tests that compare the netlist and its names against the SystemVerilog output. This netlist depended on the last central_naming branch to assure that signals in both formats had identical names.
Backwards-compatibility
No.
Documentation
This is a minor API addition (
Module.generateNetlist()) but we will add more documentation and examples of the format, etc.It will have some options as well, such as multiFile, which should parallel the
generateSynth()API.