Repository: amaranth-lang/amaranth (python)

Issue:
New IR causes DSP inference issues with Quartus
The following design (within the Details) infers 2 DSP blocks: `Info (21062): Implemented 2 DSP elements`.

<details>

```python
from amaranth import *
from amaranth.lib import wiring
from amaranth.lib.wiring import In, Out

from amaranth_boards.de10_nano import DE10NanoPlatform

class DSPMACBlock(wiring.Component):
    # using signed 18x18->36 mode
    # https://www.intel.com/content/www/us/en/docs/programmable/683375/current/independent-multiplier-mode.html

    mul_a: In(signed(18)) # A input is 18 bits
    mul_b: In(signed(18)) # B input is 18 bits
    result: Out(signed(36)) # result is len(A)+len(B)

    def elaborate(self, platform):
        m = Module()

        m.d.sync += self.result.eq(self.mul_a * self.mul_b)

        return m

class Top(Elaboratable):
    def elaborate(self, platform):
        m = Module()

        m.submodules.mac = mac = DSPMACBlock()
        m.d.comb += [
            mac.mul_a.eq(Mux(platform.request("switch", 0).i, -1, 0)), # use all bits
            mac.mul_b.eq(Mux(platform.request("switch", 1).i, -1, 0)),
            platform.request("led", 0).o.eq(mac.result[-1])
        ]

        return m

if __name__ == "__main__":
    DE10NanoPlatform().build(Top())
```

</details>

Given that this is a supported DSP mode, I would expect this to infer only 1 block. This worked properly with the old IR. If I make both operands of the `*` signed by modifying line 107 of the generated Verilog (`assign \$1 = ...`) then correctly 1 block is inferred.

Not sure if there's any difference in functionality between the two scenarios.

Versions:
* amaranth 24a392887af19a9d013252759ec209d5a91a378a
* amaranth-boards: https://github.com/amaranth-lang/amaranth-boards/commit/170675812b71ee722bcf8ccdb88409a9ad97ffe2
* Yosys 0.38 (git sha1 543faed9c8c, gcc 13.2.0 -fPIC -Os)
* Quartus x86_64 Linux 20.1.1.720

Pull request description:
back.rtlil: Opportunistically trim zero and sign extension on operands.
Fixes #1148.

Patch:
diff --git a/amaranth/back/rtlil.py b/amaranth/back/rtlil.py
index ec7b699..a6367bd 100644
--- a/amaranth/back/rtlil.py
+++ b/amaranth/back/rtlil.py
@@ -744,6 +744,16 @@ class ModuleEmitter:
         emit_assignments(proc, _nir.Net.from_const(1))
         assert pos == len(cell.assignments)
 
