From 16ec00eee7e348767f4393f189044f87f6374031 Mon Sep 17 00:00:00 2001
From: Teresa Johnson <tejohnson@google.com>
Date: Fri, 1 Nov 2019 10:31:02 -0700
Subject: [PATCH] Recommit "[ThinLTO] Handle GUID collision in import global
 processing""

This recommits cc0b9647b76178bc3869bbfff80535ad86366472 which was
reverted in d39d1a2f87aca3cfabe58ecfa5146879baa70096.

I added a fix for an issue found when testing via distributed ThinLTO,
and added a test case for that failure.
---
 .../Transforms/Utils/FunctionImportUtils.cpp  | 21 ++++++++---
 .../test/ThinLTO/X86/Inputs/guid_collision.ll | 15 ++++++++
 llvm/test/ThinLTO/X86/guid_collision.ll       | 37 +++++++++++++++++++
 3 files changed, 68 insertions(+), 5 deletions(-)
 create mode 100644 llvm/test/ThinLTO/X86/Inputs/guid_collision.ll
 create mode 100644 llvm/test/ThinLTO/X86/guid_collision.ll

diff --git a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
index 76b4635ad501..5b68efbb6d8f 100644
--- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
@@ -239,11 +239,22 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) {
   // propagateConstants hasn't been run. We can't internalize GV
   // in such case.
   if (!GV.isDeclaration() && VI && ImportIndex.withGlobalValueDeadStripping()) {
-    const auto &SL = VI.getSummaryList();
-    auto *GVS = SL.empty() ? nullptr : dyn_cast<GlobalVarSummary>(SL[0].get());
-    // At this stage "maybe" is "definitely"
-    if (GVS && (GVS->maybeReadOnly() || GVS->maybeWriteOnly()))
-      cast<GlobalVariable>(&GV)->addAttribute("thinlto-internalize");
+    if (GlobalVariable *V = dyn_cast<GlobalVariable>(&GV)) {
+      // We can have more than one local with the same GUID, in the case of
+      // same-named locals in different but same-named source files that were
+      // compiled in their respective directories (so the source file name
+      // and resulting GUID is the same). Find the one in this module.
+      // Handle the case where there is no summary found in this module. That
+      // can happen in the distributed ThinLTO backend, because the index only
+      // contains summaries from the source modules if they are being imported.
+      // We might have a non-null VI and get here even in that case if the name
+      // matches one in this module (e.g. weak or appending linkage).
+      auto* GVS = dyn_cast_or_null<GlobalVarSummary>(
+          ImportIndex.findSummaryInModule(VI, M.getModuleIdentifier()));
+      // At this stage "maybe" is "definitely"
+      if (GVS && (GVS->maybeReadOnly() || GVS->maybeWriteOnly()))
+        V->addAttribute("thinlto-internalize");
+    }
   }
 
   bool DoPromote = false;
diff --git a/llvm/test/ThinLTO/X86/Inputs/guid_collision.ll b/llvm/test/ThinLTO/X86/Inputs/guid_collision.ll
new file mode 100644
index 000000000000..3d46c7b53716
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/Inputs/guid_collision.ll
@@ -0,0 +1,15 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+; The source for the GUID for this symbol will be -:F
+source_filename = "-"
+define internal fastcc i64 @F() {
+  ret i64 0
+}
+
+@llvm.global_ctors = appending global [0 x { i32, void ()*, i8* }] zeroinitializer
+
+define i64 @G() {
+  ;%1 = load i32, i32* @dummy2, align 4
+  ret i64 0
+}
diff --git a/llvm/test/ThinLTO/X86/guid_collision.ll b/llvm/test/ThinLTO/X86/guid_collision.ll
new file mode 100644
index 000000000000..5ba39b8c0ea5
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/guid_collision.ll
@@ -0,0 +1,37 @@
+; Make sure LTO succeeds even if %t.bc contains a GlobalVariable F and
+; %t2.bc cointains a Function F with the same GUID.
+;
+; RUN: opt -module-summary %s -o %t.bc
+; RUN: opt -module-summary %p/Inputs/guid_collision.ll -o %t2.bc
+; RUN: llvm-lto2 run %t.bc %t2.bc -o %t.out -save-temps \
+; RUN: -r=%t.bc,H,px -r=%t.bc,G, -r=%t2.bc,G,px
+; RUN: llvm-dis -o - %t.out.1.3.import.bc | FileCheck %s
+
+; RUN: llvm-lto2 run %t.bc %t2.bc -o %t.out -thinlto-distributed-indexes \
+; RUN: -r=%t.bc,H,px -r=%t.bc,G, -r=%t2.bc,G,px
+; RUN: opt -function-import -import-all-index -summary-file %t.bc.thinlto.bc %t.bc -o %t.out
+; RUN: llvm-dis -o - %t.out | FileCheck %s
+
+; Sanity check that G was imported
+; CHECK: define available_externally i64 @G
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+; The source for the GUID for this symbol will be -:F
+source_filename = "-"
+
+@F = internal constant i8 0
+
+; Provide a global that has the same name as one from the module we import G
+; from, to test handling of a global variable with an entry in the distributed
+; index but not with a copy in the source module (since we can't import
+; appending linkage globals).
+@llvm.global_ctors = appending global [0 x { i32, void ()*, i8* }] zeroinitializer
+
+define i64 @H() {
+  call i64 @G()
+  ret i64 0
+}
+
+declare i64 @G()