Skip to content

Commit 4fd7f78

Browse files
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 f60b778 commit 4fd7f78

File tree

4 files changed

+92
-42
lines changed

4 files changed

+92
-42
lines changed

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

+81-24
Original file line numberDiff line numberDiff line change
@@ -1285,40 +1285,97 @@ 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))
1302-
return replaceOperand(Sel, Swapped ? 2 : 1, 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).
1297+
if (TrueVal == OldOp)
1298+
return nullptr;
1299+
1300+
std::optional<bool> IsNeverUndefCached;
1301+
auto IsNeverUndef = [&](Value *Op) {
1302+
if (!IsNeverUndefCached.has_value())
1303+
IsNeverUndefCached =
1304+
isGuaranteedNotToBeUndefOrPoison(Op, SQ.AC, &Sel, &DT);
1305+
return *IsNeverUndefCached;
1306+
};
13031307

1308+
if (Value *V = simplifyWithOpReplaced(TrueVal, OldOp, NewOp, SQ,
1309+
/* AllowRefinement= */ true)) {
1310+
// Need some guarantees about the new simplified op to ensure we don't inf
1311+
// loop.
1312+
// If we simplify to a constant, replace.
1313+
bool ShouldReplace = match(V, m_ImmConstant());
1314+
bool NeedsNoUndef = !ShouldReplace;
1315+
// Or replace if either NewOp is a constant
1316+
if (!ShouldReplace && match(NewOp, m_ImmConstant()))
1317+
ShouldReplace = true;
1318+
// Or if we end up simplifying f(Y) -> Y i.e: Old & New -> New & New ->
1319+
// New.
1320+
if (!ShouldReplace && V == NewOp)
1321+
ShouldReplace = true;
1322+
1323+
// Finally, if we are going to create a new one-use instruction, replace.
1324+
if (!ShouldReplace && isa<Instruction>(OldOp) && OldOp->hasNUses(2) &&
1325+
(!isa<Instruction>(NewOp) || !NewOp->hasOneUse()))
1326+
ShouldReplace = true;
1327+
1328+
// Unless we simplify the new instruction to a constant, need to ensure Y
1329+
// is not undef.
1330+
if (NeedsNoUndef && ShouldReplace)
1331+
ShouldReplace = IsNeverUndef(NewOp);
1332+
1333+
if (ShouldReplace)
1334+
return replaceOperand(Sel, Swapped ? 2 : 1, V);
1335+
}
1336+
// If we can't simplify, but we will either:
1337+
// 1) Create a new binop where both ops are NewOp i.e (add x, y) is "worse"
1338+
// than (add y, y) in this case, wait until the second call so we don't
1339+
// miss a one-use simplification.
1340+
// 2) Create a new one-use instruction.
1341+
// proceed.
1342+
if (TrueVal->hasOneUse() &&
1343+
(match(TrueVal, m_c_BinOp(m_Specific(OldOp), m_Specific(NewOp))) ||
1344+
(isa<Instruction>(TrueVal) && isa<Instruction>(OldOp) &&
1345+
OldOp->hasNUses(2) &&
1346+
(!isa<Instruction>(NewOp) || !NewOp->hasOneUse())))) {
1347+
auto *TrueIns = cast<Instruction>(TrueVal);
1348+
for (unsigned OpIdx = 0; OpIdx < TrueIns->getNumOperands(); ++OpIdx) {
1349+
if (TrueIns->getOperand(OpIdx) == OldOp) {
1350+
// Need to ensure NewOp is noundef (same reason as above). Wait until
1351+
// the last moment to do this check as it can be relatively expensive.
1352+
if (!IsNeverUndef(NewOp))
1353+
break;
1354+
TrueIns->setOperand(OpIdx, NewOp);
1355+
return replaceOperand(Sel, Swapped ? 2 : 1, TrueIns);
1356+
}
1357+
}
1358+
}
13041359
// Even if TrueVal does not simplify, we can directly replace a use of
13051360
// CmpLHS with CmpRHS, as long as the instruction is not used anywhere
13061361
// else and is safe to speculatively execute (we may end up executing it
13071362
// with different operands, which should not cause side-effects or trigger
13081363
// undefined behavior). Only do this if CmpRHS is a constant, as
13091364
// profitability is not clear for other cases.
13101365
// FIXME: Support vectors.
1311-
if (match(CmpRHS, m_ImmConstant()) && !match(CmpLHS, m_ImmConstant()) &&
1312-
!Cmp.getType()->isVectorTy())
1313-
if (replaceInInstruction(TrueVal, CmpLHS, CmpRHS))
1366+
if (OldOp == CmpLHS && match(NewOp, m_ImmConstant()) &&
1367+
!match(OldOp, m_ImmConstant()) && !Cmp.getType()->isVectorTy() &&
1368+
IsNeverUndef(NewOp))
1369+
if (replaceInInstruction(TrueVal, OldOp, NewOp))
13141370
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);
1371+
1372+
return nullptr;
1373+
};
1374+
1375+
if (Instruction *R = ReplaceLHSOpWithRHSOp(CmpLHS, CmpRHS))
1376+
return R;
1377+
if (Instruction *R = ReplaceLHSOpWithRHSOp(CmpRHS, CmpLHS))
1378+
return R;
13221379

