Review decision to inline mempcpy to memcpy.

Review decision to inline mempcpy to memcpy.

[Carlos O'Donell]

The upstream gcc PR/70055 requests that glibc revert commit
05a910f7b420c2b831f35ba90e61c80f001c0606 and instead work
with gcc to make the builtin mempcpy better (for various
aspects of performance).

The crux of the argument is that the compiler may be able
to do a better job of optimizing if it knows the call was
a mempcpy as opposed to memcpy + addition.

I understand the need of glibc machine maintainers to produce
a library whose performance is as optimal as they can given
the compilers we have today. This leads to decisions like those
we made to transform mempcpy to memcpy + addition.

I also understand the worries that the compiler developers see
in such a transformation. Information has been lost to the
compiler that might generated better code.

Is there a middle ground here? Should machines with mempcpy.S,
particularly x86_64 define _HAVE_STRING_ARCH_mempcpy in their
string.h? I think this question wasn't clearly answered before
the patch went in. However, the microbenchmarks show this is
a clear gain given modern compilers for x86_64. Is this because
x86_64's mempcpy.S is flawed? Does it need to be fixed as
opposed to transforming mempcpy to memcpy+addition?

Ondrej,

Did you file gcc bugs to revuew the optimization issues
with current mempcpy as suggested in [2] and [3]?

Wilco,

Were the changes in glibc to optimize mempcpy as memcpy
originally motivated by performance for ARM?

Particularly because ARM does not have an optimized
mempcpy implementation in glibc?

Cheers,
Carlos.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70055
[2] https://sourceware.org/ml/libc-alpha/2015-05/msg00778.html
[3] https://sourceware.org/ml/libc-alpha/2015-05/msg00780.html

Re: Review decision to inline mempcpy to memcpy.

[H.J. Lu]

On Thu, Mar 3, 2016 at 7:23 AM, Carlos O'Donell <carlos@redhat.com> wrote:
> The upstream gcc PR/70055 requests that glibc revert commit
> 05a910f7b420c2b831f35ba90e61c80f001c0606 and instead work
> with gcc to make the builtin mempcpy better (for various
> aspects of performance).
>
> The crux of the argument is that the compiler may be able
> to do a better job of optimizing if it knows the call was
> a mempcpy as opposed to memcpy + addition.

I opened:

https://sourceware.org/bugzilla/show_bug.cgi?id=19759

> I understand the need of glibc machine maintainers to produce
> a library whose performance is as optimal as they can given
> the compilers we have today. This leads to decisions like those
> we made to transform mempcpy to memcpy + addition.
>
> I also understand the worries that the compiler developers see
> in such a transformation. Information has been lost to the
> compiler that might generated better code.
>
> Is there a middle ground here? Should machines with mempcpy.S,
> particularly x86_64 define _HAVE_STRING_ARCH_mempcpy in their
> string.h? I think this question wasn't clearly answered before
> the patch went in. However, the microbenchmarks show this is
> a clear gain given modern compilers for x86_64. Is this because
> x86_64's mempcpy.S is flawed? Does it need to be fixed as
> opposed to transforming mempcpy to memcpy+addition?
>
> Ondrej,
>
> Did you file gcc bugs to revuew the optimization issues
> with current mempcpy as suggested in [2] and [3]?
>
> Wilco,
>
> Were the changes in glibc to optimize mempcpy as memcpy
> originally motivated by performance for ARM?

Can we make it opt-in?

> Particularly because ARM does not have an optimized
> mempcpy implementation in glibc?

It is 3 lines of code for x86-64:

#ifdef USE_AS_MEMPCPY
        add     %rdx, %rax
#endif

How hard will ARM be?

-- 
H.J.

Re: Review decision to inline mempcpy to memcpy.

[Carlos O'Donell]

On 03/03/2016 03:55 PM, H.J. Lu wrote:
> On Thu, Mar 3, 2016 at 7:23 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>> The upstream gcc PR/70055 requests that glibc revert commit
>> 05a910f7b420c2b831f35ba90e61c80f001c0606 and instead work
>> with gcc to make the builtin mempcpy better (for various
>> aspects of performance).
>>
>> The crux of the argument is that the compiler may be able
>> to do a better job of optimizing if it knows the call was
>> a mempcpy as opposed to memcpy + addition.
> 
> I opened:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=19759

I think x86_64 should define  
#define _HAVE_STRING_ARCH_mempcpy 1 in string.h?

That way the inline is avoided.

HJ, Can you show that doing that gives a speedup?

The point Ondrej makes in his original emails is that
using the inline gave better performance on x86_64.
Should we ignore such findings? Or are the microbenchmarks
we have insufficient to show the overall gains we're
talking about (in terms of the register usage)?

That is to say, should the change be made simply from
first principles that it's a good change, or do we need
a performance justification?

Cheers,
Carlos.

Re: Review decision to inline mempcpy to memcpy.

[Jakub Jelinek]

On Fri, Mar 04, 2016 at 04:48:00PM +0000, Wilco Dijkstra wrote:
> > Were the changes in glibc to optimize mempcpy as memcpy
> > originally motivated by performance for ARM?
> 
> OK, so the goal behind this was to provide the best possible out of the box performance
> in GLIBC without requiring all targets to write a lot of assembler code. For less

If people use mempcpy in their code, they do it for a reason.

> > The crux of the argument is that the compiler may be able
> > to do a better job of optimizing if it knows the call was
> > a mempcpy as opposed to memcpy + addition.
> 
> No, unfortunately even GCC6 optimizes memcpy better than mempcpy:

Must be some aarch64 backend issue then.  On x86_64 I get for memcpy (x, y,
32):
        movq    (%rsi), %rdx
        movq    %rdi, %rax
        movq    %rdx, (%rdi)
        movq    8(%rsi), %rdx
        movq    %rdx, 8(%rdi)
        movq    16(%rsi), %rdx
        movq    %rdx, 16(%rdi)
        movq    24(%rsi), %rdx
        movq    %rdx, 24(%rdi)
        ret
and for mempcpy (x, y, 32):
        movq    (%rsi), %rax
        movq    %rax, (%rdi)
        movq    8(%rsi), %rax
        movq    %rax, 8(%rdi)
        movq    16(%rsi), %rax
        movq    %rax, 16(%rdi)
        movq    24(%rsi), %rax
        movq    %rax, 24(%rdi)
        leaq    32(%rdi), %rax
        ret

> If we can get GCC to do the right thing depending of the preference of the target and

That is wrong, the preference isn't cast in stone, but you force it into the
apps.

	Jakub

Re: Review decision to inline mempcpy to memcpy.

[Wilco Dijkstra]

Hi,

(resend to post to GLIBC list too)

> Were the changes in glibc to optimize mempcpy as memcpy
> originally motivated by performance for ARM?

OK, so the goal behind this was to provide the best possible out of the box performance
in GLIBC without requiring all targets to write a lot of assembler code. For less
frequently used functions which are identical to a standard function but with a different
return value the most obvious and efficient implementation is to inline a call to the most
commonly used version. There are several good reasons to do this for mempcpy:

1. Few targets implement mempcpy.S, so currently use the slow mempcpy.c veneer.
2. On most targets merging mempcpy into memcpy looks impossible without
   slowing down memcpy as a result.
3. Adding a separate mempcpy.S implementation increases I-cache pressure as
   now you need to load mempcpy too even if memcpy is already resident in L1/L2 cache.
4. GCC doesn't optimize/inline mempcpy as well as it does memcpy (see below)

> The crux of the argument is that the compiler may be able
> to do a better job of optimizing if it knows the call was
> a mempcpy as opposed to memcpy + addition.

No, unfortunately even GCC6 optimizes memcpy better than mempcpy:

return __builtin_memcpy(x, y, 32);

        ldp     x4, x5, [x1]
        stp     x4, x5, [x0]
        ldp     x4, x5, [x1, 16]
        stp     x4, x5, [x0, 16]
        ret

return __builtin_mempcpy(x, y, 32);

        mov     x2, 32
        b       mempcpy

return mempcpy(x, y, 32);  // using GLIBC2.23 inline

        mov     x2, x0
        add     x0, x0, 32
        ldp     x4, x5, [x1]
        stp     x4, x5, [x2]
        ldp     x4, x5, [x1, 16]
        stp     x4, x5, [x2, 16]
        ret

So the only case where I can see a clear win for mempcpy is if you can do a good
merged implementation and GCC is fixed to optimize mempcpy always in exactly the
same way as memcpy. In that case just define _HAVE_STRING_ARCH_mempcpy.

If we can get GCC to do the right thing depending of the preference of the target and
library then things would be perfect.

Cheers,
Wilco