[PATCH] D33950: [mssa] If there is no definition in a block prior to an inserted use, return nullptr.

[Daniel Berlin via llvm-commits]

Could you move the original assert to the def list case?

On Wed, Jun 7, 2017, 9:47 AM Alina Sbirlea via Phabricator <
reviews at reviews.llvm.org> wrote:

> This revision was automatically updated to reflect the committed changes.
> Closed by commit rL304926: [mssa] Fix case when there is no definition in
> a block prior to an inserted use. (authored by asbirlea).
>
> Changed prior to commit:
>   https://reviews.llvm.org/D33950?vs=101625&id=101760#toc
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D33950
>
> Files:
>   llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp
>   llvm/trunk/unittests/Analysis/MemorySSA.cpp
>
>
> Index: llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp
> ===================================================================
> --- llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp
> +++ llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp
> @@ -124,17 +124,12 @@
>          return &*Iter;
>      } else {
>        // Otherwise, have to walk the all access iterator.
> -      auto Iter = MA->getReverseIterator();
> -      ++Iter;
> -      while (&*Iter != &*Defs->begin()) {
> -        if (!isa<MemoryUse>(*Iter))
> -          return &*Iter;
> -        --Iter;
> -      }
> -      // At this point it must be pointing at firstdef
> -      assert(&*Iter == &*Defs->begin() &&
> -             "Should have hit first def walking backwards");
> -      return &*Iter;
> +      auto End = MSSA->getWritableBlockAccesses(MA->getBlock())->rend();
> +      for (auto &U : make_range(++MA->getReverseIterator(), End))
> +        if (!isa<MemoryUse>(U))
> +          return cast<MemoryAccess>(&U);
> +      // Note that if MA comes before Defs->begin(), we won't hit a def.
> +      return nullptr;
>      }
>    }
>    return nullptr;
> Index: llvm/trunk/unittests/Analysis/MemorySSA.cpp
> ===================================================================
> --- llvm/trunk/unittests/Analysis/MemorySSA.cpp
> +++ llvm/trunk/unittests/Analysis/MemorySSA.cpp
> @@ -244,6 +244,52 @@
>    MSSA.verifyMemorySSA();
>  }
>
> +TEST_F(MemorySSATest, SinkLoad) {
> +  F = Function::Create(
> +      FunctionType::get(B.getVoidTy(), {B.getInt8PtrTy()}, false),
> +      GlobalValue::ExternalLinkage, "F", &M);
> +  BasicBlock *Entry(BasicBlock::Create(C, "", F));
> +  BasicBlock *Left(BasicBlock::Create(C, "", F));
> +  BasicBlock *Right(BasicBlock::Create(C, "", F));
> +  BasicBlock *Merge(BasicBlock::Create(C, "", F));
> +  B.SetInsertPoint(Entry);
> +  B.CreateCondBr(B.getTrue(), Left, Right);
> +  B.SetInsertPoint(Left, Left->begin());
> +  Argument *PointerArg = &*F->arg_begin();
> +  B.SetInsertPoint(Left);
> +  B.CreateBr(Merge);
> +  B.SetInsertPoint(Right);
> +  B.CreateBr(Merge);
> +
> +  // Load in left block
> +  B.SetInsertPoint(Left, Left->begin());
> +  LoadInst *LoadInst1 = B.CreateLoad(PointerArg);
> +  // Store in merge block
> +  B.SetInsertPoint(Merge, Merge->begin());
> +  B.CreateStore(B.getInt8(16), PointerArg);
> +
> +  setupAnalyses();
> +  MemorySSA &MSSA = *Analyses->MSSA;
> +  MemorySSAUpdater Updater(&MSSA);
> +
> +  // Mimic sinking of a load:
> +  // - clone load
> +  // - insert in "exit" block
> +  // - insert in mssa
> +  // - remove from original block
> +
> +  LoadInst *LoadInstClone = cast<LoadInst>(LoadInst1->clone());
> +  Merge->getInstList().insert(Merge->begin(), LoadInstClone);
> +  MemoryAccess * NewLoadAccess =
> +      Updater.createMemoryAccessInBB(LoadInstClone, nullptr,
> +                                     LoadInstClone->getParent(),
> +                                     MemorySSA::Beginning);
> +  Updater.insertUse(cast<MemoryUse>(NewLoadAccess));
> +  MSSA.verifyMemorySSA();
> +  Updater.removeMemoryAccess(MSSA.getMemoryAccess(LoadInst1));
> +  MSSA.verifyMemorySSA();
> +}
> +
>  TEST_F(MemorySSATest, MoveAStore) {
>    // We create a diamond where there is a in the entry, a store on one
> side, and
>    // a load at the end.  After building MemorySSA, we test updating by
> moving
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170607/6a338ef1/attachment.html>