Skip to content

Central synthesizer naming#652

Open
desmonddak wants to merge 14 commits into
intel:mainfrom
desmonddak:central_naming
Open

Central synthesizer naming#652
desmonddak wants to merge 14 commits into
intel:mainfrom
desmonddak:central_naming

Conversation

@desmonddak
Copy link
Copy Markdown
Contributor

Description & Motivation

Migrate synthesis naming to a central namer so that all synthesizers (and even WaveDumper) can agree on signal naming.
This is needed for a new synthesizer to be added (netlist).

Related Issue(s)

None.

Testing

I ran existing examples in ROHD and compared SV.

I added extensive test matrix test/naming_cases_test.dart for the combinations of naming options (reserved, mergeable, etc) for which I generated tests that pass before and after the central naming.

Two bugs were filed: bug #648 and bug #649 which are fixed in PR # 650 for the traditional SV code as well as in this PR for central naming).

I ran a very, very, very large design example and compared SV before and after.

Backwards-compatibility

Is this a breaking change that will not be backwards-compatible? If yes, how so?

Naming is slightly different due to ordering, so when we do have collisions, we will see _0, _1 on different signals. Plus the two bug fixes involving unnamed substructures and repeated const names.

Documentation

Does the change require any updates to documentation? If so, where? Are they included?

No. None needed.

Copy link
Copy Markdown
Contributor

@mkorbel1 mkorbel1 left a comment

Choose a reason for hiding this comment

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

I haven't finished reading it all yet, but first pass a few questions

Comment thread lib/src/synthesizers/synthesizer.dart Outdated
Comment thread lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart Outdated
Comment thread lib/src/synthesizers/synthesizer.dart Outdated
@desmonddak desmonddak requested a review from mkorbel1 April 20, 2026 05:32
Copy link
Copy Markdown
Contributor

@mkorbel1 mkorbel1 left a comment

Choose a reason for hiding this comment

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

This is looking pretty good, thank you for refactoring it this way, I can see how it helps your other upcoming changes!

Comment thread lib/src/synthesizers/utilities/synth_logic.dart Outdated
Comment thread lib/src/utilities/namer.dart Outdated
Comment thread lib/src/utilities/namer.dart Outdated
Comment thread lib/src/utilities/namer.dart Outdated
Comment thread lib/src/utilities/namer.dart Outdated
Comment thread lib/src/utilities/namer.dart Outdated
Comment thread lib/src/utilities/namer.dart Outdated
Comment thread lib/src/utilities/namer.dart Outdated
Comment thread test/name_test.dart Outdated
Comment thread test/signal_registry_test.dart Outdated
@desmonddak desmonddak requested a review from mkorbel1 May 3, 2026 19:47
Copy link
Copy Markdown
Contributor

@mkorbel1 mkorbel1 left a comment

Choose a reason for hiding this comment

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

Looking close now to mergeable!

Comment thread lib/src/utilities/namer.dart Outdated
Comment thread lib/src/synthesizers/utilities/synth_module_definition.dart Outdated
Comment thread test/name_test.dart Outdated
Comment thread test/signal_registry_test.dart
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants