[llvm] r222923 - Add back r222727 with a fix.

Rafael Espindola rafael.espindola at gmail.com
Fri Nov 28 08:41:24 PST 2014


Author: rafael
Date: Fri Nov 28 10:41:24 2014
New Revision: 222923

URL: http://llvm.org/viewvc/llvm-project?rev=222923&view=rev
Log:
Add back r222727 with a fix.

The original patch would fail when:

* A dst opaque type (%A) is matched with a src type (%A).
* A src opaque (%E) type is then speculatively matched with %A and the
  speculation fails afterward.
* When rolling back the speculation we would cancel the source %A to dest
  %A mapping.

The fix is to keep an explicit list of which resolutions are speculative.

Original message:

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.

Added:
    llvm/trunk/test/Linker/Inputs/type-unique-opaque.ll
    llvm/trunk/test/Linker/type-unique-opaque.ll
Modified:
    llvm/trunk/lib/Linker/LinkModules.cpp

Modified: llvm/trunk/lib/Linker/LinkModules.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=222923&r1=222922&r2=222923&view=diff
==============================================================================
--- llvm/trunk/lib/Linker/LinkModules.cpp (original)
+++ llvm/trunk/lib/Linker/LinkModules.cpp Fri Nov 28 10:41:24 2014
@@ -47,6 +47,8 @@ class TypeMapTy : public ValueMapTypeRem
   /// roll back.
   SmallVector<Type*, 16> SpeculativeTypes;
 
+  SmallVector<StructType*, 16> SpeculativeDstOpaqueTypes;
+
   /// This is a list of non-opaque structs in the source module that are mapped
   /// to an opaque struct in the destination module.
   SmallVector<StructType*, 16> SrcDefinitionsToResolve;
@@ -95,6 +97,7 @@ private:
 
 void TypeMapTy::addTypeMapping(Type *DstTy, Type *SrcTy) {
   assert(SpeculativeTypes.empty());
+  assert(SpeculativeDstOpaqueTypes.empty());
 
   // Check to see if these types are recursively isomorphic and establish a
   // mapping between them if so.
@@ -103,8 +106,14 @@ void TypeMapTy::addTypeMapping(Type *Dst
     // any speculative mappings we've established.
     for (Type *Ty : SpeculativeTypes)
       MappedTypes.erase(Ty);
+
+    SrcDefinitionsToResolve.resize(SrcDefinitionsToResolve.size() -
+                                   SpeculativeDstOpaqueTypes.size());
+    for (StructType *Ty : SpeculativeDstOpaqueTypes)
+      DstResolvedOpaqueTypes.erase(Ty);
   }
   SpeculativeTypes.clear();
+  SpeculativeDstOpaqueTypes.clear();
 }
 
 /// Recursively walk this pair of types, returning true if they are isomorphic,
@@ -139,14 +148,15 @@ bool TypeMapTy::areTypesIsomorphic(Type
 
     // 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);
+      SpeculativeDstOpaqueTypes.push_back(cast<StructType>(DstTy));
       Entry = DstTy;
       return true;
     }

Added: llvm/trunk/test/Linker/Inputs/type-unique-opaque.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/Inputs/type-unique-opaque.ll?rev=222923&view=auto
==============================================================================
--- llvm/trunk/test/Linker/Inputs/type-unique-opaque.ll (added)
+++ llvm/trunk/test/Linker/Inputs/type-unique-opaque.ll Fri Nov 28 10:41:24 2014
@@ -0,0 +1,6 @@
+%t = type { i8 }
+%t2 = type { %t*, i16 }
+
+define %t2* @f() {
+  ret %t2* null
+}

Added: llvm/trunk/test/Linker/type-unique-opaque.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/type-unique-opaque.ll?rev=222923&view=auto
==============================================================================
--- llvm/trunk/test/Linker/type-unique-opaque.ll (added)
+++ llvm/trunk/test/Linker/type-unique-opaque.ll Fri Nov 28 10:41:24 2014
@@ -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
+}





More information about the llvm-commits mailing list