[PATCH] D88764: [NewPM] Use PassInstrumentation for -verify-each

[Yuanfang Chen via Phabricator via llvm-commits]

ychen added inline comments.


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:741
+          if (DebugLogging)
+            dbgs() << "Verifying function " << F->getName() << "\n";
+
----------------
aeubanks wrote:
> ychen wrote:
> > To encode more information in the output, how about adding the suffix "for loop <loop name>" for loop pass?
> > 
> > Ditto for cgscc pass. Also, check the full string such as "Verifying module <x>", "Verifying module <x> for loop <y>" etc.
> It's a little misleading since it's verifying the entire module/function, not only the cgscc/loop.
> The proper information is already printed in the "Running pass:" when specifying `-debug-pass-manager`.
Sounds good. Add checks in the test that "Verifying .." is following "Running pass:"?


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:772
   PrintChangedIR.registerCallbacks(PIC);
+  if (VerifyEach) {
+    Verify.registerCallbacks(PIC);
----------------
remove brace


================
Comment at: llvm/test/Other/new-pass-manager-verify-each.ll:4
+; Added manually by opt at beginning
+; CHECK: Running pass: VerifierPass
+
----------------
aeubanks wrote:
> ychen wrote:
> > If it is not added by VerifyInstrumentation, maybe we don't need to check it?
> I added it because this test is basically replacing the test in new-pass-manager.ll. So I'd say this test should cover both the pass instrumentation and the verify passes added at the very beginning and end.
make sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88764