[MemorySSA] Tighten up API type signatures. NFC.

[bryant]

Summary

From what I understand, MSSA nodes are either MemoryPhis (which must only be associated with BasicBlocks), or MemoryUseOrDef (which must only be associated Instructions). Assuming that this invariant would not change in the future (which could easily not be the case if, for instance, there is desire for additional MSSA node types):

1. getMemoryAccess could only ever return meaningful values when give a BasicBlock or Instruction,
2. createMemoryAccessBefore/After could only ever return nodes for Instructions,
3. Insertion of a new MemoryUseOrDef could only occur before another MemoryUseOrDef, since MemoryPhis must always come first in a BB's AccessList.

These type changes also have the pleasant side effect of reducing pointer casting verbiage.

include/llvm/Transforms/Utils/MemorySSA.h

public:
   ~MemorySSA();
 
   MemorySSAWalker *getWalker();
 
   /// \brief Given a memory Mod/Ref'ing instruction, get the MemorySSA
   /// access associated with it. If passed a basic block gets the memory phi
   /// node that exists for that block, if there is one. Otherwise, this will get
   /// a MemoryUseOrDef.
-  MemoryAccess *getMemoryAccess(const Value *) const;
+  MemoryUseOrDef *getMemoryAccess(const Instruction *) const;
   MemoryPhi *getMemoryAccess(const BasicBlock *BB) const;
 
   void dump() const;
   void print(raw_ostream &) const;
 
   /// \brief Return true if \p MA represents the live on entry value
   ///
   /// Loads and stores from pointer arguments and other global values may be
public:
   ///
   /// Returns the new MemoryAccess.
   /// This should be called when a memory instruction is created that is being
   /// used to replace an existing memory instruction. It will *not* create PHI
   /// nodes, or verify the clobbering definition.  The clobbering definition
   /// must be non-null.
   /// Note: If a MemoryAccess already exists for I, this function will make it
   /// inaccessible and it *must* have removeMemoryAccess called on it.
