Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[InstCombine] Improve coverage of foldSelectValueEquivalence #88298

Closed
wants to merge 2 commits into from
+83 −31
Closed

[InstCombine] Improve coverage of foldSelectValueEquivalence #88298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
689dce0
[InstCombine] Add tests for expanding `foldSelectValueEquivalence`; NFC
goldsteinn Apr 10, 2024
a422148
[InstCombine] Improve coverage of `foldSelectValueEquivalence` for co…
goldsteinn Jun 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 38 additions & 21 deletions llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1288,38 +1288,55 @@ Instruction *InstCombinerImpl::foldSelectValueEquivalence(SelectInst &Sel,
Swapped = true;
}

// In X == Y ? f(X) : Z, try to evaluate f(Y) and replace the operand.
// Make sure Y cannot be undef though, as we might pick different values for
// undef in the icmp and in f(Y). Additionally, take care to avoid replacing
// X == Y ? X : Z with X == Y ? Y : Z, as that would lead to an infinite
// replacement cycle.
Value *CmpLHS = Cmp.getOperand(0), *CmpRHS = Cmp.getOperand(1);
if (TrueVal != CmpLHS && isGuaranteedNotToBeUndef(CmpRHS, SQ.AC, &Sel, &DT)) {
if (Value *V = simplifyWithOpReplaced(TrueVal, CmpLHS, CmpRHS, SQ,
/* AllowRefinement */ true))
// Require either the replacement or the simplification result to be a
// constant to avoid infinite loops.
// FIXME: Make this check more precise.
if (isa<Constant>(CmpRHS) || isa<Constant>(V))
auto ReplaceOldOpWithNewOp = [&](Value *OldOp,
Value *NewOp) -> Instruction * {
// In X == Y ? f(X) : Z, try to evaluate f(Y) and replace the operand.
// Take care to avoid replacing X == Y ? X : Z with X == Y ? Y : Z, as that
// would lead to an infinite replacement cycle.
// If we will be able to evaluate f(Y) to a constant, we can allow undef,
// otherwise Y cannot be undef as we might pick different values for undef
// in the icmp and in f(Y).
if (TrueVal == OldOp)
return nullptr;

if (Value *V = simplifyWithOpReplaced(TrueVal, OldOp, NewOp, SQ,
/* AllowRefinement=*/true)) {
// Need some guarantees about the new simplified op to ensure we don't inf
// loop.
// If we simplify to a constant, replace if we aren't creating new undef.
if (match(V, m_ImmConstant()) &&
isGuaranteedNotToBeUndef(V, SQ.AC, &Sel, &DT))
return replaceOperand(Sel, Swapped ? 2 : 1, V);

// If NewOp is a constant and OldOp is not replace iff NewOp doesn't
// contain and undef elements.
if (match(NewOp, m_ImmConstant())) {
if (isGuaranteedNotToBeUndef(NewOp, SQ.AC, &Sel, &DT))
return replaceOperand(Sel, Swapped ? 2 : 1, V);
return nullptr;
}
}

// Even if TrueVal does not simplify, we can directly replace a use of
// CmpLHS with CmpRHS, as long as the instruction is not used anywhere
// else and is safe to speculatively execute (we may end up executing it
// with different operands, which should not cause side-effects or trigger
// undefined behavior). Only do this if CmpRHS is a constant, as
// profitability is not clear for other cases.
// FIXME: Support vectors.
if (match(CmpRHS, m_ImmConstant()) && !match(CmpLHS, m_ImmConstant()) &&
!Cmp.getType()->isVectorTy())
if (replaceInInstruction(TrueVal, CmpLHS, CmpRHS))
if (OldOp == CmpLHS && match(NewOp, m_ImmConstant()) &&
!match(OldOp, m_ImmConstant()) && !Cmp.getType()->isVectorTy() &&
isGuaranteedNotToBeUndef(NewOp, SQ.AC, &Sel, &DT))
if (replaceInInstruction(TrueVal, OldOp, NewOp))
return &Sel;
}
if (TrueVal != CmpRHS && isGuaranteedNotToBeUndef(CmpLHS, SQ.AC, &Sel, &DT))
if (Value *V = simplifyWithOpReplaced(TrueVal, CmpRHS, CmpLHS, SQ,
/* AllowRefinement */ true))
if (isa<Constant>(CmpLHS) || isa<Constant>(V))
return replaceOperand(Sel, Swapped ? 2 : 1, V);
return nullptr;
};

if (Instruction *R = ReplaceOldOpWithNewOp(CmpLHS, CmpRHS))
return R;
if (Instruction *R = ReplaceOldOpWithNewOp(CmpRHS, CmpLHS))
return R;

