[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