Skip to content

Central synthesizer naming#652

Open
desmonddak wants to merge 9 commits intointel:mainfrom
desmonddak:central_naming
Open

Central synthesizer naming#652
desmonddak wants to merge 9 commits intointel: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

_name = uniquifier.getUniqueName(
initialName: module.uniqueInstanceName,
_name = parentModule.allocateSignalName(
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.

why is this called allocateSignalName instead of allocateInstanceName or something? A module is not a signal, confusing name?

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.

It should be -- in fact we have an issue that we currently don't allow an instance and a signal to have the same name which is now fixed in this version.

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.

I thought signals and instances shared a namespace in verilog? Or am I wrong?

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.

Well my search indicated they do not share a namespace. But looking at the LRM:

The module name space is introduced by the module, interface, package, program, checker,
and primitive constructs. It unifies the definition of modules, interfaces, programs, checkers,
functions, tasks, named blocks, instance names, parameters, named events, net declarations, variable
declarations, and user-defined types within the enclosing construct.

So I guess all of these need to be uniquely named wrt each other. Most CAD systems do allow netlists to have the same wire and instance names.
For example EDIF:

In EDIF (Electronic Design Interchange Format), having the same name for an instance and a
 signal (net) is generally permissible because they exist in separate name spaces within the
syntax, allowing a net and an instance to share the same identifier.

But I guess because SystemVerilog is a 'programming' language and not a netlist, they choose to treat them as having the same namespace (e.g. a module is a namespace and everything needs to be named uniquely. Which is dumb. I'd never create a map with all objects in it.

I still think we should use allocateSignalName and allocateInstanceName because we may cache them (and again, I would always use typed maps for performance and safety).

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.

I should say, I come from the Physical Design world where every format I have worked with (including my own for ChPPR) had separate namespaces. The only tricky case was ports versus wires where many systems disagree, especially when you deal with feedthrus and external connections.

Comment thread lib/src/synthesizers/synthesizer.dart Outdated
@desmonddak desmonddak requested a review from mkorbel1 April 20, 2026 05:32
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