#44 modbv

[David]

I was originally using an old 0.8-dev checkout from November where modbv happened to work well in my design.

Now I have myhdl 0.8 installed, from the .tar.gz at:
http://sourceforge.net/projects/myhdl/files/myhdl/0.8/

However, this broke my use of modbv in simple cases such as:

clk = Signal(bool(0))
x = Signal(modbv(0)[4:])
z = Signal(modbv(0)[4:])
@always(clk.posedge)
def dec():
    z.next = x - 1

To reproduce the error with 0.8, run "python test_modbv.py" with the attached code.

Since this worked on an older 0.8dev snapshot, I narrowed down the changes and created a patch which restores the expected behavior. See attached: myhdl-0.8-fix_modbv.patch

[David]

I figured I should include a dump of the error output when executing test_modbv.py

subspace(ttys000):~> python test_modbv.py                                      
Traceback (most recent call last):
  File "test_modbv.py", line 37, in <module>
    Simulation(instances()).run()
  File "/Users/dholl/Library/Python/2.7/lib/python/site-packages/myhdl/_Simulation.py", line 132, in run
    waiter.next(waiters, actives, exc)
  File "/Users/dholl/Library/Python/2.7/lib/python/site-packages/myhdl/_Waiter.py", line 141, in next
    clause = self.generator.next()
  File "/Users/dholl/Library/Python/2.7/lib/python/site-packages/myhdl/_always.py", line 105, in genfunc
    func()
  File "test_modbv.py", line 20, in dec
    z.next = x - 1
  File "/Users/dholl/Library/Python/2.7/lib/python/site-packages/myhdl/_Signal.py", line 216, in _set_next
    self._setNextVal(val)
  File "/Users/dholl/Library/Python/2.7/lib/python/site-packages/myhdl/_Signal.py", line 282, in _setNextIntbv
    self._next._handleBounds()
  File "/Users/dholl/Library/Python/2.7/lib/python/site-packages/myhdl/_intbv.py", line 83, in _handleBounds
    (self._val, self._min))
ValueError: intbv value -1 < minimum 0

[Jan Decaluwe]

Mm... subclassing support fun.
Problem is that a recent change to make intbv subclassing easier doesn't return a modbv type when slicing, when that is clearly the intention.
The test suite didn't catch this, because it only uses explcit min/max bounds instead of slicing to constrain a modbv. That is also the workaround.
I think the best solution would be to have a dedicated getitem method for the modbv subclass, instead of reverting to earlier code as in your patch.
Why would you want to change the lshift behavior?
In any case, this is a nasty bug that will warrant a 0.8.1 bug fix release soon.

[David]

As for how the problem is actually fixed, I'd defer to your insight regarding the interactions with the rest of the code base and other usage patterns. I would hope that the fix would not require changes in my current usage pattern of the simple "z.next = x - 1". (Would I need to modify my code to put getitem or [:] all over the place?)

About the change for lshift, I often dabble in cryptography where modulo arithmetic is expected, and will use bit shifts in lieu of multiplications by powers of two. (hey, they're free with bits!) From my "user perspective", I'd prefer that creating a modbv would retain that type without silently switching during any operation. (When I don't want modbv's modulo 2**N behavior, I'd specifically use intbv.)

Also, I noted that the other operations such as rshift, and, or, etc... also appear to retain the original subclassed type (yay!), so this change would bring lshift in-line with similar behavior of the other operations.

But I'm curious, why should lshift differ from the other operations in the first place? It also has that long(...) conversion in there as well whereas the other operations do not.

[Jan Decaluwe]

A lot of performance/conversion issues in MyHDL are solved by passing values through assignments, and do bound-checks or value adaptations (e.g. modulo) in the target. You will get the desired behavior such as modulo by assigning to the value of an object with a certain type. Strict subtype compliance of an operation may cause unwanted bound checks on larger values, such as with lshift. The intbv or type(self) cast on such operations means that you can slice or index a result - but I will review even this in the future to see if it's really useful, instead of simply returning an integer value.
As to long, it's probably a remainder of old Python behavior that made a difference between int and long. To be reviewed in 0.9-dev.

[Jan Decaluwe]

Fixed in development by a dedicated __getitem__ method in modbv.py