[PATCH] D54740: [NewPM] fixing asserts on deleted loop in -print-after-all

[Fedor Sergeev via Phabricator via llvm-commits]

fedor.sergeev added inline comments.


================
Comment at: include/llvm/IR/PassInstrumentation.h:158-159
+    if (Callbacks)
+      for (auto &C : Callbacks->AfterPassCallbacks)
+        C(Pass.name(), llvm::Any((const IRUnitT *)nullptr));
+  }
----------------
chandlerc wrote:
> Similarly, I think the callbacks should be different. The user can always do this binding with nullptr if that's a desirable way to model it, but generally I think different names of routines seem more appropriate.
Since this nullptr hack is not under control of the user its either the same callback with nullptr (as of right now) or a callback w/o any IR at all.

Conceptually, using different callbacks does look like a clearner solution, though I'm a bit worried that we double the amount of callbacks for a somewhat limited added value...
Need to try it out and see how it looks like.


================
Comment at: lib/Passes/StandardInstrumentations.cpp:136-137
   SmallString<20> Banner = formatv("*** IR Dump After {0} ***", PassID);
+  // TODO: handle cases when IR is nullptr (i.e. has been invalidated)
+  // but we still need to print the module due to forcePrintModuleIR.
   unwrapAndPrint(Banner, IR);
----------------
chandlerc wrote:
> FWIW, I think the way to handle this is to bind a reference to the module into the callback so that it intrinsically has access to it to print.
Thats true, just wanted to do that separately (and, btw, address the same problem for Legacy printer).


Repository:
  rL LLVM

https://reviews.llvm.org/D54740