MemorySSA: Allow movement to arbitrary places

Summary: Extend the MemorySSAUpdater API to allow movement to arbitrary places

Reviewers: davide, george.burgess.iv

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D29239

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@293363 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/Transforms/Utils/MemorySSA.h b/include/llvm/Transforms/Utils/MemorySSA.h
index 96006fd4b5227cc8bd61d7a3808dad30ef596b74..bad2a3b708cca18822dd7c9e81fd4433a8b9a8d2 100644 (file)
--- a/include/llvm/Transforms/Utils/MemorySSA.h
+++ b/include/llvm/Transforms/Utils/MemorySSA.h
@@ -689,6 +689,7 @@ protected:
   // for moves.  It does not always leave the IR in a correct state, and relies
   // on the updater to fixup what it breaks, so it is not public.
   void moveTo(MemoryUseOrDef *What, BasicBlock *BB, AccessList::iterator Where);
+  void moveTo(MemoryUseOrDef *What, BasicBlock *BB, InsertionPlace Point);
 
 private:
   class CachingWalker;

diff --git a/include/llvm/Transforms/Utils/MemorySSAUpdater.h b/include/llvm/Transforms/Utils/MemorySSAUpdater.h
index 8fac9cf3f80fd340f2384eaa7287c42692f952ae..3fb759a891a28033f154ef8bf7dc3b9d0736a6a0 100644 (file)
--- a/include/llvm/Transforms/Utils/MemorySSAUpdater.h
+++ b/include/llvm/Transforms/Utils/MemorySSAUpdater.h
@@ -24,10 +24,9 @@
 // That's it.
 //
 // For moving, first, move the instruction itself using the normal SSA
-// instruction moving API, then just call moveBefore or moveAfter with the right
-// arguments.
+// instruction moving API, then just call moveBefore, moveAfter,or moveTo with
+// the right arguments.
 //
-// walk memory instructions using a use/def graph.
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TRANSFORMS_UTILS_MEMORYSSAUPDATER_H
@@ -68,10 +67,13 @@ public:
   void insertUse(MemoryUse *Use);
   void moveBefore(MemoryUseOrDef *What, MemoryUseOrDef *Where);
   void moveAfter(MemoryUseOrDef *What, MemoryUseOrDef *Where);
-
+  void moveToPlace(MemoryUseOrDef *What, BasicBlock *BB,
+                   MemorySSA::InsertionPlace Where);
 private:
+  // Move What before Where in the MemorySSA IR.
+  template <class WhereType>
   void moveTo(MemoryUseOrDef *What, BasicBlock *BB,
-              MemorySSA::AccessList::iterator Where);
+              WhereType Where);
   MemoryAccess *getPreviousDef(MemoryAccess *);
   MemoryAccess *getPreviousDefInBlock(MemoryAccess *);
   MemoryAccess *getPreviousDefFromEnd(BasicBlock *);

diff --git a/lib/Transforms/Utils/MemorySSAUpdater.cpp b/lib/Transforms/Utils/MemorySSAUpdater.cpp
index d7140f22eaf93bfef64ab292a45a4e3371d1a1d6..511ad9c42a1badf77fc51406b4275040bcbff71a 100644 (file)
--- a/lib/Transforms/Utils/MemorySSAUpdater.cpp
+++ b/lib/Transforms/Utils/MemorySSAUpdater.cpp
@@ -349,8 +349,9 @@ void MemorySSAUpdater::fixupDefs(const SmallVectorImpl<MemoryAccess *> &Vars) {
 }
 
 // Move What before Where in the MemorySSA IR.
