[PATCH] D107034: [CSPGO] Set EnableValueProfiling flag for CSPGO to avoid IRPGO value loss in LTO

[Yolanda via Phabricator via llvm-commits]

YolandaCY added a comment.

@xur This is also reproducible using the existing test case `PGOProfile/thinlto_cspgo_gen.ll`. If evaluating the generated "`%t.2.4.opt.bc`" containing non-prevailing IRPGO flag, you will find the value is missed. (The value is still reserved in "`%t.2.3.import.bc`".)

In the LTO case, the symbol is dropped at *.preopt phase.

In summary after tracing:

1. In LTO, the value is dropped when `addRegularLTO` as it has external linkage and non-prevailing.
2. In ThinLTO, the value is first changed to `available_external` linkage by `thinLTOResolvePrevailingInModule` and `thinLTOResolvePrevailingGUID`. It is dropped later in the backend `GlobleOpt` pass by `deleteIfDead`.

Hence to keep the value we may need to identify the IRPGO variable in all above places, make sure it is reserved, and then drop it after instrumentation to avoid duplicate symbols in later linkage. My feeling is that using a module flag may avoid too much hassle across multiple places and more complexity to the symbol resolution stuff. (as suggested by Reid)
Agree that mixing `EnableValueProfiling` with IRPGO flag is improper as it seems Clang also may support value profiling (from comment of `shouldRecordFunctionAddr` ).
Maybe we can rename the module flag to "`CSIRPGO`"? We can also remove the flag after instrumentation as no need for later passes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107034