[PATCH] Gut InstCombiner::SimplifyMemTransfer.

[David Majnemer via llvm-commits]

Hi Lang,

I reviewed the InstCombine side and it looks great, all that I ask is that
you replace the `Instruction *` with `auto *` because the type is
replicated in the `dyn_cast`.

My only concern is that clang will actually create calls to the memcpy
intrinsic from within clang with pointers which exactly alias:
http://clang.llvm.org/doxygen/CGExprAgg_8cpp_source.html#l01448

I cannot confess to being an alias-analysis expert but my concern is that
annotating the memcpy as such could create some interesting paradoxes.
Those more familiar with AA should chime in.

I'm afraid I cannot provide any guidance on the SelectionDAG changes.

Thanks
-- 
David Majnemer

On Wed, Aug 12, 2015 at 10:06 PM, Lang Hames via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Another belated *ping*.
>
> - 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150812/4738ec3f/attachment.html>