Index: lib/LTO/LTO.cpp =================================================================== --- lib/LTO/LTO.cpp +++ lib/LTO/LTO.cpp @@ -164,9 +164,7 @@ } // Alias and aliasee can't be turned into available_externally. else if (!isa(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()); Index: lib/Transforms/IPO/FunctionImport.cpp =================================================================== --- lib/Transforms/IPO/FunctionImport.cpp +++ lib/Transforms/IPO/FunctionImport.cpp @@ -522,6 +522,23 @@ /// 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(&GV)) { + F->deleteBody(); + F->clearMetadata(); + } else if (GlobalVariable *V = dyn_cast(&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; @@ -532,18 +549,25 @@ auto NewLinkage = GS->second->linkage(); if (NewLinkage == GV.getLinkage()) return; - 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, + // 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 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(&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 Index: test/ThinLTO/X86/weak_resolution.ll =================================================================== --- test/ThinLTO/X86/weak_resolution.ll +++ test/ThinLTO/X86/weak_resolution.ll @@ -53,7 +53,7 @@ } ; 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 @@ -65,7 +65,7 @@ ret void } ; MOD1: define weak void @weakfunc() -; MOD2: define weak void @weakfunc() +; MOD2: declare void @weakfunc() define weak void @weakfunc() #0 { entry: ret void Index: test/tools/gold/X86/Inputs/thinlto_weak_library1.ll =================================================================== --- /dev/null +++ test/tools/gold/X86/Inputs/thinlto_weak_library1.ll @@ -0,0 +1,17 @@ +; 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 +} Index: test/tools/gold/X86/Inputs/thinlto_weak_library2.ll =================================================================== --- /dev/null +++ test/tools/gold/X86/Inputs/thinlto_weak_library2.ll @@ -0,0 +1,20 @@ +; 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 Index: test/tools/gold/X86/thinlto_weak_library.ll =================================================================== --- /dev/null +++ test/tools/gold/X86/thinlto_weak_library.ll @@ -0,0 +1,41 @@ +; 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 Index: test/tools/gold/X86/thinlto_weak_resolution.ll =================================================================== --- test/tools/gold/X86/thinlto_weak_resolution.ll +++ test/tools/gold/X86/thinlto_weak_resolution.ll @@ -13,18 +13,14 @@ ; 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" @@ -83,7 +79,7 @@ ret void } ; IMPORT: define weak void @linkoncefunc() -; IMPORT2: define linkonce void @linkoncefunc() +; IMPORT2: declare void @linkoncefunc() define linkonce void @linkoncefunc() #0 { entry: ret void @@ -95,7 +91,7 @@ ret void } ; IMPORT: define weak void @weakfunc() -; IMPORT2: define weak void @weakfunc() +; IMPORT2: declare void @weakfunc() define weak void @weakfunc() #0 { entry: ret void