[PATCH] D151965: [ThinLTO] Fix internalization decisions for weak/linkonce ODR

Fri Jun 2 12:49:08 PDT 2023

tejohnson added a comment.

In D151965#4391234 <https://reviews.llvm.org/D151965#4391234>, @steven_wu wrote:

> I think this should work for legacy API. ld64 always picks the prevailing one to be from native object file and ask LTO to reserve the symbol (so LTO cannot internalize them) so it doesn't really hit the bug mentioned. It is not the ideal solution but it is the best can be done when no symbol resolution information is available from API.
>
> It should be safe to assume atom only gets referenced once if there is only one visible and it is not in the reserved symbol list. I don't think preserved symbols will turn into prevailing, but I don't think they can be internalized in this case too. Would be good to add a test case to make sure if the weak/linkonce in the preserve GUID will get the correct behavior.

The only way I can think of to test that is to use llvm-lto with --exported-symbol=, which preserves that symbol via ThinLTOCodeGenerator::preserveSymbol. If that's what you mean, there is already a check of this case in llvm/test/ThinLTO/X86/weak_resolution.ll:

  ; When exported, we always preserve a linkonce
  ; RUN: llvm-lto -thinlto-action=promote %t.bc -thinlto-index=%t3.bc -o - --exported-symbol=_linkonceodrfuncInSingleModule | llvm-dis -o - | FileCheck %s --check-prefix=EXPORTED
  ...
  ; EXPORTED: define weak_odr void @linkonceodrfuncInSingleModule()

Let me know if you meant something else.

> LGTM.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151965