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

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev marked 2 inline comments 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);
----------------
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.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:2909
   unsigned Idx = 0;
   if (const SCEVConstant *LHSC = dyn_cast<SCEVConstant>(Ops[0])) {
 
----------------
reames wrote:
> Not related to your review, but I notice inconsistency about whether we merge adjacent constants before or after the bailout check.
We can change that separately.


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

https://reviews.llvm.org/D35990