[PATCH] D8688: Update MergedLoadStoreMotion to use MemorySSA

[Daniel Berlin via llvm-commits]

(and happy to wait if you want to review it)

On Wed, Jun 22, 2016 at 12:46 PM, Geoff Berry <gberry at codeaurora.org> wrote:

> gberry added a comment.
>
> I'm still digesting this, but here are a couple more nits I noticed.
> I don't want to hold up approval from anyone else though.
>
>
> ================
> Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:757
> @@ -606,1 +756,3 @@
> +  PA.preserve<DominatorTreeAnalysis>();
> +  PA.preserve<MemorySSAAnalysis>();
>    return PA;
> ----------------
> Isn't MemorySSA only preserved if 'UseMemorySSA' is true?
>
> ================
> 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()};
> ----------------
> getDFSNumIn/getDFSNumOut are documented as: These are an internal
> implementation detail, do not call them.
> which seems to conflict with this usage.
>
>
> http://reviews.llvm.org/D8688
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160622/f74e5218/attachment.html>