[PATCH] Gut InstCombiner::SimplifyMemTransfer.

[Daniel Berlin]

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