<div dir="ltr">Merged to 6.0 in r323737.</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 24, 2018 at 4:20 PM, Sanjay Patel via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: spatel<br>
Date: Wed Jan 24 07:20:37 2018<br>
New Revision: 323331<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=323331&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=323331&view=rev</a><br>
Log:<br>
[ValueTracking] add recursion depth param to matchSelectPattern<br>
<br>
We're getting bug reports:<br>
<a href="https://bugs.llvm.org/show_bug.cgi?id=35807" rel="noreferrer" target="_blank">https://bugs.llvm.org/show_<wbr>bug.cgi?id=35807</a><br>
<a href="https://bugs.llvm.org/show_bug.cgi?id=35840" rel="noreferrer" target="_blank">https://bugs.llvm.org/show_<wbr>bug.cgi?id=35840</a><br>
<a href="https://bugs.llvm.org/show_bug.cgi?id=36045" rel="noreferrer" target="_blank">https://bugs.llvm.org/show_<wbr>bug.cgi?id=36045</a><br>
...where we blow up the stack in value tracking because other passes are sending<br>
in selects that have an operand that is itself the select.<br>
<br>
We don't currently have a reliable way to avoid analyzing dead code that may take<br>
non-standard forms, so bail out when things go too far.<br>
<br>
This mimics the recursion depth limitations in other parts of value tracking.<br>
<br>
Unfortunately, this pushes the underlying problems for other passes (jump-threading,<br>
simplifycfg, correlated-propagation) into hiding. If someone wants to uncover those<br>
again, the first draft of this patch on Phab would do that (it would assert rather<br>
than bail out).<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D42442" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D42442</a><br>
<br>
Added:<br>
llvm/trunk/test/Analysis/<wbr>ValueTracking/select-pattern.<wbr>ll<br>
Modified:<br>
llvm/trunk/include/llvm/<wbr>Analysis/ValueTracking.h<br>
llvm/trunk/lib/Analysis/<wbr>ValueTracking.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/<wbr>Analysis/ValueTracking.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/ValueTracking.h?rev=323331&r1=323330&r2=323331&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/include/<wbr>llvm/Analysis/ValueTracking.h?<wbr>rev=323331&r1=323330&r2=<wbr>323331&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/include/llvm/<wbr>Analysis/ValueTracking.h (original)<br>
+++ llvm/trunk/include/llvm/<wbr>Analysis/ValueTracking.h Wed Jan 24 07:20:37 2018<br>
@@ -508,7 +508,8 @@ class Value;<br>
/// -> LHS = %a, RHS = i32 4, *CastOp = Instruction::SExt<br>
///<br>
SelectPatternResult matchSelectPattern(Value *V, Value *&LHS, Value *&RHS,<br>
- Instruction::CastOps *CastOp = nullptr);<br>
+ Instruction::CastOps *CastOp = nullptr,<br>
+ unsigned Depth = 0);<br>
inline SelectPatternResult<br>
matchSelectPattern(const Value *V, const Value *&LHS, const Value *&RHS,<br>
Instruction::CastOps *CastOp = nullptr) {<br>
<br>
Modified: llvm/trunk/lib/Analysis/<wbr>ValueTracking.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=323331&r1=323330&r2=323331&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>Analysis/ValueTracking.cpp?<wbr>rev=323331&r1=323330&r2=<wbr>323331&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Analysis/<wbr>ValueTracking.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/<wbr>ValueTracking.cpp Wed Jan 24 07:20:37 2018<br>
@@ -4165,17 +4165,18 @@ static SelectPatternResult matchClamp(Cm<br>
/// a < c ? min(a,b) : min(b,c) ==> min(min(a,b),min(b,c))<br>
static SelectPatternResult matchMinMaxOfMinMax(CmpInst::<wbr>Predicate Pred,<br>
Value *CmpLHS, Value *CmpRHS,<br>
- Value *TrueVal, Value *FalseVal) {<br>
+ Value *TVal, Value *FVal,<br>
+ unsigned Depth) {<br>
// TODO: Allow FP min/max with nnan/nsz.<br>
assert(CmpInst::<wbr>isIntPredicate(Pred) && "Expected integer comparison");<br>
<br>
Value *A, *B;<br>
- SelectPatternResult L = matchSelectPattern(TrueVal, A, B);<br>
+ SelectPatternResult L = matchSelectPattern(TVal, A, B, nullptr, Depth + 1);<br>
if (!SelectPatternResult::<wbr>isMinOrMax(L.Flavor))<br>
return {SPF_UNKNOWN, SPNB_NA, false};<br>
<br>
Value *C, *D;<br>
- SelectPatternResult R = matchSelectPattern(FalseVal, C, D);<br>
+ SelectPatternResult R = matchSelectPattern(FVal, C, D, nullptr, Depth + 1);<br>
if (L.Flavor != R.Flavor)<br>
return {SPF_UNKNOWN, SPNB_NA, false};<br>
<br>
@@ -4259,7 +4260,8 @@ static SelectPatternResult matchMinMaxOf<br>
static SelectPatternResult matchMinMax(CmpInst::Predicate Pred,<br>
Value *CmpLHS, Value *CmpRHS,<br>
Value *TrueVal, Value *FalseVal,<br>
- Value *&LHS, Value *&RHS) {<br>
+ Value *&LHS, Value *&RHS,<br>
+ unsigned Depth) {<br>
// Assume success. If there's no match, callers should not use these anyway.<br>
LHS = TrueVal;<br>
RHS = FalseVal;<br>
@@ -4268,7 +4270,7 @@ static SelectPatternResult matchMinMax(C<br>
if (SPR.Flavor != SelectPatternFlavor::SPF_<wbr>UNKNOWN)<br>
return SPR;<br>
<br>
- SPR = matchMinMaxOfMinMax(Pred, CmpLHS, CmpRHS, TrueVal, FalseVal);<br>
+ SPR = matchMinMaxOfMinMax(Pred, CmpLHS, CmpRHS, TrueVal, FalseVal, Depth);<br>
if (SPR.Flavor != SelectPatternFlavor::SPF_<wbr>UNKNOWN)<br>
return SPR;<br>
<br>
@@ -4332,7 +4334,8 @@ static SelectPatternResult matchSelectPa<br>
FastMathFlags FMF,<br>
Value *CmpLHS, Value *CmpRHS,<br>
Value *TrueVal, Value *FalseVal,<br>
- Value *&LHS, Value *&RHS) {<br>
+ Value *&LHS, Value *&RHS,<br>
+ unsigned Depth) {<br>
LHS = CmpLHS;<br>
RHS = CmpRHS;<br>
<br>
@@ -4448,7 +4451,7 @@ static SelectPatternResult matchSelectPa<br>
}<br>
<br>
if (CmpInst::isIntPredicate(Pred)<wbr>)<br>
- return matchMinMax(Pred, CmpLHS, CmpRHS, TrueVal, FalseVal, LHS, RHS);<br>
+ return matchMinMax(Pred, CmpLHS, CmpRHS, TrueVal, FalseVal, LHS, RHS, Depth);<br>
<br>
// According to (IEEE 754-2008 5.3.1), minNum(0.0, -0.0) and similar<br>
// may return either -0.0 or 0.0, so fcmp/select pair has stricter<br>
@@ -4569,7 +4572,11 @@ static Value *lookThroughCast(CmpInst *C<br>
}<br>
<br>
SelectPatternResult llvm::matchSelectPattern(Value *V, Value *&LHS, Value *&RHS,<br>
- Instruction::CastOps *CastOp) {<br>
+ Instruction::CastOps *CastOp,<br>
+ unsigned Depth) {<br>
+ if (Depth >= MaxDepth)<br>
+ return {SPF_UNKNOWN, SPNB_NA, false};<br>
+<br>
SelectInst *SI = dyn_cast<SelectInst>(V);<br>
if (!SI) return {SPF_UNKNOWN, SPNB_NA, false};<br>
<br>
@@ -4598,7 +4605,7 @@ SelectPatternResult llvm::matchSelectPat<br>
FMF.setNoSignedZeros();<br>
return ::matchSelectPattern(Pred, FMF, CmpLHS, CmpRHS,<br>
cast<CastInst>(TrueVal)-><wbr>getOperand(0), C,<br>
- LHS, RHS);<br>
+ LHS, RHS, Depth);<br>
}<br>
if (Value *C = lookThroughCast(CmpI, FalseVal, TrueVal, CastOp)) {<br>
// If this is a potential fmin/fmax with a cast to integer, then ignore<br>
@@ -4607,11 +4614,11 @@ SelectPatternResult llvm::matchSelectPat<br>
FMF.setNoSignedZeros();<br>
return ::matchSelectPattern(Pred, FMF, CmpLHS, CmpRHS,<br>
C, cast<CastInst>(FalseVal)-><wbr>getOperand(0),<br>
- LHS, RHS);<br>
+ LHS, RHS, Depth);<br>
}<br>
}<br>
return ::matchSelectPattern(Pred, FMF, CmpLHS, CmpRHS, TrueVal, FalseVal,<br>
- LHS, RHS);<br>
+ LHS, RHS, Depth);<br>
}<br>
<br>
/// Return true if "icmp Pred LHS RHS" is always true.<br>
<br>
Added: llvm/trunk/test/Analysis/<wbr>ValueTracking/select-pattern.<wbr>ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ValueTracking/select-pattern.ll?rev=323331&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/<wbr>Analysis/ValueTracking/select-<wbr>pattern.ll?rev=323331&view=<wbr>auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/Analysis/<wbr>ValueTracking/select-pattern.<wbr>ll (added)<br>
+++ llvm/trunk/test/Analysis/<wbr>ValueTracking/select-pattern.<wbr>ll Wed Jan 24 07:20:37 2018<br>
@@ -0,0 +1,46 @@<br>
+; RUN: opt -simplifycfg < %s -S | FileCheck %s<br>
+<br>
+; The dead code would cause a select that had itself<br>
+; as an operand to be analyzed. This would then cause<br>
+; infinite recursion and eventual crash.<br>
+<br>
+define void @PR36045(i1 %t, i32* %b) {<br>
+; CHECK-LABEL: @PR36045(<br>
+; CHECK-NEXT: entry:<br>
+; CHECK-NEXT: ret void<br>
+;<br>
+entry:<br>
+ br i1 %t, label %if, label %end<br>
+<br>
+if:<br>
+ br i1 %t, label %unreach, label %pre<br>
+<br>
+unreach:<br>
+ unreachable<br>
+<br>
+pre:<br>
+ %p = phi i32 [ 70, %if ], [ %sel, %for ]<br>
+ br label %for<br>
+<br>
+for:<br>
+ %cmp = icmp sgt i32 %p, 8<br>
+ %add = add i32 %p, 2<br>
+ %sel = select i1 %cmp, i32 %p, i32 %add<br>
+ %cmp21 = icmp ult i32 %sel, 21<br>
+ br i1 %cmp21, label %pre, label %for.end<br>
+<br>
+for.end:<br>
+ br i1 %t, label %unreach2, label %then12<br>
+<br>
+then12:<br>
+ store i32 0, i32* %b<br>
+ br label %unreach2<br>
+<br>
+unreach2:<br>
+ %spec = phi i32 [ %sel, %for.end ], [ 42, %then12 ]<br>
+ unreachable<br>
+<br>
+end:<br>
+ ret void<br>
+}<br>
+<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>