<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Aug 7, 2017 at 11:54 AM Chandler Carruth <<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Aug 7, 2017 at 11:33 AM David Blaikie via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Sat, Aug 5, 2017 at 3:49 PM Chandler Carruth via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: chandlerc<br>
Date: Sat Aug 5 15:48:37 2017<br>
New Revision: 310189<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=310189&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=310189&view=rev</a><br>
Log:<br>
[ADT] Add a much simpler loop to DenseMap::clear when the types are<br>
POD-like and we can just splat the empty key across memory.<br>
<br>
Sadly we can't optimize the normal loop well enough because we can't<br>
turn the conditional store into an unconditional store according to the<br>
memory model.<br>
<br>
This loop actually showed up in a profile of code that was calling clear<br>
as a serious source of time. =[<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/ADT/DenseMap.h<br>
<br>
Modified: llvm/trunk/include/llvm/ADT/DenseMap.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/DenseMap.h?rev=310189&r1=310188&r2=310189&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/DenseMap.h?rev=310189&r1=310188&r2=310189&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ADT/DenseMap.h (original)<br>
+++ llvm/trunk/include/llvm/ADT/DenseMap.h Sat Aug 5 15:48:37 2017<br>
@@ -107,17 +107,23 @@ public:<br>
}<br>
<br>
const KeyT EmptyKey = getEmptyKey(), TombstoneKey = getTombstoneKey();<br>
- unsigned NumEntries = getNumEntries();<br>
- for (BucketT *P = getBuckets(), *E = getBucketsEnd(); P != E; ++P) {<br>
- if (!KeyInfoT::isEqual(P->getFirst(), EmptyKey)) {<br>
- if (!KeyInfoT::isEqual(P->getFirst(), TombstoneKey)) {<br>
- P->getSecond().~ValueT();<br>
- --NumEntries;<br>
- }<br>
+ if (isPodLike<KeyT>::value && isPodLike<ValueT>::value) {<br></blockquote></div></div><div dir="ltr"><div class="gmail_quote"><div><br>What if these conditionals ^ were rolled into the general form of the loop in the else block? So that the dtor wouldn't be invoked for pod value types, even if the key wasn't pod - and the key would be assigned to EmptyKey without an isEqual check if the key is pod even if the value isn't.<br></div></div></div></blockquote><div><br></div></div></div><div dir="ltr"><div class="gmail_quote"><div>I was worried about assigning over that many keys unnecessarily if that assignment might be slow (IE, call an assignment operator).</div></div></div></blockquote></div><div dir="ltr"><div class="gmail_quote"><div><br>Not sure I quite follow how this concern follows from my question/suggestion - perhaps I missed a step or two, or my description was poorly worded?<br><br>Here's what I was thinking:<br> <br></div><div>... And as I write that out (I didn't actually get to any of the modifications/suggestions) I see that if the value type is non-pod then the key must be tested for empty/tombstone anyway, so my suggestion wouldn't save much in that case (it'd save the empty comparison if the tombstone comparison was true, if they key is pod).<br><br></div></div></div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> The POD-ness is admittedly a poor proxy for this... other ideas?</div></div></div></blockquote></div></div><div dir="ltr"><div class="gmail_quote"><div><br>is_trivially_assignable?<br> </div></div></div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> </div></div></div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ // Use a simpler loop when these are trivial types.<br>
+ for (BucketT *P = getBuckets(), *E = getBucketsEnd(); P != E; ++P)<br>
P->getFirst() = EmptyKey;<br>
+ } else {<br>
+ unsigned NumEntries = getNumEntries();<br>
+ for (BucketT *P = getBuckets(), *E = getBucketsEnd(); P != E; ++P) {<br>
+ if (!KeyInfoT::isEqual(P->getFirst(), EmptyKey)) {<br>
+ if (!KeyInfoT::isEqual(P->getFirst(), TombstoneKey)) {<br>
+ P->getSecond().~ValueT();<br>
+ --NumEntries;<br>
+ }<br>
+ P->getFirst() = EmptyKey;<br>
+ }<br>
}<br>
+ assert(NumEntries == 0 && "Node count imbalance!");<br>
}<br>
- assert(NumEntries == 0 && "Node count imbalance!");<br>
setNumEntries(0);<br>
setNumTombstones(0);<br>
}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></blockquote></div></div></div>