Repository navigation
Improve handling of signed numbers #450
Description
It is easy to make confusing mistakes with signed values in PyRTL. A broken example:
import pyrtl
a = -128
b = 127
# Compute a - b, which should be -255.
a_const = pyrtl.Const(name='a', val=a, signed=True, bitwidth=8)
b_const = pyrtl.Const(name='b', val=b, signed=True, bitwidth=8)
diff = a_const - b_const
diff.name = 'diff'
sim = pyrtl.Simulation()
sim.step()
print('expected diff is', a - b)
print('actual diff is',
pyrtl.val_to_signed_integer(sim.inspect('diff'), bitwidth=diff.bitwidth),
'bitwidth', diff.bitwidth)
When this example is run, it produces:
expected diff is -255
actual diff is 1 bitwidth 9
Note that the actual output has bitwidth 9, which is sufficient to represent -255.
This example is especially confusing because PyRTL has all the information to do the right thing, because we've marked the Consts as signed=True. To fix this example, the user currently has to call signed_sub instead of using the overloaded - operator.
The current approach is error prone because the user has to keep track of signed-ness as values propagate through their circuit, and make sure they call functions like signed_add and val_to_signed_integer at the right times.
It seems better to track signed-ness within WireVector, or within a WireVector subclass, but this raises questions about type promotion - what happens when we add a signed value and an unsigned value? My current intuition is to match Verilog's promotion behavior, but we should think about this more.
See also #417
Activity
gaborszita commentedon Mar 22, 2025
I was looking into this and noticed that this also causes issues with comparison operators. For example:
I think tracking the signed-ness within WireVector and matching Verilog's type promotion behavior is a good solution, I can work on this if it is finalized that this is what we want to do.
fdxmw commentedon Mar 22, 2025
Yes, this issue applies to comparison operators as well. Ideally, PyRTL would automatically call signed_lt when comparing two signed WireVectors with
__lt__.Tracking signed-ness within WireVector (or maybe a subclass?) sounds good idea to me, but it depends a lot on whether we can maintain backwards compatibility, and the complexity of the implementation. We should also check if @timsherwood is okay with such a change.
fdxmw commentedon Oct 30, 2025
This issue came up again in the context of pull request #475 (Pyrtl floating point library), which considered adding a
FloatWireVectorclass. We currently think we need a more complete solution to this problem that considers all the different types involved (signed/unsigned integers, floats, other future types?), with a consistent plan for how these types interact (coercion, promotion, etc), especially when different bitwidths are involved.Without some kind of "unified WireVector type philosophy," it's too easy to end up with in confusing situations where bits are interpreted incorrectly. Gabor points out this example:
If we aren't careful, this can easily invoke
WireVector.__add__, which will reinterpretbas an unsigned integer, which is almost certainly the wrong thing to do.