Skip to content
This repository was archived by the owner on Apr 23, 2020. It is now read-only.

Commit edbc0df

Browse files
committed
[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
1 parent 2a89c94 commit edbc0df

File tree

2 files changed

+32
-4
lines changed

2 files changed

+32
-4
lines changed

lib/Transforms/InstCombine/InstCombineSelect.cpp

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,24 @@ Instruction *InstCombiner::visitSelectInstWithICmp(SelectInst &SI,
598598
}
599599
}
600600

601+
// Consider:
602+
// %cmp = icmp eq i32 %x, 2147483647
603+
// %add = add nsw i32 %x, 1
604+
// %sel = select i1 %cmp, i32 -2147483648, i32 %add
605+
//
606+
// We can't replace %sel with %add unless we strip away the flags.
607+
auto StripBinOpFlags = [](Value *V) {
608+
if (auto *B = dyn_cast<BinaryOperator>(V)) {
609+
if (isa<OverflowingBinaryOperator>(B)) {
610+
B->setHasNoSignedWrap(false);
611+
B->setHasNoUnsignedWrap(false);
612+
}
613+
if (isa<PossiblyExactOperator>(B))
614+
B->setIsExact(false);
615+
}
616+
return V;
617+
};
618+
601619
// If we have an equality comparison then we know the value in one of the
602620
// arms of the select. See if substituting this value into the arm and
603621
// simplifying the result yields the same value as the other arm.
@@ -606,23 +624,23 @@ Instruction *InstCombiner::visitSelectInstWithICmp(SelectInst &SI,
606624
TrueVal ||
607625
SimplifyWithOpReplaced(FalseVal, CmpRHS, CmpLHS, TLI, DL, DT, AC) ==
608626
TrueVal)
609-
return ReplaceInstUsesWith(SI, FalseVal);
627+
return ReplaceInstUsesWith(SI, StripBinOpFlags(FalseVal));
610628
if (SimplifyWithOpReplaced(TrueVal, CmpLHS, CmpRHS, TLI, DL, DT, AC) ==
611629
FalseVal ||
612630
SimplifyWithOpReplaced(TrueVal, CmpRHS, CmpLHS, TLI, DL, DT, AC) ==
613631
FalseVal)
614-
return ReplaceInstUsesWith(SI, FalseVal);
632+
return ReplaceInstUsesWith(SI, StripBinOpFlags(FalseVal));
615633
} else if (Pred == ICmpInst::ICMP_NE) {
616634
if (SimplifyWithOpReplaced(TrueVal, CmpLHS, CmpRHS, TLI, DL, DT, AC) ==
617635
FalseVal ||
618636
SimplifyWithOpReplaced(TrueVal, CmpRHS, CmpLHS, TLI, DL, DT, AC) ==
619637
FalseVal)
620-
return ReplaceInstUsesWith(SI, TrueVal);
638+
return ReplaceInstUsesWith(SI, StripBinOpFlags(TrueVal));
621639
if (SimplifyWithOpReplaced(FalseVal, CmpLHS, CmpRHS, TLI, DL, DT, AC) ==
622640
TrueVal ||
623641
SimplifyWithOpReplaced(FalseVal, CmpRHS, CmpLHS, TLI, DL, DT, AC) ==
624642
TrueVal)
625-
return ReplaceInstUsesWith(SI, TrueVal);
643+
return ReplaceInstUsesWith(SI, StripBinOpFlags(TrueVal));
626644
}
627645

628646
// NOTE: if we wanted to, this is where to detect integer MIN/MAX

test/Transforms/InstCombine/select.ll

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1532,3 +1532,13 @@ define i32 @test_max_of_min(i32 %a) {
15321532
%s1 = select i1 %c1, i32 %s0, i32 -1
15331533
ret i32 %s1
15341534
}
1535+
1536+
define i32 @PR23757(i32 %x) {
1537+
; CHECK-LABEL: @PR23757
1538+
; CHECK: %[[add:.*]] = add i32 %x, 1
1539+
; CHECK-NEXT: ret i32 %[[add]]
1540+
%cmp = icmp eq i32 %x, 2147483647
1541+
%add = add nsw i32 %x, 1
1542+
%sel = select i1 %cmp, i32 -2147483648, i32 %add
1543+
ret i32 %sel
1544+
}

0 commit comments

Comments
 (0)