Skip to content

Commit ba2fbcf

Browse files
goldsteinngoldsteinn
committed
[InstCombine] Improve coverage of foldSelectValueEquivalence for non-constants
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). These all require that Y is not undef or poison
1 parent 3577453 commit ba2fbcf

File tree

2 files changed

+54
-15
lines changed

2 files changed

+54
-15
lines changed

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

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,6 +1300,8 @@ Instruction *InstCombinerImpl::foldSelectValueEquivalence(SelectInst &Sel,
13001300
if (TrueVal == OldOp)
13011301
return nullptr;
13021302

1303+
bool NewOpNeverUndef = false;
1304+
13031305
if (Value *V = simplifyWithOpReplaced(TrueVal, OldOp, NewOp, SQ,
13041306
/* AllowRefinement=*/true)) {
13051307
// Need some guarantees about the new simplified op to ensure we don't inf
@@ -1309,12 +1311,51 @@ Instruction *InstCombinerImpl::foldSelectValueEquivalence(SelectInst &Sel,
13091311
isGuaranteedNotToBeUndef(V, SQ.AC, &Sel, &DT))
13101312
return replaceOperand(Sel, Swapped ? 2 : 1, V);
13111313

1312-
// If NewOp is a constant and OldOp is not replace iff NewOp doesn't
1313-
// contain and undef elements.
1314-
if (match(NewOp, m_ImmConstant())) {
1315-
if (isGuaranteedNotToBeUndef(NewOp, SQ.AC, &Sel, &DT))
1316-
return replaceOperand(Sel, Swapped ? 2 : 1, V);
1314+
// We can't do any further replacement if NewOp may be undef.
1315+
if (!isGuaranteedNotToBeUndef(NewOp, SQ.AC, &Sel, &DT))
13171316
return nullptr;
1317+
1318+
// If NewOp is a constant, replace.
1319+
if (match(NewOp, m_ImmConstant()))
1320+
return replaceOperand(Sel, Swapped ? 2 : 1, V);
1321+
1322+
// If we simplified the TrueArm -> NewOp then replace.
1323+
// This handles things like `select`/`min`/`max`/`or`/`and`/etc...
1324+
if (NewOp == V)
1325+
return replaceOperand(Sel, Swapped ? 2 : 1, V);
1326+
1327+
NewOpNeverUndef = true;
1328+
}
1329+
1330+
// If we can't simplify, but we will either:
1331+
// 1) Create a new binop where both ops are NewOp i.e (add x, y) is "worse"
1332+
// than (add y, y) in this case, wait until the second call so we don't
1333+
// miss a one-use simplification.
1334+
// 2) Create a new one-use instruction.
1335+
// proceed.
1336+
if (TrueVal->hasOneUse() && isa<Instruction>(TrueVal)) {
1337+
bool BinOpMatch =
1338+
match(TrueVal, m_c_BinOp(m_Specific(OldOp), m_Specific(NewOp)));
1339+
bool NewOneUse = isa<Instruction>(OldOp) && OldOp->hasNUses(2) &&
1340+
(!isa<Instruction>(NewOp) || !NewOp->hasOneUse());
1341+
if (BinOpMatch || NewOneUse) {
1342+
bool Replaced = false;
1343+
auto *TrueIns = cast<Instruction>(TrueVal);
1344+
for (unsigned OpIdx = 0; OpIdx < TrueIns->getNumOperands(); ++OpIdx) {
1345+
if (TrueIns->getOperand(OpIdx) != OldOp)
1346+
continue;
1347+
// Need to ensure NewOp is noundef (same reason as above). Wait
1348+
// until the last moment to do this check as it can be relatively
1349+
// expensive.
1350+
if (!NewOpNeverUndef &&
1351+
!isGuaranteedNotToBeUndef(NewOp, SQ.AC, &Sel, &DT))
1352+
break;
1353+
NewOpNeverUndef = true;
1354+
TrueIns->setOperand(OpIdx, NewOp);
1355+
Replaced = true;
1356+
}
1357+
if (Replaced)
1358+
return replaceOperand(Sel, Swapped ? 2 : 1, TrueIns);
13181359
}
13191360
}
13201361

@@ -1327,7 +1368,7 @@ Instruction *InstCombinerImpl::foldSelectValueEquivalence(SelectInst &Sel,
13271368
// FIXME: Support vectors.
13281369
if (OldOp == CmpLHS && match(NewOp, m_ImmConstant()) &&
13291370
!match(OldOp, m_ImmConstant()) && !Cmp.getType()->isVectorTy() &&
1330-
isGuaranteedNotToBeUndef(NewOp, SQ.AC, &Sel, &DT))
1371+
(NewOpNeverUndef || isGuaranteedNotToBeUndef(NewOp, SQ.AC, &Sel, &DT)))
13311372
if (replaceInInstruction(TrueVal, OldOp, NewOp))
13321373
return &Sel;
13331374
return nullptr;

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

Lines changed: 7 additions & 9 deletions
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
;

0 commit comments

Comments
 (0)