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

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added inline comments.


================
Comment at: include/llvm/Analysis/LazyValueInfo.h:102
+  /// Print the \LazyValueInfoCache.
+  void printCache(const DataLayout &DL, raw_ostream &OS);
+
----------------
What's the reason for datalayout?
It's always gettable from any instruction, etc.



================
Comment at: lib/Analysis/LazyValueInfo.cpp:444
+    void printCache(raw_ostream &OS) {
+      for (auto &VC : ValueCache)
+        for (auto &BV : VC.second->BlockVals)
----------------
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.



================
Comment at: lib/Analysis/LazyValueInfo.cpp:448
+             << BV.second << "' at the beginning of BasicBlock: '"
+             << BV.first->getName() << "'\n";
+      OS << "Overdefined Values per basic block\n";
----------------
BB's don't always have names, i would use BV.first->printAsOperand(), which will also handle the nameless ones properly.



================
Comment at: lib/Analysis/LazyValueInfo.cpp:1858
+// Printer class for LazyValueInfo results.
+class LazyValueInfoPrinter : public FunctionPass {
+public:
----------------
You should add a new PM version of this as well.



================
Comment at: lib/Analysis/LazyValueInfo.cpp:1871
+  bool runOnFunction(Function &F) override {
+    errs() << "LVI for function '" << F.getName() << "':\n";
+    auto &LVI = getAnalysis<LazyValueInfoWrapperPass>().getLVI();
----------------
I do not believe we normally print to errs for printing passes.
I believe they still print to dbgs()


https://reviews.llvm.org/D30790