]> granicus.if.org Git - llvm/commitdiff
[MemorySSA] Revert r293361 and r293363, as the tests fail under asan.
authorSam McCall <sam.mccall@gmail.com>
Mon, 30 Jan 2017 09:19:50 +0000 (09:19 +0000)
committerSam McCall <sam.mccall@gmail.com>
Mon, 30 Jan 2017 09:19:50 +0000 (09:19 +0000)
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@293471 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Transforms/Utils/MemorySSA.h
include/llvm/Transforms/Utils/MemorySSAUpdater.h
lib/Transforms/Utils/MemorySSA.cpp
lib/Transforms/Utils/MemorySSAUpdater.cpp
unittests/Transforms/Utils/MemorySSA.cpp

index bad2a3b708cca18822dd7c9e81fd4433a8b9a8d2..96006fd4b5227cc8bd61d7a3808dad30ef596b74 100644 (file)
@@ -689,7 +689,6 @@ 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;
index 3fb759a891a28033f154ef8bf7dc3b9d0736a6a0..8fac9cf3f80fd340f2384eaa7287c42692f952ae 100644 (file)
 // That's it.
 //
 // For moving, first, move the instruction itself using the normal SSA
-// instruction moving API, then just call moveBefore, moveAfter,or moveTo with
-// the right arguments.
+// instruction moving API, then just call moveBefore or moveAfter with the right
+// arguments.
 //
+// walk memory instructions using a use/def graph.
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TRANSFORMS_UTILS_MEMORYSSAUPDATER_H
@@ -67,13 +68,10 @@ 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,
-              WhereType Where);
+              MemorySSA::AccessList::iterator Where);
   MemoryAccess *getPreviousDef(MemoryAccess *);
   MemoryAccess *getPreviousDefInBlock(MemoryAccess *);
   MemoryAccess *getPreviousDefFromEnd(BasicBlock *);
index af4dc028b851a562c2021d1bc0bf9495e2b0d2fc..50006cef4771d421043d56cfa37c80ed5580ca83 100644 (file)
@@ -1597,7 +1597,6 @@ void MemorySSA::insertIntoListsForBlock(MemoryAccess *NewAccess,
       Defs->push_back(*NewAccess);
     }
   }
-  BlockNumberingValid.erase(BB);
 }
 
 void MemorySSA::insertIntoListsBefore(MemoryAccess *What, const BasicBlock *BB,
@@ -1625,7 +1624,6 @@ void MemorySSA::insertIntoListsBefore(MemoryAccess *What, const BasicBlock *BB,
         Defs->insert(InsertPt->getDefsIterator(), *What);
     }
   }
-  BlockNumberingValid.erase(BB);
 }
 
 // Move What before Where in the IR.  The end result is taht What will belong to
@@ -1640,19 +1638,13 @@ void MemorySSA::moveTo(MemoryUseOrDef *What, BasicBlock *BB,
   insertIntoListsBefore(What, BB, Where);
 }
 
-void MemorySSA::moveTo(MemoryUseOrDef *What, BasicBlock *BB,
-                       InsertionPlace Point) {
-  removeFromLists(What, false);
-  What->setBlock(BB);
-  insertIntoListsForBlock(What, BB, Point);
-}
-
 MemoryPhi *MemorySSA::createMemoryPhi(BasicBlock *BB) {
   assert(!getMemoryAccess(BB) && "MemoryPhi already exists for this BB");
   MemoryPhi *Phi = new MemoryPhi(BB->getContext(), BB, NextID++);
-  // Phi's always are placed at the front of the block.
   insertIntoListsForBlock(Phi, BB, Beginning);
   ValueToMemoryAccess[BB] = Phi;
+  // Phi's always are placed at the front of the block.
+  BlockNumberingValid.erase(BB);
   return Phi;
 }
 
@@ -1673,6 +1665,7 @@ MemoryAccess *MemorySSA::createMemoryAccessInBB(Instruction *I,
                                                 InsertionPlace Point) {
   MemoryUseOrDef *NewAccess = createDefinedAccess(I, Definition);
   insertIntoListsForBlock(NewAccess, BB, Point);
+  BlockNumberingValid.erase(BB);
   return NewAccess;
 }
 
@@ -1682,6 +1675,7 @@ MemoryUseOrDef *MemorySSA::createMemoryAccessBefore(Instruction *I,
   assert(I->getParent() == InsertPt->getBlock() &&
          "New and old access must be in the same block");
   MemoryUseOrDef *NewAccess = createDefinedAccess(I, Definition);
+  BlockNumberingValid.erase(InsertPt->getBlock());
   insertIntoListsBefore(NewAccess, InsertPt->getBlock(),
                         InsertPt->getIterator());
   return NewAccess;
@@ -1693,6 +1687,7 @@ MemoryUseOrDef *MemorySSA::createMemoryAccessAfter(Instruction *I,
   assert(I->getParent() == InsertPt->getBlock() &&
          "New and old access must be in the same block");
   MemoryUseOrDef *NewAccess = createDefinedAccess(I, Definition);
+  BlockNumberingValid.erase(InsertPt->getBlock());
   insertIntoListsBefore(NewAccess, InsertPt->getBlock(),
                         ++(InsertPt->getIterator()));
   return NewAccess;
index 511ad9c42a1badf77fc51406b4275040bcbff71a..792ddef4e70e2ef9888cec586f339d1dde08a0f8 100644 (file)
@@ -243,17 +243,13 @@ void MemorySSAUpdater::insertDef(MemoryDef *MD) {
   // of that thing with us, since we are in the way of whatever was there
   // before.
   // We now define that def's memorydefs and memoryphis
-  if (DefBeforeSameBlock) {
-    for (auto UI = DefBefore->use_begin(), UE = DefBefore->use_end();
-         UI != UE;) {
-      Use &U = *UI++;
-      // Leave the uses alone
-      if (isa<MemoryUse>(U.getUser()))
-        continue;
-      U.set(MD);
-    }
+  for (auto UI = DefBefore->use_begin(), UE = DefBefore->use_end(); UI != UE;) {
+    Use &U = *UI++;
+    // Leave the uses alone
+    if (isa<MemoryUse>(U.getUser()))
+      continue;
+    U.set(MD);
   }
-
   // and that def is now our defining access.
   // We change them in this order otherwise we will appear in the use list
   // above and reset ourselves.
@@ -349,9 +345,8 @@ 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,
-                              WhereType Where) {
+                              MemorySSA::AccessList::iterator Where) {
   // Replace all our users with our defining access.
   What->replaceAllUsesWith(What->getDefiningAccess());
 
@@ -364,7 +359,6 @@ 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());
@@ -375,8 +369,4 @@ 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
index 0df476bc28c4ce7160b6d1cdc91d30dd0452f9a8..95ec3ed55e6a192e6cefa53840602c4e3b4d6172 100644 (file)
@@ -365,62 +365,6 @@ 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