[llvm] 7e7c29b - [InstCombine] Improve coverage of `foldSelectValueEquivalence` for constants
Noah Goldstein via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 6 18:03:11 PDT 2024
Author: Noah Goldstein
Date: 2024-06-06T20:02:57-05:00
New Revision: 7e7c29ba087e38056b91f1d783db0883dcc33ef7
URL: https://github.com/llvm/llvm-project/commit/7e7c29ba087e38056b91f1d783db0883dcc33ef7
DIFF: https://github.com/llvm/llvm-project/commit/7e7c29ba087e38056b91f1d783db0883dcc33ef7.diff
LOG: [InstCombine] Improve coverage of `foldSelectValueEquivalence` for constants
We don't need the `noundef` check if the new simplification is a
constant.
This cleans up regressions from folding multiuse:
`(icmp eq/ne (sub/xor x, y), 0)` -> `(icmp eq/ne x, y)`.
Closes #88298
Added:
Modified:
llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
llvm/test/Transforms/InstCombine/abs-1.ll
llvm/test/Transforms/InstCombine/select.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index d2aaa5e230545..b04e0b300f95a 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -1288,21 +1288,36 @@ 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
diff erent 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
diff erent 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
@@ -1310,16 +1325,18 @@ Instruction *InstCombinerImpl::foldSelectValueEquivalence(SelectInst &Sel,
// 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)
diff --git a/llvm/test/Transforms/InstCombine/abs-1.ll b/llvm/test/Transforms/InstCombine/abs-1.ll
index 32bd7a37053ed..0cf7cd97d8ff4 100644
--- a/llvm/test/Transforms/InstCombine/abs-1.ll
+++ b/llvm/test/Transforms/InstCombine/abs-1.ll
@@ -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
diff --git a/llvm/test/Transforms/InstCombine/select.ll b/llvm/test/Transforms/InstCombine/select.ll
index 5cc4ee360db9f..846ede45028e2 100644
--- a/llvm/test/Transforms/InstCombine/select.ll
+++ b/llvm/test/Transforms/InstCombine/select.ll
@@ -2838,8 +2838,7 @@ define <2 x i8> @select_replacement_add_eq_vec_undef_okay_todo(<2 x i8> %x, <2 x
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: [[ADD:%.*]] = xor <2 x i8> [[X]], [[Y]]
-; CHECK-NEXT: [[SEL:%.*]] = select <2 x i1> [[CMP]], <2 x i8> [[ADD]], <2 x i8> [[Z:%.*]]
+; 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
@@ -2905,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
More information about the llvm-commits
mailing list