[PATCH] D58406: Fix IR/Analysis layering issue in OptBisect

[Philip Pfaffe via Phabricator via llvm-commits]

philip.pfaffe added inline comments.


================
Comment at: include/llvm/IR/OptBisect.h:29
 
-  virtual bool shouldRunPass(const Pass *P, const Module &U) { return true; }
-  virtual bool shouldRunPass(const Pass *P, const Function &U)  {return true; }
-  virtual bool shouldRunPass(const Pass *P, const BasicBlock &U)  { return true; }
-  virtual bool shouldRunPass(const Pass *P, const Region &U)  { return true; }
-  virtual bool shouldRunPass(const Pass *P, const Loop &U)  { return true; }
-  virtual bool shouldRunPass(const Pass *P, const CallGraphSCC &U)  { return true; }
+  virtual bool shouldRunPass(const Pass *P, StringRef IRDescription) {
+    return true;
----------------
fedor.sergeev wrote:
> So, since we went over changing the interface to use StringRef for description, can we move one more step further
> in that direction and start passing StringRef for Pass?
> That would make this interface fully ready for new pass manager.
> 
> I can do it later on myself, but doing it here would lessen the churn by doing one change in interface, not two of them.
> 
> (it will not add any more overhead since pass name is always available as a string literal)
I concur with @andrew.w.kaylor that the performance hit from building the IRDescription will probably be fine. Nevertheless, it should be much cheaper to pass a Twine here instead! Additionally there should be a comment here explaining IRDescription.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58406/new/

https://reviews.llvm.org/D58406