(select (setcc lhs, rhs, CC), constant, falsev) -> (select (setcc lhs, rhs, InverseCC), falsev, constant)
This patch removes unnecessary copies
Paths
| Differential D129757
[RISCV] Optimize SELECT_CC when the true value of select is Constant ClosedPublic Authored by liaolucy on Jul 14 2022, 4:57 AM.
Details Summary (select (setcc lhs, rhs, CC), constant, falsev) -> (select (setcc lhs, rhs, InverseCC), falsev, constant) This patch removes unnecessary copies
Diff Detail
Event Timelineliaolucy retitled this revision from [RISCV] Optimize ISD::SETEQ to ISD::SETNE when the true value is zero to [RISCV] Optimize SELECT_CC when condition is eq and the true value of select is zero.Jul 14 2022, 6:14 PM
Comment Actions
liaolucy retitled this revision from [RISCV] Optimize SELECT_CC when condition is eq and the true value of select is zero to [RISCV] Optimize SELECT_CC when the true value of select is Constant.Jul 17 2022, 9:01 AM
Comment Actions Did you investigate any cases where this results in slightly worse codegen? one example would be 20100416-1.c from the GCC torture suite (-march=rv64imafdc -mabi=lp64d -O3). Overall the change seems to be positive, but there do seem to be a few cases like this where enabling this transformation results in an increase in instructions. It needn't necessarily block this patch, as it does seem like a sensible transformation to make - but it would be useful to understand why these regressions occur, and capture some representative test cases. Comment Actions
selectcc-to-shiftand.ll has an example already covering this type. define i32 @pos_sel_constants(i32 signext %a) { ; CHECK-LABEL: pos_sel_constants: ; CHECK: # %bb.0: ; CHECK-NEXT: mv a1, a0 ; CHECK-NEXT: li a0, 5 ; CHECK-NEXT: bgez a1, .LBB4_2 ; CHECK-NEXT: # %bb.1: ; CHECK-NEXT: li a0, 0 ; CHECK-NEXT: .LBB4_2: ; CHECK-NEXT: ret %tmp.1 = icmp sgt i32 %a, -1 %retval = select i1 %tmp.1, i32 5, i32 0 ret i32 %retval }
Comment Actions
Thanks, that fixes that regression. I also see increased instruction counts for other tests including: pr20100-1, pr25125, pr37102, pr68506, pr83383, pr89634 (O3, rv64imafdc, lp64d). The overall impact across the suite seems positive, but it's likely worth taking a look. Comment Actions
I used a stupid way to count valid static instructions. Dump .s file and remove comments, label and other useless information. If there is a better way I am willing to try it. pr20100-1: The number of instructions is the same, both are 70. pr37102, pr68506, pr89634: This patch has 1 more j instructions. Eg, from Pr83383.c: original: li a0, 0 beqz a4, .LBB0_2 This patch: bnez a3, .LBB0_2 newbb: li a0, 0 j .LBB0_3 -------------------------------- +1 Pr25125.c: +3 instructions. original: f: # @f # %bb.0: # %entry li a1, 0 bgtz a0, .LBB0_2 # %bb.1: # %entry lui a1, 8 xor a1, a1, a0 .LBB0_2: # %entry slli a0, a1, 48 srli a0, a0, 48 ret This patch: f: # @f # %bb.0: # %entry blez a0, .LBB0_2 # %bb.1: # %entry li a0, 0 slli a0, a0, 48 --------------------copy--------- +1 srli a0, a0, 48 --------------------copy--------- +2 ret --------------------copy--------- +3 .LBB0_2: lui a1, 8 xor a0, a0, a1 slli a0, a0, 48 srli a0, a0, 48 ret
Comment Actions I lost track of this patch for a while, but I just stumbled onto a case that would benefit. Can you rebase this? This revision is now accepted and ready to land.Oct 17 2022, 9:15 AM This revision was landed with ongoing or failed builds.Oct 17 2022, 6:24 PM Closed by commit rG7b970290c07f: [RISCV] Optimize SELECT_CC when the true value of select is Constant (authored by liaolucy). · Explain Why This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 468384 llvm/lib/Target/RISCV/RISCVISelLowering.cpp
llvm/test/CodeGen/RISCV/ctlz-cttz-ctpop.ll
llvm/test/CodeGen/RISCV/double-convert-strict.ll
llvm/test/CodeGen/RISCV/double-convert.ll
llvm/test/CodeGen/RISCV/float-convert-strict.ll
llvm/test/CodeGen/RISCV/float-convert.ll
llvm/test/CodeGen/RISCV/half-convert-strict.ll
llvm/test/CodeGen/RISCV/half-convert.ll
llvm/test/CodeGen/RISCV/rv32zbb.ll
|
There's already ISD::getSetCCInverse in ISDOpcodes.h