[llvm] r222895 - Revert "Fix overly aggressive type merging."

[Rafael Espíndola]

Thanks. I was able to reproduce this with a bootstrap on OS X. I am
currently reducing it.

On 27 November 2014 at 12:01, Duncan P. N. Exon Smith
<dexonsmith at apple.com> wrote:
> Author: dexonsmith
> Date: Thu Nov 27 11:01:10 2014
> New Revision: 222895
>
> URL: http://llvm.org/viewvc/llvm-project?rev=222895&view=rev
> Log:
> Revert "Fix overly aggressive type merging."
>
> This reverts commit r222727, which causes LTO bootstrap failures.
>
> 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/
>
> Internal bootstraps pointed at a much narrower range: r222725 is
> passing, and r222731 is failing.
>
> LTO crashes while handling libclang.dylib:
> http://lab.llvm.org:8080/green/job/clang-Rlto_master_build/533/consoleFull#-158682280549ba4694-19c4-4d7e-bec5-911270d8a58c
>
>     GEP is not of right type for indices!
>       %InfoObj.i.i = getelementptr inbounds %"class.llvm::OnDiskIterableChainedHashTable"* %.lcssa, i64 0, i32 0, i32 4, !dbg !123627
>      %"class.clang::serialization::reader::ASTIdentifierLookupTrait" = type { %"class.clang::ASTReader.31859"*, %"class.clang::serialization::ModuleFile.31870"*, %"class.clang::IdentifierInfo"* }LLVM ERROR: Broken function found, compilation aborted!
>     clang: error: linker command failed with exit code 1 (use -v to see invocation)
>
> Looks like the new algorithm doesn't merge types aggressively enough.
>
> Removed:
>     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=222895&r1=222894&r2=222895&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Linker/LinkModules.cpp (original)
> +++ llvm/trunk/lib/Linker/LinkModules.cpp Thu Nov 27 11:01:10 2014
> @@ -96,23 +96,12 @@ private:
>  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)) {
> -    SpeculativeTypes.clear();
> -    return;
> +  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]);
>    }
> -
> -  // 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();
>  }
>
> @@ -148,14 +137,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. 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.  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.
>      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;
>      }
>
> Removed: 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=222894&view=auto
> ==============================================================================
> --- llvm/trunk/test/Linker/Inputs/type-unique-opaque.ll (original)
> +++ llvm/trunk/test/Linker/Inputs/type-unique-opaque.ll (removed)
> @@ -1,6 +0,0 @@
> -%t = type { i8 }
> -%t2 = type { %t*, i16 }
> -
> -define %t2* @f() {
> -  ret %t2* null
> -}
>
> Removed: llvm/trunk/test/Linker/type-unique-opaque.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/type-unique-opaque.ll?rev=222894&view=auto
> ==============================================================================
> --- llvm/trunk/test/Linker/type-unique-opaque.ll (original)
> +++ llvm/trunk/test/Linker/type-unique-opaque.ll (removed)
> @@ -1,16 +0,0 @@
> -; 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