Skip to content

Commit

Permalink
[InstCombine] Improve coverage of foldSelectValueEquivalence for no…
Browse files Browse the repository at this point in the history 
…n-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
  • Loading branch information
@goldsteinn
goldsteinn committed Jun 5, 2024
1 parent a3c3705 commit e3d8065
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 13 deletions.
51 changes: 47 additions & 4 deletions llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1300,6 +1300,8 @@ Instruction *InstCombinerImpl::foldSelectValueEquivalence(SelectInst &Sel,
if (TrueVal == OldOp)
return nullptr;

bool NewOpNeverUndef = false;

if (Value *V = simplifyWithOpReplaced(TrueVal, OldOp, NewOp, SQ,
/* AllowRefinement= */ true)) {
// Need some guarantees about the new simplified op to ensure we don't inf
Expand All @@ -1308,11 +1310,52 @@ Instruction *InstCombinerImpl::foldSelectValueEquivalence(SelectInst &Sel,
if (match(V, m_ImmConstant()))
return replaceOperand(Sel, Swapped ? 2 : 1, V);

// If NewOp is a constant and OldOp is not replace iff NewOp doesn't
// contain and undef/poison elements.
if (match(NewOp, m_ImmConstant()) &&
isGuaranteedNotToBeUndefOrPoison(NewOp, SQ.AC, &Sel, &DT))
// We can't do any further replacement if NewOp may be undef/poison.
if (!isGuaranteedNotToBeUndefOrPoison(NewOp, SQ.AC, &Sel, &DT))
return nullptr;

// If NewOp is a constant, replace.
if (match(NewOp, m_ImmConstant()))
return replaceOperand(Sel, Swapped ? 2 : 1, V);

// If we simplified the TrueArm -> NewOp then replace.
// This handles things like `select`/`min`/`max`/`or`/`and`/etc...
if (NewOp == V)
return replaceOperand(Sel, Swapped ? 2 : 1, V);

NewOpNeverUndef = true;
}

// If we can't simplify, but we will either:
// 1) Create a new binop where both ops are NewOp i.e (add x, y) is "worse"
// than (add y, y) in this case, wait until the second call so we don't
// miss a one-use simplification.
// 2) Create a new one-use instruction.
// proceed.
if (TrueVal->hasOneUse() && isa<Instruction>(TrueVal)) {
bool BinOpMatch =
match(TrueVal, m_c_BinOp(m_Specific(OldOp), m_Specific(NewOp)));
bool NewOneUse = isa<Instruction>(OldOp) && OldOp->hasNUses(2) &&
(!isa<Instruction>(NewOp) || !NewOp->hasOneUse());
if (BinOpMatch || NewOneUse) {
bool Replaced = false;
auto *TrueIns = cast<Instruction>(TrueVal);
for (unsigned OpIdx = 0; OpIdx < TrueIns->getNumOperands(); ++OpIdx) {
if (TrueIns->getOperand(OpIdx) != OldOp)
continue;
// Need to ensure NewOp is noundef (same reason as above). Wait
// until the last moment to do this check as it can be relatively
// expensive.
if (!NewOpNeverUndef &&
!isGuaranteedNotToBeUndefOrPoison(NewOp, SQ.AC, &Sel, &DT))
break;
NewOpNeverUndef = true;
TrueIns->setOperand(OpIdx, NewOp);
Replaced = true;
}
if (Replaced)
return replaceOperand(Sel, Swapped ? 2 : 1, TrueIns);
}
}

// Even if TrueVal does not simplify, we can directly replace a use of
Expand Down
16 changes: 7 additions & 9 deletions llvm/test/Transforms/InstCombine/select-cmp-eq-op-fold.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ declare void @use.i8(i8)
define i8 @replace_with_y_noundef(i8 %x, i8 noundef %y, i8 %z) {
; CHECK-LABEL: @replace_with_y_noundef(
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X:%.*]], [[Y:%.*]]
; CHECK-NEXT: [[AND:%.*]] = and i8 [[X]], [[Y]]
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[AND]], i8 [[Z:%.*]]
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[Y]], i8 [[Z:%.*]]
; CHECK-NEXT: ret i8 [[SEL]]
;
%cmp = icmp eq i8 %x, %y
Expand All @@ -20,8 +19,7 @@ define i8 @replace_with_x_noundef(i8 noundef %x, i8 %y, i8 %z) {
; CHECK-LABEL: @replace_with_x_noundef(
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i8 [[X:%.*]], [[Y:%.*]]
; CHECK-NEXT: call void @use.i1(i1 [[CMP]])
; CHECK-NEXT: [[AND:%.*]] = or i8 [[X]], [[Y]]
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[Z:%.*]], i8 [[AND]]
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[Z:%.*]], i8 [[X]]
; CHECK-NEXT: ret i8 [[SEL]]
;
%cmp = icmp ne i8 %x, %y
Expand Down Expand Up @@ -50,7 +48,7 @@ define i8 @replace_with_y_for_new_oneuse(i8 noundef %xx, i8 noundef %y, i8 %z) {
; CHECK-LABEL: @replace_with_y_for_new_oneuse(
; CHECK-NEXT: [[X:%.*]] = mul i8 [[XX:%.*]], 13
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X]], [[Y:%.*]]
; CHECK-NEXT: [[ADD:%.*]] = add nuw i8 [[X]], [[Y]]
; CHECK-NEXT: [[ADD:%.*]] = shl nuw i8 [[Y]], 1
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[ADD]], i8 [[Z:%.*]]
; CHECK-NEXT: ret i8 [[SEL]]
;
Expand All @@ -65,7 +63,7 @@ define i8 @replace_with_y_for_new_oneuse2(i8 %xx, i8 noundef %y, i8 %z, i8 %q) {
; CHECK-LABEL: @replace_with_y_for_new_oneuse2(
; CHECK-NEXT: [[X:%.*]] = mul i8 [[XX:%.*]], 13
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X]], [[Y:%.*]]
; CHECK-NEXT: [[ADD:%.*]] = add nuw i8 [[X]], [[Q:%.*]]
; CHECK-NEXT: [[ADD:%.*]] = add nuw i8 [[Y]], [[Q:%.*]]
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[ADD]], i8 [[Z:%.*]]
; CHECK-NEXT: ret i8 [[SEL]]
;
Expand All @@ -81,7 +79,7 @@ define i8 @replace_with_x_for_new_oneuse(i8 noundef %xx, i8 noundef %yy, i8 %z,
; CHECK-NEXT: [[X:%.*]] = mul i8 [[XX:%.*]], 13
; CHECK-NEXT: [[Y:%.*]] = add i8 [[YY:%.*]], [[W:%.*]]
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X]], [[Y]]
; CHECK-NEXT: [[MUL:%.*]] = mul i8 [[X]], [[Y]]
; CHECK-NEXT: [[MUL:%.*]] = mul i8 [[X]], [[X]]
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[MUL]], i8 [[Z:%.*]]
; CHECK-NEXT: ret i8 [[SEL]]
;
Expand Down Expand Up @@ -115,7 +113,7 @@ define i8 @replace_with_x_for_simple_binop(i8 noundef %xx, i8 %yy, i8 %z, i8 %w)
; CHECK-NEXT: [[X:%.*]] = mul i8 [[XX:%.*]], 13
; CHECK-NEXT: [[Y:%.*]] = add i8 [[YY:%.*]], [[W:%.*]]
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X]], [[Y]]
; CHECK-NEXT: [[MUL:%.*]] = mul i8 [[X]], [[Y]]
; CHECK-NEXT: [[MUL:%.*]] = mul i8 [[X]], [[X]]
; CHECK-NEXT: call void @use.i8(i8 [[Y]])
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[MUL]], i8 [[Z:%.*]]
; CHECK-NEXT: ret i8 [[SEL]]
Expand Down Expand Up @@ -147,7 +145,7 @@ define i8 @replace_with_none_for_new_oneuse_fail_maybe_undef(i8 %xx, i8 %y, i8 %
define i8 @replace_with_y_for_simple_binop(i8 %x, i8 noundef %y, i8 %z) {
; CHECK-LABEL: @replace_with_y_for_simple_binop(
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X:%.*]], [[Y:%.*]]
; CHECK-NEXT: [[MUL:%.*]] = mul nsw i8 [[X]], [[Y]]
; CHECK-NEXT: [[MUL:%.*]] = mul nsw i8 [[Y]], [[Y]]
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[MUL]], i8 [[Z:%.*]]
; CHECK-NEXT: ret i8 [[SEL]]
;
Expand Down

0 comments on commit e3d8065

Please sign in to comment.