[PATCH] D35990: [SCEV] Prohibit SCEV transformations for huge SCEVs

Max Kazantsev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 29 01:41:27 PST 2019


mkazantsev marked an inline comment as done.
mkazantsev added inline comments.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:862
+
+static bool hasHugeExpression(SmallVectorImpl<const SCEV *> &Ops) {
+  return any_of(Ops, isHugeExpression);
----------------
sanjoy wrote:
> mkazantsev wrote:
> > reames wrote:
> > > Probably better to use an ArrayRef as the argument.
> > 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.
> 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`.
It makes sense, yet it was untrue for SCEV code. I've commited rL352466 to leverage this. Will rebase on top of it.


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

https://reviews.llvm.org/D35990





More information about the llvm-commits mailing list