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

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.

Reviewers: mehdi_amini, pcc

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D28806

llvm-svn: 292656

diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index 0f22207..e11ad68 100644 (file)
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -164,9 +164,7 @@ static void thinLTOResolveWeakForLinkerGUID(
     }
     // 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());

diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index 4ce50b4..b8fc79a 100644 (file)
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -540,6 +540,23 @@ llvm::EmitImportsFiles(StringRef ModulePath, StringRef OutputFilename,
 /// 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;
@@ -550,18 +567,25 @@ void llvm::thinLTOResolveWeakForLinkerModule(
     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<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

diff --git a/llvm/test/ThinLTO/X86/weak_resolution.ll b/llvm/test/ThinLTO/X86/weak_resolution.ll
index 685c91c..612cd6c 100644 (file)
--- a/llvm/test/ThinLTO/X86/weak_resolution.ll
+++ b/llvm/test/ThinLTO/X86/weak_resolution.ll
@@ -53,7 +53,7 @@ entry:
 }
 ; 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 @@ 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

diff --git a/llvm/test/tools/gold/X86/Inputs/thinlto_weak_library1.ll b/llvm/test/tools/gold/X86/Inputs/thinlto_weak_library1.ll
new file mode 100644 (file)
index 0000000..319ce6b
--- /dev/null
+++ b/llvm/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
+}

diff --git a/llvm/test/tools/gold/X86/Inputs/thinlto_weak_library2.ll b/llvm/test/tools/gold/X86/Inputs/thinlto_weak_library2.ll
new file mode 100644 (file)
index 0000000..684549a
--- /dev/null
+++ b/llvm/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

diff --git a/llvm/test/tools/gold/X86/thinlto_weak_library.ll b/llvm/test/tools/gold/X86/thinlto_weak_library.ll
new file mode 100644 (file)
index 0000000..6a04fc0
--- /dev/null
+++ b/llvm/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

diff --git a/llvm/test/tools/gold/X86/thinlto_weak_resolution.ll b/llvm/test/tools/gold/X86/thinlto_weak_resolution.ll
index 8215c42..ab609cc 100644 (file)
--- a/llvm/test/tools/gold/X86/thinlto_weak_resolution.ll
+++ b/llvm/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 @@ 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
@@ -95,7 +91,7 @@ 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