[PATCH] D38374: [memcpyopt] Memcpy-memcpy dependence isn't detected across basic blocks

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy accepted this revision.
sanjoy added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lib/Transforms/Scalar/MemCpyOptimizer.cpp:1274
     if (MemSetInst *MDep = dyn_cast<MemSetInst>(SrcDepInfo.getInst()))
       if (performMemCpyToMemSetOptzn(M, MDep)) {
         MD->removeInstruction(M);
----------------
sunfish wrote:
> sanjoy wrote:
> > IIUC, this may try to create illegal IR (def not dominating use) if it finds the non-local memset via a backedge since it tries to create a memset before the memcpy using the memset's value operand, which may not dominate the memcpy with your change.
> If the memset doesn't dominate the memcpy, the non-local dependence results reflect this. For example, in
> ```
>     for (...) {
>         memcpy(out, buffer, size);
>         memset(buffer, 0, size);
>     }
> ```
> the memcpy depends on the memset around the backend, but memdep reports two dependencies: the memcpy on the backedge, but also whatever there is on the loop entry edge, so the code in this patch requiring a single dependency doesn't accept it.
Ok, can you then add an assert that `SrcDepInfo.getInst()` dominates `M` (specifically in the cases where you computed it from a singular non-local dependency)?


Repository:
  rL LLVM

https://reviews.llvm.org/D38374