<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Feb 6, 2016 at 4:34 PM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">majnemer added a subscriber: majnemer.<br>
majnemer accepted this revision.<br>
majnemer added a reviewer: majnemer.<br>
majnemer added a comment.<br>
This revision is now accepted and ready to land.<br>
<br>
This LGTM.  An argument could be made, either for or against, that the change to minimize the number of attempts to update `DefiningBlocks` could be split out.<br>
<br></blockquote><div>i can stage it into two SVN commits :) </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:308-313<br>
@@ -267,1 +307,8 @@<br>
+    // live into it to.<br>
+    for (pred_iterator PI = pred_begin(BB), E = pred_end(BB); PI != E; ++PI) {<br>
+      BasicBlock *P = *PI;<br>
+<br>
+      // Otherwise it is, add to the worklist.<br>
+      LiveInBlockWorklist.push_back(P);<br>
+    }<br>
   }<br>
----------------<br>
Any reason not to use the range-based `predecessors`?<br></blockquote><div><br></div><div>No. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
It should make the code a little shorter:<br>
  for (BasicBlock *P : predecessors(BB))<br>
    LiveInBlockWorklist.push_back(P);<br>
<br>
I guess you could also use `append` instead:<br>
  LiveInBlockWorklist.append(pred_begin(BB), pred_end(BB));<br></blockquote><div><br></div><div>I think i'm going to do this one :)</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: test/Transforms/Util/MemorySSA/livein.ll:26<br>
@@ +25,3 @@<br>
+merge:<br>
+;CHECK: 2 = MemoryPhi({left,1},{right,liveOnEntry})<br>
+%c = load i8, i8* %0<br>
----------------<br>
Would you mind adding a space between that semicolon and the CHECK to match the others?<br>
<br>
<br>
<a href="http://reviews.llvm.org/D16950" rel="noreferrer" target="_blank">http://reviews.llvm.org/D16950</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>