[SCEV] Add a threshold to restrict number of mul operands to be inlined into SCEV

[lihuang]

Summary

[SCEV] Add a threshold to restrict number of mul operands to be inlined into SCEV

This is to solve the same slow-compile issue in SCEV that D3127 was trying to solve. But that patch is no longer active.

Without a threshold for mul ops inlining, getMulExpr could have exponential time complexity in the worst case. The case in PR18606 is an example that causes clang to hang.

However, this patch does not completely solve the slow-compile issue in SCEV. The reason is, not completely flattening the SCEV DAG will make SCEVRewriteVisitor another bottleneck, which visits the same SCEV multiple times and could take exponential time to finish. I will fix this in another patch.

[sanjoy]

lib/Analysis/ScalarEvolution.cpp

2514 ↗

(On Diff #75216)

I'd have phrased the limit as a cap on the size of Ops (either Ops.size() directly, or a cap on the number of new elements added to Ops in this loop). Any specific reason why you chose this approach over that?

test/Analysis/ScalarEvolution/max-mulops-inline.ll

24 ↗

(On Diff #75216)

Can this test case be simplified? For instance, why do we need a loop at all? Why not just have a sequence of multiplication instructions in a single basic block?

[lihuang]

lib/Analysis/ScalarEvolution.cpp

2514 ↗

(On Diff #75216)

I agree, limiting the size of Ops is the right way. My previous change was wrong, it's limiting the number of ops to be inlined in a single iteration.

test/Analysis/ScalarEvolution/max-mulops-inline.ll

24 ↗

(On Diff #75216)

Yes I removed the loop. I thought I'd need a loop to trigger SCEV analysis. Thanks!

[sanjoy]

lgtm

llvm/trunk/lib/Analysis/ScalarEvolution.cpp

 static cl::opt<bool>
 VerifySCEV("verify-scev",
            cl::desc("Verify ScalarEvolution's backedge taken counts (slow)"));
 static cl::opt<bool>
     VerifySCEVMap("verify-scev-maps",
                   cl::desc("Verify no dangling value in ScalarEvolution's "
                            "ExprValueMap (slow)"));
 
+static cl::opt<unsigned> MulOpsInlineThreshold(
+    "scev-mulops-inline-threshold", cl::Hidden,
+    cl::desc("Threshold for inlining multiplication operands into a SCEV"),
+    cl::init(1000));
+
 //===----------------------------------------------------------------------===//
 //                           SCEV class definitions
 //===----------------------------------------------------------------------===//
 
 //===----------------------------------------------------------------------===//
 // Implementation of the SCEV class.
 //
 
#endif
   // Skip over the add expression until we get to a multiply.
   while (Idx < Ops.size() && Ops[Idx]->getSCEVType() < scMulExpr)
     ++Idx;
 
   // If there are mul operands inline them all into this expression.
   if (Idx < Ops.size()) {
     bool DeletedMul = false;
     while (const SCEVMulExpr *Mul = dyn_cast<SCEVMulExpr>(Ops[Idx])) {
+      if (Ops.size() > MulOpsInlineThreshold)
+        break;
       // If we have an mul, expand the mul operands onto the end of the operands
       // list.
       Ops.erase(Ops.begin()+Idx);
       Ops.append(Mul->op_begin(), Mul->op_end());
       DeletedMul = true;
     }
 
     // If we deleted at least one mul, we added operands to the end of the list,

llvm/trunk/test/Analysis/ScalarEvolution/max-mulops-inline.ll

+; RUN: opt -analyze -scalar-evolution -scev-mulops-inline-threshold=1 < %s | FileCheck --check-prefix=CHECK1 %s
+; RUN: opt -analyze -scalar-evolution -scev-mulops-inline-threshold=10 < %s | FileCheck --check-prefix=CHECK10 %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@a = local_unnamed_addr global i32 0, align 4
+@b = local_unnamed_addr global i32 0, align 4
+
+define i32 @main() local_unnamed_addr {
+
+; CHECK1: %mul.1 = mul nsw i32 %mul, %mul
+; CHECK1: -->  ((%a.promoted * %a.promoted) * (%a.promoted * %a.promoted))
+
+; CHECK10: %mul.1 = mul nsw i32 %mul, %mul
+; CHECK10: -->  (%a.promoted * %a.promoted * %a.promoted * %a.promoted)
+
+entry:
+  %a.promoted = load i32, i32* @a, align 4
+  %mul = mul nsw i32 %a.promoted, %a.promoted
+  %mul.1 = mul nsw i32 %mul, %mul
+  %mul.2 = mul nsw i32 %mul.1, %mul.1
+  %mul.3 = mul nsw i32 %mul.2, %mul.2
+  %mul.4 = mul nsw i32 %mul.3, %mul.3
+  %mul.5 = mul nsw i32 %mul.4, %mul.4
+  store i32 %mul.5, i32* @a, align 4
+  store i32 31, i32* @b, align 4
+  ret i32 0
+}