Re: [PATCH] elf: Remove -fno-tree-loop-distribute-patterns usage on dl-support

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 09/08/22 16:34, H.J. Lu wrote:
> On Tue, Aug 9, 2022 at 12:32 PM Adhemerval Zanella Netto
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 09/08/22 16:06, H.J. Lu wrote:
>>> On Tue, Aug 9, 2022 at 11:48 AM Adhemerval Zanella Netto
>>> <adhemerval.zanella@linaro.org> wrote:
>>>>
>>>>
>>>>
>>>> On 09/08/22 15:31, H.J. Lu wrote:
>>>>> On Tue, Aug 9, 2022 at 5:11 AM Adhemerval Zanella Netto
>>>>> <adhemerval.zanella@linaro.org> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 08/08/22 19:16, H.J. Lu wrote:
>>>>>>
>>>>>>>> --- a/sysdeps/x86_64/multiarch/memset-evex-unaligned-erms.S
>>>>>>>> +++ b/sysdeps/x86_64/multiarch/memset-evex-unaligned-erms.S
>>>>>>>> @@ -44,4 +44,7 @@
>>>>>>>>
>>>>>>>>  # define USE_LESS_VEC_MASK_STORE       1
>>>>>>>>  # include "memset-vec-unaligned-erms.S"
>>>>>>>> +# if !defined NO_MULTIARCH && MINIMUM_X86_ISA_LEVEL >= 4
>>>>>>>> +strong_alias (__memset_evex_unaligned, __memset_generic)
>>>>>>>> +# endif
>>>>>>>>  #endif
>>>>>>>
>>>>>>> This will define 2 __memset_generic for -march=x86-64-v4.
>>>>>>
>>>>>> Are you sure? Check with a x86_64 build with -march=x86-64-v4 I am seeing:
>>>>>>
>>>>>> $ readelf -Ws string/memset*.os | grep -w __memset_generic | wc -l
>>>>>> 1
>>>>>>
>>>>>> And it is indeed provided only by string/memset-evex-unaligned-erms.os.
>>>>>
>>>>> You are right.  Since dl-symbol-hacks.h defines memset to an alias,
>>>>> should it be moved to sysdes/generic and each arch can provide a
>>>>> suitable alias?
>>>>
>>>> The dl-symbol-hacks.h is meant to be used solely by loader code (that's
>>>> why I have added to elf/), but I don't have a strong preference.
>>>
>>> The default should be no alias.  Only IFUNC targets need to define a
>>> proper alias.
>>
>> Alright, although I added the alias on all targets to avoid adding another
>> hook to specify whether memset is implemented by IFUNC (to enable the
>> symbol redirection instead of use the default symbol name).  I am not sure
>> if making arch-specific will simplify the required code here.
> 
> But your patch adds alias to all targets.

Yes, because otherwise we will need something like HAVE_MEMSET_IFUNC to add:

#if defined SHARED && HAVE_MEMSET_IFUNC
asm ("memset = __memset_generic");
#endif

I don't have a strong preference, although it is another this will require
to have kernel-features.h (which we usually add the HAVE_*) on elf objects.