<div dir="ltr">(and happy to wait if you want to review it)<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 22, 2016 at 12:46 PM, Geoff Berry <span dir="ltr"><<a href="mailto:gberry@codeaurora.org" target="_blank">gberry@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">gberry added a comment.<br>
<br>
I'm still digesting this, but here are a couple more nits I noticed.<br>
I don't want to hold up approval from anyone else though.<br>
<br>
<br>
================<br>
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:757<br>
@@ -606,1 +756,3 @@<br>
+  PA.preserve<DominatorTreeAnalysis>();<br>
+  PA.preserve<MemorySSAAnalysis>();<br>
   return PA;<br>
----------------<br>
Isn't MemorySSA only preserved if 'UseMemorySSA' is true?<br>
<br>
================<br>
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:982<br>
@@ +981,3 @@<br>
+  DomTreeNode *StartNode = DT->getNode(Start->getBlock());<br>
+  std::pair<unsigned, unsigned> StartDFS = {StartNode->getDFSNumIn(),<br>
+                                            StartNode->getDFSNumOut()};<br>
----------------<br>
getDFSNumIn/getDFSNumOut are documented as: These are an internal implementation detail, do not call them.<br>
which seems to conflict with this usage.<br>
<br>
<br>
<a href="http://reviews.llvm.org/D8688" rel="noreferrer" target="_blank">http://reviews.llvm.org/D8688</a><br>
<br>
<br>
<br>
</blockquote></div><br></div>