[PATCH] D15776: Filtering IR printing for print-after-all/print-before-all

[Weiming Zhao via llvm-commits]

weimingz marked an inline comment as not done.

================
Comment at: lib/Analysis/LoopPass.cpp:45-47
@@ -44,3 +44,5 @@
   bool runOnLoop(Loop *L, LPPassManager &) override {
-    P.run(*L);
+    if (L->getHeader() &&
+        isFunctionInPrintList(L->getHeader()->getParent()->getName()))
+      P.run(*L);
     return false;
----------------
joker.eph wrote:
> Not clear to me: either there is a valid use case where `PrintLoopPassWrapper::runOnLoop()` is called with `getHeader()` returning null or not, can you clarify?
> 
> In the absence of such valid case, please turn this into an assert or just remove the check.
Could be a valid case. Below is PrintLoopPass::run()
PrintLoopPass::run(Loop &L) {
   ..
  for (auto *Block : L.blocks())
    if (Block)
      Block->print(OS);
    else
      OS << "Printing <null> block";
..
}


================
Comment at: lib/IR/LegacyPassManager.cpp:126
@@ +125,3 @@
+      PrintFuncNames.insert("*");
+  }
+
----------------
joker.eph wrote:
> Is it possible to write it:
> 
> ```
> static std::set<std::string> PrintFuncNames(PrintFuncsList.begin(), PrintFuncsList.end());
> ```
> 
> ?
> 
> Also, why not an `unordered_set`?
Yes, we can write in one line. 

yes, I meant to use unordered_set, but forgot the change. Thanks!

================
Comment at: lib/IR/LegacyPassManager.cpp:129
@@ +128,3 @@
+  if (PrintFuncNames.count("*"))
+    return true;
+  return PrintFuncNames.count(FunctionName);
----------------
joker.eph wrote:
> I'm not sure I see the need for the "*" in the set? The empty case returning true seems enough to me to provide exactly the same behavior.
Need that because in Module print, we need to test if the filter-list is empty or not.  (An empty list means the whole module will be printed).

isFunctionInPrintList() only checks if the argument string is in the list or not. Alternatively, we can let it return a pair of bools, one indicates if the list is empty or not, the other tells if the arg is in list or not. But this is not intuitive. 


Repository:
  rL LLVM

http://reviews.llvm.org/D15776