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

[Dan Gohman via Phabricator via llvm-commits]

sunfish marked 3 inline comments as done.
sunfish added inline comments.


================
Comment at: lib/Transforms/Scalar/MemCpyOptimizer.cpp:1274
     if (MemSetInst *MDep = dyn_cast<MemSetInst>(SrcDepInfo.getInst()))
       if (performMemCpyToMemSetOptzn(M, MDep)) {
         MD->removeInstruction(M);
----------------
sanjoy wrote:
> 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)?
Makes sense. I've now added the asserts.


Repository:
  rL LLVM

https://reviews.llvm.org/D38374