<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Aug 2, 2016 at 3:02 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 nits; thanks!<br>
<br>
<br>
================<br>
Comment at: lib/Transforms/Utils/MemorySSA.cpp:180<br>
@@ +179,3 @@<br>
+      case Intrinsic::lifetime_end:<br>
+        {<br>
<span class="">+        if (AA.isMustAlias(MemoryLocation(II->getArgOperand(1)), Loc))<br>
</span>----------------<br>
Please kill these brackets<br>
<br>
================<br>
Comment at: lib/Transforms/Utils/MemorySSA.cpp:181<br>
@@ +180,3 @@<br>
+        {<br>
<span class="">+        if (AA.isMustAlias(MemoryLocation(II->getArgOperand(1)), Loc))<br>
+          return true;<br>
</span>----------------<br>
`return AA.isMustAlias(...);`?</blockquote><div><br></div><div>This is what i get for copying code. 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: test/Transforms/Util/MemorySSA/lifetime-simple.ll:2<br>
@@ +1,3 @@<br>
<span class="">+; RUN: opt -basicaa -print-memoryssa -verify-memoryssa -analyze < %s 2>&1 | FileCheck %s<br>
+; RUN: opt -aa-pipeline=basic-aa -passes='print<memoryssa>,verify<memoryssa>' -disable-output < %s 2>&1 | FileCheck %s<br>
+define i8 @test(i8* %P, i8* %Q) {<br>
</span>----------------<br>
Please add a newline after this line<br>
<br>
================<br>
Comment at: test/Transforms/Util/MemorySSA/lifetime-simple.ll:10<br>
@@ +9,3 @@<br>
<span class="">+  call void @llvm.lifetime.start(i64 32, i8* %P)<br>
+; CHECK-DAG:  MemoryUse(liveOnEntry)<br>
+  %0 = load i8, i8* %P<br>
</span>----------------<br>
Is there a reason these aren't vanilla CHECKs?<br></blockquote><div><br></div><div>I was trying to get it to order them without having to use CHECK-NEXT, but re-reading, it won't work.</div><div><br></div><div>I'll go back to the CHECK-NEXT style.</div><div><br></div><div> <br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">(If not, please also add the `; CHECK-NEXT: %0 = load ...` after each check for MemoryDef/MemoryUse, for consistency with the other tests)<br></blockquote><div><br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
<a href="https://reviews.llvm.org/D23076" rel="noreferrer" target="_blank">https://reviews.llvm.org/D23076</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>