[ThinLTO] Keep common symbols in ThinLTO modules

Summary:
Skip the merging of common symbols for ThinLTO modules, they will be
merged by the final native object link. Trying to merge the symbols and
add to a combined module will incorrectly enable the common symbol to be
internalized in the ThinLTO module. Additionally, we will not want to
create a combined module for ThinLTO distributed builds.

This fixes failures in 7 cpu2006 benchmarks from the new LTO API in
ThinLTO mode.

Reviewers: mehdi_amini

Subscribers: pcc, llvm-commits, mehdi_amini

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

llvm-svn: 279023

4 files changed, 59 insertions, 1 deletions

diff --git a/llvm/include/llvm/LTO/LTO.h b/llvm/include/llvm/LTO/LTO.h
index 1511047047b..96281f20617 100644
--- a/llvm/include/llvm/LTO/LTO.h
+++ b/llvm/include/llvm/LTO/LTO.h
@@ -228,6 +228,10 @@ public:
   StringRef getSourceFileName() const {
     return Obj->getModule().getSourceFileName();
   }
+
+  MemoryBufferRef getMemoryBufferRef() const {
+    return Obj->getMemoryBufferRef();
+  }
 };
 
 /// A ThinBackend defines what happens after the thin-link phase during ThinLTO.
diff --git a/llvm/test/tools/gold/X86/Inputs/common_thinlto.ll b/llvm/test/tools/gold/X86/Inputs/common_thinlto.ll
new file mode 100644
index 00000000000..f9310584340
--- /dev/null
+++ b/llvm/test/tools/gold/X86/Inputs/common_thinlto.ll
@@ -0,0 +1,13 @@
+source_filename = "common2.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@P = external global i8* (...)*, align 8
+
+; Function Attrs: nounwind uwtable
+define void @foo() #0 {
+entry:
+  %0 = load i8* (...)*, i8* (...)** @P, align 8
+  %call = call i8* (...) %0()
+  ret void
+}
diff --git a/llvm/test/tools/gold/X86/common_thinlto.ll b/llvm/test/tools/gold/X86/common_thinlto.ll
new file mode 100644
index 00000000000..b4ea181151d
--- /dev/null
+++ b/llvm/test/tools/gold/X86/common_thinlto.ll
@@ -0,0 +1,38 @@
+; RUN: opt -module-summary %s -o %t1.o
+; RUN: opt -module-summary %p/Inputs/common_thinlto.ll -o %t2.o
+
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold.so \
+; RUN:    --plugin-opt=save-temps \
+; RUN:    --plugin-opt=thinlto \
+; RUN:    -shared %t1.o %t2.o -o %t3
+
+; RUN: llvm-dis %t1.o.2.internalize.bc -o - | FileCheck %s --check-prefix=INTERNALIZE
+; We should not have internalized P, and it should still be common.
+; INTERNALIZE: @P = common global
+
+; RUN: llvm-dis %t1.o.4.opt.bc -o - | FileCheck %s --check-prefix=OPT
+; bar should still exist (if we had internalized P it would look dead).
+; OPT: @bar
+
+; RUN: llvm-nm %t3.o | FileCheck %s --check-prefix=NM
+; NM: bar
+
+source_filename = "common1.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@P = common global i8* (...)* null, align 8
+
+define i32 @main() {
+entry:
+  store i8* (...)* bitcast (i8* ()* @bar to i8* (...)*), i8* (...)** @P, align 8
+  %call = call i32 (...) @foo()
+  ret i32 0
+}
+
+declare i32 @foo(...)
+
+define internal i8* @bar() {
+entry:
+  ret i8* null
+}
diff --git a/llvm/tools/gold/gold-plugin.cpp b/llvm/tools/gold/gold-plugin.cpp
index 144a8eb90da..9ca28266d6e 100644
--- a/llvm/tools/gold/gold-plugin.cpp
+++ b/llvm/tools/gold/gold-plugin.cpp
@@ -572,6 +572,8 @@ static void addModule(LTO &Lto, claimed_file &F, const void *View) {
             toString(ObjOrErr.takeError()).c_str());
 
   InputFile &Obj = **ObjOrErr;
+  bool HasThinLTOSummary =
+      hasGlobalValueSummary(Obj.getMemoryBufferRef(), diagnosticHandler);
 
   unsigned SymNum = 0;
   std::vector<SymbolResolution> Resols(F.syms.size());
@@ -617,7 +619,8 @@ static void addModule(LTO &Lto, claimed_file &F, const void *View) {
         (IsExecutable || !Res.DefaultVisibility))
       R.FinalDefinitionInLinkageUnit = true;
 
-    if (ObjSym.getFlags() & object::BasicSymbolRef::SF_Common) {
+    if ((ObjSym.getFlags() & object::BasicSymbolRef::SF_Common) &&
+        !HasThinLTOSummary) {
       // We ignore gold's resolution for common symbols. A common symbol with
       // the correct size and alignment is added to the module by the pre-opt
       // module hook if any common symbol prevailed.