[PATCH] D8688: Update MergedLoadStoreMotion to use MemorySSA

Daniel Berlin via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 22 12:54:28 PDT 2016


dberlin added inline comments.

================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:757
@@ -606,1 +756,3 @@
+  PA.preserve<DominatorTreeAnalysis>();
+  PA.preserve<MemorySSAAnalysis>();
   return PA;
----------------
gberry wrote:
> Isn't MemorySSA only preserved if 'UseMemorySSA' is true?
Yes, fixed.


================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:982
@@ +981,3 @@
+  DomTreeNode *StartNode = DT->getNode(Start->getBlock());
+  std::pair<unsigned, unsigned> StartDFS = {StartNode->getDFSNumIn(),
+                                            StartNode->getDFSNumOut()};
----------------
gberry wrote:
> getDFSNumIn/getDFSNumOut are documented as: These are an internal implementation detail, do not call them.
> which seems to conflict with this usage.
I will change the docs for them in a separate patch.

The change to make updateDFSNumbers public, etc, was done specifically to enable this usage.
See the thread titlte "[PATCH] Make updateDFSNumbers API public" from last year, and there was a review where i computed them separately, and it was suggested to just make it public.


http://reviews.llvm.org/D8688





More information about the llvm-commits mailing list