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

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: include/llvm/Analysis/LazyValueInfo.h:102
+  /// Print the \LazyValueInfoCache.
+  void printCache(const DataLayout &DL, raw_ostream &OS);
+
----------------
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);`


================
Comment at: lib/Analysis/LazyValueInfo.cpp:444
+    void printCache(raw_ostream &OS) {
+      for (auto &VC : ValueCache)
+        for (auto &BV : VC.second->BlockVals)
----------------
dberlin wrote:
> 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 have not looked at this specific case, of course.
> 
> 
> But, for example, the iteration order here is *completely* undefined, meaning you have to be prepared for the output to appear in any order.
> 
Yes, since the order would be completely undefined, I used the `CHECK-DAG` part here along with the `CHECK-LABEL`. 
Thanks, I will take a look at the asm printer interface used in the other print functions.


================
Comment at: lib/Analysis/LazyValueInfo.cpp:1871
+  bool runOnFunction(Function &F) override {
+    errs() << "LVI for function '" << F.getName() << "':\n";
+    auto &LVI = getAnalysis<LazyValueInfoWrapperPass>().getLVI();
----------------
dberlin wrote:
> I do not believe we normally print to errs for printing passes.
> I believe they still print to dbgs()
So, couple of ones still print to errs, like the `AliasSetPrinter` (that's the one I followed for this). It looks like most printing passes print to dbgs(), I can change to that.


https://reviews.llvm.org/D30790