From cfff88c03cf9e9b72906a41fd11e06721d54f293 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 10 Sep 2020 18:45:53 +0200 Subject: [PATCH] [InstCombine] Simplify select operand based on equality condition For selects of the type X == Y ? A : B, check if we can simplify A by using the X == Y equality and replace the operand if that's possible. We already try to do this in InstSimplify, but will only fold if the result of the simplification is the same as B, in which case the select can be dropped entirely. Here the select will be retained, just one operand simplified. As we are performing an actual replacement here, we don't have problems with refinement / poison values. Differential Revision: https://reviews.llvm.org/D87480 --- .../InstCombine/InstCombineSelect.cpp | 30 ++++++++++++++----- llvm/test/Transforms/InstCombine/rem.ll | 3 +- .../InstCombine/select-binop-cmp.ll | 15 ++++------ llvm/test/Transforms/InstCombine/select.ll | 15 ++++------ 4 files changed, 35 insertions(+), 28 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp index 378132011aba..ce473410f4ca 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp @@ -1165,15 +1165,32 @@ 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 Value *foldSelectValueEquivalence(SelectInst &Sel, ICmpInst &Cmp, - const SimplifyQuery &Q) { +static Instruction *foldSelectValueEquivalence(SelectInst &Sel, ICmpInst &Cmp, + const SimplifyQuery &Q, + InstCombiner &IC) { if (!Cmp.isEquality()) return nullptr; // Canonicalize the pattern to ICMP_EQ by swapping the select operands. Value *TrueVal = Sel.getTrueValue(), *FalseVal = Sel.getFalseValue(); - if (Cmp.getPredicate() == ICmpInst::ICMP_NE) + bool Swapped = false; + 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(FalseVal); if (!FalseInst) @@ -1198,12 +1215,11 @@ static Value *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 FalseVal; + return IC.replaceInstUsesWith(Sel, FalseVal); } // Restore poison-generating flags if the transform did not apply. @@ -1439,8 +1455,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 (Value *V = foldSelectValueEquivalence(SI, *ICI, SQ)) - return replaceInstUsesWith(SI, V); + if (Instruction *NewSel = foldSelectValueEquivalence(SI, *ICI, SQ, *this)) + return NewSel; 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 2b9f5326dd15..37d81f2ebf6a 100644 --- a/llvm/test/Transforms/InstCombine/rem.ll +++ b/llvm/test/Transforms/InstCombine/rem.ll @@ -50,8 +50,7 @@ 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: [[TMP1:%.*]] = zext i1 [[DOTNOT]] to i5 -; CHECK-NEXT: [[REM:%.*]] = add i5 [[TMP1]], [[X]] +; CHECK-NEXT: [[REM:%.*]] = select i1 [[DOTNOT]], i5 0, i5 [[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 4173c31b2acb..aa450f8af8b7 100644 --- a/llvm/test/Transforms/InstCombine/select-binop-cmp.ll +++ b/llvm/test/Transforms/InstCombine/select-binop-cmp.ll @@ -564,12 +564,10 @@ 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 } -; 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( +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( ; CHECK-NEXT: [[A:%.*]] = icmp eq <2 x i8> [[X:%.*]], -; 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: [[C:%.*]] = select <2 x i1> [[A]], <2 x i8> [[Z:%.*]], <2 x i8> [[Y:%.*]] ; CHECK-NEXT: ret <2 x i8> [[C]] ; %A = icmp eq <2 x i8> %x, @@ -604,11 +602,10 @@ define i32 @select_add_icmp_bad(i32 %x, i32 %y, i32 %z) { ret i32 %C } -define i32 @select_and_icmp_bad(i32 %x, i32 %y, i32 %z) { -; CHECK-LABEL: @select_and_icmp_bad( +define i32 @select_and_icmp_zero(i32 %x, i32 %y, i32 %z) { +; CHECK-LABEL: @select_and_icmp_zero( ; CHECK-NEXT: [[A:%.*]] = icmp eq i32 [[X:%.*]], 0 -; CHECK-NEXT: [[B:%.*]] = and i32 [[X]], [[Z:%.*]] -; CHECK-NEXT: [[C:%.*]] = select i1 [[A]], i32 [[B]], i32 [[Y:%.*]] +; CHECK-NEXT: [[C:%.*]] = select i1 [[A]], i32 0, 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 d9a4f4bdbd47..c4c282e9cacf 100644 --- a/llvm/test/Transforms/InstCombine/select.ll +++ b/llvm/test/Transforms/InstCombine/select.ll @@ -2606,8 +2606,7 @@ 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: [[ADD:%.*]] = add i8 [[X]], 1 -; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[ADD]], i8 [[Y:%.*]] +; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 2, i8 [[Y:%.*]] ; CHECK-NEXT: ret i8 [[SEL]] ; %cmp = icmp eq i8 %x, 1 @@ -2620,8 +2619,7 @@ 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: [[ADD:%.*]] = add i8 [[X]], 1 -; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[Y:%.*]], i8 [[ADD]] +; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[Y:%.*]], i8 2 ; CHECK-NEXT: ret i8 [[SEL]] ; %cmp = icmp ne i8 %x, 1 @@ -2634,8 +2632,7 @@ 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: [[ADD:%.*]] = add nuw i8 [[X]], 1 -; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[ADD]], i8 [[Y:%.*]] +; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 2, i8 [[Y:%.*]] ; CHECK-NEXT: ret i8 [[SEL]] ; %cmp = icmp eq i8 %x, 1 @@ -2647,8 +2644,7 @@ 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: [[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 @@ -2661,8 +2657,7 @@ 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: [[SHL:%.*]] = shl i8 [[Y]], 1 -; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[SHL]], i8 [[Z:%.*]] +; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i8 [[X]], i8 [[Z:%.*]] ; CHECK-NEXT: ret i8 [[SEL]] ; %shr = lshr exact i8 %x, 1