Re: [Xen-devel] [PATCH v2 for-next 2/2] x86/string: Use compiler __builtin_str*() where possible

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 12/05/17 16:42, Julien Grall wrote:
> Hi Andrew,
>
> On 12/05/17 16:30, Andrew Cooper wrote:
>> On 12/05/17 15:56, Jan Beulich wrote:
>>>>>> On 12.05.17 at 16:34, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> --- a/xen/include/asm-x86/string.h
>>>> +++ b/xen/include/asm-x86/string.h
>>>> @@ -10,4 +10,12 @@
>>>>  #define __HAVE_ARCH_MEMSET
>>>>  #define memset(s, c, n)       __builtin_memset(s, c, n)
>>>>
>>>> +#define strcmp(s1, s2)        __builtin_strcmp(s1, s2)
>>>> +#define strncmp(s1, s2, n)    __builtin_strncmp(s1, s2, n)
>>>> +#define strcasecmp(s1, s2)    __builtin_strcasecmp(s1, s2)
>>>> +#define strchr(s1, c)         __builtin_strchr(s1, c)
>>>> +#define strrchr(s1, c)        __builtin_strrchr(s1, c)
>>>> +#define strstr(s1, s2)        __builtin_strstr(s1, s2)
>>>> +#define strlen(s1)            __builtin_strlen(s1)
>>> If the lack of __HAVE_ARCH_* additions is intentional here,
>>
>> Yes - it is deliberate.
>>
>>> why do you keep them for mem*()?
>>
>> We have x86-specific implementation of mem*(), while we use the common
>> implementation of str*().
>>
>> Defining __HAVE_ARCH_STR* causes the common implementation to be
>> omitted, resulting in a link failure.
>>
>>
>> Given that all supported compilers have these builtins, I think it might
>> be better to make this adjustment in common code.  The arguments for
>> using them in x86 are the same as ARM.
>>
>> Julien/Stefano: Thoughts?
>
> Using our own implementation rather than the built-in version gives us
> the liberty to do optimization based on the processor using alternative.
>
> I know that we already use built-in for arch_fetch_and_add, but I am
> planning to revert that as we want to take advantage of new atomics
> instruction in ARMv8.1.

Of course, my "adjustment to common code" would only apply in the case
that an arch doesn't define __HAVE_ARCH_*.

If an arch has an override, that would take priority.

>
> Furthemore, someone mentioned a potential legal issue to linked
> against the built-in. Anyone heard about that?

The main use of these are optimising things like strlen("literal") at
compile time.  The use of -fno-bultin disables this optimisation, but as
we dont link against any runtime libraries, we still provide the
underling implementation in the case that __builtin_*() fall back to a
straight function call.

I can't see any possibility for legal issues here.  It is all down to
exactly which instructions your compiler wants to assemble.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel