[PATCH] D17201: [SCEV] Introduce a guarded backedge taken count and use it in LAA and LV

[Andrew Trick via llvm-commits]

atrick added inline comments.

================
Comment at: include/llvm/Analysis/ScalarEvolution.h:557
@@ -545,1 +556,3 @@
+      /// otherwise it's all CouldNotCompute.
+      SCEVUnionPredicate Pred;
 
----------------
sanjoy wrote:
> sbaranga wrote:
> > sanjoy wrote:
> > > Now that you're embedding `SCEVUnionPredicate` in every
> > > `ExitNotTakenInfo`, I'm somewhat worried about the `SmallVector<const
> > > SCEVPredicate *, 16> Preds;` :)
> > > 
> > > Firstly, what is the typical number of predicates we see?  16 seems
> > > higher that what I would expect.
> > > 
> > > Secondly, a better (in terms of memory consumption) solution would be
> > > to have a `std::unique_ptr<SCEVUnionPredicate>` here, where a null
> > > value means "always true" so that in the non-predicated case we only
> > > use up one word.
> > > 
> > > The most compact solution (I think we should go with this) is to:
> > > 
> > >  - Extract out a `struct ExitNotTakenExtras` that contains a
> > >    `SmallVector` of `ExitNotTaken` s and a `SCEVUnionPredicate`
> > > 
> > >  - Change `PointerIntPair<ExitNotTakenInfo*, 1> NextExit` to
> > >    `PointerIntPair<ExitNotTakenExtras *, 1> NextExit` where it is
> > >    usually null, but if it is non-null you either have multiple exits
> > >    or have some SCEV predicates.
> > > 
> > > Normally I wouldn't worry so much about space, but `ExitNotTakenInfo`
> > > has clearly been optimized for space usage, so we should try not to
> > > break that.
> > Ok, the most common size of the SCEVUnionPredicate there should be 0, so going with your solution makes sense to me.
> > 
> > On the other hand, it does seem a bit strange to optimize this for memory consumption. I wouldn't expect it to make that much of a difference.
> My //guess// is that there are enough `ExitNotTakenInfo` instances floating around in a typical compilation that its size becomes important.  But maybe @atrick has a more cogent answer?
I don't have a better answer but agree that SCEV instances need to be size conscious, just as other IR data types do. I would avoid allocating and initializing 16 words that are unlikely to be used.


http://reviews.llvm.org/D17201