Could you move the original assert to the def list case?<div><br><div class="gmail_quote"><div dir="ltr">On Wed, Jun 7, 2017, 9:47 AM Alina Sbirlea via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This revision was automatically updated to reflect the committed changes.<br>
Closed by commit rL304926: [mssa] Fix case when there is no definition in a block prior to an inserted use. (authored by asbirlea).<br>
<br>
Changed prior to commit:<br>
  <a href="https://reviews.llvm.org/D33950?vs=101625&id=101760#toc" rel="noreferrer" target="_blank">https://reviews.llvm.org/D33950?vs=101625&id=101760#toc</a><br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D33950" rel="noreferrer" target="_blank">https://reviews.llvm.org/D33950</a><br>
<br>
Files:<br>
  llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp<br>
  llvm/trunk/unittests/Analysis/MemorySSA.cpp<br>
<br>
<br>
Index: llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp<br>
===================================================================<br>
--- llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp<br>
+++ llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp<br>
@@ -124,17 +124,12 @@<br>
         return &*Iter;<br>
     } else {<br>
       // Otherwise, have to walk the all access iterator.<br>
-      auto Iter = MA->getReverseIterator();<br>
-      ++Iter;<br>
-      while (&*Iter != &*Defs->begin()) {<br>
-        if (!isa<MemoryUse>(*Iter))<br>
-          return &*Iter;<br>
-        --Iter;<br>
-      }<br>
-      // At this point it must be pointing at firstdef<br>
-      assert(&*Iter == &*Defs->begin() &&<br>
-             "Should have hit first def walking backwards");<br>
-      return &*Iter;<br>
+      auto End = MSSA->getWritableBlockAccesses(MA->getBlock())->rend();<br>
+      for (auto &U : make_range(++MA->getReverseIterator(), End))<br>
+        if (!isa<MemoryUse>(U))<br>
+          return cast<MemoryAccess>(&U);<br>
+      // Note that if MA comes before Defs->begin(), we won't hit a def.<br>
+      return nullptr;<br>
     }<br>
   }<br>
   return nullptr;<br>
Index: llvm/trunk/unittests/Analysis/MemorySSA.cpp<br>
===================================================================<br>
--- llvm/trunk/unittests/Analysis/MemorySSA.cpp<br>
+++ llvm/trunk/unittests/Analysis/MemorySSA.cpp<br>
@@ -244,6 +244,52 @@<br>
   MSSA.verifyMemorySSA();<br>
 }<br>
<br>
+TEST_F(MemorySSATest, SinkLoad) {<br>
+  F = Function::Create(<br>
+      FunctionType::get(B.getVoidTy(), {B.getInt8PtrTy()}, false),<br>
+      GlobalValue::ExternalLinkage, "F", &M);<br>
+  BasicBlock *Entry(BasicBlock::Create(C, "", F));<br>
+  BasicBlock *Left(BasicBlock::Create(C, "", F));<br>
+  BasicBlock *Right(BasicBlock::Create(C, "", F));<br>
+  BasicBlock *Merge(BasicBlock::Create(C, "", F));<br>
+  B.SetInsertPoint(Entry);<br>
+  B.CreateCondBr(B.getTrue(), Left, Right);<br>
+  B.SetInsertPoint(Left, Left->begin());<br>
+  Argument *PointerArg = &*F->arg_begin();<br>
+  B.SetInsertPoint(Left);<br>
+  B.CreateBr(Merge);<br>
+  B.SetInsertPoint(Right);<br>
+  B.CreateBr(Merge);<br>
+<br>
+  // Load in left block<br>
+  B.SetInsertPoint(Left, Left->begin());<br>
+  LoadInst *LoadInst1 = B.CreateLoad(PointerArg);<br>
+  // Store in merge block<br>
+  B.SetInsertPoint(Merge, Merge->begin());<br>
+  B.CreateStore(B.getInt8(16), PointerArg);<br>
+<br>
+  setupAnalyses();<br>
+  MemorySSA &MSSA = *Analyses->MSSA;<br>
+  MemorySSAUpdater Updater(&MSSA);<br>
+<br>
+  // Mimic sinking of a load:<br>
+  // - clone load<br>
+  // - insert in "exit" block<br>
+  // - insert in mssa<br>
+  // - remove from original block<br>
+<br>
+  LoadInst *LoadInstClone = cast<LoadInst>(LoadInst1->clone());<br>
+  Merge->getInstList().insert(Merge->begin(), LoadInstClone);<br>
+  MemoryAccess * NewLoadAccess =<br>
+      Updater.createMemoryAccessInBB(LoadInstClone, nullptr,<br>
+                                     LoadInstClone->getParent(),<br>
+                                     MemorySSA::Beginning);<br>
+  Updater.insertUse(cast<MemoryUse>(NewLoadAccess));<br>
+  MSSA.verifyMemorySSA();<br>
+  Updater.removeMemoryAccess(MSSA.getMemoryAccess(LoadInst1));<br>
+  MSSA.verifyMemorySSA();<br>
+}<br>
+<br>
 TEST_F(MemorySSATest, MoveAStore) {<br>
   // We create a diamond where there is a in the entry, a store on one side, and<br>
   // a load at the end.  After building MemorySSA, we test updating by moving<br>
<br>
<br>
</blockquote></div></div>