<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Mar 7, 2016, at 6:30 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Mon, Mar 7, 2016 at 1:45 AM, Mehdi Amini via llvm-commits <span dir="ltr" class=""><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>></span> wrote:<br class=""><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 class="">
> On Mar 6, 2016, at 5:31 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" class="">dexonsmith@apple.com</a>> wrote:<br class="">
><br class="">
> Sorry, I thought I'd responded back in January. This approach SGTM.<br class="">
><br class="">
> I have some ideas about field choice below. If you've profiled -flto -g<br class="">
> with something non-trivial, then I'm happy for you to commit (LGTM!), but<br class="">
> you might be able to extract even better performance.<br class="">
><br class="">
>> On 2016-Mar-05, at 23:13, Mehdi AMINI <<a href="mailto:mehdi.amini@apple.com" class="">mehdi.amini@apple.com</a>> wrote:<br class="">
>><br class="">
>> joker.eph updated this revision to Diff 49892.<br class="">
>> joker.eph updated the summary for this revision.<br class="">
>> joker.eph added a comment.<br class="">
>><br class="">
>> Change the order of arguments as suggested by Eric.<br class="">
>><br class="">
>> Duncan?<br class="">
>><br class="">
>><br class="">
>> <a href="http://reviews.llvm.org/D16571" rel="noreferrer" target="_blank" class="">http://reviews.llvm.org/D16571</a><br class="">
>><br class="">
>> Files:<br class="">
>> lib/IR/LLVMContextImpl.h<br class="">
>><br class="">
>> Index: lib/IR/LLVMContextImpl.h<br class="">
>> ===================================================================<br class="">
>> --- lib/IR/LLVMContextImpl.h<br class="">
>> +++ lib/IR/LLVMContextImpl.h<br class="">
>> @@ -365,8 +365,7 @@<br class="">
>> ExtraData == RHS->getRawExtraData();<br class="">
>> }<br class="">
>> unsigned getHashValue() const {<br class="">
>> - return hash_combine(Tag, Name, File, Line, Scope, BaseType, SizeInBits,<br class="">
>> - AlignInBits, OffsetInBits, Flags, ExtraData);<br class="">
>> + return hash_combine(Name, File, Line, Scope, BaseType);<br class="">
><br class="">
> I suspect the most important distinguishers are:<br class="">
> - Tag<br class="">
> - Name<br class="">
> - BaseType<br class="">
> - Scope<br class="">
><br class="">
> This tells you the kind of the derived type (pointer, typedef, etc.),<br class="">
> its name (if any), what it's derived from, and where it's defined. In<br class="">
> other words, I suggest dropping File/Line (which is often left blank)<br class="">
> and adding back Tag.<br class="">
><br class="">
> However, you should confirm that dropping File/Line doesn't hurt<br class="">
> performance of -flto=full.<br class="">
<br class="">
</div></div>Unfortunately it does:<br class="">
<br class="">
!317 = !DIDerivedType(tag: DW_TAG_typedef, name: "size_type", file: !234, line: 97, baseType: !268)<br class="">
!578 = !DIDerivedType(tag: DW_TAG_typedef, name: "size_type", file: !544, line: 1709, baseType: !268)<br class="">
!15757 = !DIDerivedType(tag: DW_TAG_typedef, name: "size_type", file: !15753, line: 36, baseType: !268)<br class="">
....<br class="">
<br class="">
<br class="">
These are corresponding to declaration like:<br class="">
<br class="">
typedef typename __alloc_traits::size_type size_type;<br class="">
<br class="">
in classes like std::vector_base.<br class="">
<br class="">
There are similar corner cases with the other types.<br class=""></blockquote><div class=""><br class=""></div><div class="">What's the behavior when you drop file/line from these?<br class=""><br class="">We could probably manufacture correct C++ code that still tickles those problems:<br class=""><br class="">foo.def<br class="">typedef int x;<br class=""><br class="">a.h:<br class="">struct a {<br class=""> #include "foo.def"<br class="">};<br class=""><br class="">b.h:<br class="">struct b {<br class=""> #include "bar.def"<br class="">};<br class=""><br class="">So maybe this is a bug we should fix regardless? But maybe that's difficult... </div></div></div></div></div></blockquote><div><br class=""></div><div>There's no correctness issue involved. The worst would be that more hashes would collide in the map and you need to probe longer.</div><div>(If this does not answer, I'm not sure I understood what you meant)</div><div><br class=""></div><div><br class=""></div><div>-- </div><div>Mehdi</div><div><br class=""></div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888" class=""><br class="">
--<br class="">
Mehdi<br class="">
</font></span><div class="HOEnZb"><div class="h5"><br class="">
<br class="">
<br class="">
<br class="">
><br class="">
>> }<br class="">
>> };<br class="">
>><br class="">
>> @@ -422,9 +421,7 @@<br class="">
>> Identifier == RHS->getIdentifier();<br class="">
>> }<br class="">
>> unsigned getHashValue() const {<br class="">
>> - return hash_combine(Tag, Name, File, Line, Scope, BaseType, SizeInBits,<br class="">
>> - AlignInBits, OffsetInBits, Flags, Elements, RuntimeLang,<br class="">
>> - VTableHolder, TemplateParams, Identifier);<br class="">
>> + return hash_combine(Name, File, Line, Scope, SizeInBits, Elements);<br class="">
><br class="">
> I feel like you can safely drop SizeInBits and Elements. I think it's<br class="">
> unlikely the other four fields will collide.<br class="">
><br class="">
> Again, you should profile -flto=full with -g before dropping this.<br class="">
><br class="">
>> }<br class="">
>> };<br class="">
>><br class="">
>> @@ -518,10 +515,7 @@<br class="">
>> Variables == RHS->getRawVariables();<br class="">
>> }<br class="">
>> unsigned getHashValue() const {<br class="">
>> - return hash_combine(Scope, Name, LinkageName, File, Line, Type,<br class="">
>> - IsLocalToUnit, IsDefinition, ScopeLine, ContainingType,<br class="">
>> - Virtuality, VirtualIndex, Flags, IsOptimized,<br class="">
>> - TemplateParams, Declaration, Variables);<br class="">
>> + return hash_combine(File, Line, Type, Scope);<br class="">
>> }<br class="">
>> };<br class="">
><br class="">
> It feels strange to leave both Name and LinkageName out here. I'm not<br class="">
> sure it's wrong though. As long as you've profiled this.<br class="">
><br class="">
>><br class="">
>><br class="">
>><br class="">
>> <D16571.49892.patch><br class="">
<br class="">
_______________________________________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="">
</div></div></blockquote></div><br class=""></div></div>
</div></blockquote></div><br class=""></body></html>