Skip to content

Commit 41ce9ab

Browse files
goldsteinngoldsteinn
committed
[InstCombine] Improve coverage of foldSelectValueEquivalence
1) We don't need the `noundef` check if the new simplification is a constant. 2) There are a few more cases we can easily cover. - If f(Y) simplifies to Y - If replace X with Y makes X one-use (and make Y non one-use). This cleans up regressions from folding multiuse: `(icmp eq/ne (sub/xor x, y), 0)` -> `(icmp eq/ne x, y)`.
1 parent 08867a3 commit 41ce9ab

File tree

3 files changed

+70
-50
lines changed

3 files changed

+70
-50
lines changed

‎llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

+51-23
Original file line numberDiff line numberDiff line change
@@ -1285,21 +1285,47 @@ Instruction *InstCombinerImpl::foldSelectValueEquivalence(SelectInst &Sel,
12851285
Swapped = true;
12861286
}
12871287

1288-
// In X == Y ? f(X) : Z, try to evaluate f(Y) and replace the operand.
1289-
// Make sure Y cannot be undef though, as we might pick different values for
1290-
// undef in the icmp and in f(Y). Additionally, take care to avoid replacing
1291-
// X == Y ? X : Z with X == Y ? Y : Z, as that would lead to an infinite
1292-
// replacement cycle.
12931288
Value *CmpLHS = Cmp.getOperand(0), *CmpRHS = Cmp.getOperand(1);
1294-
if (TrueVal != CmpLHS &&
1295-
isGuaranteedNotToBeUndefOrPoison(CmpRHS, SQ.AC, &Sel, &DT)) {
1296-
if (Value *V = simplifyWithOpReplaced(TrueVal, CmpLHS, CmpRHS, SQ,
1297-
/* AllowRefinement */ true))
1298-
// Require either the replacement or the simplification result to be a
1299-
// constant to avoid infinite loops.
1300-
// FIXME: Make this check more precise.
1301-
if (isa<Constant>(CmpRHS) || isa<Constant>(V))
1289+
auto ReplaceLHSOpWithRHSOp = [&](Value *OldOp,
1290+
Value *NewOp) -> Instruction * {
1291+
// In X == Y ? f(X) : Z, try to evaluate f(Y) and replace the operand.
1292+
// Take care to avoid replacing X == Y ? X : Z with X == Y ? Y : Z, as that
1293+
// would lead to an infinite replacement cycle.
1294+
// If we will be able to evaluate f(Y) to a constant, we can allow undef,
1295+
// otherwise Y cannot be undef as we might pick different values for undef
1296+
// in the icmp and in f(Y). Additionally
1297+
if (TrueVal == OldOp)
1298+
return nullptr;
1299+
1300+
if (Value *V = simplifyWithOpReplaced(TrueVal, OldOp, NewOp, SQ,
1301+
/* AllowRefinement */ true)) {
1302+
// Need some gurantees about the new simplified op to ensure we don't inf
1303+
// loop.
1304+
// If we simplify to a constant, replace.
1305+
bool ShouldReplace = isa<Constant>(V);
1306+
bool NeedsNoUndef = !ShouldReplace;
1307+
// Or replace if either NewOp is a constant
1308+
if (!ShouldReplace && isa<Constant>(NewOp))
1309+
ShouldReplace = true;
1310+
// Or if we end up simplifying f(Y) -> Y i.e: Old & New -> New & New ->
1311+
// New.
1312+
if (!ShouldReplace && V == NewOp)
1313+
ShouldReplace = true;
1314+
1315+
// Finally, if we are going to create a new one-use instruction, replace.
1316+
if (!ShouldReplace && isa<Instruction>(OldOp) && OldOp->hasNUses(2) &&
1317+
(!isa<Instruction>(NewOp) || !NewOp->hasOneUse()))
1318+
ShouldReplace = true;
1319+
1320+
// Unless we simplify the new instruction to a constant, need to ensure Y
1321+
// is not undef.
1322+
if (NeedsNoUndef && ShouldReplace)
1323+
ShouldReplace =
1324+
isGuaranteedNotToBeUndefOrPoison(NewOp, SQ.AC, &Sel, &DT);
1325+
1326+
if (ShouldReplace)
13021327
return replaceOperand(Sel, Swapped ? 2 : 1, V);
1328+
}
13031329

13041330
// Even if TrueVal does not simplify, we can directly replace a use of
13051331
// CmpLHS with CmpRHS, as long as the instruction is not used anywhere
@@ -1308,17 +1334,19 @@ Instruction *InstCombinerImpl::foldSelectValueEquivalence(SelectInst &Sel,
13081334
// undefined behavior). Only do this if CmpRHS is a constant, as
13091335
// profitability is not clear for other cases.
13101336
// FIXME: Support vectors.
1311-
if (match(CmpRHS, m_ImmConstant()) && !match(CmpLHS, m_ImmConstant()) &&
1312-
!Cmp.getType()->isVectorTy())
1313-
if (replaceInInstruction(TrueVal, CmpLHS, CmpRHS))
1337+
if (OldOp == CmpLHS && match(NewOp, m_ImmConstant()) &&
1338+
!match(OldOp, m_ImmConstant()) && !Cmp.getType()->isVectorTy() &&
1339+
isGuaranteedNotToBeUndefOrPoison(NewOp, SQ.AC, &Sel, &DT))
1340+
if (replaceInInstruction(TrueVal, OldOp, NewOp))
13141341
return &Sel;
1315-
}
1316-
if (TrueVal != CmpRHS &&
1317-
isGuaranteedNotToBeUndefOrPoison(CmpLHS, SQ.AC, &Sel, &DT))
1318-
if (Value *V = simplifyWithOpReplaced(TrueVal, CmpRHS, CmpLHS, SQ,
1319-
/* AllowRefinement */ true))
1320-
if (isa<Constant>(CmpLHS) || isa<Constant>(V))
1321-
return replaceOperand(Sel, Swapped ? 2 : 1, V);
1342+
1343+
return nullptr;
1344+
};
1345+
1346+
if (Instruction *R = ReplaceLHSOpWithRHSOp(CmpLHS, CmpRHS))
1347+
return R;
1348+
if (Instruction *R = ReplaceLHSOpWithRHSOp(CmpRHS, CmpLHS))
1349+
return R;
13221350

13231351
auto *FalseInst = dyn_cast<Instruction>(FalseVal);
13241352
if (!FalseInst)

‎llvm/test/Transforms/InstCombine/abs-1.ll

+2-5
Original file line numberDiff line numberDiff line change
@@ -852,11 +852,8 @@ define i8 @abs_diff_signed_sgt_nuw_extra_use3(i8 %a, i8 %b) {
852852

853853
define i32 @abs_diff_signed_slt_swap_wrong_pred1(i32 %a, i32 %b) {
854854
; CHECK-LABEL: @abs_diff_signed_slt_swap_wrong_pred1(
855-
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[B:%.*]], [[A:%.*]]
856-
; CHECK-NEXT: [[SUB_AB:%.*]] = sub nsw i32 [[A]], [[B]]
857-
; CHECK-NEXT: [[SUB_AB1:%.*]] = sub nsw i32 [[B]], [[A]]
858-
; CHECK-NEXT: [[COND:%.*]] = select i1 [[CMP]], i32 [[SUB_AB]], i32 [[SUB_AB1]]
859-
; CHECK-NEXT: ret i32 [[COND]]
855+
; CHECK-NEXT: [[SUB_AB1:%.*]] = sub nsw i32 [[B:%.*]], [[A:%.*]]
856+
; CHECK-NEXT: ret i32 [[SUB_AB1]]
860857
;
861858
%cmp = icmp eq i32 %a, %b
862859
%sub_ba = sub nsw i32 %b, %a

‎llvm/test/Transforms/InstCombine/select.ll

+17-22
Original file line numberDiff line numberDiff line change
@@ -2787,8 +2787,7 @@ define <2 x i8> @select_replacement_add_eq_vec_nonuniform(<2 x i8> %x, <2 x i8>
27872787
define <2 x i8> @select_replacement_add_eq_vec_poison(<2 x i8> %x, <2 x i8> %y) {
27882788
; CHECK-LABEL: @select_replacement_add_eq_vec_poison(
27892789
; CHECK-NEXT: [[CMP:%.*]] = icmp eq <2 x i8> [[X:%.*]], <i8 1, i8 poison>
2790-
; CHECK-NEXT: [[ADD:%.*]] = add <2 x i8> [[X]], <i8 1, i8 1>
2791-
; CHECK-NEXT: [[SEL:%.*]] = select <2 x i1> [[CMP]], <2 x i8> [[ADD]], <2 x i8> [[Y:%.*]]
2790+
; CHECK-NEXT: [[SEL:%.*]] = select <2 x i1> [[CMP]], <2 x i8> <i8 2, i8 poison>, <2 x i8> [[Y:%.*]]
27922791
; CHECK-NEXT: ret <2 x i8> [[SEL]]
27932792
;
27942793
%cmp = icmp eq <2 x i8> %x, <i8 1, i8 poison>
@@ -2839,8 +2838,7 @@ define i8 @select_replacement_sub_noundef(i8 %x, i8 noundef %y, i8 %z) {
28392838
define i8 @select_replacement_sub(i8 %x, i8 %y, i8 %z) {
28402839
; CHECK-LABEL: @select_replacement_sub(
28412840
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X:%.*]], [[Y:%.*]]
2842-
; CHECK-NEXT: [[SUB:%.*]] = sub i8 [[X]], [[Y]]
2843-
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[SUB]], i8 [[Z:%.*]]
2841+
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 0, i8 [[Z:%.*]]
28442842
; CHECK-NEXT: ret i8 [[SEL]]
28452843
;
28462844
%cmp = icmp eq i8 %x, %y
@@ -4430,10 +4428,9 @@ define i32 @src_no_trans_select_and_ne0_xor_or(i32 %x, i32 %y) {
44304428

44314429
define i32 @src_no_trans_select_xor_eq0_xor_and(i32 %x, i32 %y) {
44324430
; CHECK-LABEL: @src_no_trans_select_xor_eq0_xor_and(
4433-
; CHECK-NEXT: [[XOR:%.*]] = xor i32 [[X:%.*]], [[Y:%.*]]
4434-
; CHECK-NEXT: [[XOR0:%.*]] = icmp eq i32 [[X]], [[Y]]
4431+
; CHECK-NEXT: [[XOR0:%.*]] = icmp eq i32 [[X:%.*]], [[Y:%.*]]
44354432
; CHECK-NEXT: [[AND:%.*]] = and i32 [[X]], [[Y]]
4436-
; CHECK-NEXT: [[COND:%.*]] = select i1 [[XOR0]], i32 [[XOR]], i32 [[AND]]
4433+
; CHECK-NEXT: [[COND:%.*]] = select i1 [[XOR0]], i32 0, i32 [[AND]]
44374434
; CHECK-NEXT: ret i32 [[COND]]
44384435
;
44394436
%xor = xor i32 %x, %y
@@ -4445,10 +4442,9 @@ define i32 @src_no_trans_select_xor_eq0_xor_and(i32 %x, i32 %y) {
44454442

44464443
define i32 @src_no_trans_select_xor_eq0_xor_or(i32 %x, i32 %y) {
44474444
; CHECK-LABEL: @src_no_trans_select_xor_eq0_xor_or(
4448-
; CHECK-NEXT: [[XOR:%.*]] = xor i32 [[X:%.*]], [[Y:%.*]]
4449-
; CHECK-NEXT: [[XOR0:%.*]] = icmp eq i32 [[X]], [[Y]]
4445+
; CHECK-NEXT: [[XOR0:%.*]] = icmp eq i32 [[X:%.*]], [[Y:%.*]]
44504446
; CHECK-NEXT: [[OR:%.*]] = or i32 [[X]], [[Y]]
4451-
; CHECK-NEXT: [[COND:%.*]] = select i1 [[XOR0]], i32 [[XOR]], i32 [[OR]]
4447+
; CHECK-NEXT: [[COND:%.*]] = select i1 [[XOR0]], i32 0, i32 [[OR]]
44524448
; CHECK-NEXT: ret i32 [[COND]]
44534449
;
44544450
%xor = xor i32 %x, %y
@@ -4460,11 +4456,11 @@ define i32 @src_no_trans_select_xor_eq0_xor_or(i32 %x, i32 %y) {
44604456

44614457
define i32 @src_no_trans_select_xor_eq0_and_xor(i32 %x, i32 %y) {
44624458
; CHECK-LABEL: @src_no_trans_select_xor_eq0_and_xor(
4463-
; CHECK-NEXT: [[XOR:%.*]] = xor i32 [[X:%.*]], [[Y:%.*]]
4464-
; CHECK-NEXT: [[XOR0:%.*]] = icmp eq i32 [[X]], [[Y]]
4465-
; CHECK-NEXT: [[AND:%.*]] = and i32 [[X]], [[Y]]
4466-
; CHECK-NEXT: [[COND:%.*]] = select i1 [[XOR0]], i32 [[AND]], i32 [[XOR]]
4467-
; CHECK-NEXT: ret i32 [[COND]]
4459+
; CHECK-NEXT: [[COND:%.*]] = xor i32 [[XOR:%.*]], [[Y:%.*]]
4460+
; CHECK-NEXT: [[XOR0:%.*]] = icmp eq i32 [[XOR]], [[Y]]
4461+
; CHECK-NEXT: [[AND:%.*]] = and i32 [[XOR]], [[Y]]
4462+
; CHECK-NEXT: [[COND1:%.*]] = select i1 [[XOR0]], i32 [[AND]], i32 [[COND]]
4463+
; CHECK-NEXT: ret i32 [[COND1]]
44684464
;
44694465
%xor = xor i32 %x, %y
44704466
%xor0 = icmp eq i32 %xor, 0
@@ -4476,11 +4472,11 @@ define i32 @src_no_trans_select_xor_eq0_and_xor(i32 %x, i32 %y) {
44764472
; https://alive2.llvm.org/ce/z/SBe8ei
44774473
define i32 @src_no_trans_select_xor_eq0_or_xor(i32 %x, i32 %y) {
44784474
; CHECK-LABEL: @src_no_trans_select_xor_eq0_or_xor(
4479-
; CHECK-NEXT: [[XOR:%.*]] = xor i32 [[X:%.*]], [[Y:%.*]]
4480-
; CHECK-NEXT: [[XOR0:%.*]] = icmp eq i32 [[X]], [[Y]]
4481-
; CHECK-NEXT: [[OR:%.*]] = or i32 [[X]], [[Y]]
4482-
; CHECK-NEXT: [[COND:%.*]] = select i1 [[XOR0]], i32 [[OR]], i32 [[XOR]]
4483-
; CHECK-NEXT: ret i32 [[COND]]
4475+
; CHECK-NEXT: [[COND:%.*]] = xor i32 [[XOR:%.*]], [[Y:%.*]]
4476+
; CHECK-NEXT: [[XOR0:%.*]] = icmp eq i32 [[XOR]], [[Y]]
4477+
; CHECK-NEXT: [[OR:%.*]] = or i32 [[XOR]], [[Y]]
4478+
; CHECK-NEXT: [[COND1:%.*]] = select i1 [[XOR0]], i32 [[OR]], i32 [[COND]]
4479+
; CHECK-NEXT: ret i32 [[COND1]]
44844480
;
44854481
%xor = xor i32 %x, %y
44864482
%xor0 = icmp eq i32 %xor, 0
@@ -4494,8 +4490,7 @@ define i32 @src_no_trans_select_xor_eq0_or_xor(i32 %x, i32 %y) {
44944490
define i32 @src_select_xxory_eq0_xorxy_y(i32 %x, i32 %y) {
44954491
; CHECK-LABEL: @src_select_xxory_eq0_xorxy_y(
44964492
; CHECK-NEXT: [[XOR0:%.*]] = icmp eq i32 [[X:%.*]], [[Y:%.*]]
4497-
; CHECK-NEXT: [[XOR:%.*]] = select i1 [[XOR0]], i32 [[X]], i32 0
4498-
; CHECK-NEXT: [[COND:%.*]] = xor i32 [[XOR]], [[Y]]
4493+
; CHECK-NEXT: [[COND:%.*]] = select i1 [[XOR0]], i32 0, i32 [[Y]]
44994494
; CHECK-NEXT: ret i32 [[COND]]
45004495
;
45014496
%xor = xor i32 %x, %y

0 commit comments

Comments
 (0)