[PATCH] D53230: [RISCV] Introduce codegen patterns for RV64M-only instructions

[Bruce Hoult via Phabricator via llvm-commits]

brucehoult added inline comments.


================
Comment at: lib/Target/RISCV/RISCVInstrInfoM.td:57
+def : Pat<(sdiv (sexti32 GPR:$rs1), (sexti32 GPR:$rs2)),
+          (DIVW GPR:$rs1, GPR:$rs2)>;
+def : Pat<(sext_inreg (sdiv (sexti32 GPR:$rs1),
----------------
efriedma wrote:
> Are you sure `(sdiv (sexti32 GPR:$rs1), (sexti32 GPR:$rs2))` is sufficient?  I think it returns the wrong result for something like `INT_MIN/-1` (which should be a positive 64-bit value).
> 
> The pattern where the result is sign-extended is fine, I think.  And the corresponding "REMW" pattern is also fine.
I'm not sure exactly what you're saying here.

A 32 bit INT_MIN/-1 which is 0x8000_0000/0xffffffff is defined to give INT_MIN 0x8000_0000 (which a DIVW will). On a 64 bit processor the end result will be 0xffff_ffff_8000_0000.

A 32 bit INT_MIN converted to 64 bit will be 0xffff_ffff_8000_0000, which when divided by a 64 bit -1 will give 0x0000_0001_0000_0000, which is incorrect for a 32 bit calculation.

Arguably, INT_MIN/-1 could have been defined to be 0 (the LSBs of the correct result), but it wasn't.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53230/new/

https://reviews.llvm.org/D53230