auto *FalseInst = dyn_cast<Instruction>(FalseVal);
if (!FalseInst)
Expand Down
7 changes: 2 additions & 5 deletions llvm/test/Transforms/InstCombine/abs-1.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -852,11 +852,8 @@ define i8 @abs_diff_signed_sgt_nuw_extra_use3(i8 %a, i8 %b) {

define i32 @abs_diff_signed_slt_swap_wrong_pred1(i32 %a, i32 %b) {
; CHECK-LABEL: @abs_diff_signed_slt_swap_wrong_pred1(
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[SUB_BA:%.*]] = sub nsw i32 [[B]], [[A]]
; CHECK-NEXT: [[SUB_AB:%.*]] = sub nsw i32 [[A]], [[B]]
; CHECK-NEXT: [[COND:%.*]] = select i1 [[CMP]], i32 [[SUB_BA]], i32 [[SUB_AB]]
; CHECK-NEXT: ret i32 [[COND]]
; CHECK-NEXT: [[SUB_AB:%.*]] = sub nsw i32 [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: ret i32 [[SUB_AB]]
;
%cmp = icmp eq i32 %a, %b
%sub_ba = sub nsw i32 %b, %a
Expand Down
48 changes: 43 additions & 5 deletions llvm/test/Transforms/InstCombine/select.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ define i1 @test39(i1 %cond, double %x) {
; CHECK-LABEL: @test39(
; CHECK-NEXT: ret i1 true
;
%s = select i1 %cond, double %x, double 0x7FF0000000000000 ; RHS = +infty
%s = select i1 %cond, double %x, double 0x7FF0000000000000 ; RHS = +infty
%cmp = fcmp ule double %x, %s
ret i1 %cmp
}
Expand Down Expand Up @@ -1364,7 +1364,7 @@ define i32 @PR23757_ne(i32 %x, ptr %p) {
; CHECK-NEXT: ret i32 -2147483648
;
%cmp = icmp ne i32 %x, 2147483647
store i1 %cmp, ptr %p ; thwart predicate canonicalization
store i1 %cmp, ptr %p ; thwart predicate canonicalization
%add = add nsw i32 %x, 1
%sel = select i1 %cmp, i32 -2147483648, i32 %add
ret i32 %sel
Expand All @@ -1378,7 +1378,7 @@ define i32 @PR23757_ne_swapped(i32 %x, ptr %p) {
; CHECK-NEXT: ret i32 [[ADD]]
;
%cmp = icmp ne i32 %x, 2147483647
store i1 %cmp, ptr %p ; thwart predicate canonicalization
store i1 %cmp, ptr %p ; thwart predicate canonicalization
%add = add nsw i32 %x, 1
%sel = select i1 %cmp, i32 %add, i32 -2147483648
ret i32 %sel
Expand Down Expand Up @@ -2809,6 +2809,45 @@ define <2 x i8> @select_replacement_add_eq_vec_undef(<2 x i8> %x, <2 x i8> %y) {
ret <2 x i8> %sel
}

define <2 x i8> @select_replacement_add_eq_vec_undef_okay(<2 x i8> %x, <2 x i8> %y) {
; CHECK-LABEL: @select_replacement_add_eq_vec_undef_okay(
; CHECK-NEXT: [[CMP:%.*]] = icmp eq <2 x i8> [[X:%.*]], <i8 1, i8 1>
; CHECK-NEXT: [[SEL:%.*]] = select <2 x i1> [[CMP]], <2 x i8> <i8 2, i8 undef>, <2 x i8> [[Y:%.*]]
; CHECK-NEXT: ret <2 x i8> [[SEL]]
;
%cmp = icmp eq <2 x i8> %x, <i8 1, i8 1>
%add = add <2 x i8> %x, <i8 1, i8 undef>
%sel = select <2 x i1> %cmp, <2 x i8> %add, <2 x i8> %y
ret <2 x i8> %sel
}


define <2 x i8> @select_replacement_add_eq_vec_undef_okay_todo(<2 x i8> %x, <2 x i8> %y) {
; CHECK-LABEL: @select_replacement_add_eq_vec_undef_okay_todo(
; CHECK-NEXT: [[CMP:%.*]] = icmp eq <2 x i8> [[X:%.*]], <i8 1, i8 undef>
; CHECK-NEXT: [[ADD:%.*]] = add <2 x i8> [[X]], <i8 1, i8 undef>
; CHECK-NEXT: [[SEL:%.*]] = select <2 x i1> [[CMP]], <2 x i8> [[ADD]], <2 x i8> [[Y:%.*]]
; CHECK-NEXT: ret <2 x i8> [[SEL]]
;
%cmp = icmp eq <2 x i8> %x, <i8 1, i8 undef>
%add = add <2 x i8> %x, <i8 1, i8 undef>
%sel = select <2 x i1> %cmp, <2 x i8> %add, <2 x i8> %y
ret <2 x i8> %sel
}

define <2 x i8> @select_replacement_xor_eq_vec(<2 x i8> %x, <2 x i8> %y, <2 x i8> %z) {
; CHECK-LABEL: @select_replacement_xor_eq_vec(
; CHECK-NEXT: [[CMP:%.*]] = icmp eq <2 x i8> [[X:%.*]], [[Y:%.*]]
; CHECK-NEXT: [[SEL:%.*]] = select <2 x i1> [[CMP]], <2 x i8> zeroinitializer, <2 x i8> [[Z:%.*]]
; CHECK-NEXT: ret <2 x i8> [[SEL]]
;
%cmp = icmp eq <2 x i8> %x, %y
%add = xor <2 x i8> %x, %y
%sel = select <2 x i1> %cmp, <2 x i8> %add, <2 x i8> %z
ret <2 x i8> %sel
}


define i8 @select_replacement_add_ne(i8 %x, i8 %y) {
; CHECK-LABEL: @select_replacement_add_ne(
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i8 [[X:%.*]], 1
Expand Down Expand Up @@ -2865,8 +2904,7 @@ define i8 @select_replacement_sub_noundef_but_may_be_poison(i8 %x, i8 noundef %y
define i8 @select_replacement_sub(i8 %x, i8 %y, i8 %z) {
; CHECK-LABEL: @select_replacement_sub(
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X:%.*]], [[Y:%.*]]
; CHECK-NEXT: [[SUB:%.*]] = sub i8 [[X]], [[Y]]
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[SUB]], i8 [[Z:%.*]]
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 0, i8 [[Z:%.*]]
; CHECK-NEXT: ret i8 [[SEL]]
;
%cmp = icmp eq i8 %x, %y
Expand Down
Loading