[llvm] r222727 - Fix overly aggressive type merging.
Duncan P. N. Exon Smith
dexonsmith at apple.com
Thu Nov 27 08:57:45 PST 2014
> On 2014 Nov 27, at 08:46, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>
> 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?
>
Now that I've looked at the error more closely, I'm pretty confident
this is the problem, so I'm going to revert right away to get our
builds green. Your commit makes type merging less aggressive, and
the error from the GEP could be fixed by making it more aggressive.
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!
Let me know if you need help reproducing -- although we're all on
holiday down here, so turnaround might be poor :(.
>> 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
>
More information about the llvm-commits
mailing list