[PATCH] D135427: [LTO] Make local linkage GlobalValue in non-prevailing COMDAT available_externally

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked 2 inline comments as done.
MaskRay added inline comments.


================
Comment at: llvm/test/LTO/Resolution/X86/comdat-mixed-lto.ll:19
 ; RUN: llvm-dis %t3.0.0.preopt.bc -o - | FileCheck %s
-; CHECK: define internal void @__cxx_global_var_init() section ".text.startup" {
+; CHECK: declare dso_local void @__cxx_global_var_init() section ".text.startup"
 ; CHECK: define available_externally dso_local void @testglobfunc() section ".text.startup" {
----------------
tejohnson wrote:
> Any idea where this is getting dropped to a declaration before optimization?
I am very familiar with IRMover but the logic is in
```
LTO.cpp RegularLTO.Mover->move
...
IRMover.cpp:594 Expected<Constant *> NewProto = linkGlobalValueProto(SGV, ForIndirectSymbol);
IRMover.cpp:1038 NewGV = copyGlobalValueProto(SGV, ShouldLink || ForIndirectSymbol);
```
`shouldLink` returns false for the available_externally GlobalObject.

Then D28737 ensures the global_ctors entry is dropped (since the comdat object is available_externally).


================
Comment at: llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll:22
+; IMPORT2: define available_externally i32 @g() unnamed_addr [[ATTR]] {
+; IMPORT2: define available_externally dso_local void @g_internal() unnamed_addr {
 
----------------
tejohnson wrote:
> I assume the rest of the cases above (i.e. besides g_internal) already worked before this patch, including g_private since it is not local (see question about this below), is that correct?
Objects except g_internal/g_private work before this patch. g_internal/g_private work with this patch.


================
Comment at: llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll:42
 
-define linkonce_odr i32 @f(i8*) unnamed_addr comdat($c1) {
+ at g_private = global i32 43, comdat($g)
+
----------------
tejohnson wrote:
> Was this meant to have private linkage?
`g` (comdat key) is meant to have private linkage. A comdat with a local linkage key should not be used for portability.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135427