This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] x86-64: Implement strcpy family IFUNC selectors in C
On Tue, Jun 13, 2017 at 12:54 PM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
> On 11/06/2017 20:33, H.J. Lu wrote:
>> On Sun, Jun 11, 2017 at 12:46 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Sun, Jun 11, 2017 at 11:22 AM, Florian Weimer
>>> <fweimer@gapps.redhat.com> wrote:
>>>> On 06/11/2017 03:50 PM, H.J. Lu wrote:
>>>>> All internal calls within libc.so can use IFUNC on x86-64 since unlike
>>>>> x86, x86-64 doesn't need to reserve a register to make a PLT call.
>>>>
>>>> Shouldn't this rather say, “because, unlike x86, x86-64 supports PC-relative
>>>> addressing to access the GOT entry”? That's probably the root cause of the
>>> Both are true. The end result is there is no extra needed for call
>>> via PLT.
>>>
>>>> performance difference.
>>>>
>> Here is the patch with updated commit message. I also renamed
>> ifunc-strcpy.h to ifunc-unaligned-ssse3.h so that it can be used for
>> strcat.
>>
>> Any other comments?
>>
>
> LGTM with just some remarks below.
>
>> +
>> +# include <sysdep.h>
>> +# define __stpcpy __stpcpy_sse2
>> +
>> +# undef weak_alias
>> +# define weak_alias(__stpcpy, stpcpy)
>> +# undef libc_hidden_def
>> +# define libc_hidden_def(__stpcpy)
>> +# undef libc_hidden_builtin_def
>> +# define libc_hidden_builtin_def(stpcpy)
>> +#endif
>> +
>> +#define USE_AS_STPCPY
>> +#include "../stpcpy.S"
>
> I would recommend use full path.
It has been done already.
>> +
>> +/* Define multiple versions only for the definition in libc. */
>> +#if IS_IN (libc)
>> +# define _HAVE_STRING_ARCH_stpcpy 1
>
> I think we will need to revise this later, since the idea is to remove string2.h.
>
Sure.
>> diff --git a/sysdeps/x86_64/multiarch/stpncpy-c.c b/sysdeps/x86_64/multiarch/stpncpy-c.c
>> index 2fde77d..62f69b5 100644
>> --- a/sysdeps/x86_64/multiarch/stpncpy-c.c
>> +++ b/sysdeps/x86_64/multiarch/stpncpy-c.c
>> @@ -1,8 +1,7 @@
>> #define STPNCPY __stpncpy_sse2
>> -#ifdef SHARED
>> +#undef weak_alias
>> +#define weak_alias(__stpncpy, stpncpy)
>> #undef libc_hidden_def
>> -#define libc_hidden_def(name) \
>> - __hidden_ver1 (__stpncpy_sse2, __GI___stpncpy, __stpncpy_sse2);
>> -#endif
>> +#define libc_hidden_def(stpncpy)
>>
>> -#include "stpncpy.c"
>> +#include <string/stpncpy.c>
>
> Shouldn't we add a copyright header on this file (same for sysdeps/x86_64/multiarch/strncpy-c.c)?
A separate patch?
>> +
>> +#include "../strcpy.S"
>
> I would recommend use full path as well.
It has been done already.
--
H.J.