Index: lib/Transforms/IPO/FunctionImport.cpp =================================================================== --- lib/Transforms/IPO/FunctionImport.cpp +++ lib/Transforms/IPO/FunctionImport.cpp @@ -489,6 +489,15 @@ DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName() << "` from " << GV.getLinkage() << " to " << NewLinkage << "\n"); GV.setLinkage(NewLinkage); + // Remove functions imported as available externally defs from comdats, + // 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)"); + GO->setComdat(nullptr); + } }; // Process functions and global now Index: test/tools/gold/X86/Inputs/thinlto_alias.ll =================================================================== --- test/tools/gold/X86/Inputs/thinlto_alias.ll +++ test/tools/gold/X86/Inputs/thinlto_alias.ll @@ -1,5 +1,16 @@ -target triple = "x86_64-unknown-linux-gnu" -define weak void @weakfunc() { -entry: - ret void +$c2 = comdat any + +define weak_odr i32 @f1(i8*) unnamed_addr comdat($c2) { + ret i32 40 +} + +define linkonce_odr i32 @f2(i8*) unnamed_addr comdat($c2) { + ret i32 41 +} + +@r2 = global i32(i8*)* @f1 +@a2 = alias i32(i8*), i32(i8*)* @f1 +define i32 @g() { + %i = call i32 @f2(i8* null) + ret i32 %i } Index: test/tools/gold/X86/thinlto_alias.ll =================================================================== --- test/tools/gold/X86/thinlto_alias.ll +++ test/tools/gold/X86/thinlto_alias.ll @@ -1,30 +1,38 @@ ; RUN: opt -module-summary %s -o %t.o ; RUN: opt -module-summary %p/Inputs/thinlto_alias.ll -o %t2.o -; Ensure that a preempted weak symbol that is linked in as a local -; copy is handled properly. Specifically, the local copy will be promoted, -; and internalization should be able to use the original non-promoted -; name to locate the summary (otherwise internalization will abort because -; it expects to locate summaries for all definitions). -; Note that gold picks the first copy of weakfunc() as the prevailing one, -; so listing %t2.o first is sufficient to ensure that this copy is -; preempted. ; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold.so \ +; RUN: -shared \ ; RUN: --plugin-opt=thinlto \ +; RUN: --plugin-opt=-import-instr-limit=0 \ ; RUN: --plugin-opt=save-temps \ -; RUN: -o %t3.o %t2.o %t.o +; RUN: -o %t3.o %t.o %t2.o + +; Check that we have weak f1/f2 symbols ; RUN: llvm-nm %t3.o | FileCheck %s +; CHECK-DAG: W f1 +; CHECK-DAG: W f2 + +; The %t.o copies are prevailing, and should both now be weak_odr. ; RUN: llvm-dis %t.o.opt.bc -o - | FileCheck --check-prefix=OPT %s +; OPT: define weak_odr i32 @f1(i8*) unnamed_addr #0 comdat($c1) { +; OPT: define weak_odr i32 @f2(i8*) unnamed_addr #0 comdat($c1) { + +; The %t2.o copies are preempted, but we have to keep the weak_odr f1 since +; it is referenced by an alias. The linkonce_odr f2 should be inlined and +; eliminated. ; RUN: llvm-dis %t2.o.opt.bc -o - | FileCheck --check-prefix=OPT2 %s +; OPT2-NOT: @f2 +; OPT2: define weak_odr i32 @f1(i8*) unnamed_addr #0 comdat($c2) { -; CHECK-NOT: U f -; OPT: define hidden void @weakfunc.llvm.0() -; OPT2: define weak void @weakfunc() +$c1 = comdat any -target triple = "x86_64-unknown-linux-gnu" +define weak_odr i32 @f1(i8*) unnamed_addr comdat($c1) { + ret i32 42 +} -@weakfuncAlias = alias void (...), bitcast (void ()* @weakfunc to void (...)*) -define weak void @weakfunc() { -entry: - ret void +define linkonce_odr i32 @f2(i8*) unnamed_addr comdat($c1) { + ret i32 43 } + +@a1 = alias i32 (i8*), i32 (i8*)* @f1 Index: test/tools/gold/X86/thinlto_linkonceresolution.ll =================================================================== --- test/tools/gold/X86/thinlto_linkonceresolution.ll +++ test/tools/gold/X86/thinlto_linkonceresolution.ll @@ -1,8 +1,8 @@ ; RUN: opt -module-summary %s -o %t.o ; RUN: opt -module-summary %p/Inputs/thinlto_linkonceresolution.ll -o %t2.o -; Ensure the plugin ensures that for ThinLTO the prevailing copy of a -; linkonce symbol is changed to weak to ensure it is not eliminated. +; Ensure that for ThinLTO the prevailing copy of a linkonce symbol is +; changed to weak so that it is not eliminated. ; Note that gold picks the first copy of f() as the prevailing one, ; so listing %t2.o first is sufficient to ensure that this copy is ; preempted. Also, set the import-instr-limit to 0 to prevent f() from @@ -18,10 +18,12 @@ ; RUN: llvm-dis %t2.o.opt.bc -o - | FileCheck --check-prefix=OPT2 %s ; Ensure that f() is defined in resulting object file, and also -; confirm the weak linkage directly in the saved opt bitcode files. +; confirm the weak linkage directly in the saved opt bitcode file. ; CHECK-NOT: U f -; OPT: declare hidden void @f() ; OPT2: define weak_odr hidden void @f() +; The preempted copy of f() should be dropped after inlining into g(), +; as it would have been changed to available_externally. +; OPT-NOT: @f() target triple = "x86_64-unknown-linux-gnu" define i32 @g() { 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,11 +13,6 @@ ; RUN: llvm-nm %t3.o | FileCheck %s ; CHECK: weakfunc -; All of the preempted functions should have been eliminated (the plugin will -; not link them in). -; RUN: llvm-dis %t2.o.opt.bc -o - | FileCheck --check-prefix=OPT2 %s -; OPT2-NOT: @ - ; RUN: llvm-dis %t.o.opt.bc -o - | FileCheck --check-prefix=OPT %s target triple = "x86_64-unknown-linux-gnu" Index: tools/gold/gold-plugin.cpp =================================================================== --- tools/gold/gold-plugin.cpp +++ tools/gold/gold-plugin.cpp @@ -703,7 +703,8 @@ getModuleForFile(LLVMContext &Context, claimed_file &F, const void *View, StringRef Name, raw_fd_ostream *ApiFile, StringSet<> &Internalize, std::vector &Keep, - StringMap &Realign) { + StringMap &Realign, + bool KeepPreempted = false) { MemoryBufferRef BufferRef(StringRef((const char *)View, F.filesize), Name); ErrorOr> ObjOrErr = object::IRObjectFile::create(BufferRef, Context); @@ -793,8 +794,12 @@ case LDPR_RESOLVED_IR: case LDPR_RESOLVED_EXEC: case LDPR_RESOLVED_DYN: + break; + case LDPR_PREEMPTED_IR: case LDPR_PREEMPTED_REG: + if (KeepPreempted) + Keep.push_back(GV); break; case LDPR_UNDEF: @@ -1153,11 +1158,12 @@ static void linkInModule(LLVMContext &Context, IRMover &L, claimed_file &F, const void *View, StringRef Name, raw_fd_ostream *ApiFile, StringSet<> &Internalize, - bool SetName = false) { + bool SetName = false, bool KeepPreempted = false) { std::vector Keep; StringMap Realign; std::unique_ptr M = getModuleForFile(Context, F, View, Name, ApiFile, - Internalize, Keep, Realign); + Internalize, Keep, Realign, + KeepPreempted); if (!M.get()) return; if (!options::triple.empty()) @@ -1208,7 +1214,12 @@ IRMover L(*NewModule.get()); StringSet<> Dummy; - linkInModule(Context, L, F, View, Name, ApiFile, Dummy, true); + // Keep preempted symbols to enable linkonce_odr inlining within this + // module, and to ensure that the module matches the index summaries + // generated earlier for correct internalization, etc. Preempted symbols + // will be dropped as possible during thinLTOResolveWeakForLinkerModule. + linkInModule(Context, L, F, View, Name, ApiFile, Dummy, true, + /* KeepPreempted = */ true); if (renameModuleForThinLTO(*NewModule, CombinedIndex)) message(LDPL_FATAL, "Failed to rename module for ThinLTO");