[PATCH] D82698: [NewPM] make parsePassPipeline parse adaptor-wrapped user passes

[Yuanfang Chen via Phabricator via llvm-commits]

ychen marked an inline comment as done.
ychen added inline comments.


================
Comment at: llvm/test/Other/pass-pipeline-parsing.ll:182
+; CHECK-ADAPTORS: Running pass: ModuleToFunctionPassAdaptor<{{.*}}NoOpFunctionPass>
+; CHECK-ADAPTORS: Running pass: ModuleToFunctionPassAdaptor<{{.*}}FunctionToLoopPassAdaptor<{{.*}}NoOpLoopPass>{{.*}}>
+; CHECK-ADAPTORS: Running pass: ModuleToPostOrderCGSCCPassAdaptor<{{.*}}NoOpCGSCCPass>
----------------
aeubanks wrote:
> ychen wrote:
> > aeubanks wrote:
> > > ychen wrote:
> > > > aeubanks wrote:
> > > > > ychen wrote:
> > > > > > aeubanks wrote:
> > > > > > > what about a separate check for
> > > > > > > Running pass: FunctionToLoopPassAdaptor<{{.*}}NoOpLoopPass>
> > > > > > > and NoOpLoopPass itself?
> > > > > > I'm happy to do that. What is the motivation?
> > > > > To make sure that the FunctionToLoopPassAdaptor itself isn't skipped for some reason, and the same with the NoOpLoopPass.
> > > > Is line 191 what you refer to?
> > > no, that one is a different no-op-loop in -passes='module(no-op-function,no-op-loop,no-op-cgscc,cgscc(no-op-function,no-op-loop),function(no-op-loop))'.
> > > 
> > > line 191 refers to the very last no-op-loop, I meant the first one. basically I think every adaptor "Running pass" message should be checked.
> > > 1 for the first no-op-function (module -> function), 2 for the first no-op-loop (module -> function -> loop), 1 for the first no-op-cgscc (module -> cgscc), 1 for the second no-op-function (cgscc -> function), 2 for the second no-op-loop (cgscc -> function -> loop), and 1 for the last no-op-loop (function -> loop)
> > First no-op-loop is checked by line 182.
> > 
> > Aren't these covered by the existing `Running pass:`?
> > 
> > Sorry, I'm still confused. 
> Ah I think I see what I'm missing. I was expecting each adaptor pass to print "Running pass:" if it were to run the pass it's adapting, but it doesn't. I think that's a good idea since the adaptor may run but for some reason it might not run its pass (e.g. if it decides to skip it for some reason). WDYT? Maybe that's out of scope for this change.
I agree. We should fix this o/w, we will see  `Running pass`: for skipped pass. Feel free to submit a patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82698