[PATCH] D87216: [NewPM] Support --print-before/after in NPM

aeubanks added inline comments. Comment at: llvm/lib/Passes/PassBuilder.cpp:462 + if (!PIC->hasPassName(P)) +report_fatal_error("unrecognized pass name: " + P); +} hoy wrote: > @aeubanks This seems not working with MIR passes under newpm. Would i

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

hoy added inline comments. Comment at: llvm/lib/Passes/PassBuilder.cpp:462 + if (!PIC->hasPassName(P)) +report_fatal_error("unrecognized pass name: " + P); +} @aeubanks This seems not working with MIR passes under newpm. Would it make sense to i

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2f0de582949d: [NewPM] Support --print-before/after in NPM (authored by aeubanks). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

aeubanks updated this revision to Diff 309338. aeubanks added a comment. forward declare class Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87216/new/ https://reviews.llvm.org/D87216 Files: llvm/include/llvm/IR/IRPrintingPasses.h llvm/include

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

ychen added a comment. In D87216#2431626 , @aeubanks wrote: > In D87216#2431508 , @ychen wrote: > >> It is very unfortunate that we have to manage and translate between two sets >> of names (one pass name and one t

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

aeubanks added a comment. In D87216#2431508 , @ychen wrote: > It is very unfortunate that we have to manage and translate between two sets > of names (one pass name and one type name). This makes me wonder if we just > keep the pass name as the return va

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

ychen accepted this revision. ychen added a comment. It is very unfortunate that we have to manage and translate between two sets of names (one pass name and one type name). This makes me wonder if we just keep the pass name as the return value of PassInfoMixin::name and get rid of class name e

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

jamieschmeiser accepted this revision. jamieschmeiser added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87216/new/ https://reviews.llvm.org/D87216 _

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

aeubanks updated this revision to Diff 309052. aeubanks added a comment. add comments to new methods Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87216/new/ https://reviews.llvm.org/D87216 Files: llvm/include/llvm/IR/IRPrintingPasses.h llvm/i

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

jamieschmeiser added a comment. I previously saw unrelated changes showing up in the differences here but this no longer seems to be the case so that comment can be ignored. Comment at: llvm/include/llvm/IR/PassInstrumentation.h:126 + void addClassToPassName(StringRef Class

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

aeubanks added a comment. In D87216#2428359 , @jamieschmeiser wrote: > I see you have made the requested changes (nit: clang-tidy complained about > the capitalization of the function) but why are there so many other, > unrelated changes shown? Is ther

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

aeubanks updated this revision to Diff 309000. aeubanks added a comment. clean up some more unnecessary llvm:: add comments to function declarations in PrintPasses.h Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87216/new/ https://reviews.llvm.org/

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

aeubanks updated this revision to Diff 308995. aeubanks added a comment. fix capitalization remove extra declaration and fixup includes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87216/new/ https://reviews.llvm.org/D87216 Files: llvm/include/

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

jamieschmeiser added a comment. I see you have made the requested changes (nit: clang-tidy complained about the capitalization of the function) but why are there so many other, unrelated changes shown? Is there a problem with the patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

aeubanks added inline comments. Comment at: llvm/lib/Passes/PassBuilder.cpp:449 + // We currently only use these for --print-before/after. + if (PIC && (!printBeforePasses().empty() || !printAfterPasses().empty())) { +#define MODULE_PASS(NAME, CREATE_PASS)

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

aeubanks updated this revision to Diff 308797. aeubanks added a comment. address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87216/new/ https://reviews.llvm.org/D87216 Files: llvm/include/llvm/IR/PassInstrumentation.h llvm/in

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

jamieschmeiser added a comment. I agree that having the callbacks ask for the names is an improvement as it is cleaner and allows other callbacks to use this feature if desired. Generally, things look good. I have a couple of minor concerns mentioned in the code but I think it would be accept

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

aeubanks added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87216/new/ https://reviews.llvm.org/D87216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

aeubanks added a comment. In D87216#2356770 , @jamieschmeiser wrote: > The changes are specific to -print-before and -print-after (which is the > intended target and this work originated before -print-changed) but could the > change be made universal?

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

aeubanks updated this revision to Diff 306307. aeubanks added a comment. rebase now we don't add a new parameter to callbacks, instead let individual instrumentations ask PassInstrumentationCallbacks for pass names Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

jamieschmeiser requested changes to this revision. jamieschmeiser added a comment. This revision now requires changes to proceed. The changes are specific to -print-before and -print-after (which is the intended target and this work originated before -print-changed) but could the change be made

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

aeubanks updated this revision to Diff 300555. aeubanks added a comment. move map into PassInstrumentationCallbacks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87216/new/ https://reviews.llvm.org/D87216 Files: llvm/include/llvm/IR/PassInstrume

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

aeubanks updated this revision to Diff 290159. aeubanks added a comment. remove some includes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87216/new/ https://reviews.llvm.org/D87216 Files: clang/lib/CodeGen/BackendUtil.cpp llvm/include/llvm/I

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

aeubanks created this revision. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, steven_wu, hiraditya, mgorny. Herald added projects: clang, LLVM. aeubanks requested review of this revision. This changes --print-before/after to be a list of strings rather than legacy passes. (this