Fix PR 28933

[• dberlin]

Summary

This fixes PR 28933 by making sure GVNHoist does not try to recreate memory
accesses when it has not actually moved them.

[sebpop]

LGTM. Thanks Danny!

lib/Transforms/Scalar/GVNHoist.cpp

for (const HoistingPointInfo &HP : HPL) {
       Instruction *Repl = nullptr;
       for (Instruction *I : InstructionsToHoist)
         if (I->getParent() == HoistPt)
           // If there are two instructions in HoistPt to be hoisted in place:
           // update Repl to be the first one, such that we can rename the uses
           // of the second based on the first.
           if (!Repl || firstInBB(I, Repl))
             Repl = I;
-
+      bool MoveAccess = true;
       if (Repl) {
         // Repl is already in HoistPt: it remains in place.
         assert(allOperandsAvailable(Repl, HoistPt) &&
                "instruction depends on operands that are not available");
+        MoveAccess = false;
       } else {
         // When we do not find Repl in HoistPt, select the first in the list
         // and move it to HoistPt.
         Repl = InstructionsToHoist.front();
 
         // We can move Repl in HoistPt only when all operands are available.
         // The order in which hoistings are done may influence the availability
         // of operands.
for (const HoistingPointInfo &HP : HPL) {
         // Move the instruction at the end of HoistPt.
         Instruction *Last = HoistPt->getTerminator();
         Repl->moveBefore(Last);
 
         DFSNumber[Repl] = DFSNumber[Last]++;
       }
 
       MemoryAccess *NewMemAcc = nullptr;
+      if (MoveAccess) {
         if (MemoryAccess *MA = MSSA->getMemoryAccess(Repl)) {
           if (MemoryUseOrDef *OldMemAcc = dyn_cast<MemoryUseOrDef>(MA)) {
             // The definition of this ld/st will not change: ld/st hoisting is
             // legal when the ld/st is not moved past its current definition.
             MemoryAccess *Def = OldMemAcc->getDefiningAccess();
             NewMemAcc = MSSA->createMemoryAccessInBB(Repl, Def, HoistPt,
                                                      MemorySSA::End);
             OldMemAcc->replaceAllUsesWith(NewMemAcc);
             MSSA->removeMemoryAccess(OldMemAcc);
           }
         }
+      } else {
+        NewMemAcc = MSSA->getMemoryAccess(Repl);
+      }
 
       if (isa<LoadInst>(Repl))
         ++NL;
       else if (isa<StoreInst>(Repl))
         ++NS;
       else if (isa<CallInst>(Repl))
         ++NC;
       else // Scalar