chipsalliance / chisel

Chisel: A Modern Hardware Design Language
https://www.chisel-lang.org/
Apache License 2.0
4.01k stars 602 forks source link

[Binder] Generate wrong IR in some cases of SInt #3963

Open SpriteOvO opened 7 months ago

SpriteOvO commented 7 months ago

https://github.com/ucb-bar/berkeley-hardfloat/blob/b3c8a38c286101973b3bc071f7918392343faba7/hardfloat/src/main/scala/primitives.scala#L76

val shift = (BigInt(-1)<<numInVals.toInt).S>>in

Right outputs

node roundMask_shift = dshr(asSInt(UInt<65>(0h10000000000000000)), roundMask_lsbs_2)

Binder generates

node roundMask_shift = dshr(SInt<65>(0), roundMask_lsbs_2)
SpriteOvO commented 7 months ago

There are 2 problems:

  1. We used mlirIntegerAttrGet to construct constants in MLIR, and the C-API accepts int64_t for a value. The reproducible example has a 65-bit value, therefore overflowed to 0.

    Upstream PR for adding a workaround: llvm/circt#6893

  2. There is a negative sign in the literal for UInt. Not sure if this is causing any problems. Could simply get an absolute value to ensure consistent output.

    val shift_s_zero = (BigInt(0)).S >> in
    val shift_s_neg1 = (BigInt(-1)).S >> in
    val shift_s_pos1 = (BigInt(1)).S >> in
    
    # Expected outputs
    node shift_zero = dshr(asSInt(UInt<1>(0h0)), in)
    node shift_neg1 = dshr(asSInt(UInt<1>(0h1)), in)
    node shift_pos1 = dshr(asSInt(UInt<2>(0h1)), in)
    
    # Binder outputs
    node shift_zero = dshr(asSInt(UInt<1>(0)), in)
    node shift_neg1 = dshr(asSInt(UInt<1>(-1)), in)
    node shift_pos1 = dshr(asSInt(UInt<2>(1)), in)