[PATCH] D30790: [LVI] Add an LVI printer pass to capture test LVI cache after transformations

[Anna Thomas via Phabricator via llvm-commits]

anna marked an inline comment as done.
anna added a comment.

Hi folks,
Daniel added a suggestion regarding the printing of the LVI:

  FWIW: I would generally recommend you do this as an annotation to the basic blocks/values, using the asm printer interface, and adding them as comments.
  
  This would produce output similar to print-predicateinfo, print-memoryssa, etc, where the values are right above the instructions they belong to.
  
  This usually makes it significantly easier to test and understand what is going on, IME.

I had a look at the `print-predicateinfo`, and agree with Daniel - adding annotations as comments before the instruction makes it easier to see what's the transformation (along with the LVI values). However, in the LVI case, an instruction would have a list of LatticeValues corresponding to different basic blocks. Also, the LatticeValueCache structure enables printing the annotation per instruction (the cache key is ssa value). The overdefined cache structure enables printing per basic block. So, the print would be something like this:

For `test2` example (see annotations added as comments):

  entry:
  ; ' i32 %n ' is overdefined
    br label %loop
  
  loop
  ;  %iv2 = phi i32 [ %n, %entry ], [ %iv2.next, %backedge ] '  is overdefined
  ; 'constantrange<0, -2147483647>' at the beginning of 'loop'
  ; 'constantrange<0, -2147483648>' at the beginning of 'backedge'
    %iv = phi i32 [0, %entry], [%iv.next, %backedge] <-- the values corresponding to the `%iv` per basic block, are annotated before the instruction.
    %iv2 = phi i32 [%n, %entry], [%iv2.next, %backedge]
    %cnd1 = icmp sge i32 %iv, 0
    %cnd2 = icmp sgt i32 %iv2, 0
    %cnd = and i1 %cnd1, %cnd2
    br i1 %cnd, label %backedge, label %exit
  
  backedge:
  ; 'constantrange<1, -2147483647>' at the beginning of  'backedge'
    %iv.next = add nsw i32 %iv, 1
    %iv2.next = sub nsw i32 %iv2, 1
    %cont1 = icmp slt i32 %iv.next, 400
    %cont2 = icmp sgt i32 %iv2.next, 0
    %cont = and i1 %cont1, %cont2
    br i1 %cont, label %loop, label %exit
  
  exit:
    ret i8 0

I think it looks clearer than the `CHECK-DAG` and `CHECK-LABEL` combination I have for `test2` currently.

Does this look better?



================
Comment at: include/llvm/Analysis/LazyValueInfo.h:102
+  /// Print the \LazyValueInfoCache.
+  void printCache(const DataLayout &DL, raw_ostream &OS);
+
----------------
dberlin wrote:
> anna wrote:
> > dberlin wrote:
> > > What's the reason for datalayout?
> > > It's always gettable from any instruction, etc.
> > > 
> > We need it here for getting the `LazyValueInfoImpl` within this function. I don't think we have any other way for getting the datalayout.
> > 
> > see: `getImpl(PImpl, AC, &DL, DT).printCache(OS);`
> Oh, yeah, this is just silly.
> It looks like LazyValueInfo doesn't have DL, but all the impl's require it.
> This looks like it was just the simplest conversion possible.
> 
> I would just add it to the class LazyValueInfo interface, since everything else is there.
> 
> 
Thanks, added as a separate NFC.


https://reviews.llvm.org/D30790