13231380
auto *FalseInst = dyn_cast<Instruction>(FalseVal);
13241381
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 [[A:%.*]], [[B:%.*]]
856-
; CHECK-NEXT: [[SUB_BA:%.*]] = sub nsw i32 [[B]], [[A]]
857-
; CHECK-NEXT: [[SUB_AB:%.*]] = sub nsw i32 [[A]], [[B]]
858-
; CHECK-NEXT: [[COND:%.*]] = select i1 [[CMP]], i32 [[SUB_BA]], i32 [[SUB_AB]]
859-
; CHECK-NEXT: ret i32 [[COND]]
855+
; CHECK-NEXT: [[SUB_AB:%.*]] = sub nsw i32 [[A:%.*]], [[B:%.*]]
856+
; CHECK-NEXT: ret i32 [[SUB_AB]]
860857
;
861858
%cmp = icmp eq i32 %a, %b
862859
%sub_ba = sub nsw i32 %b, %a

llvm/test/Transforms/InstCombine/select-cmp-eq-op-fold.ll

+7-9
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@ declare void @use.i8(i8)
66
define i8 @replace_with_y_noundef(i8 %x, i8 noundef %y, i8 %z) {
77
; CHECK-LABEL: @replace_with_y_noundef(
88
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X:%.*]], [[Y:%.*]]
9-
; CHECK-NEXT: [[AND:%.*]] = and i8 [[X]], [[Y]]
10-
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[AND]], i8 [[Z:%.*]]
9+
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[Y]], i8 [[Z:%.*]]
1110
; CHECK-NEXT: ret i8 [[SEL]]
1211
;
1312
%cmp = icmp eq i8 %x, %y
@@ -20,8 +19,7 @@ define i8 @replace_with_x_noundef(i8 noundef %x, i8 %y, i8 %z) {
2019
; CHECK-LABEL: @replace_with_x_noundef(
2120
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i8 [[X:%.*]], [[Y:%.*]]
2221
; CHECK-NEXT: call void @use.i1(i1 [[CMP]])
23-
; CHECK-NEXT: [[AND:%.*]] = or i8 [[X]], [[Y]]
24-
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[Z:%.*]], i8 [[AND]]
22+
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[Z:%.*]], i8 [[X]]
2523
; CHECK-NEXT: ret i8 [[SEL]]
2624
;
2725
%cmp = icmp ne i8 %x, %y
@@ -50,7 +48,7 @@ define i8 @replace_with_y_for_new_oneuse(i8 noundef %xx, i8 noundef %y, i8 %z) {
5048
; CHECK-LABEL: @replace_with_y_for_new_oneuse(
5149
; CHECK-NEXT: [[X:%.*]] = mul i8 [[XX:%.*]], 13
5250
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X]], [[Y:%.*]]
53-
; CHECK-NEXT: [[ADD:%.*]] = add nuw i8 [[X]], [[Y]]
51+
; CHECK-NEXT: [[ADD:%.*]] = shl nuw i8 [[Y]], 1
5452
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[ADD]], i8 [[Z:%.*]]
5553
; CHECK-NEXT: ret i8 [[SEL]]
5654
;
@@ -65,7 +63,7 @@ define i8 @replace_with_y_for_new_oneuse2(i8 %xx, i8 noundef %y, i8 %z, i8 %q) {
6563
; CHECK-LABEL: @replace_with_y_for_new_oneuse2(
6664
; CHECK-NEXT: [[X:%.*]] = mul i8 [[XX:%.*]], 13
6765
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X]], [[Y:%.*]]
68-
; CHECK-NEXT: [[ADD:%.*]] = add nuw i8 [[X]], [[Q:%.*]]
66+
; CHECK-NEXT: [[ADD:%.*]] = add nuw i8 [[Y]], [[Q:%.*]]
6967
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[ADD]], i8 [[Z:%.*]]
7068
; CHECK-NEXT: ret i8 [[SEL]]
7169
;
@@ -81,7 +79,7 @@ define i8 @replace_with_x_for_new_oneuse(i8 noundef %xx, i8 noundef %yy, i8 %z,
8179
; CHECK-NEXT: [[X:%.*]] = mul i8 [[XX:%.*]], 13
8280
; CHECK-NEXT: [[Y:%.*]] = add i8 [[YY:%.*]], [[W:%.*]]
8381
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X]], [[Y]]
84-
; CHECK-NEXT: [[MUL:%.*]] = mul i8 [[X]], [[Y]]
82+
; CHECK-NEXT: [[MUL:%.*]] = mul i8 [[X]], [[X]]
8583
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[MUL]], i8 [[Z:%.*]]
8684
; CHECK-NEXT: ret i8 [[SEL]]
8785
;
@@ -115,7 +113,7 @@ define i8 @replace_with_x_for_simple_binop(i8 noundef %xx, i8 %yy, i8 %z, i8 %w)
115113
; CHECK-NEXT: [[X:%.*]] = mul i8 [[XX:%.*]], 13
116114
; CHECK-NEXT: [[Y:%.*]] = add i8 [[YY:%.*]], [[W:%.*]]
117115
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X]], [[Y]]
118-
; CHECK-NEXT: [[MUL:%.*]] = mul i8 [[X]], [[Y]]
116+
; CHECK-NEXT: [[MUL:%.*]] = mul i8 [[X]], [[X]]
119117
; CHECK-NEXT: call void @use.i8(i8 [[Y]])
120118
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[MUL]], i8 [[Z:%.*]]
121119
; CHECK-NEXT: ret i8 [[SEL]]
@@ -147,7 +145,7 @@ define i8 @replace_with_none_for_new_oneuse_fail_maybe_undef(i8 %xx, i8 %y, i8 %
147145
define i8 @replace_with_y_for_simple_binop(i8 %x, i8 noundef %y, i8 %z) {
148146
; CHECK-LABEL: @replace_with_y_for_simple_binop(
149147
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X:%.*]], [[Y:%.*]]
150-
; CHECK-NEXT: [[MUL:%.*]] = mul nsw i8 [[X]], [[Y]]
148+
; CHECK-NEXT: [[MUL:%.*]] = mul nsw i8 [[Y]], [[Y]]
151149
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[MUL]], i8 [[Z:%.*]]
152150
; CHECK-NEXT: ret i8 [[SEL]]
153151
;

llvm/test/Transforms/InstCombine/select.ll

+2-4
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

0 commit comments

Comments
 (0)