[PATCH] D38374: [memcpyopt] Memcpy-memcpy dependence isn't detected across basic blocks
Dan Gohman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 28 16:15:03 PDT 2017
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.
More information about the llvm-commits