[SCEV] Prohibit SCEV transformations for huge SCEVs

[mkazantsev]

Summary

Currently SCEV attempts to limit transformations so that they do not work with
big SCEVs (that may take almost infinite compile time). But for this, it uses heuristics
such as recursion depth and number of operands, which do not give us a guarantee
that we don't actually have big SCEVs. This situation is still possible, though it is not
likely to happen. However, the bug PR33494 showed a bunch of simple corner case
tests where we still produce huge SCEVs, even not reaching big recursion depth etc.

This patch introduces a concept of 'huge' SCEVs. A SCEV is huge if its expression
size (intoduced in D35989) exceeds some threshold value. We prohibit optimizing
transformations if any of SCEVs we are dealing with is huge. This gives us a reliable
check that we don't spend too much time working with them.

As the next step, we can possibly get rid of old limiting mechanisms, such as recursion
depth thresholds.

[mkazantsev]

Reused smart version of any_of

[mkazantsev]

Some tests from https://bugs.llvm.org/show_bug.cgi?id=33494 fail when huge expressions threshold is set to low values (like 100). It is not automatically this patch's problem, but this should be fixed before this is merged.

[mkazantsev]

UPD: The mentioned crash is actually assertion failure on cast<SCEVAddRecExpr>. Something happens to be non-addrec after we limit transformations by huge scevs. This bug also should be (in theory) reproducible without this patch, but I don't have an example.

[mkazantsev]

We need it to fix PR40292. Reclaiming.

[mkazantsev]

Updated test to preserve current behavior (it creates *really* big SCEVs in test_05).

[reames]

You need some tests highlighting the limit working in practice. These can be easily written by using non-default threshold values.

lib/Analysis/ScalarEvolution.cpp

813

Please add a comment here explaining what "huge" means and is used for. This the obvious point for someone reading the code to try to find the explanation.

817

Probably better to use an ArrayRef as the argument.

2741

Not related to your review, but I notice inconsistency about whether we merge adjacent constants before or after the bailout check.

[mkazantsev]

lib/Analysis/ScalarEvolution.cpp

817

SCEV uses SmallVectorImpl to pass argument lists in all places. What you propose is inconsistent with the current code. We can do a global refactoring if there is any benefit from it.

2741

We can change that separately.

[mkazantsev]

Added comments.

[reames]

Still needs tests

[sanjoy]

lib/Analysis/ScalarEvolution.cpp

817

SCEV uses SmallVectorImpl in many places because it needs to modify the container in place (e.g. getAddExpr will reduce the size of SmallVectorImpl in place when folding constants). Functions that do not modify the input sequence in place should use ArrayRef.

[mkazantsev]

lib/Analysis/ScalarEvolution.cpp

817

It makes sense, yet it was untrue for SCEV code. I've commited rL352466 to leverage this. Will rebase on top of it.

[mkazantsev]

Added some tests to show that it works, rebased, replaced with ArrayRef.

[reames]

LGTM

lib/Analysis/ScalarEvolution.cpp

MaxExtDepth("scalar-evolution-max-ext-depth", cl::Hidden,
                 cl::desc("Maximum depth of recursive SExt/ZExt"),
                 cl::init(8));
 
 static cl::opt<unsigned>
     MaxAddRecSize("scalar-evolution-max-add-rec-size", cl::Hidden,
                   cl::desc("Max coefficients in AddRec during evolving"),
                   cl::init(16));
 
+static cl::opt<unsigned>
+    HugeExprThreshold("scalar-evolution-huge-expr-threshold", cl::Hidden,
+                  cl::desc("Size of the expression which is considered huge"),
+                  cl::init(4096));
+
 //===----------------------------------------------------------------------===//
 //                           SCEV class definitions
 //===----------------------------------------------------------------------===//
 
 //===----------------------------------------------------------------------===//
 // Implementation of the SCEV class.
 //
 
static inline int sizeOfSCEV(const SCEV *S) {
   };
 
   FindSCEVSize F;
   SCEVTraversal<FindSCEVSize> ST(F);
   ST.visitAll(S);
   return F.Size;
 }
 
+static bool isHugeExpression(const SCEV *S) {
+  return S->getExpressionSize() >= HugeExprThreshold;
+}
+
+static bool hasHugeExpression(SmallVectorImpl<const SCEV *> &Ops) {
+  return any_of(Ops, isHugeExpression);
+}
+
 namespace {
 
 struct SCEVDivision : public SCEVVisitor<SCEVDivision, void> {
 public:
   // Computes the Quotient and Remainder of the division of Numerator by
   // Denominator.
   static void divide(ScalarEvolution &SE, const SCEV *Numerator,
                      const SCEV *Denominator, const SCEV **Quotient,
if (LHSC->getValue()->isZero()) {
       Ops.erase(Ops.begin());
       --Idx;
     }
 
     if (Ops.size() == 1) return Ops[0];
   }
 
   // Limit recursion calls depth.
-  if (Depth > MaxArithDepth)
+  if (Depth > MaxArithDepth || hasHugeExpression(Ops))
     return getOrCreateAddExpr(Ops, Flags);
 
   // Okay, check to see if the same value occurs in the operand list more than
   // once.  If so, merge them together into an multiply expression.  Since we
   // sorted the list, these values are required to be adjacent.
   Type *Ty = Ops[0]->getType();
   bool FoundMatch = false;
   for (unsigned i = 0, e = Ops.size(); i != e-1; ++i)

 #endif
 
   // Sort by complexity, this groups all similar expression types together.
   GroupByComplexity(Ops, &LI, DT);
 
   Flags = StrengthenNoWrapFlags(this, scMulExpr, Ops, Flags);
 
   // Limit recursion calls depth.
-  if (Depth > MaxArithDepth)
+  if (Depth > MaxArithDepth || hasHugeExpression(Ops))
     return getOrCreateMulExpr(Ops, Flags);
 
   // If there are any constants, fold them together.
   unsigned Idx = 0;
   if (const SCEVConstant *LHSC = dyn_cast<SCEVConstant>(Ops[0])) {
 
     // C1*(C2+V) -> C1*C2 + C1*V
     if (Ops.size() == 2)
         if (const SCEVAddExpr *Add = dyn_cast<SCEVAddExpr>(Ops[1]))
           // If any of Add's ops are Adds or Muls with a constant,
           // apply this transformation as well.
           if (Add->getNumOperands() == 2)
             // TODO: There are some cases where this transformation is not
for (unsigned OtherIdx = Idx+1;
       const SCEVAddRecExpr *OtherAddRec =
         dyn_cast<SCEVAddRecExpr>(Ops[OtherIdx]);
       if (!OtherAddRec || OtherAddRec->getLoop() != AddRecLoop)
         continue;
 
       // Limit max number of arguments to avoid creation of unreasonably big
       // SCEVAddRecs with very complex operands.
       if (AddRec->getNumOperands() + OtherAddRec->getNumOperands() - 1 >
-          MaxAddRecSize)
+          MaxAddRecSize || isHugeExpression(AddRec) ||
+          isHugeExpression(OtherAddRec))
         continue;
 
       bool Overflow = false;
       Type *Ty = AddRec->getType();
       bool LargerThan64Bits = getTypeSizeInBits(Ty) > 64;
       SmallVector<const SCEV*, 7> AddRecOps;
       for (int x = 0, xe = AddRec->getNumOperands() +
              OtherAddRec->getNumOperands() - 1; x != xe && !Overflow; ++x) {