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

Leonard Chan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 28 16:57:03 PDT 2019


leonardchan added a comment.

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.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155





More information about the llvm-commits mailing list