[PATCH] Gut InstCombiner::SimplifyMemTransfer.

[Lang Hames via llvm-commits]

Hi David, Hal,

Thanks very much for the review!

The clang issue should be fixed, but I think it's orthogonal to this patch:
Since the memcpys that clang emits aren't annotated they'll be treated
conservatively by my patch (i.e. not optimized).

Unless I've misunderstood that, I think it should be safe to land this once
I make the auto* fix that David suggested?

Cheers,
Lang.


On Sun, Aug 16, 2015 at 3:23 AM, Hal Finkel <hfinkel at anl.gov> wrote:

> ----- Original Message -----
> > From: "Lang Hames via llvm-commits" <llvm-commits at lists.llvm.org>
> > To: "Chandler Carruth" <chandlerc at gmail.com>
> > Cc: "Commit Messages and Patches for LLVM" <llvm-commits at lists.llvm.org>
> > Sent: Wednesday, August 12, 2015 9:06:52 PM
> > Subject: Re: [PATCH] Gut InstCombiner::SimplifyMemTransfer.
> >
> >
> >
> > Another belated *ping*.
> >
>
> The SDAG change LGTM.
>
> The InstCombine change also LGTM, but it seems that we need to fix Clang
> before we can make this change. Let's add this under a flag for testing.
>
>  -Hal
>
> >
> >
> > - Lang.
> >
> >
> > On Tue, Jun 30, 2015 at 2:16 PM, Lang Hames < lhames at gmail.com >
> > wrote:
> >
> >
> >
> > Belated ping. :)
> >
> >
> > - Lang.
> >
> > On Tue, May 12, 2015 at 2:11 PM, Lang Hames < lhames at gmail.com >
> > wrote:
> >
> >
> >
> > Gentle ping.
> >
> >
> > - Lang.
> >
> >
> >
> >
> > On Tue, Apr 28, 2015 at 11:51 AM, Lang Hames < lhames at gmail.com >
> > wrote:
> >
> >
> >
> > Ping / delayed reply.
> >
> >
> > DannyB - thanks for the extra context. :)
> >
> >
> > Chandler - any thoughts? Assuming you still want to keep this
> > transform, what do you think of my instcombine change? (i.e. tagging
> > the load/store pointers as non-aliasing for memcpy).
> >
> >
> > Cheers,
> > Lang.
> >
> >
> >
> >
> >
> >
> > On Tue, Apr 21, 2015 at 9:21 PM, Daniel Berlin < dberlin at dberlin.org
> > > wrote:
> >
> >
> > On Wed, Apr 8, 2015 at 10:38 PM, Chandler Carruth <
> > chandlerc at gmail.com > wrote:
> > > On Wed, Apr 8, 2015 at 10:15 PM Lang Hames < lhames at gmail.com >
> > > wrote:
> > >>
> > >> Hi David, Chandler,
> > >>
> > >> The attached patch guts the SimplifyMemTransfer method, which
> > >> turns small
> > >> memcpys (1/2/4/8 bytes) into load/store pairs. Turning memcpys
> > >> into
> > >> load/store pairs loses information, since we can no longer assume
> > >> the source
> > >> and dest are non-overlapping. This is leading to some suboptimal
> > >> expansions
> > >> for small memcpys on AArch64 when -mno-unaligned-access is turned
> > >> on (see
> > >> r234462). I suspect other architectures would suffer similar
> > >> issues.
> > >>
> > >> I assume this transform is an old workaround to simplify other
> > >> non-memcpy-aware IR transforms. These days I think most IR
> > >> transforms can
> > >> reason sensibly about memcpys, so I'm hoping this is safe to
> > >> remove. FWIW,
> > >> removing it didn't hit any regression tests except those that were
> > >> verifying
> > >> that this optimisation was being applied, but then you wouldn't
> > >> really
> > >> expect it to hit any others.
> > >
> > >
> > > Heh. I tried to remove it before and it regressed a *lot* of
> > > performance.
> > > Have you measured it? I think there are many places that don't
> > > today reason
> > > about memcpy but do reason about loads and stores. Here is a
> > > partial list:
> > >
> > > - GVN
> >
> > Note:
> >
> > It reasons about memory intrinsics, including memcpy and memset, and
> > will pull values out to forward.
> >
> > for memset, if the load is in the memset size, it will compute what
> > the value is.
> > For memcpy, it handles anything that is constant memory.
> >
> >
> > (Certainly, loads and stores are handled better, but ...)
> >
> >
> >
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150818/4c8b20fe/attachment.html>