<div dir="ltr">Hi Adrian,<div><br></div><div>After this commit, there's sometimes assert fail on large bitcodes on my environment.</div><div>According to my result, two types seems the same, and I take out the assert fail can still build and run.</div>
<div>Is any possible that the contained MDNode is not the same (since it is a pointer comparison indeed) but this is reasonable?</div><div>Thanks!</div><div><br></div><div><br></div><div>Below is the detail information:</div>
<div><br></div><div>I try to dump, and it shows: (The contents are the same)</div><div><br></div><div><div>Ty:</div><div>[ DW_TAG_class_type ] [Vector2<int>] [line 12, size 64, align 32, offset 0] [def] [from ]</div>
<div>resolve(Ty.getRef()):</div><div>[ DW_TAG_class_type ] [Vector2<int>] [line 12, size 64, align 32, offset 0] [def] [from ]</div></div><div><br></div><div>and I dump the MDNode</div><div><br></div><div><div>!833 = metadata !{i32 786434, metadata !834, metadata !59, metadata !"list<Vector2<int>, std::allocator<Vector2<int> > >", i32 438, i64 128, i64 64, i32 0, i32 0, null, metadata !835, i32 0, null, metadata !970, metadata !"_ZTSSt4listI7Vector2IiESaIS1_EE"} ; [ DW_TAG_class_type ] [list<Vector2<int>, std::allocator<Vector2<int> > >] [line 438, size 128, align 64, offset 0] [def] [from ]</div>
</div><div><br></div><div>_ZTSSt4listI7Vector2IiESaIS1_EE is "typeinfo name of ..."<br></div><div><br></div><div>In other cases, the error is on the "typeinfo name of ..." either.</div><div><br></div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-03-25 5:39 GMT+08:00 Adrian Prantl <span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
On Mar 24, 2014, at 2:08 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<br>
<br>
> On Mon, Mar 17, 2014 at 7:53 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>> On Mon, Mar 17, 2014 at 7:35 PM, Adrian Prantl <<a href="mailto:aprantl@apple.com">aprantl@apple.com</a>> wrote:<br>
>>> Author: adrian<br>
>>> Date: Mon Mar 17 21:35:03 2014<br>
>>> New Revision: 204107<br>
>>><br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=204107&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=204107&view=rev</a><br>
>>> Log:<br>
>>> DwarfDebug: Only unique retained types instead of all types.<br>
>>> This is a follow-up to r203983 based on feedback from dblaikie and mren (Thanks!)<br>
>>> No functionality change.<br>
>><br>
>> Thanks!<br>
>><br>
>>><br>
>>> Modified:<br>
>>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>
>>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp<br>
>>><br>
>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=204107&r1=204106&r2=204107&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=204107&r1=204106&r2=204107&view=diff</a><br>
>>> ==============================================================================<br>
>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)<br>
>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Mon Mar 17 21:35:03 2014<br>
>>> @@ -846,8 +846,13 @@ void DwarfDebug::beginModule() {<br>
>>> for (unsigned i = 0, e = EnumTypes.getNumElements(); i != e; ++i)<br>
>>> CU->getOrCreateTypeDIE(EnumTypes.getElement(i));<br>
>>> DIArray RetainedTypes = CUNode.getRetainedTypes();<br>
>>> - for (unsigned i = 0, e = RetainedTypes.getNumElements(); i != e; ++i)<br>
>>> - CU->getOrCreateTypeDIE(RetainedTypes.getElement(i));<br>
>>> + for (unsigned i = 0, e = RetainedTypes.getNumElements(); i != e; ++i) {<br>
>>> + DIType Ty(RetainedTypes.getElement(i));<br>
>>> + // The retained types array by design contains pointers to<br>
>>> + // MDNodes rather than DIRefs. Unique them here.<br>
>>> + DIType UniqueTy(resolve(Ty.getRef()));<br>
>>> + CU->getOrCreateTypeDIE(UniqueTy);<br>
>><br>
>> One thought I've had here: should we, rather than resolving the<br>
>> references and emitting them, avoid emitting the type if the resolved<br>
>> version isn't from this compilation unit?<br>
>><br>
>> I'm not sure there's a good reason here, but I'm just tossing it<br>
>> around in my head - I wonder if we could get into confusing situations<br>
>> (with types as you've described in other recent threads - where two<br>
>> ODR-correct definitions, written in different files, may end up<br>
>> emitted under the "wrong" compilation unit). I'm not sure if this<br>
>> would actually be bad - but, for example, with my line table work it<br>
>> might be a tiny bit more verbose to emit a type from the wrong<br>
>> compilation unit since it wouldn't take advantage of the comp_dir as<br>
>> much (if the type was actually in a header in that directory) or might<br>
>> introduce an extra file name in the line table that would otherwise<br>
>> have been avoided.<br>
>><br>
>> This is ultra low priority - just sanity checking to see if maybe<br>
>> there are issues with emitting the type from the wrong unit.<br>
>><br>
><br>
> I think you're correct here. The way the code is written we're adding<br>
> retained types as we go over the compile unit. We should probably do a<br>
> check if the current compile unit is the same as the compile unit for<br>
> the type that we're about to emit since the type merging would have<br>
> put it in doubt which compile unit a retained type actually belonged<br>
> to.<br>
><br>
>>> + }<br>
>>> // Emit imported_modules last so that the relevant context is already<br>
>>> // available.<br>
>>> for (unsigned i = 0, e = ImportedEntities.getNumElements(); i != e; ++i)<br>
>>><br>
>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp<br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=204107&r1=204106&r2=204107&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=204107&r1=204106&r2=204107&view=diff</a><br>
>>> ==============================================================================<br>
>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (original)<br>
>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp Mon Mar 17 21:35:03 2014<br>
>>> @@ -971,6 +971,8 @@ DIE *DwarfUnit::getOrCreateTypeDIE(const<br>
>>><br>
>>> DIType Ty(TyNode);<br>
>>> assert(Ty.isType());<br>
>>> + assert(*&Ty == resolve(Ty.getRef()) &&<br>
>><br>
>> Why's the "*&Ty" required? That seems a bit strange/subtle...<br>
>><br>
><br>
> Agreed. Adrian?<br>
<br>
</div></div>Absolutely. Removed in r204673.<br>
<span class="HOEnZb"><font color="#888888"><br>
-- adrian<br>
</font></span><div class="HOEnZb"><div class="h5">_______________________________________________<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>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr">Best Regards,<div>WenHan Gu (Nowar)</div></div>
</div>