[ThinLTO] Drop non-prevailing non-ODR weak to declarations

[tejohnson]

Summary

Allow non-ODR weak/linkonce non-prevailing copies to be marked
as available_externally in the index. Add support for dropping these to
declarations in the backend.

[mehdi_amini]

LGTM.

lib/LTO/LTO.cpp

for (auto &S : GVSummaryList) {
     // aspects into separate routines.
     if (isPrevailing(GUID, S.get())) {
       if (GlobalValue::isLinkOnceLinkage(OriginalLinkage))
         S->setLinkage(GlobalValue::getWeakLinkage(
             GlobalValue::isLinkOnceODRLinkage(OriginalLinkage)));
     }
     // Alias and aliasee can't be turned into available_externally.
     else if (!isa<AliasSummary>(S.get()) &&
-             !GlobalInvolvedWithAlias.count(S.get()) &&
-             (GlobalValue::isLinkOnceODRLinkage(OriginalLinkage) ||
-              GlobalValue::isWeakODRLinkage(OriginalLinkage)))
+             !GlobalInvolvedWithAlias.count(S.get()))
       S->setLinkage(GlobalValue::AvailableExternallyLinkage);
     if (S->linkage() != OriginalLinkage)
       recordNewLinkage(S->modulePath(), GUID, S->linkage());
   }
 }
 
 // Resolve Weak and LinkOnce values in the \p Index.
 //

lib/Transforms/IPO/FunctionImport.cpp

llvm::EmitImportsFiles(StringRef ModulePath, StringRef OutputFilename,
   for (auto &ILI : ModuleImports)
     ImportsOS << ILI.first() << "\n";
   return std::error_code();
 }
 
 /// Fixup WeakForLinker linkages in \p TheModule based on summary analysis.
 void llvm::thinLTOResolveWeakForLinkerModule(
     Module &TheModule, const GVSummaryMapTy &DefinedGlobals) {
+  auto ConvertToDeclaration = [](GlobalValue &GV) {
+    DEBUG(dbgs() << "Converting to a declaration: `" << GV.getName() << "\n");
+    if (Function *F = dyn_cast<Function>(&GV)) {
+      F->deleteBody();
+      F->clearMetadata();
+    } else if (GlobalVariable *V = dyn_cast<GlobalVariable>(&GV)) {
+      V->setInitializer(nullptr);
+      V->setLinkage(GlobalValue::ExternalLinkage);
+      V->clearMetadata();
+    } else
+      // For now we don't resolve or drop aliases. Once we do we'll
+      // need to add support here for creating either a function or
+      // variable declaration, and return the new GlobalValue* for
+      // the caller to use.
+      assert(false && "Expected function or variable");
+  };
+
   auto updateLinkage = [&](GlobalValue &GV) {
     if (!GlobalValue::isWeakForLinker(GV.getLinkage()))
       return;
     // See if the global summary analysis computed a new resolved linkage.
     const auto &GS = DefinedGlobals.find(GV.getGUID());
     if (GS == DefinedGlobals.end())
       return;
     auto NewLinkage = GS->second->linkage();
     if (NewLinkage == GV.getLinkage())
       return;
+    // Check for a non-prevailing def that has interposable linkage
+    // (e.g. non-odr weak or linkonce). In that case we can't simply
+    // convert to available_externally, since it would lose the
+    // interposable property and possibly get inlined. Simply drop
+    // the definition in that case.
+    if (GlobalValue::isAvailableExternallyLinkage(NewLinkage) &&
+        GlobalValue::isInterposableLinkage(GV.getLinkage()))
+      ConvertToDeclaration(GV);
+    else {
       DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName() << "` from "
                    << GV.getLinkage() << " to " << NewLinkage << "\n");
       GV.setLinkage(NewLinkage);
-    // Remove functions converted to available_externally from comdats,
+    }
+    // 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);
-    if (GO && GO->isDeclarationForLinker() && GO->hasComdat()) {
-      assert(GO->hasAvailableExternallyLinkage() &&
-             "Expected comdat on definition (possibly available external)");
+    if (GO && GO->isDeclarationForLinker() && GO->hasComdat())
       GO->setComdat(nullptr);
-    }
   };
 
   // Process functions and global now
   for (auto &GV : TheModule)
     updateLinkage(GV);
   for (auto &GV : TheModule.globals())
     updateLinkage(GV);
   for (auto &GV : TheModule.aliases())

