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.
Add regression test for _BusSubsetForStructSlice.instanceNameKey.
Each SynthModuleDefinition pass creates fresh _BusSubsetForStructSlice
instances for any submodule with a LogicStructure output port. Without
the instanceNameKey override those instances use 'this' as the cache key,
so the namer allocates a new suffix every pass ('struct_slice' → 'struct_slice_0').
Restoring _destination and overriding instanceNameKey => _destination pins
the cache to the stable destination Logic, keeping names consistent.
Fixes: _BusSubsetForStructSlice._destination removed in 249b210.
Records which Logic the namer chose as the source of each signal name (an additive reverse map; does not influence naming). Lets source-trace and cross-probe callers attribute a merged net to its declared signal rather than an arbitrary internal signal that merged into the same net.
| } | ||
|
|
||
| /// Returns a synthesized netlist JSON representation of this [Module]. | ||
| String generateNetlist( |
There was a problem hiding this comment.
i've been sort of regretting having generateSynth at all, maybe long term can deprecate it, but it keeps being convenient. this expansion of the pattern is maybe not great in that regard. what if we instead upgraded the generateSynth to accept a Synthesizer and default to SystemVerilogSynthesizer to maintain backwards compatibility? thoughts?
There was a problem hiding this comment.
Please look at the module_services branch. I move away from generateSynth -- not deprecate (yet), but I use a 'Service' that has its own options and output instead of in the Module api
There was a problem hiding this comment.
This was ONLY done to make this branch reviewable at this point in time (this 'era' of having generateSynth).
There was a problem hiding this comment.
Should we omit it then from this PR if it's going to be removed/deprecated soon anyways?
There was a problem hiding this comment.
Tests need it. We will deprecate in module_services_api.
There was a problem hiding this comment.
im concerned that once we merge this if we do a release it will become an API that needs deprecation
There was a problem hiding this comment.
This PR is designed to come before module_services_api which we are still not completely sure of.
meanwhile, netlist needs tests.
So we could simply mark this as testing only for now.
There was a problem hiding this comment.
This is why this PR is from branch netlist_pre. The _pre means before module_services. I tried to peel apart the changes into digestible PRs, but this means things like this need to happen: APIs need to be there for a period and then disappear. I cannot design a perfect merge order, especially when we can't quite agree on the services api.
There was a problem hiding this comment.
I marked it @visibleForTesting.
|
|
||
| /// Replaces a `$concat` of adjacent `$slice` outputs from the same source | ||
| /// with one wider `$slice`. | ||
| static void collapseConcatOfAdjacentSlices( |
There was a problem hiding this comment.
question: is this already all merged with main and benefits from the common collapsing in the synth stack?
There was a problem hiding this comment.
I believe netlist passes contains operations that are supersets of the common collapsing that was just added to the synth stack.
- Adjacent $slice outputs feeding a $concat can collapse into one wider $slice; example module: [netlist_test.dart:79]
- Transparent $slice/$buf alias chains can collapse by tracing bits back to the external source; example module: [netlist_test.dart:93
- The netlist pass file also has cleanup passes for trivial concat aliases and unconsumed transparent cells in [netlist_passes.dart:322
Similarly, there is code for structure field extraction and concatenation that also need collapse.
I've factored out the common collapsing code to leverage what is in main, a first layer of optimization.
I think what we can do is generate examples that trigger these passes and see if we want to push these optimizations down into the synth layer. I have a feeling they would subsume the current collapsing code since they are more general.
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.