[PATCH] D19821: [EarlyCSE] Use MemorySSA if available.

George Burgess IV via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 20 12:56:01 PDT 2016


george.burgess.iv added a comment.

> George, can you stare at these quickly and see if any of your caching changes/etc will help?


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. One of the big goals of the new cache/walker is to allow us to drop as little as possible.

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.

If you'd like, I'm happy to profile/poke around and give you a more definitive answer.


================
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:500
@@ -485,1 +499,3 @@
 
+// Determine if the memory referenced by LaterInst is from the same heap version
+// as EarlierInst.
----------------
Nit: Please use ///

================
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:571
@@ +570,3 @@
+      MSSAWalker->getClobberingMemoryAccess(EarlierInst);
+  if (EarlierHeapGen == LaterHeapGen)
+    return true;
----------------
Nit: `return EarlierHeapGen == LaterHeapGen`?


http://reviews.llvm.org/D19821





More information about the llvm-commits mailing list