test/ThinLTO/X86/weak_resolution.ll

 ; MOD1: define weak_odr void @linkonceodrfunc()
 ; MOD2: define available_externally void @linkonceodrfunc()
 define linkonce_odr void @linkonceodrfunc() #0 {
 entry:
   ret void
 }
 ; MOD1: define weak void @linkoncefunc()
 ; MOD1-INT: define weak void @linkoncefunc()
-; MOD2: define linkonce void @linkoncefunc()
+; MOD2: declare void @linkoncefunc()
 define linkonce void @linkoncefunc() #0 {
 entry:
   ret void
 }
 ; MOD1: define weak_odr void @weakodrfunc()
 ; MOD2: define available_externally void @weakodrfunc()
 define weak_odr void @weakodrfunc() #0 {
 entry:
   ret void
 }
 ; MOD1: define weak void @weakfunc()
-; MOD2: define weak void @weakfunc()
+; MOD2: declare void @weakfunc()
 define weak void @weakfunc() #0 {
 entry:
   ret void
 }
 
 ; MOD1: define weak_odr void @linkonceodrfuncInSingleModule()
 ; MOD1-INT: define internal void @linkonceodrfuncInSingleModule()
 ; EXPORTED: define weak_odr void @linkonceodrfuncInSingleModule()
 define linkonce_odr void @linkonceodrfuncInSingleModule() #0 {
 entry:
   ret void
 }

test/tools/gold/X86/Inputs/thinlto_weak_library1.ll

+; ModuleID = 'thinlto_weak_library1.c'
+source_filename = "thinlto_weak_library1.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: nounwind uwtable
+define weak i32 @f() local_unnamed_addr {
+entry:
+  ret i32 1
+}
+
+; Function Attrs: norecurse nounwind readnone uwtable
+define i32 @test1() local_unnamed_addr {
+entry:
+  %call = tail call i32 @f()
+  ret i32 %call
+}

test/tools/gold/X86/Inputs/thinlto_weak_library2.ll

+; ModuleID = 'thinlto_weak_library2.c'
+source_filename = "thinlto_weak_library2.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: nounwind uwtable
+define weak i32 @f() local_unnamed_addr {
+entry:
+  ret i32 2
+}
+
+; Function Attrs: nounwind uwtable
+define void @test2() local_unnamed_addr {
+entry:
+  tail call i32 (...) @test1()
+  tail call i32 @f()
+  ret void
+}
+
+declare i32 @test1(...) local_unnamed_addr

test/tools/gold/X86/thinlto_weak_library.ll

