[PATCH] D44524: Add an analysis printer for must execute reasoning

[Anna Thomas via Phabricator via llvm-commits]

anna added a comment.

In https://reviews.llvm.org/D44524#1040307, @reames wrote:

> In https://reviews.llvm.org/D44524#1040018, @anna wrote:
>
> > Just to note, other printer passes like print-memoryssa, print-lvi, print-predicate-info, we print the annotations as comments just above the IR (using the asm printer interface). It's easier to see which instruction we are referring to and quickly spot the loops they were part of , *if loops are properly annotated in the IR*. It would be useful when debugging large code base IR and spotting the missing cases.
> >  IMO, the missing cases may be hard to spot in this printer. One workaround  is to print the number of loops where it's not guaranteed to execute.  That can be a separate add on after landing this as well.
> >
> > From a testing perspective of the utility, this version works well for me.
>
>
> This is a good idea.  I'd copied the basic structure of this from the mem deref printer, but I like the annotation version better.  Mind if I submit this one (so that I can start getting some basic testing around this code), and then return to the annotation as a follow on?  I'll change over mem-deref while I'm at it.


SGTM.



================
Comment at: lib/Analysis/MustExecute.cpp:25
+    DenseMap<Value*, SmallVector<Loop*, 4> > MustExec;
+    SmallVector<Value *, 4> Ordering;
+
----------------
reames wrote:
> anna wrote:
> > Any reason for 4? Given we are looking at all instructions within all loops that's guaranteed to execute, it can be a much larger number. 
> 4 is simply the initial size and chosen fairly randomly.  The SmallVector will resize if needed.  Suggestions?
Yes, it would resize. It's just that this feels we would definitely resize. I don't have another good suggestion though. We could do something like `Ordering.reserve(F.size())`, i.e. reserve the number of instructions in the function, but that's just wasted space... 
And I don't think there's an easy way to get num instructions in all loops.
So, we can just stick with 4 I guess.


Repository:
  rL LLVM

https://reviews.llvm.org/D44524