<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">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>I was worried about assigning over that many keys unnecessarily if that assignment might be slow (IE, call an assignment operator). The POD-ness is admittedly a poor proxy for this... other ideas?</div><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>