-  MemoryAccess *createMemoryAccessBefore(Instruction *I,
+  MemoryUseOrDef *createMemoryAccessBefore(Instruction *I,
                                            MemoryAccess *Definition,
-                                         MemoryAccess *InsertPt);
-  MemoryAccess *createMemoryAccessAfter(Instruction *I,
+                                           MemoryUseOrDef *InsertPt);
+  MemoryUseOrDef *createMemoryAccessAfter(Instruction *I,
                                           MemoryAccess *Definition,
                                           MemoryAccess *InsertPt);
 
   /// \brief Remove a MemoryAccess from MemorySSA, including updating all
   /// definitions and uses.
   /// This should be called when a memory instruction that has a MemoryAccess
   /// associated with it is erased from the program.  For example, if a store or
   /// load is simply erased (not replaced), removeMemoryAccess should be called
   /// on the MemoryAccess for that store/load.
   void removeMemoryAccess(MemoryAccess *);

lib/Transforms/Scalar/GVNHoist.cpp

void partitionCandidates(SmallVecImplInsn &InstructionsToHoist,
     int NBBsOnAllPaths = MaxNumberOfBBSInPath;
 
     SmallVecImplInsn::iterator II = InstructionsToHoist.begin();
     SmallVecImplInsn::iterator Start = II;
     Instruction *HoistPt = *II;
     BasicBlock *HoistBB = HoistPt->getParent();
     MemoryUseOrDef *UD;
     if (K != InsKind::Scalar)
-      UD = cast<MemoryUseOrDef>(MSSA->getMemoryAccess(HoistPt));
+      UD = MSSA->getMemoryAccess(HoistPt);
 
     for (++II; II != InstructionsToHoist.end(); ++II) {
       Instruction *Insn = *II;
       BasicBlock *BB = Insn->getParent();
       BasicBlock *NewHoistBB;
       Instruction *NewHoistPt;
 
       if (BB == HoistBB) {
for (++II; II != InstructionsToHoist.end(); ++II) {
         // unsafe to hoist loads to a place where there may be a path not
         // loading from the same address: for instance there may be a branch on
         // which the address of the load may not be initialized.
         if ((HoistBB == NewHoistBB || BB == NewHoistBB ||
              hoistingFromAllPaths(NewHoistBB, WL)) &&
             // Also check that it is safe to move the load or store from HoistPt
             // to NewHoistPt, and from Insn to NewHoistPt.
             safeToHoistLdSt(NewHoistPt, HoistPt, UD, K, NBBsOnAllPaths) &&
-            safeToHoistLdSt(NewHoistPt, Insn,
-                            cast<MemoryUseOrDef>(MSSA->getMemoryAccess(Insn)),
+            safeToHoistLdSt(NewHoistPt, Insn, MSSA->getMemoryAccess(Insn),
                             K, NBBsOnAllPaths)) {
           // Extend HoistPt to NewHoistPt.
           HoistPt = NewHoistPt;
           HoistBB = NewHoistBB;
           continue;
         }
       }
 
       // At this point it is not safe to extend the current hoisting to
       // NewHoistPt: save the hoisting list so far.
       if (std::distance(Start, II) > 1)
         HPL.push_back({HoistBB, SmallVecInsn(Start, II)});
 
       // Start over from BB.
       Start = II;
       if (K != InsKind::Scalar)
-        UD = cast<MemoryUseOrDef>(MSSA->getMemoryAccess(*Start));
+        UD = MSSA->getMemoryAccess(*Start);
       HoistPt = Insn;
       HoistBB = BB;
       NBBsOnAllPaths = MaxNumberOfBBSInPath;
     }
 
     // Save the last partition.
     if (std::distance(Start, II) > 1)
       HPL.push_back({HoistBB, SmallVecInsn(Start, II)});

lib/Transforms/Utils/MemorySSA.cpp

if (Point == Beginning) {
 
     Accesses->insert(AI, NewAccess);
   } else {
     Accesses->push_back(NewAccess);
   }
   BlockNumberingValid.erase(BB);
   return NewAccess;
 }
-MemoryAccess *MemorySSA::createMemoryAccessBefore(Instruction *I,
+
+MemoryUseOrDef *MemorySSA::createMemoryAccessBefore(Instruction *I,
                                                     MemoryAccess *Definition,
-                                                  MemoryAccess *InsertPt) {
+                                                    MemoryUseOrDef *InsertPt) {
   assert(I->getParent() == InsertPt->getBlock() &&
          "New and old access must be in the same block");
   MemoryUseOrDef *NewAccess = createDefinedAccess(I, Definition);
   auto *Accesses = getOrCreateAccessList(InsertPt->getBlock());
   Accesses->insert(AccessList::iterator(InsertPt), NewAccess);
   BlockNumberingValid.erase(InsertPt->getBlock());
   return NewAccess;
 }
 
-MemoryAccess *MemorySSA::createMemoryAccessAfter(Instruction *I,
+MemoryUseOrDef *MemorySSA::createMemoryAccessAfter(Instruction *I,
                                                    MemoryAccess *Definition,
                                                    MemoryAccess *InsertPt) {
   assert(I->getParent() == InsertPt->getBlock() &&
          "New and old access must be in the same block");
   MemoryUseOrDef *NewAccess = createDefinedAccess(I, Definition);
   auto *Accesses = getOrCreateAccessList(InsertPt->getBlock());
   Accesses->insertAfter(AccessList::iterator(InsertPt), NewAccess);
   BlockNumberingValid.erase(InsertPt->getBlock());
   return NewAccess;
 }
if (MemoryPhi *Phi = getMemoryAccess(&B)) {
       assert(Phi->getNumOperands() == static_cast<unsigned>(std::distance(
                                           pred_begin(&B), pred_end(&B))) &&
              "Incomplete MemoryPhi Node");
       for (unsigned I = 0, E = Phi->getNumIncomingValues(); I != E; ++I)
         verifyUseInDefs(Phi->getIncomingValue(I), Phi);
     }
 
     for (Instruction &I : B) {
-      if (MemoryAccess *MA = getMemoryAccess(&I)) {
-        assert(isa<MemoryUseOrDef>(MA) &&
-               "Found a phi node not attached to a bb");
-        verifyUseInDefs(cast<MemoryUseOrDef>(MA)->getDefiningAccess(), MA);
+      if (MemoryUseOrDef *MA = getMemoryAccess(&I)) {
+        verifyUseInDefs(MA->getDefiningAccess(), MA);
       }
     }
   }
 }
 
-MemoryAccess *MemorySSA::getMemoryAccess(const Value *I) const {
-  return ValueToMemoryAccess.lookup(I);
+MemoryUseOrDef *MemorySSA::getMemoryAccess(const Instruction *I) const {
+  return cast_or_null<MemoryUseOrDef>(ValueToMemoryAccess.lookup(I));
 }
 
 MemoryPhi *MemorySSA::getMemoryAccess(const BasicBlock *BB) const {
-  return cast_or_null<MemoryPhi>(getMemoryAccess((const Value *)BB));
+  return cast_or_null<MemoryPhi>(ValueToMemoryAccess.lookup(cast<Value>(BB)));
 }
 
 /// Perform a local numbering on blocks so that instruction ordering can be
 /// determined in constant time.
 /// TODO: We currently just number in order.  If we numbered by N, we could
 /// allow at least N-1 sequences of insertBefore or insertAfter (and at least
 /// log2(N) sequences of mixed before and after) without needing to invalidate
 /// the numbering.

unittests/Transforms/Utils/MemorySSA.cpp

TEST_F(MemorySSATest, TestTripleStore) {
 
   setupAnalyses();
   MemorySSA &MSSA = *Analyses->MSSA;
   MemorySSAWalker *Walker = Analyses->Walker;
 
   unsigned I = 0;
   for (StoreInst *V : {S1, S2, S3}) {
     // Everything should be clobbered by its defining access
-    MemoryAccess *DefiningAccess =
-        cast<MemoryUseOrDef>(MSSA.getMemoryAccess(V))->getDefiningAccess();
+    MemoryAccess *DefiningAccess = MSSA.getMemoryAccess(V)->getDefiningAccess();
     MemoryAccess *WalkerClobber = Walker->getClobberingMemoryAccess(V);
     EXPECT_EQ(DefiningAccess, WalkerClobber)
         << "Store " << I << " doesn't have the correct clobbering access";
     // EXPECT_EQ expands such that if we increment I above, it won't get
     // incremented except when we try to print the error message.
     ++I;
   }
 }