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

[Dan Gohman via Phabricator via llvm-commits]

sunfish added a comment.

In https://reviews.llvm.org/D38374#883899, @dberlin wrote:

> In https://reviews.llvm.org/D38374#883874, @sunfish wrote:
>
> > In https://reviews.llvm.org/D38374#883738, @dberlin wrote:
> >
> > > 1. non-local calls in GVN are the cause of pretty much all of it's massive slowdown.
> > >
> > >   Have you measured the effect this has on larger memory-using testcases?
> >
> >
> > I've measured compile times on some C++ and Rust testcases, and it doesn't appear to significantly slow down -O2. memcpyopt remains under 1% of the time, according to -time-passes.
>
>
> I'm not opposed if we are willing to revert it.


Is that approval? :-)

> 
> 
>>> 2. If you are interested, someone already ported it to memoryssa: https://reviews.llvm.org/D26739
>> 
>> That's interesting, though I'm still interested in the focused and incremental patch here.
> 
> Okay. You said"It might be desirable to use MemorySSA, however MemCpyOpt is currently using MemoryDependenceAnalysis for everything, and it seems desirable to avoid using a mix of both within the same pass, and updating the whole pass to use MemorySSA is a bigger project than I'm hoping to take on here."
> 
> 1. There's now a way to do that, with pretty much no work.
> 2. Chandler specifically requested that passes be mixed with an option to turn it on off.
> 
>   So ....

Indeed maybe it wouldn't be as much work as I assumed. That said, I'm still interested in pursuing the current incremental, focused, and simple patch first.


Repository:
  rL LLVM

https://reviews.llvm.org/D38374