[PATCH] D59477: [RISCV] Custom lower SHL_PARTS, SRA_PARTS, SRL_PARTS

[Lewis Revill via Phabricator via llvm-commits]

lewis-revill added inline comments.


================
Comment at: lib/Target/RISCV/RISCVISelLowering.cpp:557
+  //   Lo = Lo << (Shamt-XLEN)
+  //   Hi = (Hi << Shamt) | ((Lo >>u 1) >>u (XLEN-1 - Shamt))
+  // else:
----------------
lewis-revill wrote:
> lewis-revill wrote:
> > I'm sure there's an off-by-one error in my reasoning here, but aren't we already safe to assume that `XLEN-Shamt` can fit in the bits available for srl? Since we checked for if `Shamt-XLEN < 0`, the only case where `XLEN-Shamt` couldn't fit would be where `XLEN-Shamt == XLEN` IE when `Shamt-XLEN == 0`, which is handled by the basic case below.
> > 
> > I'm assuming this was the reasoning for the `XLEN-1` stuff and the `(Lo >>u 1)` here, if not can someone explain it to me?
> I see at least part of my error. It should read 'IE when `Shamt == 0``. Either way I'm not seeing how it should come up?
Okay sorry for the noise, I see why this is an issue now. `Shamt == 0` is indeed handled by this case. Could we have some comment to that effect in this function? And likewise for `lowerShiftRightParts`?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59477