+template <class WhereType>
 void MemorySSAUpdater::moveTo(MemoryUseOrDef *What, BasicBlock *BB,
-                              MemorySSA::AccessList::iterator Where) {
+                              WhereType Where) {
   // Replace all our users with our defining access.
   What->replaceAllUsesWith(What->getDefiningAccess());
 
@@ -363,6 +364,7 @@ void MemorySSAUpdater::moveTo(MemoryUseOrDef *What, BasicBlock *BB,
   else
     insertUse(cast<MemoryUse>(What));
 }
+
 // Move What before Where in the MemorySSA IR.
 void MemorySSAUpdater::moveBefore(MemoryUseOrDef *What, MemoryUseOrDef *Where) {
   moveTo(What, Where->getBlock(), Where->getIterator());
@@ -373,4 +375,8 @@ void MemorySSAUpdater::moveAfter(MemoryUseOrDef *What, MemoryUseOrDef *Where) {
   moveTo(What, Where->getBlock(), ++Where->getIterator());
 }
 
+void MemorySSAUpdater::moveToPlace(MemoryUseOrDef *What, BasicBlock *BB,
+                                   MemorySSA::InsertionPlace Where) {
+  return moveTo(What, BB, Where);
+}
 } // namespace llvm

diff --git a/unittests/Transforms/Utils/MemorySSA.cpp b/unittests/Transforms/Utils/MemorySSA.cpp
index 95ec3ed55e6a192e6cefa53840602c4e3b4d6172..0df476bc28c4ce7160b6d1cdc91d30dd0452f9a8 100644 (file)
--- a/unittests/Transforms/Utils/MemorySSA.cpp
+++ b/unittests/Transforms/Utils/MemorySSA.cpp
@@ -365,6 +365,62 @@ TEST_F(MemorySSATest, MoveAStoreUpdaterMove) {
   MSSA.verifyMemorySSA();
 }
 
+TEST_F(MemorySSATest, MoveAStoreAllAround) {
+  // 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
+  // the store from the side block to the entry block, then to the other side
+  // block, then to before the load.  This does not destroy the old access.
+  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);
+  Argument *PointerArg = &*F->arg_begin();
+  StoreInst *EntryStore = B.CreateStore(B.getInt8(16), PointerArg);
+  B.CreateCondBr(B.getTrue(), Left, Right);
+  B.SetInsertPoint(Left);
+  auto *SideStore = B.CreateStore(B.getInt8(16), PointerArg);
+  BranchInst::Create(Merge, Left);
+  BranchInst::Create(Merge, Right);
+  B.SetInsertPoint(Merge);
+  auto *MergeLoad = B.CreateLoad(PointerArg);
+  setupAnalyses();
+  MemorySSA &MSSA = *Analyses->MSSA;
+  MemorySSAUpdater Updater(&MSSA);
+
+  // Move the store
+  auto *EntryStoreAccess = MSSA.getMemoryAccess(EntryStore);
+  auto *SideStoreAccess = MSSA.getMemoryAccess(SideStore);
+  // Before, the load will point to a phi of the EntryStore and SideStore.
+  auto *LoadAccess = cast<MemoryUse>(MSSA.getMemoryAccess(MergeLoad));
+  EXPECT_TRUE(isa<MemoryPhi>(LoadAccess->getDefiningAccess()));
+  MemoryPhi *MergePhi = cast<MemoryPhi>(LoadAccess->getDefiningAccess());
+  EXPECT_EQ(MergePhi->getIncomingValue(1), EntryStoreAccess);
+  EXPECT_EQ(MergePhi->getIncomingValue(0), SideStoreAccess);
+  // Move the store before the entry store
+  SideStore->moveBefore(*EntryStore->getParent(), EntryStore->getIterator());
+  Updater.moveBefore(SideStoreAccess, EntryStoreAccess);
+  // After, it's a phi of the entry store.
+  EXPECT_EQ(MergePhi->getIncomingValue(0), EntryStoreAccess);
+  EXPECT_EQ(MergePhi->getIncomingValue(1), EntryStoreAccess);
+  MSSA.verifyMemorySSA();
+  // Now move the store to the right branch
+  SideStore->moveBefore(*Right, Right->begin());
+  Updater.moveToPlace(SideStoreAccess, Right, MemorySSA::Beginning);
+  MSSA.verifyMemorySSA();
+  EXPECT_EQ(MergePhi->getIncomingValue(0), EntryStoreAccess);
+  EXPECT_EQ(MergePhi->getIncomingValue(1), SideStoreAccess);
+  // Now move it before the load
+  SideStore->moveBefore(MergeLoad);
+  Updater.moveBefore(SideStoreAccess, LoadAccess);
+  EXPECT_EQ(MergePhi->getIncomingValue(0), EntryStoreAccess);
+  EXPECT_EQ(MergePhi->getIncomingValue(1), EntryStoreAccess);
+  MSSA.verifyMemorySSA();
+}
+
 TEST_F(MemorySSATest, RemoveAPhi) {
   // We create a diamond where there is a store on one side, and then a load
   // after the merge point.  This enables us to test a bunch of different