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

[Anna Thomas via Phabricator via llvm-commits]

anna added a comment.

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.



================
Comment at: lib/Analysis/MustExecute.cpp:25
+    DenseMap<Value*, SmallVector<Loop*, 4> > MustExec;
+    SmallVector<Value *, 4> Ordering;
+
----------------
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. 


================
Comment at: lib/Analysis/MustExecute.cpp:85
+  for (Value *V: Ordering) {
+    V->print(OS);
+    OS << "\t(mustexec in ";
----------------
I think we can use `printAsOperand` so that nameless values are handled properly.


================
Comment at: lib/Analysis/MustExecute.cpp:87
+    OS << "\t(mustexec in ";
+    bool first = true;
+    for (const Loop *L : MustExec.lookup(V)) {
----------------
Perhaps rename this to exactlyOneLoop and  define that as  `MustExec[V]->size()  == 1`  


================
Comment at: test/Analysis/MustExecute/loop-header.ll:4
+; CHECK: Printing analysis 'Instructions which execute on loop entry' for function 'header_with_icf':
+; CHECK: The following are guaranteed to execute (for the respective loops):
+; CHECK:   %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]	(mustexec in loop)
----------------
>From a usability perspective in large IR where multiple transforms already done, we will have print statements like this:
```
; %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]  (mustexec in loop.split.unroll.split.split.crit_edge.... , another loop header name etc)
```
It seems difficult to read. If the usecase of the printer is to test existing code, it's fine.
However, printing this from large IR can make it pretty difficult to parse.
How about something like:
```
; %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]  (mustexec in 3 loops: <header names> of all 3 loops)
```
This number 3 can be retrieved from `MustExec[V]->size()`


Repository:
  rL LLVM

https://reviews.llvm.org/D44524