fpga: Fixes to fixed-point arithmetic

firmware/fpga/amaranth_future/fixed.py

@@ -26,14 +26,16 @@ def __init__(self, i_or_f_width, f_width = None, /, *, signed):
         self.signed = bool(signed)
 
     @staticmethod
-    def cast(shape, f_width=0):
+    def cast(shape, f_width=0, signed=None):
         if not isinstance(shape, hdl.Shape):
             raise TypeError(f"Object {shape!r} cannot be converted to a fixed.Shape")
+        if signed == None:
+            signed = shape.signed
 
         # i_width is what's left after subtracting f_width and sign bit, but can't be negative.
-        i_width = max(0, shape.width - shape.signed - f_width)
+        i_width = max(0, shape.width - signed - f_width)
 
-        return Shape(i_width, f_width, signed = shape.signed)
+        return Shape(i_width, f_width, signed = signed)
 
     def as_shape(self):
         return hdl.Shape(self.signed + self.i_width + self.f_width, self.signed)
@@ -77,12 +79,14 @@ def __init__(self, *args):
 
 class Value(hdl.ValueCastable):
     def __init__(self, shape, target):
+        if target.shape().width != shape.as_shape().width:
+            raise ValueError(f'attempting to create {shape} ({shape.as_shape().width} bits) from {target.shape()} value')
         self._shape = shape
         self._target = target
 
     @staticmethod
-    def cast(value, f_width=0):
-        return Shape.cast(value.shape(), f_width)(value)
+    def cast(value, f_width=0, signed=None):
+        return Shape.cast(value.shape(), f_width, signed)(value)
 
     def round(self, f_width=0):
         # If we're increasing precision, extend with more fractional bits.
@@ -91,7 +95,7 @@ def round(self, f_width=0):
 
         # If we're reducing precision, perform convergent rounding (round to even).
         elif f_width < self.f_width:
-            return Shape(self.i_width, f_width, signed = self.signed)( convergent_round(self.raw(), self.f_width - f_width) )
+            return Shape(self.i_width + 1, f_width, signed = self.signed)( convergent_round(self.raw(), self.f_width - f_width) )
 
         return self
 
@@ -115,12 +119,12 @@ def as_value(self):
 
     def raw(self):
         """
-        Adding an `s ( )` signedness wrapper in `as_value` when needed
+        Adding an `s ( )` or `u ( )` signedness wrapper in `as_value` when needed
         breaks lib.wiring for some reason. In the future, raw() and
         `as_value()` should be combined.
         """
-        if self.signed:
-            return self._target.as_signed()
+        if self._target.shape().signed != self.signed:
+            return self._target.as_signed() if self.signed else self._target.as_unsigned()
         return self._target
 
     def shape(self):
@@ -217,7 +221,7 @@ def __lshift__(self, other):
                 raise ValueError("Shift amount cannot be negative")
 
             if other > self.f_width:
-                return Value.cast(hdl.Cat(hdl.Const(0, other - self.f_width), self.raw()))
+                return Value.cast(hdl.Cat(hdl.Const(0, other - self.f_width), self.raw()), signed = self.signed)
             else:
                 return Value.cast(self.raw(), self.f_width - other)
 
@@ -234,7 +238,13 @@ def __rshift__(self, other):
             if other < 0:
                 raise ValueError("Shift amount cannot be negative")
 
-            return Value.cast(self.raw(), self.f_width + other)
+            value = self.raw()
+
+            if other > self.i_width:
+                extend = value[-1] if self.signed else hdl.Const(0)
+                value = hdl.Cat(value, extend.replicate(other - self.i_width))
+
+            return Value.cast(value, self.f_width + other, self.signed)
 
         elif not isinstance(other, hdl.Value):
             raise TypeError("Shift amount must be an integer value")

firmware/fpga/dsp/fir.py

@@ -308,7 +308,7 @@ def fixed_reg(value, *args, **kwargs):
         muls = []
         for i, tap in enumerate(self.taps):
             tap_fixed = fixed.Const(tap)
-            muls.append([ fixed.Value.cast(mcm.output.p[c][f"{i}"], tap_fixed.f_width + self.shape.f_width) for c in range(self.num_channels) ])
+            muls.append([ fixed.Value.cast(mcm.output.p[c][f"{i}"], self.shape.f_width) >> tap_fixed.f_width for c in range(self.num_channels) ])
 
         # Implement adder line.
         with m.If(~self.output.valid | self.output.ready):

firmware/fpga/dsp/fir_mac16.py

@@ -434,7 +434,7 @@ def elaborate(self, platform):
                 m.d.sync += sum_carry_q.eq(self.sum_carry)
 
         m.submodules.dsp = dsp = iCE40Multiplier(
-            o_width=shape_out.as_shape().width,
+            o_width=shape_out.as_shape().width, p_width=shape_out.as_shape().width,
             always_ready=self.always_ready)
 
         valid_cnt = Signal(depth, init=1)

firmware/fpga/dsp/round.py

@@ -5,13 +5,23 @@
 # SPDX-License-Identifier: BSD-3-Clause
 
 def convergent_round(value, discarded_bits):
+    shape = value.shape()
+    if discarded_bits > shape.width - shape.signed:
+        raise ValueError(f'cannot discard {discarded_bits} bits from a value with {shape.width - shape.signed} non-sign bits')
     retained = value[discarded_bits:]
     discarded = value[:discarded_bits]
     msb_discarded = discarded[-1]
     rest_discarded = discarded[:-1]
-    lsb_retained = retained[0]
+    lsb_retained = retained[0] if len(retained) else 0
     # Round up:
     # - If discarded > 0.5
     # - If discarded == 0.5 and retained is odd
     round_up = msb_discarded & (rest_discarded.any() | lsb_retained)
+    if shape.signed:
+        retained = retained.as_signed()
+    if len(retained) - shape.signed == 0:
+        # the returned result's width should always be 1 more than retained,
+        # but in this case Amaranth addition produces 2 bits more
+        value = (retained + round_up)[:-1]
+        return value.as_signed() if shape.signed else value
     return retained + round_up