amaranth icon indicating copy to clipboard operation
amaranth copied to clipboard

New IR causes DSP inference issues with Quartus

Open tpwrules opened this issue 1 year ago • 1 comments

The following design (within the Details) infers 2 DSP blocks: Info (21062): Implemented 2 DSP elements.

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())

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

tpwrules avatar Feb 16 '24 21:02 tpwrules

This worked properly with the old IR.

(Technically we use both IRs right now.)

whitequark avatar Feb 19 '24 20:02 whitequark