[PATCH] D22652: GVH-hoist: only clone GEPs (PR28606)

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: llvm/lib/Transforms/Scalar/GVNHoist.cpp:586
@@ -585,3 +585,3 @@
       Gep = dyn_cast<GetElementPtrInst>(St->getPointerOperand());
       Val = dyn_cast<Instruction>(St->getValueOperand());
     }
----------------
sebpop wrote:
> majnemer wrote:
> > We could put the check here:
> >   ...
> >   GetElementPtrInst *Val = nullptr;
> >   if (auto *St = dyn_cast<StoreInst>(Repl)) {
> >     Gep = ...
> >     Val = dyn_cast<GetElementPtrInst>(St->getValueOperand());
> I think this is not possible: we need to check whether the stored value is available at HoistPt.
> If we dyn_cast Val to GEP this early we would only hoist GEP stores. 
Oh, I understand now.  You are trying to move the address computation closer to the memory operation...

================
Comment at: llvm/lib/Transforms/Scalar/GVNHoist.cpp:592-594
@@ -591,5 +591,5 @@
 
     // PHIs may only be inserted at the start of a block.
     if (Val && isa<PHINode>(Val))
       return false;
 
----------------
This can be removed now, we won't try to clone it.


https://reviews.llvm.org/D22652