m-labs / migen

A Python toolbox for building complex digital hardware
https://m-labs.hk/migen
Other
1.23k stars 210 forks source link

Migen Namer __main__ Decoration #29

Open cr1901 opened 9 years ago

cr1901 commented 9 years ago

This isn't necessarily a bug, but I do feel it makes Verilog identifiers that Migen generates more difficult to read.

Migen has a tendency to decorate Verilog identifiers with the __main__ prefix if declared in a top level module. Strictly speaking, these identifiers do not appear to be necessary, and only increase identifier length/verbosity.

The following gist demonstrates the behavior: https://gist.github.com/cr1901/c20cbc5b1eb0d8d43578

In the first .py file of the above gist, ModA() receives no decoration, but signals declared in ModTop() and ModB() receive __main__ decorators. The behavior is not consistent and seems to be related to whether ClockDomains are manually defined. In the second .py file of the above gist, the ModA clock domain is removed, the sys clock domain is inferred in ModTop() and verilog.convert, and the __main__ decoration in the generated Verilog disappears. Except for the always@ for ModA's clock, the two files are functionally identical, but the first generates more verbose identifiers.

sbourdeauducq commented 9 years ago

Here is how the Migen namer sees your code (generate those figures with debug=True in namer.py). Pink-colored nodes are the names which are selected to appear. tree There is a conflict for the clk/rst names which appear twice, and the algorithm correctly solves it at the upper-most level - at the distinction between __main__ and moda.

Now the question is why the tracer generates this distinction. Can you have a look?

whitequark commented 5 years ago

Triage: fixed in nMigen, which features a simpler namer design.

sbourdeauducq commented 5 years ago

Hmm, I'd say that currently nMigen generally has worse names than Migen in Verilog output (even with https://github.com/YosysHQ/yosys/pull/726 patched)

whitequark commented 5 years ago

@sbourdeauducq I think you should file an issue with specific complaints and/or improvement suggestions, then.