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

[Li Huang via llvm-commits]

lihuang marked 2 inline comments as done.
lihuang added inline comments.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:2514
     while (const SCEVMulExpr *Mul = dyn_cast<SCEVMulExpr>(Ops[Idx])) {
+      if (Mul->getNumOperands() > MaxMulOpsToInline)
+        break;
----------------
sanjoy wrote:
> 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?
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. 


================
Comment at: test/Analysis/ScalarEvolution/max-mulops-inline.ll:24
+
+for.cond1.preheader:                              ; preds = %for.body3, %entry
+  %mul.lcssa5 = phi i32 [ %a.promoted4, %entry ], [ %mul.5, %for.body3 ]
----------------
sanjoy wrote:
> 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?
Yes I removed the loop. I thought I'd need a loop to trigger SCEV analysis. Thanks!


https://reviews.llvm.org/D25794