<div dir="ltr">FWIW: THis looks reasonable to me.<div>If you can give me the program, i'd love it.</div><div><br></div><div>I have been working on some patches to dramatically speed up memdep for gvn's use case:<br>Two possible approaches<div>1. by using quadtrees to store and lookup the nearest affecting loads/stores for a given load/store</div><div>2. by using quadtrees to store and lookup the nearest loads/stores to a given bb. A lot of memdep time is spent doing the walks, not the dependence checks.</div><div><br></div><div>Quadtrees are used because it can do "nearest" lookups *very* quickly, and what we want is the nearest loads/stores using the DFS entry/exit numbers of each statement as ordered in the dominator tree as coordinates.</div><div><br></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 29, 2014 at 5:00 PM, Robin Morisset <span dir="ltr"><<a href="mailto:morisset@google.com" target="_blank">morisset@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I don't think I know this code well enough to give the LGTM, but the general approach looks good.<br>
Also, thank you for the detailed testing/explanation.<br>
<br>
I just have a few questions/comments:<br>
- You say that there were no regressions in the test-suite. Was there any compile-time improvement ?<br>
- From my understanding of your patch, it makes the limit in GVN redundant. If so, could the limit in GVN be removed ? It looks especially brittle to have a magic number in GVN, that just happens to be the same as a named limit in MemoryDependencyAnalysis and does exactly the same job.<br>
- Have you tried different values for the limit (not super important, it was maybe already tuned before adding to GVN, but I could not find quickly its history in the log) ?<br>
<br>
nitpick: it is easier to review changes in Phabricator if all the context is included. This is normally done automatically when using arcanist, but can also be had with git/svn:<br>
git diff -U999999 other-branch<br>
svn diff --diff-cmd=diff -x -U999999<br>
(from <a href="http://llvm.org/docs/Phabricator.html" target="_blank">http://llvm.org/docs/Phabricator.html</a>)<br>
<br>
<a href="http://reviews.llvm.org/D5532" target="_blank">http://reviews.llvm.org/D5532</a><br>
<br>
<br>
<br>
_______________________________________________<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>
</blockquote></div><br></div>