[llvm] r222727 - Fix overly aggressive type merging.

[Duncan P. N. Exon Smith]

Our public bots are failing to bootstrap:

Last passing @ r222698:
http://lab.llvm.org:8080/green/job/clang-Rlto_master_build/532/

First failing @ r222843:
http://lab.llvm.org:8080/green/job/clang-Rlto_master_build/533/

Our internal bootstraps are failing too, and they somehow came up
with a much tighter range: r222725 is passing, and r222731 is
failing.

This (r222727) looks like the only possible commit in the narrow
range pointed at by our internal builders that could have caused
a problem.

Here's the failure -- LTO crashes while handling libclang.dylib:

http://lab.llvm.org:8080/green/job/clang-Rlto_master_build/533/consoleFull#-158682280549ba4694-19c4-4d7e-bec5-911270d8a58c

Do you mind if I speculatively revert?

> On 2014 Nov 24, at 21:59, Rafael Espindola <rafael.espindola at gmail.com> wrote:
> 
> Author: rafael
> Date: Mon Nov 24 23:59:24 2014
> New Revision: 222727
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=222727&view=rev
> Log:
> 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=222727&r1=222726&r2=222727&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Linker/LinkModules.cpp (original)
> +++ llvm/trunk/lib/Linker/LinkModules.cpp Mon Nov 24 23:59:24 2014
> @@ -107,12 +107,23 @@ void TypeMapTy::addTypeMapping(Type *Dst
> 
>   // 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
> 
>     // 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;
>     }
> 
> 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=222727&view=auto
> ==============================================================================
> --- llvm/trunk/test/Linker/Inputs/type-unique-opaque.ll (added)
> +++ llvm/trunk/test/Linker/Inputs/type-unique-opaque.ll Mon Nov 24 23:59: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=222727&view=auto
> ==============================================================================
> --- llvm/trunk/test/Linker/type-unique-opaque.ll (added)
> +++ llvm/trunk/test/Linker/type-unique-opaque.ll Mon Nov 24 23:59: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
> +}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits