(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 -------------------------------- +1Pr25125.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