<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Aug 5, 2016 at 12:04 PM, George Burgess IV <span dir="ltr"><<a href="mailto:george.burgess.iv@gmail.com" target="_blank">george.burgess.iv@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">george.burgess.iv accepted this revision.<br>
george.burgess.iv added a comment.<br>
This revision is now accepted and ready to land.<br>
<br>
LGTM with comments, thanks.<br>
<br>
<br>
================<br>
Comment at: include/llvm/Transforms/Utils/<wbr>MemorySSA.h:594<br>
@@ +593,3 @@<br>
<span class="">+  /// dominates Use \p B.<br>
+  bool dominates(const MemoryAccess *A, const Use &B) const;<br>
+<br>
</span>----------------<br>
Do we see this being used outside of verification?</blockquote><div><br></div><div>Yes, actually.</div><div>It also matches what Dominators.h provides, and that gets used.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> If not, make this private, please.<br>
<br>
================<br>
Comment at: lib/Transforms/Utils/<wbr>MemorySSA.cpp:1872<br>
@@ -1891,1 +1871,3 @@<br>
<span class="">+      for (const Use &U : MP->uses())<br>
+        assert(dominates(MP, U) && "Memory PHI does not dominate it's uses");<br>
     for (Instruction &I : B) {<br>
</span>----------------<br>
Looks like this definition is provided if `NDEBUG` is defined. Please braces around the assert. :)<br>
<br>
================<br>
Comment at: lib/Transforms/Utils/<wbr>MemorySSA.cpp:1879<br>
@@ -1913,1 +1878,3 @@<br>
<span class="">+      for (const Use &U : MD->uses())<br>
+        assert(dominates(MD, U) && "Memory Def does not dominate it's uses");<br>
     }<br>
</span>----------------<br>
Same as above<br>
<br>
<br>
<a href="https://reviews.llvm.org/D23184" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D23184</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>