[PATCH] D90365: [Debugify] Port -debugify-each to NewPM

Thu Oct 29 11:23:16 PDT 2020

MaskRay added a comment.

In D90365#2362581 <https://reviews.llvm.org/D90365#2362581>, @aeubanks wrote:

> Thanks for looking into this!
>
> I was previously worried that modifying the IR in pass instrumentation would be bad, but maybe it's ok? The biggest thing I was worried about was unintentionally invalidating analyses, but it looks like both debugify passes preserve all analyses, so it might be ok.

Yeah I was a bit concerned as well. The code says `PreservedAnalyses::all()` so may be it is fine? The dirty part is the const_cast.

> I'm not so sure of moving the flags into Debugify.cpp since they are debugging tools, but I'm not 100% against it.

> DebugifyEachInstrumentation should probably be part of StandardInstrumentations so that all users of StandardInstrumentations will get it for free (tying into moving the flags to not be opt-specific). DebugifyEachInstrumentation::registerCallbacks can check if DebugifyEach is set or not to determine whether or not to actually register callbacks, which is cleaner than checking in NewPMDriver.cpp.

-debugify-export writes a file. So even if it is moved to StandardInstrumentations, all the users (clang/opt newPM/opt legacyPM) will need to do something with the option.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90365