<div dir="ltr">I like this pattern of %p/foo.ll and %p/Inputs/foo.ll<div><br></div><div>Very tidy :)</div><div><br></div><div>-- Sean Silva</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 24, 2014 at 9:59 PM, Rafael Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rafael<br>
Date: Mon Nov 24 23:59:24 2014<br>
New Revision: 222727<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=222727&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=222727&view=rev</a><br>
Log:<br>
Fix overly aggressive type merging.<br>
<br>
If we find out that two types are *not* isomorphic, we learn nothing about<br>
opaque sub types in both the source and destination.<br>
<br>
Added:<br>
    llvm/trunk/test/Linker/Inputs/type-unique-opaque.ll<br>
    llvm/trunk/test/Linker/type-unique-opaque.ll<br>
Modified:<br>
    llvm/trunk/lib/Linker/LinkModules.cpp<br>
<br>
Modified: llvm/trunk/lib/Linker/LinkModules.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=222727&r1=222726&r2=222727&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=222727&r1=222726&r2=222727&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Linker/LinkModules.cpp (original)<br>
+++ llvm/trunk/lib/Linker/LinkModules.cpp Mon Nov 24 23:59:24 2014<br>
@@ -107,12 +107,23 @@ void TypeMapTy::addTypeMapping(Type *Dst<br>
<br>
   // Check to see if these types are recursively isomorphic and establish a<br>
   // mapping between them if so.<br>
-  if (!areTypesIsomorphic(DstTy, SrcTy)) {<br>
-    // Oops, they aren't isomorphic.  Just discard this request by rolling out<br>
-    // any speculative mappings we've established.<br>
-    for (unsigned i = 0, e = SpeculativeTypes.size(); i != e; ++i)<br>
-      MappedTypes.erase(SpeculativeTypes[i]);<br>
+  if (areTypesIsomorphic(DstTy, SrcTy)) {<br>
+    SpeculativeTypes.clear();<br>
+    return;<br>
   }<br>
+<br>
+  // Oops, they aren't isomorphic. Just discard this request by rolling out<br>
+  // any speculative mappings we've established.<br>
+  unsigned Removed = 0;<br>
+  for (unsigned I = 0, E = SpeculativeTypes.size(); I != E; ++I) {<br>
+    Type *SrcTy = SpeculativeTypes[I];<br>
+    auto Iter = MappedTypes.find(SrcTy);<br>
+    auto *DstTy = dyn_cast<StructType>(Iter->second);<br>
+    if (DstTy && DstResolvedOpaqueTypes.erase(DstTy))<br>
+      Removed++;<br>
+    MappedTypes.erase(Iter);<br>
+  }<br>
+  SrcDefinitionsToResolve.resize(SrcDefinitionsToResolve.size() - Removed);<br>
   SpeculativeTypes.clear();<br>
 }<br>
<br>
@@ -147,14 +158,14 @@ bool TypeMapTy::areTypesIsomorphic(Type<br>
<br>
     // Mapping a non-opaque source type to an opaque dest.  If this is the first<br>
     // type that we're mapping onto this destination type then we succeed.  Keep<br>
-    // the dest, but fill it in later.  This doesn't need to be speculative.  If<br>
-    // this is the second (different) type that we're trying to map onto the<br>
-    // same opaque type then we fail.<br>
+    // the dest, but fill it in later. If this is the second (different) type<br>
+    // that we're trying to map onto the same opaque type then we fail.<br>
     if (cast<StructType>(DstTy)->isOpaque()) {<br>
       // We can only map one source type onto the opaque destination type.<br>
       if (!DstResolvedOpaqueTypes.insert(cast<StructType>(DstTy)).second)<br>
         return false;<br>
       SrcDefinitionsToResolve.push_back(SSTy);<br>
+      SpeculativeTypes.push_back(SrcTy);<br>
       Entry = DstTy;<br>
       return true;<br>
     }<br>
<br>
Added: llvm/trunk/test/Linker/Inputs/type-unique-opaque.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/Inputs/type-unique-opaque.ll?rev=222727&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/Inputs/type-unique-opaque.ll?rev=222727&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Linker/Inputs/type-unique-opaque.ll (added)<br>
+++ llvm/trunk/test/Linker/Inputs/type-unique-opaque.ll Mon Nov 24 23:59:24 2014<br>
@@ -0,0 +1,6 @@<br>
+%t = type { i8 }<br>
+%t2 = type { %t*, i16 }<br>
+<br>
+define %t2* @f() {<br>
+  ret %t2* null<br>
+}<br>
<br>
Added: llvm/trunk/test/Linker/type-unique-opaque.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/type-unique-opaque.ll?rev=222727&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/type-unique-opaque.ll?rev=222727&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Linker/type-unique-opaque.ll (added)<br>
+++ llvm/trunk/test/Linker/type-unique-opaque.ll Mon Nov 24 23:59:24 2014<br>
@@ -0,0 +1,16 @@<br>
+; RUN: llvm-link -S %s %p/Inputs/type-unique-opaque.ll | FileCheck %s<br>
+<br>
+; Test that a failed attempt at merging %u2 and %t2 (for the other file) will<br>
+; not cause %u and %t to get merged.<br>
+<br>
+; CHECK: %u = type opaque<br>
+; CHECK: define %u* @g() {<br>
+<br>
+%u = type opaque<br>
+%u2 = type { %u*, i8 }<br>
+<br>
+declare %u2* @f()<br>
+<br>
+define %u* @g() {<br>
+  ret %u* null<br>
+}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>