MyHDL

Conversion problem with shadow signal

Hi,
I have noticed that the use of shadow signals lead often to conversion problems. I have not found the exact conditions yet, so I have no small reproducible example.

But in my github repo https://github.com/bonfireprocessor/bonfire-core (which is just WIP…)
I have a branch “slice_problem” which exemplifies the problem.
To reproduce clone the repo, checkot branch slice_problem and than run tb_run.py.
The resulitng trace should be:

 File "tb_run.py", line 30, in <module>
    test(tb_alu.tb(c_shifter_mode="pipelined"),trace=True)
  File "/usr/local/lib/python2.7/dist-packages/myhdl/_block.py", line 196, in __call__
    self.srcline, *args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/myhdl/_block.py", line 218, in __init__
    self.subs = _flatten(func(*args, **kwargs))
  File "/home/thomas/development/bonfire/bonfire-core/tb/tb_alu.py", line 39, in tb
    inst.convert(hdl='VHDL',std_logic_ports=True,path='vhdl_gen', name="alu_" + c_shifter_mode )
  File "/usr/local/lib/python2.7/dist-packages/myhdl/_block.py", line 342, in convert
    return converter(self)
  File "/usr/local/lib/python2.7/dist-packages/myhdl/conversion/_toVHDL.py", line 255, in __call__
    _convertGens(genlist, siglist, memlist, vfile)
  File "/usr/local/lib/python2.7/dist-packages/myhdl/conversion/_toVHDL.py", line 600, in _convertGens
    v.visit(tree)
  File "/usr/lib/python2.7/ast.py", line 241, in visit
    return visitor(node)
  File "/usr/local/lib/python2.7/dist-packages/myhdl/conversion/_toVHDL.py", line 1403, in visit_Module
    self.visit(stmt)
  File "/usr/lib/python2.7/ast.py", line 241, in visit
    return visitor(node)
  File "/usr/local/lib/python2.7/dist-packages/myhdl/conversion/_toVHDL.py", line 1770, in visit_FunctionDef
    senslist = compressSensitivityList(self.tree.senslist)
  File "/usr/local/lib/python2.7/dist-packages/myhdl/conversion/_toVHDL.py", line 1762, in compressSensitivityList
    name = item._name.split('(', 1)[0]
AttributeError: 'NoneType' object has no attribute 'split'

When doing the same with the master branch, everthing works.
The difference is line 78 in alu.py:

branch slice_problem:

shift_inst=shift_pipelined(clock,reset,self.op1_i,shifter_out,self.op2_i(5,0), \
                       shift_right,fill_v,shift_en,shift_ready, 3 if c_shifter_mode=="pipelined" else 0 )

branch master:

 shift_amount=Signal(intbv(0)[5:])

 shift_inst=shift_pipelined(clock,reset,self.op1_i,shifter_out,shift_amount, \
                       shift_right,fill_v,shift_en,shift_ready, 3 if c_shifter_mode=="pipelined" else 0 )
                      
 @always_comb
            def shift_comb():

                shift_valid.next = shift_ready
                shift_amount.next = self.op2_i[5:0]
.....

From simulation perspective both variants run the same.
Is there any rule when I can use a shadow signal?
BTW: The conversion call in the test bench (tb.alu.py) is only for testing convertiblty of the design.

I can see a major difference :
self.op2_i is defined as Signal(modbv(0)[xlen:])
shift_amount is defined as Signal(intbv(0)[5:])
How does it convert if self.op2_i is defined as Signal(intbv(0)[5:]) ?

Ok, you say this is the reason that it don‘t convert. When this limitation exist, I accept it. But it is not obvious: The design simulates correct, and MyHDL doc states the modbv is a subclass of intbv. So „casting“ modbv to intbv should be semantically valid.

I‘m considering using only modbv in my desgin. It is close to the semantics of signed/unsigned types in VHDL and hardware usually also has wrap-around behavior

I dont say this is the reason. I asked you if this solve the problem. Does it ?

I believe this is not a good idea to code everything with modbv. Doing so, you might miss important bugs when simulating your design. At least, bugs will be harder to catch.

Imagine you have a counter which must not go higher than a given value. This given value is used as max when instantiating intbv. If, because of a bug, the counter is always incrementing, the simulator will raise an exception at some point. With modbv, you will never get an exception.
This is just an example.

Don’t forget that modbv with max different from a power of 2 are usable in simulation but not in conversion.

Your code takes a ShadowSignal from a ShadowSignal and the converter has a problem with that; which IMHO it shouldn’t have.
The second ShadowSignal operation is in left_shift_pipelined() when c_pipe_stage > 0. As you specified (c_shifter_mode == "pipelined") .
I applied the same workaround in left_shift_pipelined() as you did in you master branch at a higher level.
Note that c_shifter_mode == "comb" converts fine with the higher-level ShadowSignal.

A work-around is to comment out these two lines in
_analyze: def _analyzeSigs(hierarchy, hdl=‘Verilog’):

        if isinstance(s, _SliceSignal):
            continue

This will make the conversion work. I have no idea whether it is safe (for other code).
Or (worse?) perhaps (probably) not for Verilog at all.

Thanks for your effort. I think you made the workaround in the line

stage_0=left_shift_comb(d_i,stage0_out,shift_i(c_pipe_stage,0),fill_i,c_pipe_stage,0)

in left_shift_pipelined. For me its is important to understand that I made nothing “intentionally” wrong.

It is just an implementation limitation (or bug…)

Sorry for my late reply, I was a bit busy the last two weeks and did not do much with my MyHDL design. With josyb’s answer it is clear that the nesting of shadow signals is that root cause for the problem.

Ok, to clarify, with “everything” I meant more that I would try to avoid mixing modbv and intbv when creating fixed bit width structures like a processor data path or a bus system.

I agree with you, that when modelling a Signal holding a number range (like a natural subtype in VHDL) and Synthesis inferring the required implementation bit width intbv is the way to go.

But the design in question is an ALU for a processor with a fixed bit length (actually only xlen=32 is supported, I may later extend it to 64 Bit). I exactly expect arithmetic wrapping around at 2**xlen, so I think modbv is the right choice.

The question is now how to model the connections (e.g buses/multiplexers/etc) between the components. This is an area where a feel also a bit uncomfortable with MyHDL. Both “bit array” types (modbv and intbv) have an integer arithmetic semantic. I cannot say something is just a “bunch of bits” like a std_logic_vector in VHDL. But a bus or a mux is usually just switching a “bunch of bits”.

Yes, that was the line where a ShadowSignal was taken from a ShadowSignal.
Let’s call it an imperfection?

@ThomasHornschuh I understand your point of view. I might be wrong, but I believe you will not benefit the full power of MyHDL going this way.

From my point of view, a bus (data or address) should be modelled using intbv(). Bus signals are written and read as is. I mean there are no addition, shifting or any kind of operation on bus signals. So, there is no reason to not use intbv().
Even for the ALU, I’m not convinced modbv() is the way to go.