[PATCH] Gut InstCombiner::SimplifyMemTransfer.

[Hal Finkel via llvm-commits]

----- 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