From d9788e9372793481a887ba21c0c773990364d611 Mon Sep 17 00:00:00 2001
From: Rafael Espindola <rafael.espindola@gmail.com>
Date: Mon, 1 Dec 2014 16:32:20 +0000
Subject: [PATCH] Partial revert of r222986.

The explicit set of destination types is not fully redundant when lazy loading
since the TypeFinder will not find types used only in function bodies.

This keeps the logic to drop the name of mapped types since it still helps
with avoiding further renaming.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@223043 91177308-0d34-0410-b5e6-96231b3b80d8
---
 include/llvm/Linker/Linker.h        |  1 +
 lib/Linker/LinkModules.cpp          | 25 ++++++++++++++++++++-----
 test/Linker/type-unique-src-type.ll | 22 ++++++++++++++++++++++
 3 files changed, 43 insertions(+), 5 deletions(-)
 create mode 100644 test/Linker/type-unique-src-type.ll

diff --git a/include/llvm/Linker/Linker.h b/include/llvm/Linker/Linker.h
index c3b55c4b635..88d41c1b0a0 100644
--- a/include/llvm/Linker/Linker.h
+++ b/include/llvm/Linker/Linker.h
@@ -46,6 +46,7 @@ public:
 private:
   void init(Module *M, DiagnosticHandlerFunction DiagnosticHandler);
   Module *Composite;
+  SmallPtrSet<StructType *, 32> IdentifiedStructTypes;
   DiagnosticHandlerFunction DiagnosticHandler;
 };
 
diff --git a/lib/Linker/LinkModules.cpp b/lib/Linker/LinkModules.cpp
index e1dd2e12d72..176df428cb6 100644
--- a/lib/Linker/LinkModules.cpp
+++ b/lib/Linker/LinkModules.cpp
@@ -58,8 +58,9 @@ class TypeMapTy : public ValueMapTypeRemapper {
   SmallPtrSet<StructType*, 16> DstResolvedOpaqueTypes;
 
 public:
-  TypeMapTy() {}
+  TypeMapTy(TypeSet &Set) : DstStructTypesSet(Set) {}
 
+  TypeSet &DstStructTypesSet;
   /// Indicate that the specified type in the destination module is conceptually
   /// equivalent to the specified type in the source module.
   void addTypeMapping(Type *DstTy, Type *SrcTy);
@@ -225,6 +226,13 @@ void TypeMapTy::linkDefinedTypeBodies() {
 }
 
 Type *TypeMapTy::get(Type *Ty) {
+#ifndef NDEBUG
+  for (auto &Pair : MappedTypes) {
+    assert(!(Pair.first != Ty && Pair.second == Ty) &&
+           "mapping to a source type");
+  }
+#endif
+
   // If we already have an entry for this type, return it.
   Type **Entry = &MappedTypes[Ty];
   if (*Entry)
@@ -310,6 +318,7 @@ Type *TypeMapTy::get(Type *Ty) {
   if (STy->isOpaque()) {
     // A named structure type from src module is used. Add it to the Set of
     // identified structs in the destination module.
+    DstStructTypesSet.insert(STy);
     return *Entry = STy;
   }
 
@@ -317,6 +326,7 @@ Type *TypeMapTy::get(Type *Ty) {
   StructType *DTy = StructType::create(STy->getContext());
   // A new identified structure type was created. Add it to the set of
   // identified structs in the destination module.
+  DstStructTypesSet.insert(DTy);
   *Entry = DTy;
 
   SmallVector<Type*, 4> ElementTypes;
@@ -402,9 +412,9 @@ class ModuleLinker {
   Linker::DiagnosticHandlerFunction DiagnosticHandler;
 
 public:
-  ModuleLinker(Module *dstM, Module *srcM,
+  ModuleLinker(Module *dstM, TypeSet &Set, Module *srcM,
                Linker::DiagnosticHandlerFunction DiagnosticHandler)
-      : DstM(dstM), SrcM(srcM),
+      : DstM(dstM), SrcM(srcM), TypeMap(Set),
         ValMaterializer(TypeMap, DstM, LazilyLinkFunctions),
         DiagnosticHandler(DiagnosticHandler) {}
 
@@ -816,7 +826,7 @@ void ModuleLinker::computeTypeMapping() {
       // we prefer to take the '%C' version. So we are then left with both
       // '%C.1' and '%C' being used for the same types. This leads to some
       // variables using one type and some using the other.
-      if (!SrcStructTypesSet.count(DST))
+      if (!SrcStructTypesSet.count(DST) && TypeMap.DstStructTypesSet.count(DST))
         TypeMap.addTypeMapping(DST, ST);
   }
 
@@ -1578,6 +1588,10 @@ bool ModuleLinker::run() {
 void Linker::init(Module *M, DiagnosticHandlerFunction DiagnosticHandler) {
   this->Composite = M;
   this->DiagnosticHandler = DiagnosticHandler;
+
+  TypeFinder StructTypes;
+  StructTypes.run(*M, true);
+  IdentifiedStructTypes.insert(StructTypes.begin(), StructTypes.end());
 }
 
 Linker::Linker(Module *M, DiagnosticHandlerFunction DiagnosticHandler) {
@@ -1599,7 +1613,8 @@ void Linker::deleteModule() {
 }
 
 bool Linker::linkInModule(Module *Src) {
-  ModuleLinker TheLinker(Composite, Src, DiagnosticHandler);
+  ModuleLinker TheLinker(Composite, IdentifiedStructTypes, Src,
+                         DiagnosticHandler);
   return TheLinker.run();
 }
 
diff --git a/test/Linker/type-unique-src-type.ll b/test/Linker/type-unique-src-type.ll
new file mode 100644
index 00000000000..45669045702
--- /dev/null
+++ b/test/Linker/type-unique-src-type.ll
@@ -0,0 +1,22 @@
+; RUN: llvm-as %s -o %t.bc
+; RUN: llvm-link -S %t.bc -o - | FileCheck %s
+; RUN: llvm-link -S %s -o - | FileCheck %s
+
+; Test that we don't try to map %C.0 and C and then try to map %C to a new type.
+; This used to happen when lazy loading since we wouldn't then identify %C
+; as a destination type until it was too late.
+
+; CHECK: %C.0 = type { %B }
+; CHECK-NEXT: %B = type { %A }
+; CHECK-NEXT: %A = type { i8 }
+; CHECK-NEXT: %C = type { %B }
+
+%A   = type { i8 }
+%B   = type { %A }
+%C   = type { %B }
+%C.0 = type { %B }
+define void @f1() {
+  getelementptr %C* null, i64 0, i32 0, i32 0
+  ret void
+}
+@g1 = external global %C.0