[llvm] b768546 - Revert "[InstCombine] Simplify select operand based on equality condition"
Benjamin Kramer via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 15 03:26:12 PDT 2020
Author: Benjamin Kramer
Date: 2020-09-15T12:22:47+02:00
New Revision: b768546fe0cc1d320857a6e080d4c796efb0c00c
URL: https://github.com/llvm/llvm-project/commit/b768546fe0cc1d320857a6e080d4c796efb0c00c
DIFF: https://github.com/llvm/llvm-project/commit/b768546fe0cc1d320857a6e080d4c796efb0c00c.diff
LOG: Revert "[InstCombine] Simplify select operand based on equality condition"
This reverts commit cfff88c03cf9e9b72906a41fd11e06721d54f293. Sends
instcombine into an infinite loop.
```
define i1 @foo(i32 %arg, i32 %arg1) {
bb:
%tmp = udiv i32 %arg, %arg1
%tmp2 = mul nsw i32 %tmp, %arg1
%tmp3 = icmp eq i32 %tmp2, %arg
%tmp4 = select i1 %tmp3, i32 %tmp, i32 undef
%tmp5 = icmp sgt i32 %tmp4, 255
ret i1 %tmp5
}
```
Added:
Modified:
llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
llvm/test/Transforms/InstCombine/rem.ll
llvm/test/Transforms/InstCombine/select-binop-cmp.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 ce473410f4ca..378132011aba 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -1165,32 +1165,15 @@ static Instruction *canonicalizeAbsNabs(SelectInst &Sel, ICmpInst &Cmp,
///
/// We can't replace %sel with %add unless we strip away the flags.
/// TODO: Wrapping flags could be preserved in some cases with better analysis.
-static Instruction *foldSelectValueEquivalence(SelectInst &Sel, ICmpInst &Cmp,
- const SimplifyQuery &Q,
- InstCombiner &IC) {
+static Value *foldSelectValueEquivalence(SelectInst &Sel, ICmpInst &Cmp,
+ const SimplifyQuery &Q) {
if (!Cmp.isEquality())
return nullptr;
// Canonicalize the pattern to ICMP_EQ by swapping the select operands.
Value *TrueVal = Sel.getTrueValue(), *FalseVal = Sel.getFalseValue();
- bool Swapped = false;
- if (Cmp.getPredicate() == ICmpInst::ICMP_NE) {
+ if (Cmp.getPredicate() == ICmpInst::ICMP_NE)
std::swap(TrueVal, FalseVal);
- Swapped = true;
- }
-
- // In X == Y ? f(X) : Z, try to evaluate f(X) 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.
- Value *CmpLHS = Cmp.getOperand(0), *CmpRHS = Cmp.getOperand(1);
- if (TrueVal != CmpLHS)
- if (Value *V = SimplifyWithOpReplaced(TrueVal, CmpLHS, CmpRHS, Q,
- /* AllowRefinement */ true))
- return IC.replaceOperand(Sel, Swapped ? 2 : 1, V);
- if (TrueVal != CmpRHS)
- if (Value *V = SimplifyWithOpReplaced(TrueVal, CmpRHS, CmpLHS, Q,
- /* AllowRefinement */ true))
- return IC.replaceOperand(Sel, Swapped ? 2 : 1, V);
auto *FalseInst = dyn_cast<Instruction>(FalseVal);
if (!FalseInst)
@@ -1215,11 +1198,12 @@ static Instruction *foldSelectValueEquivalence(SelectInst &Sel, ICmpInst &Cmp,
// We have an 'EQ' comparison, so the select's false value will propagate.
// Example:
// (X == 42) ? 43 : (X + 1) --> (X == 42) ? (X + 1) : (X + 1) --> X + 1
+ Value *CmpLHS = Cmp.getOperand(0), *CmpRHS = Cmp.getOperand(1);
if (SimplifyWithOpReplaced(FalseVal, CmpLHS, CmpRHS, Q,
/* AllowRefinement */ false) == TrueVal ||
SimplifyWithOpReplaced(FalseVal, CmpRHS, CmpLHS, Q,
/* AllowRefinement */ false) == TrueVal) {
- return IC.replaceInstUsesWith(Sel, FalseVal);
+ return FalseVal;
}
// Restore poison-generating flags if the transform did not apply.
@@ -1455,8 +1439,8 @@ tryToReuseConstantFromSelectInComparison(SelectInst &Sel, ICmpInst &Cmp,
/// Visit a SelectInst that has an ICmpInst as its first operand.
Instruction *InstCombinerImpl::foldSelectInstWithICmp(SelectInst &SI,
ICmpInst *ICI) {
- if (Instruction *NewSel = foldSelectValueEquivalence(SI, *ICI, SQ, *this))
- return NewSel;
+ if (Value *V = foldSelectValueEquivalence(SI, *ICI, SQ))
+ return replaceInstUsesWith(SI, V);
if (Instruction *NewSel = canonicalizeMinMaxWithConstant(SI, *ICI, *this))
return NewSel;
diff --git a/llvm/test/Transforms/InstCombine/rem.ll b/llvm/test/Transforms/InstCombine/rem.ll
index 37d81f2ebf6a..2b9f5326dd15 100644
--- a/llvm/test/Transforms/InstCombine/rem.ll
+++ b/llvm/test/Transforms/InstCombine/rem.ll
@@ -50,7 +50,8 @@ define i8 @big_divisor(i8 %x) {
define i5 @biggest_divisor(i5 %x) {
; CHECK-LABEL: @biggest_divisor(
; CHECK-NEXT: [[DOTNOT:%.*]] = icmp eq i5 [[X:%.*]], -1
-; CHECK-NEXT: [[REM:%.*]] = select i1 [[DOTNOT]], i5 0, i5 [[X]]
+; CHECK-NEXT: [[TMP1:%.*]] = zext i1 [[DOTNOT]] to i5
+; CHECK-NEXT: [[REM:%.*]] = add i5 [[TMP1]], [[X]]
; CHECK-NEXT: ret i5 [[REM]]
;
%rem = urem i5 %x, -1
diff --git a/llvm/test/Transforms/InstCombine/select-binop-cmp.ll b/llvm/test/Transforms/InstCombine/select-binop-cmp.ll
index aa450f8af8b7..4173c31b2acb 100644
--- a/llvm/test/Transforms/InstCombine/select-binop-cmp.ll
+++ b/llvm/test/Transforms/InstCombine/select-binop-cmp.ll
@@ -564,10 +564,12 @@ define <2 x i8> @select_xor_icmp_vec_bad(<2 x i8> %x, <2 x i8> %y, <2 x i8> %z)
ret <2 x i8> %C
}
-define <2 x i8> @select_xor_icmp_vec_undef(<2 x i8> %x, <2 x i8> %y, <2 x i8> %z) {
-; CHECK-LABEL: @select_xor_icmp_vec_undef(
+; TODO: support for undefs, check for an identity constant does not handle them yet
+define <2 x i8> @select_xor_icmp_vec_bad_2(<2 x i8> %x, <2 x i8> %y, <2 x i8> %z) {
+; CHECK-LABEL: @select_xor_icmp_vec_bad_2(
; CHECK-NEXT: [[A:%.*]] = icmp eq <2 x i8> [[X:%.*]], <i8 0, i8 undef>
-; CHECK-NEXT: [[C:%.*]] = select <2 x i1> [[A]], <2 x i8> [[Z:%.*]], <2 x i8> [[Y:%.*]]
+; CHECK-NEXT: [[B:%.*]] = xor <2 x i8> [[X]], [[Z:%.*]]
+; CHECK-NEXT: [[C:%.*]] = select <2 x i1> [[A]], <2 x i8> [[B]], <2 x i8> [[Y:%.*]]
; CHECK-NEXT: ret <2 x i8> [[C]]
;
%A = icmp eq <2 x i8> %x, <i8 0, i8 undef>
@@ -602,10 +604,11 @@ define i32 @select_add_icmp_bad(i32 %x, i32 %y, i32 %z) {
ret i32 %C
}
-define i32 @select_and_icmp_zero(i32 %x, i32 %y, i32 %z) {
-; CHECK-LABEL: @select_and_icmp_zero(
+define i32 @select_and_icmp_bad(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: @select_and_icmp_bad(
; CHECK-NEXT: [[A:%.*]] = icmp eq i32 [[X:%.*]], 0
-; CHECK-NEXT: [[C:%.*]] = select i1 [[A]], i32 0, i32 [[Y:%.*]]
+; CHECK-NEXT: [[B:%.*]] = and i32 [[X]], [[Z:%.*]]
+; CHECK-NEXT: [[C:%.*]] = select i1 [[A]], i32 [[B]], i32 [[Y:%.*]]
; CHECK-NEXT: ret i32 [[C]]
;
%A = icmp eq i32 %x, 0
diff --git a/llvm/test/Transforms/InstCombine/select.ll b/llvm/test/Transforms/InstCombine/select.ll
index c4c282e9cacf..d9a4f4bdbd47 100644
--- a/llvm/test/Transforms/InstCombine/select.ll
+++ b/llvm/test/Transforms/InstCombine/select.ll
@@ -2606,7 +2606,8 @@ define i32 @pr47322_more_poisonous_replacement(i32 %arg) {
define i8 @select_replacement_add_eq(i8 %x, i8 %y) {
; CHECK-LABEL: @select_replacement_add_eq(
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X:%.*]], 1
-; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 2, i8 [[Y:%.*]]
+; CHECK-NEXT: [[ADD:%.*]] = add i8 [[X]], 1
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[ADD]], i8 [[Y:%.*]]
; CHECK-NEXT: ret i8 [[SEL]]
;
%cmp = icmp eq i8 %x, 1
@@ -2619,7 +2620,8 @@ define i8 @select_replacement_add_ne(i8 %x, i8 %y) {
; CHECK-LABEL: @select_replacement_add_ne(
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i8 [[X:%.*]], 1
; CHECK-NEXT: call void @use(i1 [[CMP]])
-; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[Y:%.*]], i8 2
+; CHECK-NEXT: [[ADD:%.*]] = add i8 [[X]], 1
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[Y:%.*]], i8 [[ADD]]
; CHECK-NEXT: ret i8 [[SEL]]
;
%cmp = icmp ne i8 %x, 1
@@ -2632,7 +2634,8 @@ define i8 @select_replacement_add_ne(i8 %x, i8 %y) {
define i8 @select_replacement_add_nuw(i8 %x, i8 %y) {
; CHECK-LABEL: @select_replacement_add_nuw(
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X:%.*]], 1
-; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 2, i8 [[Y:%.*]]
+; CHECK-NEXT: [[ADD:%.*]] = add nuw i8 [[X]], 1
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[ADD]], i8 [[Y:%.*]]
; CHECK-NEXT: ret i8 [[SEL]]
;
%cmp = icmp eq i8 %x, 1
@@ -2644,7 +2647,8 @@ define i8 @select_replacement_add_nuw(i8 %x, i8 %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: [[SEL:%.*]] = select i1 [[CMP]], i8 0, i8 [[Z:%.*]]
+; CHECK-NEXT: [[SUB:%.*]] = sub i8 [[X]], [[Y]]
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[SUB]], i8 [[Z:%.*]]
; CHECK-NEXT: ret i8 [[SEL]]
;
%cmp = icmp eq i8 %x, %y
@@ -2657,7 +2661,8 @@ define i8 @select_replacement_shift(i8 %x, i8 %y, i8 %z) {
; CHECK-LABEL: @select_replacement_shift(
; CHECK-NEXT: [[SHR:%.*]] = lshr exact i8 [[X:%.*]], 1
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[SHR]], [[Y:%.*]]
-; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[X]], i8 [[Z:%.*]]
+; CHECK-NEXT: [[SHL:%.*]] = shl i8 [[Y]], 1
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[SHL]], i8 [[Z:%.*]]
; CHECK-NEXT: ret i8 [[SEL]]
;
%shr = lshr exact i8 %x, 1
More information about the llvm-commits
mailing list