<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jun 20, 2016 at 12:21 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 added a comment.<br>
<br>
Looks good overall -- just a few nits.<br>
<br>
<br>
================<br>
Comment at: include/llvm/Transforms/Utils/MemorySSA.h:553<br>
@@ +552,3 @@<br>
+  /// It will return the new MemoryAccess.<br>
+  MemoryAccess *createMemoryAccess(Instruction *I, MemoryAccess *Definition,<br>
+                                   const BasicBlock *BB, InsertionPlace Point);<br>
----------------<br>
`createMemoryAccessInBB`, maybe? Naming is hard.<br></blockquote><div><br></div><div>Fixed</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
================<br>
Comment at: include/llvm/Transforms/Utils/MemorySSA.h:557<br>
@@ +556,3 @@<br>
+  /// MemoryAccess.<br>
+  //<br>
+  /// Returns the new MemoryAccess.<br>
----------------<br>
///<br></blockquote><div><br></div><div>Fixed </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
================<br>
Comment at: lib/Transforms/Utils/MemorySSA.cpp:589<br>
@@ +588,3 @@<br>
+/// order and existence of memory affecting instructions.<br>
+<br>
+void MemorySSA::verifyOrdering(Function &F) const {<br>
----------------<br>
Please remove this newline<br></blockquote><div>Fixed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
================<br>
Comment at: lib/Transforms/Utils/MemorySSA.cpp:595<br>
@@ +594,3 @@<br>
+  for (BasicBlock &B : F) {<br>
+    SmallVector<MemoryAccess *, 32> ActualAccesses;<br>
+    const AccessListType *AL = getBlockAccesses(&B);<br>
----------------<br>
Bring this out of the loop + clear on each iteration, please.<br></blockquote><div><br></div><div>Fixed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
================<br>
Comment at: lib/Transforms/Utils/MemorySSA.cpp:602<br>
@@ +601,3 @@<br>
+      MemoryAccess *MA = getMemoryAccess(&I);<br>
+      assert((!MA || AL != nullptr) && "We have memory affecting instructions "<br>
+                                       "in this block but they are not in the "<br>
----------------<br>
Varying compare-against-null styles?</blockquote><div><br></div><div>Fixed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<a href="http://reviews.llvm.org/D21463" rel="noreferrer" target="_blank">http://reviews.llvm.org/D21463</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>