From 86911440c2af76382db3a5354eb3acd76d8e927e Mon Sep 17 00:00:00 2001
From: Rafael Espindola <rafael.espindola@gmail.com>
Date: Tue, 25 Nov 2014 05:59:24 +0000
Subject: [PATCH] Fix overly aggressive type merging.

If we find out that two types are *not* isomorphic, we learn nothing about
opaque sub types in both the source and destination.

llvm-svn: 222727
---
 llvm/lib/Linker/LinkModules.cpp               | 27 +++++++++++++++++++--------
 llvm/test/Linker/Inputs/type-unique-opaque.ll |  6 ++++++
 llvm/test/Linker/type-unique-opaque.ll        | 16 ++++++++++++++++
 3 files changed, 41 insertions(+), 8 deletions(-)
 create mode 100644 llvm/test/Linker/Inputs/type-unique-opaque.ll
 create mode 100644 llvm/test/Linker/type-unique-opaque.ll

diff --git a/llvm/lib/Linker/LinkModules.cpp b/llvm/lib/Linker/LinkModules.cpp
index a8dc324..e57a37f 100644
--- a/llvm/lib/Linker/LinkModules.cpp
+++ b/llvm/lib/Linker/LinkModules.cpp
@@ -107,12 +107,23 @@ void TypeMapTy::addTypeMapping(Type *DstTy, Type *SrcTy) {
 
   // Check to see if these types are recursively isomorphic and establish a
   // mapping between them if so.
-  if (!areTypesIsomorphic(DstTy, SrcTy)) {
-    // Oops, they aren't isomorphic.  Just discard this request by rolling out
-    // any speculative mappings we've established.
-    for (unsigned i = 0, e = SpeculativeTypes.size(); i != e; ++i)
-      MappedTypes.erase(SpeculativeTypes[i]);
+  if (areTypesIsomorphic(DstTy, SrcTy)) {
+    SpeculativeTypes.clear();
+    return;
+  }
+
+  // Oops, they aren't isomorphic. Just discard this request by rolling out
+  // any speculative mappings we've established.
+  unsigned Removed = 0;
+  for (unsigned I = 0, E = SpeculativeTypes.size(); I != E; ++I) {
+    Type *SrcTy = SpeculativeTypes[I];
+    auto Iter = MappedTypes.find(SrcTy);
+    auto *DstTy = dyn_cast<StructType>(Iter->second);
+    if (DstTy && DstResolvedOpaqueTypes.erase(DstTy))
+      Removed++;
+    MappedTypes.erase(Iter);
   }
+  SrcDefinitionsToResolve.resize(SrcDefinitionsToResolve.size() - Removed);
   SpeculativeTypes.clear();
 }
 
@@ -147,14 +158,14 @@ bool TypeMapTy::areTypesIsomorphic(Type *DstTy, Type *SrcTy) {
 
     // Mapping a non-opaque source type to an opaque dest.  If this is the first
     // type that we're mapping onto this destination type then we succeed.  Keep
-    // the dest, but fill it in later.  This doesn't need to be speculative.  If
-    // this is the second (different) type that we're trying to map onto the
-    // same opaque type then we fail.
+    // the dest, but fill it in later. If this is the second (different) type
+    // that we're trying to map onto the same opaque type then we fail.
     if (cast<StructType>(DstTy)->isOpaque()) {
       // We can only map one source type onto the opaque destination type.
       if (!DstResolvedOpaqueTypes.insert(cast<StructType>(DstTy)).second)
         return false;
       SrcDefinitionsToResolve.push_back(SSTy);
+      SpeculativeTypes.push_back(SrcTy);
       Entry = DstTy;
       return true;
     }
diff --git a/llvm/test/Linker/Inputs/type-unique-opaque.ll b/llvm/test/Linker/Inputs/type-unique-opaque.ll
new file mode 100644
index 0000000..872b601
--- /dev/null
+++ b/llvm/test/Linker/Inputs/type-unique-opaque.ll
@@ -0,0 +1,6 @@
+%t = type { i8 }
+%t2 = type { %t*, i16 }
+
+define %t2* @f() {
+  ret %t2* null
+}
diff --git a/llvm/test/Linker/type-unique-opaque.ll b/llvm/test/Linker/type-unique-opaque.ll
new file mode 100644
index 0000000..b4f6966
--- /dev/null
+++ b/llvm/test/Linker/type-unique-opaque.ll
@@ -0,0 +1,16 @@
+; RUN: llvm-link -S %s %p/Inputs/type-unique-opaque.ll | FileCheck %s
+
+; Test that a failed attempt at merging %u2 and %t2 (for the other file) will
+; not cause %u and %t to get merged.
+
+; CHECK: %u = type opaque
+; CHECK: define %u* @g() {
+
+%u = type opaque
+%u2 = type { %u*, i8 }
+
+declare %u2* @f()
+
+define %u* @g() {
+  ret %u* null
+}
-- 
2.7.4