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

["H.J. Lu" <hjl.tools@gmail.com>]

On Tue, Aug 9, 2022 at 12:40 PM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> 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.

There are

elf/Makefile:CFLAGS-dl-tunables.c += -fno-tree-loop-distribute-patterns
elf/Makefile:CFLAGS-rtld.c += -fno-tree-loop-distribute-patterns
elf/Makefile:CFLAGS-dl-support.c = -fno-tree-loop-distribute-patterns
include/libc-symbols.h:    __attribute__ ((__optimize__
("-fno-tree-loop-distribute-patterns")))
string/test-string.h:    __attribute__ ((__optimize__
("-fno-tree-loop-distribute-patterns")))

There should be no check on SHARED.   It can be something like

#define SYMBOL_STR1(s) #s
#define SYMBOL_STR(s) SYMBOL_STR1(s)

#ifdef DEFAULT_MEMSET
asm ("memset = " _SYMBOL_STR (DEFAULT_MEMSET));
#endif


-- 
H.J.