[PATCH] D119089: [RISCV] Teach RISCVDAGToDAGISel::selectShiftMask to replace sub from constant with neg.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

There are a few small regressions in this as noted. Not sure exactly how to fix them yet.



================
Comment at: llvm/test/CodeGen/RISCV/rotl-rotr.ll:259
+; RV32ZBB-NEXT:    li a3, 32
+; RV32ZBB-NEXT:    sub t0, a3, a2
+; RV32ZBB-NEXT:    sll a3, a4, a7
----------------
This sub could be a t0, a2, -32 if we changed the bltz that uses t0 to check bgtz.


================
Comment at: llvm/test/CodeGen/RISCV/rotl-rotr.ll:271
+; RV32ZBB-NEXT:    sub t0, t0, a2
+; RV32ZBB-NEXT:    sub a6, a6, t0
+; RV32ZBB-NEXT:    srli a4, a4, 1
----------------
There appears to be a missed optimization here. We have (31 - (64 - a2)). Using two subs.  This could be simplified to (a2 + (-33)) using an addi.


================
Comment at: llvm/test/CodeGen/RISCV/shifts.ll:158
+; RV32I-NEXT:    li a7, 32
+; RV32I-NEXT:    sub t1, a7, a2
+; RV32I-NEXT:    sll t0, a5, a6
----------------
After we form the neg, this sub is only used by two branches. The sub could be reversed to an addi if the branch conditions were inverted.

This is also i128 shift expansion on rv32. Clang doesn't support __int128 on rv32 so this codegen doesn't matter a lot.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119089