Re: [PATCH] Prefer mempcpy to memcpy on x86_64 target (PR middle-end/81657).

["Martin Liška" <mliska@suse.cz>]

On 03/14/2018 01:57 PM, H.J. Lu wrote:
> On Wed, Mar 14, 2018 at 5:54 AM, Martin Liška <mliska@suse.cz> wrote:
>> On 03/13/2018 04:23 PM, Jakub Jelinek wrote:
>>> On Tue, Mar 13, 2018 at 04:19:21PM +0100, Martin Liška wrote:
>>>>> Yes, see e.g. TARGET_LIBC_HAS_FUNCTION target hook,
>>>>> where in particular linux_libc_has_function deals with various C libraries.
>>>>> Of course, in this case you need another target hook, that is dependent both
>>>>> on the target backend and C library.
>>>>>
>>>>> It would be nice to make the target hook a little bit more generic as well,
>>>>> e.g. pass it enum builtin_function and query if it is fast, slow or
>>>>> unknown, or even some kind of cost, where the caller could ask for cost of
>>>>> BUILT_IN_MEMCPY and BUILT_IN_MEMPCPY and decide based on the relative costs.
>>>>
>>>> Let me start with simple return enum value of FAST,SLOW,UNKNOWN. I've added new hook
>>>> definition to gcc/config/gnu-user.h that will point to gnu_libc_function_implementation.
>>>> I would like to implement the function in gcc/targhooks.c, but I don't know how to
>>>> make ifdef according to target?
>>>
>>> Put there just the default implementation (everything is UNKNOWN?).
>>>
>>>> One another issue is that built_in_function is enum defined in tree.h. Thus I'll replace the
>>>> callback argument with int, that will be casted. One last issue: am I right that I'll have to define
>>>> TARGET_LIBC_FUNCTION_IMPLEMENTATION in each config file (similar to no_c99_libc_has_function)?
>>>
>>> And define the i386/x86_64 glibc one in config/i386/*.h, check there
>>> OPTION_GLIBC and only in that case return something other than UNKNOWN.
>>>
>>> And redefine TARGET_LIBC_FUNCTION_IMPLEMENTATION only in that case.
>>>
>>>       Jakub
>>>
>>
>> Hi.
>>
>> I'm sending V2 that can survive bootstrap and regression tests on both x86_64 and ppc64le.
>>
>> Martin
> 
> diff --git a/gcc/testsuite/gcc.c-torture/execute/builtins/mempcpy.c
> b/gcc/testsuite/gcc.c-torture/execute/builtins/mempcpy.c
> index d82e2232d7b..91e1c87f83f 100644
> --- a/gcc/testsuite/gcc.c-torture/execute/builtins/mempcpy.c
> +++ b/gcc/testsuite/gcc.c-torture/execute/builtins/mempcpy.c
> @@ -62,7 +62,7 @@ main_test (void)
>    mempcpy (p + 5, s3, 1);
>    if (memcmp (p, "ABCDEFg", 8))
>      abort ();
> -  mempcpy (p + 6, s1 + 1, l1);
> +  memcpy (p + 6, s1 + 1, l1);
>    if (memcmp (p, "ABCDEF2", 8))
>      abort ();
>  }
> 
> This is a mempcpy test.  Why is mempcpy changed to memcpy?
> 

Because this mempcpy is not optimized out to memcpy and it then aborts.
It's proper to leave here mempcpy I believe.

Martin