+; Test to ensure that ThinLTO sorts the modules before passing to the
+; final native link based on the linker's determination of which
+; object within a static library contains the prevailing def of a symbol.
+
+; First generate bitcode with a module summary index for each file
+; RUN: opt -module-summary %s -o %t.o
+; RUN: opt -module-summary %p/Inputs/thinlto_weak_library1.ll -o %t2.o
+; RUN: opt -module-summary %p/Inputs/thinlto_weak_library2.ll -o %t3.o
+
+; Although the objects are ordered "%t2.o %t3.o" in the library, the
+; linker selects %t3.o first since it satisfies a strong reference from
+; %t.o. It later selects %t2.o based on the strong ref from %t3.o.
+; Therefore, %t3.o's copy of @f is prevailing, and we need to link
+; %t3.o before %t2.o in the final native link.
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold.so \
+; RUN:    --plugin-opt=thinlto \
+; RUN:    --plugin-opt=save-temps \
+; RUN:    -m elf_x86_64 \
+; RUN:    -o %t4 \
+; RUN:    %t.o \
+; RUN:    --start-lib %t2.o %t3.o --end-lib
+
+; Make sure we completely dropped the definition of the non-prevailing
+; copy of f() (and didn't simply convert to available_externally, which
+; would incorrectly enable inlining).
+; RUN: llvm-dis %t2.o.1.promote.bc -o - | FileCheck %s
+; CHECK: declare i32 @f()
+
+; ModuleID = 'thinlto_weak_library.c'
+source_filename = "thinlto_weak_library.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: nounwind uwtable
+define i32 @main() local_unnamed_addr {
+entry:
+  tail call void (...) @test2()
+  ret i32 0
+}
+
+declare void @test2(...) local_unnamed_addr

test/tools/gold/X86/thinlto_weak_resolution.ll

 ; RUN: opt -module-summary %s -o %t.o
 ; RUN: opt -module-summary %p/Inputs/thinlto_weak_resolution.ll -o %t2.o
 
 ; Verify that prevailing weak for linker symbol is kept.
 ; Note that gold picks the first copy of a function as the prevailing one,
 ; so listing %t.o first is sufficient to ensure that its copies are prevailing.
 ; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold.so \
 ; RUN:     --plugin-opt=thinlto \
 ; RUN:     --plugin-opt=save-temps \
 ; RUN:     -shared \
 ; RUN:     -o %t3.o %t.o %t2.o
 
 ; RUN: llvm-nm %t3.o | FileCheck %s
 ; CHECK: weakfunc
 
-; Most of the preempted functions should have been eliminated (the plugin will
-; set linkage of odr functions to available_externally and linkonce functions
-; are removed by globaldce). FIXME: Need to introduce combined index linkage
-; that means "drop this function" so we can avoid importing linkonce functions
-; and drop weak functions.
+; The preempted functions should have been eliminated (the plugin will
+; set linkage of odr functions to available_externally, and convert
+; linkonce and weak to declarations).
 ; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck --check-prefix=OPT2 %s
 ; OPT2-NOT: @
-; OPT2: @weakfunc
-; OPT2-NOT: @
 
 ; RUN: llvm-dis %t.o.3.import.bc -o - | FileCheck --check-prefix=IMPORT %s
-; RUN llvm-dis %t2.o.3.import.bc -o - | FileCheck --check-prefix=IMPORT2 %s
+; RUN: llvm-dis %t2.o.3.import.bc -o - | FileCheck --check-prefix=IMPORT2 %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 
 define i32 @main() #0 {
 entry:
   call void @linkonceodralias()

 
 ; IMPORT: define weak_odr void @linkonceodrfunc()
 ; IMPORT2: define available_externally void @linkonceodrfunc()
 define linkonce_odr void @linkonceodrfunc() #0 {
 entry:
   ret void
 }
 ; IMPORT: define weak void @linkoncefunc()
-; IMPORT2: define linkonce void @linkoncefunc()
+; IMPORT2: declare void @linkoncefunc()
 define linkonce void @linkoncefunc() #0 {
 entry:
   ret void
 }
 ; IMPORT: define weak_odr void @weakodrfunc()
 ; IMPORT2: define available_externally void @weakodrfunc()
 define weak_odr void @weakodrfunc() #0 {
 entry:
   ret void
 }
 ; IMPORT: define weak void @weakfunc()
-; IMPORT2: define weak void @weakfunc()
+; IMPORT2: declare void @weakfunc()
 define weak void @weakfunc() #0 {
 entry:
   ret void
 }
 
 ; IMPORT: weak_odr void @linkonceodrfuncInSingleModule()
 define linkonce_odr void @linkonceodrfuncInSingleModule() #0 {
 entry:
   ret void
 }