<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jun 20, 2016 at 12:56 PM, George Burgess IV <span dir="ltr"><<a href="mailto:george.burgess.iv@gmail.com" target="_blank">george.burgess.iv@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">george.burgess.iv added a comment.<br>
<span class=""><br>
> George, can you stare at these quickly and see if any of your caching changes/etc will help?<br>
<br>
<br>
</span>That depends on what exactly is slowing the benchmarks down so much. If our usage pattern is query -> remove -> query -> remove, then our cache may become useless, since (worst case) we drop the entire thing on each removal. If we primarily query defs, then this pattern gives us the same effectively-n^2 behavior of MemDep.</blockquote><div><br></div><div><br></div><div>With the caveat that this will never occur for loads unless you remove the stores in the way of those specific loads ;)</div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> One of the big goals of the new cache/walker is to allow us to drop as little as possible.<br></blockquote><div><br></div><div>The nice part about walkers is that we  can use different walkers for different purposes.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
In terms of pure walker/cache speed, the current walker is happy to do a lot of potentially useless work walking phis we can't optimize; the one I'm working on will do as little work as possible in that case. Also, the current walker potentially does a lot of domtree queries when caching results, whereas the one I'm working on does none (except in asserts). Glancing at some of the benchmarks, I'm not sure if any of that is what's slowing us down here, though.<br>
<br>
If you'd like, I'm happy to profile/poke around and give you a more definitive answer.<br>
<br>
<br>
================<br>
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:500<br>
@@ -485,1 +499,3 @@<br>
<br>
+// Determine if the memory referenced by LaterInst is from the same heap version<br>
+// as EarlierInst.<br>
----------------<br>
Nit: Please use ///<br>
<br>
================<br>
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:571<br>
@@ +570,3 @@<br>
+      MSSAWalker->getClobberingMemoryAccess(EarlierInst);<br>
+  if (EarlierHeapGen == LaterHeapGen)<br>
+    return true;<br>
----------------<br>
Nit: `return EarlierHeapGen == LaterHeapGen`?<br>
<br>
<br>
<a href="http://reviews.llvm.org/D19821" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19821</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>