At IMEC we use myhdl extensively for system exploration and design. Recently we migrated all our projects to the newest (github) version, and rewote the designs to use the block-decorator. Unfortunately this has a side effect in the vcd simulation results, for example
u_instA = some_moduleA(signal1…)
would end up as an entry ‘u_instA’ in the vcd file, but after migration the name was changed to use the module name + some number, like ‘some_moduleA_2’
This breaks our verification flow (where we use the vcd file to verify the output of the RTL simulation)
After a little digging we came up with a minor modification to mydhl_misc.instances() to restore the previous behaviour. Basically we extend the loop over all instances to copy the variable name to _Block instances, (overriding the name that was generated before). I am aware that this bypasses _uniqueify_name() in myhdl._block
Would that break something else in the flow?
I aim for inclusion in the mainline, so would an option be needed to make this behaviour switchable ?
I understand your work-flow is broken but your modification is not future proof.
I believe @hgomersall is the one to talk with about your problem. If I am right, he made the last modifications on naming inside MyHDL.
Yes, it would break the requirements for unique names!
We deal with the name mangling problem of signals with some code that adds annotations to the output v* file, like this. I think a robust way to handle naming differences on modules is something like that.
The reason it doesn’t work in general is the python namespace doesn’t map cleanly to V* names, so any convention has a risk of a collision which needs to be detected and worked around. Annotations in the file allow you to have a string->string mapping which can accept any naming convention to any other naming convention.
Further info - there is more complication around having multiple objects that point to the same @block instance - which name of the object should be used in the V* file?
Also, how to handle class @blocks? Clearly, they can’t be self_something with any sense of robustness.
I’ve found myself fighting this problem with signals (hence the annotation solution), but IMO we need a more robust solution.
The current naming strategy is designed to result in correct behaviour of the converted code, with parsing of that code a secondary concern.
Per block the name is unique (as it is the key of the local name-space dict), But apparently the name must be unique over the entire design. It would be trivially enforced by checking (as in uniqueify routine). Is this needed for the vhdl/verilog generation? The proposed solution seems to be OK for simulation (as was also the case prior to the @block decorator).
Our solution fails for blocks that explicitly return instances rather than calling the instances() function. I can also envision a solution where the block decorator would call the ‘rename to unique instance name’ function. That would be slightly more work, but fool-proof and still maintain the same naming convention as verilog and vhdl.
I came to the conclusion that any renaming needs to be done while still inside the block, otherwise the code will be converted to byte-code and all local variable names are ‘lost’. All our blocks use return instances() as last line, so I opted to modify that function that is located in file myhdl/_misc.py. To allow backwards compatible naming the default is left untouched. In our case we want the old naming behaviour (use instance names) so we set an extra attribute in the block class, prior to defining the top-level testbench. Example:
This will generate a vcd file as before the introduction of the block decorators. We don’t use the toVerilog or toVhdl functionality, so it might be that uniqueness is required globally, not only within the module. In that case we could use block.inst_naming = ‘unique instance name’ and if a name is already occupied it will be appended by a number to make it unique.
The proposed modification is only in myhdl/_misc.py to the instances() function:
def instances():
import myhdl._block as blk
# inst_naming is one of following cases
# 'instance name' => use instance name (old behavior prior to block decorator)
# 'unique instance name' => use instance name, but make globally unique by appending number
# all others => use block-name + unique number (default)
#
# using instance-names only works when a block uses return instances()
# outside block the code is already converted to byte-code and local
# variable names (instance-names) are 'lost. Byte-code manipulation is not very portable
inst_naming = blk.block.inst_naming if hasattr(blk.block, 'inst_naming') else None
f = inspect.currentframe()
d = inspect.getouterframes(f)[1][0].f_locals
l = []
for k, v in d.items():
# use instance names instead of block-names
if isinstance(v, blk._Block) and inst_naming:
if inst_naming == 'instance name':
blk._name_set.discard(v.name) # discard old name
name = k
v.name = v.__name__ = name
elif inst_naming == 'unique instance name':
blk._name_set.discard(v.name) # discard old name
name = blk._uniqueify_name(k)
v.name = v.__name__ = name
# default is to use unique block-name, generated at block creation
if _isGenSeq(v):
l.append(v)
return l
@paulM , currently, uniqueness is required globally.
IMO, the unique name should append a number for all cases if any (no number IMO implies a single instance). That said, there are so many dodgy names that are applied (because of the fundamental issues with python naming you’ve identifies), there is a much bigger question is around naming that I’d be happy to ignore some rough corners for the time being.
There seems to be something broken with the idea of naming an instantiation from some reference to it. They’re fundamentally different things. It’s not really anything to do with byte-code manipulation; rather it is to do with python scoping rules and namespaces. Better conceptually would be to use its ID, which is at least unique.
In my view, the way to do what you want is to do it explicitly, with additional attributes on the object (for example, vcd_name). You could then use these attributes in the VCD generation code (which is where the modification can be made). That is, modify the VCD stuff by inspecting the block; if the attribute is present, use it, else default to the previous behaviour.
This would be a clean solution that doesn’t tread all over the rest of the code base and I think should be accepted.
If you wanted to do this quickly, you could replace block to use your own Block class (probably inherited from Block) that implements the instance naming behaviour you desire. Then you just need to add a single import line to all your files.
The proposed solution does exact MEP114, but without explicit assigning a inst.name = ‘inst’. Instead it modifies the instances() routine (when inst is still in locals) The proposed solution only touches the myhdl._misc.py file. And behaviour can be chosen by setting an optional attribute in the block class. So the solution is backwards compatible with the current naming solution (default) but also with the situation prior to the block decorator, and also with globally unique naming.
@hgomersall: The proposed solution does not interfere with the current situation, but extends it. As far as I can see, the name only has to be locally unique (in the block) and that is naturally guaranteed, as the inst name is the variable name in the local name space. Even though the proposed solution does allow for a globally uniqueness, can you point out where that is needed? Our design uses several identical instance names on different hierarchy levels (blocks) but that simulates without problems.
@paulM I almost never use instances(), mostly because the PyDev Eclipse plug-in otherwise flags the local instances with a warning unused variable, and I like to avoid these warnings.
Anyway, your proposed modification for instances() looks fine, to me at least. @hgomersall With hindsight, using instance names (as before the @block was bestowed upon us) should have been kept the default. I don’t understand where the uniquefy requirement came from. Because of using module derived naming?
@josyb I think the requirement comes from the verilog generation. Myhdl does not care about input widths etc, but verilog requires them to be explicitly specified. When re-using a block with different input widths the name has to be different. Verilog does not have support for hierarchical block-definitions and as a consequence the name has to be globally unique. Note that parameterized verilog instances will be translated to unique names by the verilog compiler, so also in that case the name-correspondence is lost. Anyway, the proposed solution supports both. It will default to globally unique names, but you can also set inst_naming to ‘instance name’ or ‘unique instance name’
@josyb To suppress warnings I use a call to a dummy function ‘no_warn’ just before the return instances() line. The function arguments are the instances that are defined (causing the unused_variable warning).
def no_warn(*args):
'''dummy function to suppress unused variable warnings'''
pass
I think the requirement comes from the verilog generation. MyHDL does not care about input widths etc, but verilog requires them to be explicitly specified. When re-using a block with different input widths the name has to be different.
If the behaviour was kept all names were unique ,it is only because of the switch to use the module name instead of the variable name that this (…) numbering - uniquefying has become a necessity. Or am I overlooking something?
To suppress warnings I use a call to a dummy function ‘no_warn’ just before the return instances() line. The function arguments are the instances that are defined (causing the unused_variable warning).
Q:If the behaviour was kept all names were unique ,it is only because of the switch to use the module name instead of the variable name that this (…) numbering - uniquefying has become a necessity. Or am I overlooking something?
A: Correct
Q:In which case I have to enumerate them anyway?
A: Yes, but I did not want to change return instances() argument list at the time to take dummy arguments, just to silence the editor warnings.