[llvm] r293471 - [MemorySSA] Revert r293361 and r293363, as the tests fail under asan.

Sam McCall via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 30 01:19:51 PST 2017


Author: sammccall
Date: Mon Jan 30 03:19:50 2017
New Revision: 293471

URL: http://llvm.org/viewvc/llvm-project?rev=293471&view=rev
Log:
[MemorySSA] Revert r293361 and r293363, as the tests fail under asan.

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

Modified: llvm/trunk/include/llvm/Transforms/Utils/MemorySSA.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/MemorySSA.h?rev=293471&r1=293470&r2=293471&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/Utils/MemorySSA.h (original)
+++ llvm/trunk/include/llvm/Transforms/Utils/MemorySSA.h Mon Jan 30 03:19:50 2017
@@ -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;

Modified: llvm/trunk/include/llvm/Transforms/Utils/MemorySSAUpdater.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/MemorySSAUpdater.h?rev=293471&r1=293470&r2=293471&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/Utils/MemorySSAUpdater.h (original)
+++ llvm/trunk/include/llvm/Transforms/Utils/MemorySSAUpdater.h Mon Jan 30 03:19:50 2017
@@ -24,9 +24,10 @@
 // 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 *);

Modified: llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp?rev=293471&r1=293470&r2=293471&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp Mon Jan 30 03:19:50 2017
@@ -1597,7 +1597,6 @@ void MemorySSA::insertIntoListsForBlock(
       Defs->push_back(*NewAccess);
     }
   }
-  BlockNumberingValid.erase(BB);
 }
 
 void MemorySSA::insertIntoListsBefore(MemoryAccess *What, const BasicBlock *BB,
@@ -1625,7 +1624,6 @@ void MemorySSA::insertIntoListsBefore(Me
         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 *W
   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::createMemoryAcc
                                                 InsertionPlace Point) {
   MemoryUseOrDef *NewAccess = createDefinedAccess(I, Definition);
   insertIntoListsForBlock(NewAccess, BB, Point);
+  BlockNumberingValid.erase(BB);
   return NewAccess;
 }
 
@@ -1682,6 +1675,7 @@ MemoryUseOrDef *MemorySSA::createMemoryA
   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::createMemoryA
   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;

Modified: llvm/trunk/lib/Transforms/Utils/MemorySSAUpdater.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/MemorySSAUpdater.cpp?rev=293471&r1=293470&r2=293471&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/MemorySSAUpdater.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/MemorySSAUpdater.cpp Mon Jan 30 03:19:50 2017
@@ -243,17 +243,13 @@ void MemorySSAUpdater::insertDef(MemoryD
   // 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 S
 }
 
 // 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(MemoryUseO
   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(MemoryU
   moveTo(What, Where->getBlock(), ++Where->getIterator());
 }
 
-void MemorySSAUpdater::moveToPlace(MemoryUseOrDef *What, BasicBlock *BB,
-                                   MemorySSA::InsertionPlace Where) {
-  return moveTo(What, BB, Where);
-}
 } // namespace llvm

Modified: llvm/trunk/unittests/Transforms/Utils/MemorySSA.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/MemorySSA.cpp?rev=293471&r1=293470&r2=293471&view=diff
==============================================================================
--- llvm/trunk/unittests/Transforms/Utils/MemorySSA.cpp (original)
+++ llvm/trunk/unittests/Transforms/Utils/MemorySSA.cpp Mon Jan 30 03:19:50 2017
@@ -365,62 +365,6 @@ TEST_F(MemorySSATest, MoveAStoreUpdaterM
   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




More information about the llvm-commits mailing list