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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

I do not understand this part:

> Whereas the symbol from thin module has external attribute, and is associated to __profc_assign* who is non-prevailing and should not be inserted. While there is a prevailing __profc_assign* existing in symbol table, there is no __profd_assign*, thus reported as unresolved symbol.

Summary of the issue:

- The linker command line looks like `lld-link ... regular.o thin.o`.
- `regular.o` (regular LTO) emits `__prof[cd]_assign`. `__profc_assign` as the comdat leader is prevailing. profc is linkonce_odr while profd is private.
- `thin.o` (Thin LTO) emits `__prof[cd]_assign`. `__profc_assign` as the comdat leader is non-prevailing. Both profc and profd are linkonce_odr.

Does your change make `regular.o __profc_assign` linkonce_odr?
Which variable in a non-prevailing comdat is referenced, causing an undefined symbol error?

> I think by correcting the profile var name with a hash suffix when IRPGO enabled should avoid such inconsistency in different translation units. And the linker error helped exposing the issue.

Any idea why a hash suffix isn't used?
See `canRenameComdatFunc`, is the address of `assign` taken?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107034