[PATCH] D117465: [RISCV] Add patterns to MIR sign-extension removal pass.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVSExtWRemoval.cpp:95
   case RISCV::LHU:
   case RISCV::LB:
----------------
Please upload patches with full context using -U999999 as noted here https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface


================
Comment at: llvm/lib/Target/RISCV/RISCVSExtWRemoval.cpp:112
+  // mask sets or resets bits 63:31
+  case RISCV::ANDI:
+    return countLeadingZeros((uint64_t)MI.getOperand(2).getImm()) > 32;
----------------
ANDI was already handled in an if statement right below this switch. That code is now dead.


================
Comment at: llvm/lib/Target/RISCV/RISCVSExtWRemoval.cpp:180
     case RISCV::XORI: {
+      // |Quotient| and |Remainder| are always <= |Dividend|. If D fits in
+      // 32-bit, then so do Q & R. Sign-division doesn't work because of
----------------
This doesn't work for DIVU

0xffffffffffffffff / 2 = 0x7fffffffffffffff


0xffffffffffffffff has all sign bits, but 0x7fffffffffffffff only has 1 sign bit.


It doesn't work for REMU either
0xffffffffffffffff % 0x8000000000000000 = 0x7fffffffffffffff


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

https://reviews.llvm.org/D117465