[PATCH] D63155: [clang][NewPM] Fix broken profile test

[Rong Xu via Phabricator via llvm-commits]

xur added a comment.

In D63155#1563266 <https://reviews.llvm.org/D63155#1563266>, @chandlerc wrote:

> In D63155#1563240 <https://reviews.llvm.org/D63155#1563240>, @leonardchan wrote:
>
> > In D63155#1563229 <https://reviews.llvm.org/D63155#1563229>, @xur wrote:
> >
> > > This patch does not make sense to me.
> > >
> > > The reason of failing with -fexperimental-new-pass-manager is because we don't do PGO instrumentation at -O0. This is due to the fact that PGO instrumentation/use passes are under PassBuilder::buildPerModuleDefaultPipeline.
> > >
> > > This patch add a pass
> > >
> > >   MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
> > >
> > > which only gives you the wrong signal  of instrumentation is done.
> > >
> > > I wrote pass PGOInstrumentationGenCreateVar only for some trick interactions for thinlto under ldd for CSPGO.
> > >  Regular FDO should not use it.
> > >
> > > The right fix is to enable PGO instrumentation and use in pass builder for O0.
> > >
> > > I would like to request to revert this patch.
> >
> >
> > As an alternative, could I instead request that we remove the BackendUtil changes and just mark the runs in gcc-flag-compatibility.c with `-fno-experimental-new-pass-manager`. This way we could clarify that under the new PM, we shouldn't run PGO at -O0? If not, I'll revert this patch as is.
>
>
> No, I think we should be doing PGO at O0 in the new PM if we do so in the old PM.
>
> I think Rong is saying that the *way* you're enabling PGO at O0 isn't correct to fix this test case. That seems plausible to me at least, and I think reverting and figuring out what the right way to do it is a fine approach.


Just check the IR, we still not doing instrumentation at O0. This patch just mask the error.

> I just think we also need to understand why *no other test failed*, and why the only test we have for doing PGO at O0 is this one and it could be "fixed" but such a deeply unrelated change....

We have special code to do PGO at O0 in old pass manager. This is not done in the new pass manager.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155