+    def shorten_operand(self, value, *, signed):
+        value = list(value)
+        if signed:
+            while len(value) > 1 and value[-1] == value[-2]:
+                value.pop()
+        else:
+            while len(value) > 0 and value[-1] == _nir.Net.from_const(0):
+                value.pop()
+        return _nir.Value(value)
+
     def emit_operator(self, cell_idx, cell):
         UNARY_OPERATORS = {
             "-":    "$neg",
@@ -782,17 +792,75 @@ class ModuleEmitter:
         if len(cell.inputs) == 1:
             cell_type = UNARY_OPERATORS[cell.operator]
             operand, = cell.inputs
+            signed = False
+            if cell.operator == "-":
+                # For arithmetic operands, we trim the extra sign or zero extension on the operands
+                # to make the output prettier, and to fix inference problems in some not very smart
+                # synthesis tools.
+                operand_u = self.shorten_operand(operand, signed=False)
+                operand_s = self.shorten_operand(operand, signed=True)
+                # The operator will work when lowered with either signedness.  Pick whichever
+                # is prettier.
+                if len(operand_s) < len(operand_u):
+                    signed = True
+                    operand = operand_s
+                else:
+                    signed = False
+                    operand = operand_u
             self.builder.cell(cell_type, ports={
                 "A": self.sigspec(operand),
                 "Y": self.cell_wires[cell_idx].name
             }, parameters={
-                "A_SIGNED": False,
+                "A_SIGNED": signed,
                 "A_WIDTH": len(operand),
                 "Y_WIDTH": cell.width,
             }, src_loc=cell.src_loc)
         elif len(cell.inputs) == 2:
             cell_type, a_signed, b_signed = BINARY_OPERATORS[cell.operator]
             operand_a, operand_b = cell.inputs
+            if cell.operator in ("+", "-", "*", "==", "!="):
+                # Arithmetic operators that will work with any signedness, but we have to choose
+                # a common one for both operands. Prefer signed in case of mixed signedness.
+                operand_a_u = self.shorten_operand(operand_a, signed=False)
+                operand_b_u = self.shorten_operand(operand_b, signed=False)
+                operand_a_s = self.shorten_operand(operand_a, signed=True)
+                operand_b_s = self.shorten_operand(operand_b, signed=True)
+                if operand_a.is_const:
+                    # In case of constant operand, choose whichever shortens the other one better.
+                    signed = len(operand_b_s) < len(operand_b_u)
+                elif operand_b.is_const:
+                    signed = len(operand_a_s) < len(operand_a_u)
+                elif (len(operand_a_s) < len(operand_a) and len(operand_a_u) == len(operand_a)):
+                    # Operand A can only be shortened by signed. Pick it.
+                    signed = True
+                elif (len(operand_b_s) < len(operand_b) and len(operand_b_u) == len(operand_b)):
+                    # Operand B can only be shortened by signed. Pick it.
+                    signed = True
+                else:
+                    # Otherwise, use unsigned shortening.
+                    signed = False
+                if signed:
+                    operand_a = operand_a_s
+                    operand_b = operand_b_s
+                else:
+                    operand_a = operand_a_u
+                    operand_b = operand_b_u
+                a_signed = b_signed = signed
+            if cell.operator[0] in "us":
+                # Signedness forced, just shorten.
+                operand_a = self.shorten_operand(operand_a, signed=a_signed)
+                operand_b = self.shorten_operand(operand_b, signed=b_signed)
+            if cell.operator == "<<":
+                # We can pick the signedness for left operand, but right is fixed.
+                operand_a_u = self.shorten_operand(operand_a, signed=False)
+                operand_a_s = self.shorten_operand(operand_a, signed=True)
+                if len(operand_a_s) < len(operand_a_u):
+                    a_signed = True
+                    operand_a = operand_a_s
+                else:
+                    a_signed = False
+                    operand_a = operand_a_u
+                operand_b = self.shorten_operand(operand_b, signed=b_signed)
             if cell.operator in ("u//", "s//", "u%", "s%"):
                 result = self.builder.wire(cell.width)
                 self.builder.cell(cell_type, ports={
diff --git a/amaranth/hdl/_nir.py b/amaranth/hdl/_nir.py
index 72803b9..828e858 100644
--- a/amaranth/hdl/_nir.py
+++ b/amaranth/hdl/_nir.py
@@ -161,6 +161,10 @@ class Value(tuple):
         else:
             return f"(cat {' '.join(chunks)})"
 
+    @property
+    def is_const(self):
+        return all(net.is_const for net in self)
+
     __str__ = __repr__
 
 


Test patch:
diff --git a/tests/test_back_rtlil.py b/tests/test_back_rtlil.py
index 47d7b4b..480a3f2 100644
--- a/tests/test_back_rtlil.py
+++ b/tests/test_back_rtlil.py
@@ -123,16 +123,16 @@ class RHSTestCase(RTLILTestCase):
             wire width 1 $8
             cell $neg $9
                 parameter \A_SIGNED 0
-                parameter \A_WIDTH 9
+                parameter \A_WIDTH 8
                 parameter \Y_WIDTH 9
-                connect \A { 1'0 \i8u [7:0] }
+                connect \A \i8u [7:0]
                 connect \Y $1
             end
             cell $neg $10
-                parameter \A_SIGNED 0
-                parameter \A_WIDTH 9
+                parameter \A_SIGNED 1
+                parameter \A_WIDTH 8
                 parameter \Y_WIDTH 9
-                connect \A { \i8s [7] \i8s [7:0] }
+                connect \A \i8s [7:0]
                 connect \Y $2
             end
             cell $not $11
@@ -229,61 +229,61 @@ class RHSTestCase(RTLILTestCase):
             cell $add $5
                 parameter \A_SIGNED 0
                 parameter \B_SIGNED 0
-                parameter \A_WIDTH 9
-                parameter \B_WIDTH 9
+                parameter \A_WIDTH 8
+                parameter \B_WIDTH 8
                 parameter \Y_WIDTH 9
-                connect \A { 1'0 \i8ua [7:0] }
-                connect \B { 1'0 \i8ub [7:0] }
+                connect \A \i8ua [7:0]
+                connect \B \i8ub [7:0]
                 connect \Y $1
             end
             cell $add $6
-                parameter \A_SIGNED 0
-                parameter \B_SIGNED 0
-                parameter \A_WIDTH 10
-                parameter \B_WIDTH 10
+                parameter \A_SIGNED 1
+                parameter \B_SIGNED 1
+                parameter \A_WIDTH 9
+                parameter \B_WIDTH 8
                 parameter \Y_WIDTH 10
-                connect \A { 2'00 \i8ua [7:0] }
-                connect \B { \i8sb [7] \i8sb [7] \i8sb [7:0] }
+                connect \A { 1'0 \i8ua [7:0] }
+                connect \B \i8sb [7:0]
                 connect \Y \o2
             end
             cell $add $7
-                parameter \A_SIGNED 0
-                parameter \B_SIGNED 0
-                parameter \A_WIDTH 9
-                parameter \B_WIDTH 9
+                parameter \A_SIGNED 1
+                parameter \B_SIGNED 1
+                parameter \A_WIDTH 8
+                parameter \B_WIDTH 8
                 parameter \Y_WIDTH 9
-                connect \A { \i8sa [7] \i8sa [7:0] }
-                connect \B { \i8sb [7] \i8sb [7:0] }
+                connect \A \i8sa [7:0]
+                connect \B \i8sb [7:0]
                 connect \Y $2
             end
             cell $sub $8
                 parameter \A_SIGNED 0
                 parameter \B_SIGNED 0
-                parameter \A_WIDTH 9
-                parameter \B_WIDTH 9
+                parameter \A_WIDTH 8
+                parameter \B_WIDTH 8
                 parameter \Y_WIDTH 9
-                connect \A { 1'0 \i8ua [7:0] }
-                connect \B { 1'0 \i8ub [7:0] }
+                connect \A \i8ua [7:0]
+                connect \B \i8ub [7:0]
                 connect \Y $3
             end
             cell $sub $9
-                parameter \A_SIGNED 0
-                parameter \B_SIGNED 0
-                parameter \A_WIDTH 10
-                parameter \B_WIDTH 10
+                parameter \A_SIGNED 1
+                parameter \B_SIGNED 1
+                parameter \A_WIDTH 9
+                parameter \B_WIDTH 8
                 parameter \Y_WIDTH 10
-                connect \A { 2'00 \i8ua [7:0] }
-                connect \B { \i8sb [7] \i8sb [7] \i8sb [7:0] }
+                connect \A { 1'0 \i8ua [7:0] }
+                connect \B \i8sb [7:0]
                 connect \Y \o5
             end
             cell $sub $10
-                parameter \A_SIGNED 0
-                parameter \B_SIGNED 0
-                parameter \A_WIDTH 9
-                parameter \B_WIDTH 9
+                parameter \A_SIGNED 1
+                parameter \B_SIGNED 1
+                parameter \A_WIDTH 8
+                parameter \B_WIDTH 8
                 parameter \Y_WIDTH 9
-                connect \A { \i8sa [7] \i8sa [7:0] }
-                connect \B { \i8sb [7] \i8sb [7:0] }
+                connect \A \i8sa [7:0]
+                connect \B \i8sb [7:0]
                 connect \Y $4
             end
             connect \o1 { 1'0 $1 [8:0] }
@@ -293,6 +293,82 @@ class RHSTestCase(RTLILTestCase):
         end
         """)
 
+    def test_operator_add_imm(self):
+        i8u = Signal(8)
+        i8s = Signal(signed(8))
+        o1 = Signal(10)
+        o2 = Signal(10)
+        o3 = Signal(10)
+        o4 = Signal(10)
+        m = Module()
+        m.d.comb += [
+            o1.eq(i8u + 3),
+            o2.eq(i8s + 3),
+            o3.eq(3 + i8u),
+            o4.eq(3 + i8s),
+        ]
+        self.assertRTLIL(m, [i8u, i8s, o1, o2, o3, o4], R"""
+        attribute \generator "Amaranth"
+        attribute \top 1
+        module \top
+            wire width 8 input 0 \i8u
+            wire width 8 input 1 signed \i8s
+            wire width 10 output 2 \o1
+            wire width 10 output 3 \o2
+            wire width 10 output 4 \o3
+            wire width 10 output 5 \o4
+            wire width 9 $1
+            wire width 9 $2
+            wire width 9 $3
+            wire width 9 $4
+            cell $add $5
+                parameter \A_SIGNED 0
+                parameter \B_SIGNED 0
+                parameter \A_WIDTH 8
+                parameter \B_WIDTH 2
+                parameter \Y_WIDTH 9
+                connect \A \i8u [7:0]
+                connect \B 2'11
+                connect \Y $1
+            end
+            cell $add $6
+                parameter \A_SIGNED 1
+                parameter \B_SIGNED 1
+                parameter \A_WIDTH 8
+                parameter \B_WIDTH 3
+                parameter \Y_WIDTH 9
+                connect \A \i8s [7:0]
+                connect \B 3'011
+                connect \Y $2
+            end
+            cell $add $7
+                parameter \A_SIGNED 0
+                parameter \B_SIGNED 0
+                parameter \A_WIDTH 2
+                parameter \B_WIDTH 8
+                parameter \Y_WIDTH 9
+                connect \A 2'11
+                connect \B \i8u [7:0]
+                connect \Y $3
+            end
+            cell $add $8
+                parameter \A_SIGNED 1
+                parameter \B_SIGNED 1
+                parameter \A_WIDTH 3
+                parameter \B_WIDTH 8
+                parameter \Y_WIDTH 9
+                connect \A 3'011
+                connect \B \i8s [7:0]
+                connect \Y $4
+            end
+            connect \o1 { 1'0 $1 [8:0] }
+            connect \o2 { $2 [8] $2 [8:0] }
+            connect \o3 { 1'0 $3 [8:0] }
+            connect \o4 { $4 [8] $4 [8:0] }
+        end
+        """)
+
+
     def test_operator_mul(self):
         i4ua = Signal(4)
         i4ub = Signal(4)
@@ -324,31 +400,31 @@ class RHSTestCase(RTLILTestCase):
             cell $mul $4
                 parameter \A_SIGNED 0
                 parameter \B_SIGNED 0
-                parameter \A_WIDTH 8
-                parameter \B_WIDTH 8
+                parameter \A_WIDTH 4
+                parameter \B_WIDTH 4
                 parameter \Y_WIDTH 8
-                connect \A { 4'0000 \i4ua [3:0] }
-                connect \B { 4'0000 \i4ub [3:0] }
+                connect \A \i4ua [3:0]
+                connect \B \i4ub [3:0]
                 connect \Y $1
             end
             cell $mul $5
-                parameter \A_SIGNED 0
-                parameter \B_SIGNED 0
-                parameter \A_WIDTH 8
-                parameter \B_WIDTH 8
+                parameter \A_SIGNED 1
+                parameter \B_SIGNED 1
+                parameter \A_WIDTH 5
+                parameter \B_WIDTH 4
                 parameter \Y_WIDTH 8
-                connect \A { 4'0000 \i4ua [3:0] }
-                connect \B { \i4sb [3] \i4sb [3] \i4sb [3] \i4sb [3] \i4sb [3:0] }
+                connect \A { 1'0 \i4ua [3:0] }
+                connect \B \i4sb [3:0]
                 connect \Y $2
             end
             cell $mul $6
-                parameter \A_SIGNED 0
-                parameter \B_SIGNED 0
-                parameter \A_WIDTH 8
-                parameter \B_WIDTH 8
+                parameter \A_SIGNED 1
+                parameter \B_SIGNED 1
+                parameter \A_WIDTH 4
+                parameter \B_WIDTH 4
                 parameter \Y_WIDTH 8
-                connect \A { \i4sa [3] \i4sa [3] \i4sa [3] \i4sa [3] \i4sa [3:0] }
-                connect \B { \i4sb [3] \i4sb [3] \i4sb [3] \i4sb [3] \i4sb [3:0] }
+                connect \A \i4sa [3:0]
+                connect \B \i4sb [3:0]
                 connect \Y $3
             end
             connect \o1 { 1'0 $1 [7:0] }
@@ -438,18 +514,18 @@ class RHSTestCase(RTLILTestCase):
                 parameter \A_SIGNED 1
                 parameter \B_SIGNED 1
                 parameter \A_WIDTH 5
-                parameter \B_WIDTH 5
+                parameter \B_WIDTH 4
                 parameter \Y_WIDTH 5
                 connect \A { 1'0 \i4ua [3:0] }
-                connect \B { \i4sb [3] \i4sb [3:0] }
+                connect \B \i4sb [3:0]
                 connect \Y $14
             end
             wire width 1 $16
             cell $reduce_bool $17
                 parameter \A_SIGNED 0
-                parameter \A_WIDTH 5
+                parameter \A_WIDTH 4
                 parameter \Y_WIDTH 1
-                connect \A { \i4sb [3] \i4sb [3:0] }
+                connect \A \i4sb [3:0]
                 connect \Y $16
             end
             cell $mux $18
@@ -464,10 +540,10 @@ class RHSTestCase(RTLILTestCase):
             cell $divfloor $20
                 parameter \A_SIGNED 1
                 parameter \B_SIGNED 1
-                parameter \A_WIDTH 5
+                parameter \A_WIDTH 4
                 parameter \B_WIDTH 5
                 parameter \Y_WIDTH 5
-                connect \A { \i4sa [3] \i4sa [3:0] }
+                connect \A \i4sa [3:0]
                 connect \B { 1'0 \i4ub [3:0] }
                 connect \Y $19
             end
@@ -491,19 +567,19 @@ class RHSTestCase(RTLILTestCase):
             cell $divfloor $25
                 parameter \A_SIGNED 1
                 parameter \B_SIGNED 1
-                parameter \A_WIDTH 5
-                parameter \B_WIDTH 5
+                parameter \A_WIDTH 4
+                parameter \B_WIDTH 4
                 parameter \Y_WIDTH 5
-                connect \A { \i4sa [3] \i4sa [3:0] }
-                connect \B { \i4sb [3] \i4sb [3:0] }
+                connect \A \i4sa [3:0]
+                connect \B \i4sb [3:0]
                 connect \Y $24
             end
             wire width 1 $26
             cell $reduce_bool $27
                 parameter \A_SIGNED 0
-                parameter \A_WIDTH 5
+                parameter \A_WIDTH 4
                 parameter \Y_WIDTH 1
-                connect \A { \i4sb [3] \i4sb [3:0] }
+                connect \A \i4sb [3:0]
                 connect \Y $26
             end
             cell $mux $28
@@ -546,18 +622,18 @@ class RHSTestCase(RTLILTestCase):
                 parameter \A_SIGNED 1
                 parameter \B_SIGNED 1
                 parameter \A_WIDTH 5
-                parameter \B_WIDTH 5
+                parameter \B_WIDTH 4
                 parameter \Y_WIDTH 5
                 connect \A { 1'0 \i4ua [3:0] }
-                connect \B { \i4sb [3] \i4sb [3:0] }
+                connect \B \i4sb [3:0]
                 connect \Y $34
             end
             wire width 1 $36
             cell $reduce_bool $37
                 parameter \A_SIGNED 0
-                parameter \A_WIDTH 5
+                parameter \A_WIDTH 4
                 parameter \Y_WIDTH 1
-                connect \A { \i4sb [3] \i4sb [3:0] }
+                connect \A \i4sb [3:0]
                 connect \Y $36
             end
             cell $mux $38
@@ -572,10 +648,10 @@ class RHSTestCase(RTLILTestCase):
             cell $modfloor $40
                 parameter \A_SIGNED 1
                 parameter \B_SIGNED 1
-                parameter \A_WIDTH 5
+                parameter \A_WIDTH 4
                 parameter \B_WIDTH 5
                 parameter \Y_WIDTH 5
-                connect \A { \i4sa [3] \i4sa [3:0] }
+                connect \A \i4sa [3:0]
                 connect \B { 1'0 \i4ub [3:0] }
                 connect \Y $39
             end
@@ -666,20 +742,20 @@ class RHSTestCase(RTLILTestCase):
             cell $shl $5
                 parameter \A_SIGNED 0
                 parameter \B_SIGNED 0
-                parameter \A_WIDTH 15
+                parameter \A_WIDTH 8
                 parameter \B_WIDTH 3
                 parameter \Y_WIDTH 15
-                connect \A { 7'0000000 \i8ua [7:0] }
+                connect \A \i8ua [7:0]
                 connect \B \i3 [2:0]
                 connect \Y $1
             end
             cell $shl $6
-                parameter \A_SIGNED 0
+                parameter \A_SIGNED 1
                 parameter \B_SIGNED 0
-                parameter \A_WIDTH 15
+                parameter \A_WIDTH 8
                 parameter \B_WIDTH 3
                 parameter \Y_WIDTH 15
-                connect \A { \i8sa [7] \i8sa [7] \i8sa [7] \i8sa [7] \i8sa [7] \i8sa [7] \i8sa [7] \i8sa [7:0] }
+                connect \A \i8sa [7:0]
                 connect \B \i3 [2:0]
                 connect \Y $2
             end
@@ -822,13 +898,13 @@ class RHSTestCase(RTLILTestCase):
                     connect \Y $1
                 end
                 cell $eqop $5
-                    parameter \A_SIGNED 0
-                    parameter \B_SIGNED 0
+                    parameter \A_SIGNED 1
+                    parameter \B_SIGNED 1
                     parameter \A_WIDTH 9
-                    parameter \B_WIDTH 9
+                    parameter \B_WIDTH 8
                     parameter \Y_WIDTH 1
                     connect \A { 1'0 \i8ua [7:0] }
-                    connect \B { \i8sb [7] \i8sb [7:0] }
+                    connect \B \i8sb [7:0]
                     connect \Y $2
                 end
                 cell $eqop $6
@@ -895,10 +971,10 @@ class RHSTestCase(RTLILTestCase):
                     parameter \A_SIGNED 1
                     parameter \B_SIGNED 1
                     parameter \A_WIDTH 9
-                    parameter \B_WIDTH 9
+                    parameter \B_WIDTH 8
                     parameter \Y_WIDTH 1
                     connect \A { 1'0 \i8ua [7:0] }
-                    connect \B { \i8sb [7] \i8sb [7:0] }
+                    connect \B \i8sb [7:0]
                     connect \Y $2
                 end
                 cell $cmpop $6