[PATCH] D79893: [SCEV] Constant fold MultExpr before applying depth limit.

[Denis Antrushin via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:2960
       Ops[0] = getConstant(Fold);
+      if (Ops.size() == 2)
+        return Ops[0];
----------------
mkazantsev wrote:
> Is this change realy necessary? Looks unrelated, please commit it separately unless there is a reason to have it here. I'm also sure that similar change can be done in Add and some other SCEVs.
This exactly is copied from getAddExpr (the only other place where depth limits are applied)


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:2982
             const SCEV *Mul = getMulExpr(Ops[0], AddOp, SCEV::FlagAnyWrap,
                                          Depth + 1);
             if (!isa<SCEVMulExpr>(Mul)) AnyFolded = true;
----------------
mkazantsev wrote:
> We can potentially recurse here with unlimited depth, which is undersiable. Can you please avoid this?
Again, the idea was that just constant folding should not take too much time even if going deep. This also allows to keep code clean and simple.
Ok, I'll change it to check for all constant ops


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79893/new/

https://reviews.llvm.org/D79893