[PATCH] D22966: GVN-hoist: compute MSSA once per function (PR28670)

[Sebastian Pop via llvm-commits]

sebpop added a comment.

It looks like the bug is because we do not update the block of a memory access: this patch fixes a crash where we were still referring to the block that used to contain an instruction that got hoisted.

  --- a/llvm/lib/Transforms/Scalar/GVNHoist.cpp
  +++ b/llvm/lib/Transforms/Scalar/GVNHoist.cpp
  @@ -307,7 +307,8 @@ private:
   
       for (User *U : Def->users())
         if (auto *MU = dyn_cast<MemoryUse>(U)) {
  -        BasicBlock *UBB = MU->getBlock();
  +        // FIXME: MU->getBlock() does not get updated when we move the instruction.
  +        BasicBlock *UBB = MU->getMemoryInst()->getParent();
           // Only analyze uses in BB.
           if (BB != UBB)
             continue;

Why not having a MemoryAccess contain a reference to the instruction itself when we have one, and for MemoryPhi nodes we could reference the BB in which they occur:

  --- a/llvm/include/llvm/Transforms/Utils/MemorySSA.h
  +++ b/llvm/include/llvm/Transforms/Utils/MemorySSA.h
  @@ -167,7 +167,7 @@ protected:
   private:
     MemoryAccess(const MemoryAccess &);
     void operator=(const MemoryAccess &);
  -  BasicBlock *Block;
  +  Value *InstructionOrBlock;
   };
   
   template <>


https://reviews.llvm.org/D22966