Incorrect codegen for signed operations mixed with unsigned

[poemonsense]

Incorrect codegen for signed operations mixed with unsigned

[dtzSiFive]

Fixed with llvm/circt#6966 , tested the GCD example fails the assertion before and does not after.

This should work when bumping Chisel to use the next CIRCT release including that change.

[seldridge]

Nice find, and thanks for working with the Verilator folks on getting this tracked down.

I think few people are using SInt in Chisel. But it should be correct.

Yes, I expect low impact as SInt has very limited use. We'll get this fixed.

[poemonsense]

BTW, why does Chisel/CIRCT adds an extra sign bit when converting UInt to SInt? It seems without this extra sign bit, the conversion still holds.

[poemonsense]

@seldridge Hi, do you have a timeline when this bug will be fixed? Thanks

[seldridge]

Sorry, I forgot about this. I reduced this just now to: llvm/circt#6961

@dtzSiFive identified that there was a very similar bug related to arithmetic shift right which was fixed by adding the EBForceResultSigned binary flag. This same approach should work for division here. See: llvm/circt@31070d0

I'll try to get a fix in this week. If you're so motivated to send a patch earlier, that is always appreciated.

[poemonsense]

Thank you very much on working this. Unluckily we still don't have enough human resources on understanding the circt. Personally I've tried my best to learn Chisel/CIRCT, but it seems the complexity of CIRCT makes me struggling. Hopefully next time I can submit a patch to CIRCT myself.

[poemonsense]

Fixed with llvm/circt#6966 , tested the GCD example fails the assertion before and does not after.

This should work when bumping Chisel to use the next CIRCT release including that change.

Thanks very much for all of you. We're continuously tracking the latest Chisel.

I'm closing this issue now. Please feel free to open this again if there're still issues to be cleared.

[seldridge]

For completeness, the output of the erroneous example will now produce:

// Generated by CIRCT firtool-1.73.0-56-g3ef492c80-dirty
module Test1(
  input         clock,
                reset,
  input  [15:0] io_in0,
                io_in1,
                io_in2,
  output [15:0] io_result
);

  reg [16:0] result;
  always @(posedge clock)
    result <= {1'h0, io_in0} + $signed($signed({io_in1[15], io_in1}) / $signed({io_in2[15], io_in2}));
  assign io_result = result[15:0];
endmodule