From edbc0df974cebd05a76f17023c7e7c057f709441 Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Fri, 5 Jun 2015 09:57:57 +0000 Subject: [PATCH] [InstCombine] Rephrase fix to SimplifyWithOpReplaced I don't have the IR which is causing the build bot breakage but I can postulate as to why they are timing out: 1. SimplifyWithOpReplaced was stripping flags from the simplified value. 2. visitSelectInstWithICmp was overriding SimplifyWithOpReplaced because it's simplification wasn't correct. 3. InstCombine would revisit the add instruction and note that it can rederive the flags. 4. By modifying the value, we chose to revisit instructions which reuse the value. One of the instructions is the original select, causing LLVM to never reach fixpoint. Instead, strip the flags only when we are sure we are going to perform the simplification. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@239141 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../InstCombine/InstCombineSelect.cpp | 26 ++++++++++++++++--- test/Transforms/InstCombine/select.ll | 10 +++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/lib/Transforms/InstCombine/InstCombineSelect.cpp b/lib/Transforms/InstCombine/InstCombineSelect.cpp index d2fbcdd3991..68ddb27ec4a 100644 --- a/lib/Transforms/InstCombine/InstCombineSelect.cpp +++ b/lib/Transforms/InstCombine/InstCombineSelect.cpp @@ -598,6 +598,24 @@ Instruction *InstCombiner::visitSelectInstWithICmp(SelectInst &SI, } } + // Consider: + // %cmp = icmp eq i32 %x, 2147483647 + // %add = add nsw i32 %x, 1 + // %sel = select i1 %cmp, i32 -2147483648, i32 %add + // + // We can't replace %sel with %add unless we strip away the flags. + auto StripBinOpFlags = [](Value *V) { + if (auto *B = dyn_cast(V)) { + if (isa(B)) { + B->setHasNoSignedWrap(false); + B->setHasNoUnsignedWrap(false); + } + if (isa(B)) + B->setIsExact(false); + } + return V; + }; + // If we have an equality comparison then we know the value in one of the // arms of the select. See if substituting this value into the arm and // simplifying the result yields the same value as the other arm. @@ -606,23 +624,23 @@ Instruction *InstCombiner::visitSelectInstWithICmp(SelectInst &SI, TrueVal || SimplifyWithOpReplaced(FalseVal, CmpRHS, CmpLHS, TLI, DL, DT, AC) == TrueVal) - return ReplaceInstUsesWith(SI, FalseVal); + return ReplaceInstUsesWith(SI, StripBinOpFlags(FalseVal)); if (SimplifyWithOpReplaced(TrueVal, CmpLHS, CmpRHS, TLI, DL, DT, AC) == FalseVal || SimplifyWithOpReplaced(TrueVal, CmpRHS, CmpLHS, TLI, DL, DT, AC) == FalseVal) - return ReplaceInstUsesWith(SI, FalseVal); + return ReplaceInstUsesWith(SI, StripBinOpFlags(FalseVal)); } else if (Pred == ICmpInst::ICMP_NE) { if (SimplifyWithOpReplaced(TrueVal, CmpLHS, CmpRHS, TLI, DL, DT, AC) == FalseVal || SimplifyWithOpReplaced(TrueVal, CmpRHS, CmpLHS, TLI, DL, DT, AC) == FalseVal) - return ReplaceInstUsesWith(SI, TrueVal); + return ReplaceInstUsesWith(SI, StripBinOpFlags(TrueVal)); if (SimplifyWithOpReplaced(FalseVal, CmpLHS, CmpRHS, TLI, DL, DT, AC) == TrueVal || SimplifyWithOpReplaced(FalseVal, CmpRHS, CmpLHS, TLI, DL, DT, AC) == TrueVal) - return ReplaceInstUsesWith(SI, TrueVal); + return ReplaceInstUsesWith(SI, StripBinOpFlags(TrueVal)); } // NOTE: if we wanted to, this is where to detect integer MIN/MAX diff --git a/test/Transforms/InstCombine/select.ll b/test/Transforms/InstCombine/select.ll index e4bc96cff17..b05f2ca2c4c 100644 --- a/test/Transforms/InstCombine/select.ll +++ b/test/Transforms/InstCombine/select.ll @@ -1532,3 +1532,13 @@ define i32 @test_max_of_min(i32 %a) { %s1 = select i1 %c1, i32 %s0, i32 -1 ret i32 %s1 } + +define i32 @PR23757(i32 %x) { +; CHECK-LABEL: @PR23757 +; CHECK: %[[add:.*]] = add i32 %x, 1 +; CHECK-NEXT: ret i32 %[[add]] + %cmp = icmp eq i32 %x, 2147483647 + %add = add nsw i32 %x, 1 + %sel = select i1 %cmp, i32 -2147483648, i32 %add + ret i32 %sel +} -- 2.34.1