[llvm-branch-commits] [llvm-branch] r259066 - Merging r258971:

[Hans Wennborg via llvm-branch-commits]

Author: hans
Date: Thu Jan 28 12:23:25 2016
New Revision: 259066

URL: http://llvm.org/viewvc/llvm-project?rev=259066&view=rev
Log:
Merging r258971:
------------------------------------------------------------------------
r258971 | spatel | 2016-01-27 11:22:45 -0800 (Wed, 27 Jan 2016) | 26 lines

[SimplifyCFG] limit recursion depth when speculating instructions (PR26308)

This is a fix for:
https://llvm.org/bugs/show_bug.cgi?id=26308

With the switch to using the TTI cost model in:
http://reviews.llvm.org/rL228826
...it became possible to hit a zero-cost cycle of instructions (gep -> phi -> gep...), 
so we need a cap for the recursion in DominatesMergePoint().

A recursion depth parameter was already added for a different reason in:
http://reviews.llvm.org/rL255660
...so we can just set a limit for it.

I pulled "10" out of the air and made it an independent parameter that we can play with.
It might be higher than it needs to be given the currently low default value of 
PHINodeFoldingThreshold (2). That's the starting cost value that we enter the recursion
with, and most instructions have cost set to TCC_Basic (1), so I don't think we're going
to speculate more than 2 instructions with the current parameters.

As noted in the review and the TODO comment, we can do better than just limiting recursion
depth.

Differential Revision: http://reviews.llvm.org/D16637


------------------------------------------------------------------------

Modified:
    llvm/branches/release_38/   (props changed)
    llvm/branches/release_38/lib/Transforms/Utils/SimplifyCFG.cpp
    llvm/branches/release_38/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll

Propchange: llvm/branches/release_38/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Jan 28 12:23:25 2016
@@ -1,3 +1,3 @@
 /llvm/branches/Apple/Pertwee:110850,110961
 /llvm/branches/type-system-rewrite:133420-134817
-/llvm/trunk:155241,257645,257648,257730,257775,257791,257875,257886,257902,257905,257925,257929-257930,257940,257942,257977,257979,257997,258168,258184,258207,258221,258273,258325,258406,258416,258428,258436,258471,258690,258729,258891
+/llvm/trunk:155241,257645,257648,257730,257775,257791,257875,257886,257902,257905,257925,257929-257930,257940,257942,257977,257979,257997,258168,258184,258207,258221,258273,258325,258406,258416,258428,258436,258471,258690,258729,258891,258971

Modified: llvm/branches/release_38/lib/Transforms/Utils/SimplifyCFG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_38/lib/Transforms/Utils/SimplifyCFG.cpp?rev=259066&r1=259065&r2=259066&view=diff
==============================================================================
--- llvm/branches/release_38/lib/Transforms/Utils/SimplifyCFG.cpp (original)
+++ llvm/branches/release_38/lib/Transforms/Utils/SimplifyCFG.cpp Thu Jan 28 12:23:25 2016
@@ -90,6 +90,11 @@ static cl::opt<bool> SpeculateOneExpensi
     cl::desc("Allow exactly one expensive instruction to be speculatively "
              "executed"));
 
+static cl::opt<unsigned> MaxSpeculationDepth(
+    "max-speculation-depth", cl::Hidden, cl::init(10),
+    cl::desc("Limit maximum recursion depth when calculating costs of "
+             "speculatively executed instructions"));
+
 STATISTIC(NumBitMaps, "Number of switch instructions turned into bitmaps");
 STATISTIC(NumLinearMaps, "Number of switch instructions turned into linear mapping");
 STATISTIC(NumLookupTables, "Number of switch instructions turned into lookup tables");
@@ -269,6 +274,13 @@ static bool DominatesMergePoint(Value *V
                                 unsigned &CostRemaining,
                                 const TargetTransformInfo &TTI,
                                 unsigned Depth = 0) {
+  // It is possible to hit a zero-cost cycle (phi/gep instructions for example),
+  // so limit the recursion depth.
+  // TODO: While this recursion limit does prevent pathological behavior, it
+  // would be better to track visited instructions to avoid cycles.
+  if (Depth == MaxSpeculationDepth)
+    return false;
+
   Instruction *I = dyn_cast<Instruction>(V);
   if (!I) {
     // Non-instructions all dominate instructions, but not all constantexprs

Modified: llvm/branches/release_38/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_38/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll?rev=259066&r1=259065&r2=259066&view=diff
==============================================================================
--- llvm/branches/release_38/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll (original)
+++ llvm/branches/release_38/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll Thu Jan 28 12:23:25 2016
@@ -1302,3 +1302,35 @@ l6:
 ; CHECK: entry
 ; CHECK-NEXT: switch
 }
+
+; Speculation depth must be limited to avoid a zero-cost instruction cycle.
+
+; CHECK-LABEL: @PR26308(
+; CHECK:       cleanup4:
+; CHECK-NEXT:  br label %cleanup4
+
+define i32 @PR26308(i1 %B, i64 %load) {
+entry:
+  br label %while.body
+
+while.body:
+  br label %cleanup
+
+cleanup:
+  %cleanup.dest.slot.0 = phi i1 [ false, %while.body ]
+  br i1 %cleanup.dest.slot.0, label %for.cond, label %cleanup4
+
+for.cond:
+  %e.0 = phi i64* [ undef, %cleanup ], [ %incdec.ptr, %for.cond2 ]
+  %pi = ptrtoint i64* %e.0 to i64
+  %incdec.ptr = getelementptr inbounds i64, i64* %e.0, i64 1
+  br label %for.cond2
+
+for.cond2:
+  %storemerge = phi i64 [ %pi, %for.cond ], [ %load, %for.cond2 ]
+  br i1 %B, label %for.cond2, label %for.cond
+
+cleanup4:
+  br label %while.body
+}
+