[PATCH] [MemoryDependenceAnalysis] Fix compile time slowdown

Robin Morisset morisset at google.com
Mon Sep 29 17:00:35 PDT 2014


I don't think I know this code well enough to give the LGTM, but the general approach looks good.
Also, thank you for the detailed testing/explanation.

I just have a few questions/comments:
- You say that there were no regressions in the test-suite. Was there any compile-time improvement ?
- 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.
- 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) ?

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:
git diff -U999999 other-branch
svn diff --diff-cmd=diff -x -U999999
(from http://llvm.org/docs/Phabricator.html)

http://reviews.llvm.org/D5532






More information about the llvm-commits mailing list