[ThinLTO] Teach ThinLTO about auto hide symbols

[steven_wu]

Summary

For symbols that has linkonce_odr linkage and unnamed_addr, it can be
auto hide by linker to avoid weak external symbols. Teach ThinLTO to
perform auto hide so it can safely promote linkonce_odr to weak symbols
without breaking this nice property.

[tejohnson]

LGTM although I have a suggestion on where to move the new code.

lib/Transforms/IPO/FunctionImport.cpp

693 ↗

(On Diff #133641)

Suggest moving the new check down into this else, which is the only condition under which it could be true, to avoid unnecessary checking.

[steven_wu]

Move the check later so it is only check when needed.

llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp

auto updateLinkage = [&](GlobalValue &GV) {
     if (GlobalValue::isAvailableExternallyLinkage(NewLinkage) &&
         GlobalValue::isInterposableLinkage(GV.getLinkage())) {
       if (!convertToDeclaration(GV))
         // FIXME: Change this to collect replaced GVs and later erase
         // them from the parent module once thinLTOResolveWeakForLinkerGUID is
         // changed to enable this for aliases.
         llvm_unreachable("Expected GV to be converted");
     } else {
+      // If the original symbols has global unnamed addr and linkonce_odr linkage,
+      // it should be an auto hide symbol. Add hidden visibility to the symbol to
+      // preserve the property.
+      if (GV.hasLinkOnceODRLinkage() && GV.hasGlobalUnnamedAddr() &&
+          NewLinkage == GlobalValue::WeakODRLinkage)
+        GV.setVisibility(GlobalValue::HiddenVisibility);
+
       DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName() << "` from "
                    << GV.getLinkage() << " to " << NewLinkage << "\n");
       GV.setLinkage(NewLinkage);
     }
     // Remove declarations from comdats, including available_externally
     // as this is a declaration for the linker, and will be dropped eventually.
     // It is illegal for comdats to contain declarations.
     auto *GO = dyn_cast_or_null<GlobalObject>(&GV);

llvm/trunk/test/ThinLTO/X86/linkonce_odr_unnamed_addr.ll

+; This test ensures that when linkonce_odr + unnamed_addr symbols promoted to
+; weak symbols, it preserves the auto hide property.
+
+; RUN: opt -module-summary %s -o %t.bc
+; RUN: opt -module-summary %s -o %t2.bc
+; RUN: llvm-lto -thinlto-action=thinlink -o %t3.bc %t.bc %t2.bc
+; RUN: llvm-lto -thinlto-action=promote %t.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s
+
+; CHECK: @linkonceodrunnamed = weak_odr hidden unnamed_addr constant i32 0
+@linkonceodrunnamed = linkonce_odr unnamed_addr constant i32 0

llvm/trunk/test/ThinLTO/X86/linkonce_resolution_comdat.ll

 ; This test ensures that we drop the preempted copy of @f from %t2.bc from its
 ; comdat after making it available_externally. If not we would get a
 ; verification error.
 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/linkonce_resolution_comdat.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto-action=run %t1.bc %t2.bc -exported-symbol=f -exported-symbol=g -thinlto-save-temps=%t3.
 
 ; RUN: llvm-dis %t3.0.3.imported.bc -o - | FileCheck %s --check-prefix=IMPORT1
 ; RUN: llvm-dis %t3.1.3.imported.bc -o - | FileCheck %s --check-prefix=IMPORT2
 ; Copy from first module is prevailing and converted to weak_odr, copy
 ; from second module is preempted and converted to available_externally and
 ; removed from comdat.
-; IMPORT1: define weak_odr i32 @f(i8*) unnamed_addr comdat($c1) {
+; IMPORT1: define weak_odr hidden i32 @f(i8*) unnamed_addr comdat($c1) {
 ; IMPORT2: define available_externally i32 @f(i8*) unnamed_addr {
 
 ; RUN: llvm-nm -o - < %t1.bc.thinlto.o | FileCheck %s --check-prefix=NM1
 ; NM1: W f
 
 ; RUN: llvm-nm -o - < %t2.bc.thinlto.o | FileCheck %s --check-prefix=NM2
 ; f() would have been turned into available_externally since it is preempted,
 ; and inlined into g()