[PATCH] D59477: [RISCV] Custom lower SHL_PARTS, SRA_PARTS, SRL_PARTS
Lewis Revill via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 3 08:08:05 PDT 2019
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:
----------------
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?
================
Comment at: lib/Target/RISCV/RISCVISelLowering.cpp:560
+ // Lo = 0
+ // Hi = Lo << Shamt
+
----------------
Nitpick: shouldn't this read `Hi = Lo << (Shamt-XLEN)`
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59477/new/
https://reviews.llvm.org/D59477
More information about the llvm-commits
mailing list