Conversation
mkorbel1
left a comment
There was a problem hiding this comment.
I haven't finished reading it all yet, but first pass a few questions
|
|
||
| _name = uniquifier.getUniqueName( | ||
| initialName: module.uniqueInstanceName, | ||
| _name = parentModule.allocateSignalName( |
There was a problem hiding this comment.
why is this called allocateSignalName instead of allocateInstanceName or something? A module is not a signal, confusing name?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I thought signals and instances shared a namespace in verilog? Or am I wrong?
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
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.dartfor 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
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
No. None needed.