antonblanchard / vlsiffra

Create fast and efficient standard cell based adders, multipliers and multiply-adders.
Apache License 2.0
104 stars 9 forks source link

Verilog incompatibility with Openroad #17

Closed hermanschmit closed 1 year ago

hermanschmit commented 1 year ago

When I use: vlsi-multiplier --bits=8 to generate an eight-bit multiplier, and then try to read into OpenRoad for physical design (directly, without going thru yosys), I get a verilog syntax error.

The error is on this line:

  assign \$1  = + (* src = "/usr/local/google/home/schmit/pythonenv/vlsiffra/lib/python3.10/site-packages/vlsiffra/multiplier.py\:175" *) { b_registered, 1'h0 };

I think there are two problems. (1) the leading "+" and (2) the size mismatch between RHS and LHS.

1) I don't know why there is this leading "+". Is this an amaranth problem? 2) I think this is more straightforward: \$1 is defined as a 10 bit wire, and b_registered is 8 bits. The concatenation does not produce a ten bit quantity.

I think bug (2) is pretty evident. (from multiplier.py):

    multiplicand = Signal(self._bits + 2)

    # Add a zero in the LSB of the multiplier and multiplicand
    self.m.d.comb += [
        multiplier.eq(Cat(Const(0), self.a_registered, Const(0), Const(0))),
        multiplicand.eq(Cat(Const(0), self.b_registered)),
    ]

I suppose there is a possibility that these two things are related. Maybe the leading + is there because there is a width mismatch?

hermanschmit commented 1 year ago

I submitted a push request to fix this. Indeed, if you fix the width mismatch, the leading + goes away.