[PATCH] Gut InstCombiner::SimplifyMemTransfer.

[Lang Hames]

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 ...)
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150630/b24815b6/attachment.html>