* [PATCH][aarch64] Enable ifunc resolver attribute by default
@ 2017-06-12 15:02 Steve Ellcey
2017-09-04 14:40 ` Szabolcs Nagy
0 siblings, 1 reply; 6+ messages in thread
From: Steve Ellcey @ 2017-06-12 15:02 UTC (permalink / raw)
To: gcc-patches
I recently noticed that the GCC 'resolver' attribute used for ifunc's is not
on by default for aarch64 even though all the infrastructure to support it is
in place. I made memcpy an ifunc on aarch64 in glibc and am looking at
possibly using it for libatomic too. For this reason I would like to enable
it by default. Note that the memcpy ifunc works even when this is not enabled
because glibc enables ifuncs by using the assembly language .type psuedo-op to
set the resolver attribute when GCC cannot do it with an attribute. Using
an ifunc in libatomic does require this to be enabled and I do not see any
reason not to have it enabled by default.
Tested with no regressions, OK to check in?
Steve Ellcey
sellcey@cavium.com
2017-06-12 Steve Ellcey <sellcey@cavium.com>
* config.gcc (aarch64*-*-linux*): Enable IFUNC by default.
diff --git a/gcc/config.gcc b/gcc/config.gcc
index a311cd95..e4caca4 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -974,6 +974,7 @@ aarch64*-*-freebsd*)
tmake_file="${tmake_file} aarch64/t-aarch64 aarch64/t-aarch64-freebsd"
;;
aarch64*-*-linux*)
+ default_gnu_indirect_function=yes
tm_file="${tm_file} dbxelf.h elfos.h gnu-user.h linux.h glibc-stdint.h"
tm_file="${tm_file} aarch64/aarch64-elf.h aarch64/aarch64-linux.h"
tmake_file="${tmake_file} aarch64/t-aarch64 aarch64/t-aarch64-linux"
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][aarch64] Enable ifunc resolver attribute by default
2017-06-12 15:02 [PATCH][aarch64] Enable ifunc resolver attribute by default Steve Ellcey
@ 2017-09-04 14:40 ` Szabolcs Nagy
2017-09-05 17:09 ` Steve Ellcey
0 siblings, 1 reply; 6+ messages in thread
From: Szabolcs Nagy @ 2017-09-04 14:40 UTC (permalink / raw)
To: sellcey, gcc-patches; +Cc: nd
On 12/06/17 16:02, Steve Ellcey wrote:
> I recently noticed that the GCC 'resolver' attribute used for ifunc's is not
> on by default for aarch64 even though all the infrastructure to support it is
> in place. I made memcpy an ifunc on aarch64 in glibc and am looking at
> possibly using it for libatomic too. For this reason I would like to enable
> it by default. Note that the memcpy ifunc works even when this is not enabled
> because glibc enables ifuncs by using the assembly language .type psuedo-op to
> set the resolver attribute when GCC cannot do it with an attribute. Using
> an ifunc in libatomic does require this to be enabled and I do not see any
> reason not to have it enabled by default.
>
> Tested with no regressions, OK to check in?
>
this is not the right default for bionic, uclibc and musl
(gcc does not distinguish between supporting ifunc in the
compiler vs runtime, so when ifunc is enabled it is assumed
the c runtime will have support too, hence libatomic and
libgcc starts using ifuncs which breaks at runtime)
so don't change the default if target matches
*-*-*android*|*-*-*uclibc*|*-*-*musl*)
(i think the default should be kept "no" for these targets
independently of cpu arch, so the current logic that is
repeated many places in config.gcc is suboptimal.
and i think the attribute syntax should be always supported
and this setting should only mean that ifunc use is allowed
in the runtime libraries.)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][aarch64] Enable ifunc resolver attribute by default
2017-09-04 14:40 ` Szabolcs Nagy
@ 2017-09-05 17:09 ` Steve Ellcey
2017-09-05 17:20 ` Szabolcs Nagy
2017-09-21 12:37 ` Joseph Myers
0 siblings, 2 replies; 6+ messages in thread
From: Steve Ellcey @ 2017-09-05 17:09 UTC (permalink / raw)
To: Szabolcs Nagy, gcc-patches; +Cc: nd
[-- Attachment #1: Type: text/plain, Size: 1759 bytes --]
On Mon, 2017-09-04 at 15:40 +0100, Szabolcs Nagy wrote:
> this is not the right default for bionic, uclibc and musl
>
> (gcc does not distinguish between supporting ifunc in the
> compiler vs runtime, so when ifunc is enabled it is assumed
> the c runtime will have support too, hence libatomic and
> libgcc starts using ifuncs which breaks at runtime)
>
> so don't change the default if target matches
> *-*-*android*|*-*-*uclibc*|*-*-*musl*)
>
> (i think the default should be kept "no" for these targets
> independently of cpu arch, so the current logic that is
> repeated many places in config.gcc is suboptimal.
I cleaned up config.gcc so default_gnu_indirect_function is set in a
single place now and has the right defaults for android/uclibc/musl.
> and i think the attribute syntax should be always supported
> and this setting should only mean that ifunc use is allowed
> in the runtime libraries.)
I think that might be a reasonable thing to do but should be a
separate patch from this change, so I have not done anything
with that.
I retested on aarch64 but I did not test any of the other platforms
where I moved the setting of default_gnu_indirect_function, but I
don't think I changed any defaults.
Steve Ellcey
sellcey@cavium.com
2017-09-05  Steve Ellcey  <sellcey@cavium.com>
        * config.gcc: Add new case statement to set
        default_gnu_indirect_function.  Remove it from x86_64-*-linux*,
        i[34567]86-*, powerpc*-*-linux*spe*, powerpc*-*-linux*, s390-*-linux*,
        s390x-*-linux* case statements.   Added aarch64 to the list of
        supported architectures.
[-- Attachment #2: ifunc.gcc.patch --]
[-- Type: text/x-patch, Size: 3169 bytes --]
diff --git a/gcc/config.gcc b/gcc/config.gcc
index cc56c57..1a1b2fe 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -1516,14 +1516,6 @@ i[34567]86-*-linux* | i[34567]86-*-kfreebsd*-gnu | i[34567]86-*-gnu* | i[34567]8
i[34567]86-*-linux*)
tm_file="${tm_file} linux.h linux-android.h"
extra_options="${extra_options} linux-android.opt"
- # Assume modern glibc if not targeting Android nor uclibc.
- case ${target} in
- *-*-*android*|*-*-*uclibc*|*-*-*musl*)
- ;;
- *)
- default_gnu_indirect_function=yes
- ;;
- esac
if test x$enable_targets = xall; then
tm_file="${tm_file} i386/x86-64.h i386/gnu-user-common.h i386/gnu-user64.h i386/linux-common.h i386/linux64.h"
tm_defines="${tm_defines} TARGET_BI_ARCH=1"
@@ -1582,14 +1574,6 @@ x86_64-*-linux* | x86_64-*-kfreebsd*-gnu)
x86_64-*-linux*)
tm_file="${tm_file} linux.h linux-android.h i386/linux-common.h i386/linux64.h"
extra_options="${extra_options} linux-android.opt"
- # Assume modern glibc if not targeting Android nor uclibc.
- case ${target} in
- *-*-*android*|*-*-*uclibc*|*-*-*musl*)
- ;;
- *)
- default_gnu_indirect_function=yes
- ;;
- esac
;;
x86_64-*-kfreebsd*-gnu)
tm_file="${tm_file} kfreebsd-gnu.h i386/kfreebsd-gnu64.h"
@@ -2455,7 +2439,6 @@ powerpc*-*-linux*spe*)
tm_file="${tm_file} powerpcspe/linux.h glibc-stdint.h"
tmake_file="${tmake_file} powerpcspe/t-ppcos powerpcspe/t-linux"
tm_file="${tm_file} powerpcspe/linuxspe.h powerpcspe/e500.h"
- default_gnu_indirect_function=yes
;;
powerpc*-*-linux*)
tm_file="${tm_file} dbxelf.h elfos.h gnu-user.h freebsd-spec.h rs6000/sysv4.h"
@@ -2535,14 +2518,6 @@ powerpc*-*-linux*)
if test x${enable_secureplt} = xyes; then
tm_file="rs6000/secureplt.h ${tm_file}"
fi
- # Assume modern glibc if not targeting Android nor uclibc.
- case ${target} in
- *-*-*android*|*-*-*uclibc*|*-*-*musl*)
- ;;
- *)
- default_gnu_indirect_function=yes
- ;;
- esac
;;
powerpc-wrs-vxworksspe)
tm_file="${tm_file} elfos.h freebsd-spec.h powerpcspe/sysv4.h"
@@ -2664,7 +2639,6 @@ rx-*-elf*)
tmake_file="${tmake_file} rx/t-rx"
;;
s390-*-linux*)
- default_gnu_indirect_function=yes
tm_file="s390/s390.h dbxelf.h elfos.h gnu-user.h linux.h glibc-stdint.h s390/linux.h"
c_target_objs="${c_target_objs} s390-c.o"
cxx_target_objs="${cxx_target_objs} s390-c.o"
@@ -2674,7 +2648,6 @@ s390-*-linux*)
tmake_file="${tmake_file} s390/t-s390"
;;
s390x-*-linux*)
- default_gnu_indirect_function=yes
tm_file="s390/s390x.h s390/s390.h dbxelf.h elfos.h gnu-user.h linux.h glibc-stdint.h s390/linux.h"
tm_p_file="linux-protos.h s390/s390-protos.h"
c_target_objs="${c_target_objs} s390-c.o"
@@ -3120,6 +3093,20 @@ case ${target} in
;;
esac
+# Assume the existence of indirect function support and allow the use of the
+# resolver attribute.
+case ${target} in
+*-*-linux*android*|*-*-linux*uclibc*|*-*-linux*musl*)
+ ;;
+*-*-linux*)
+ case ${target} in
+ aarch64*-* | i[34567]86-* | powerpc*-* | s390*-* | x86_64-*)
+ default_gnu_indirect_function=yes
+ ;;
+ esac
+ ;;
+esac
+
# Build mkoffload tool
case ${target} in
*-intelmic-* | *-intelmicemul-*)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][aarch64] Enable ifunc resolver attribute by default
2017-09-05 17:09 ` Steve Ellcey
@ 2017-09-05 17:20 ` Szabolcs Nagy
2017-09-21 12:37 ` Joseph Myers
1 sibling, 0 replies; 6+ messages in thread
From: Szabolcs Nagy @ 2017-09-05 17:20 UTC (permalink / raw)
To: sellcey, gcc-patches; +Cc: nd
On 05/09/17 18:09, Steve Ellcey wrote:
> On Mon, 2017-09-04 at 15:40 +0100, Szabolcs Nagy wrote:
>
>> this is not the right default for bionic, uclibc and musl
>>
>> (gcc does not distinguish between supporting ifunc in the
>> compiler vs runtime, so when ifunc is enabled it is assumed
>> the c runtime will have support too, hence libatomic and
>> libgcc starts using ifuncs which breaks at runtime)
>>
>> so don't change the default if target matches
>> *-*-*android*|*-*-*uclibc*|*-*-*musl*)
>>
>> (i think the default should be kept "no" for these targets
>> independently of cpu arch, so the current logic that is
>> repeated many places in config.gcc is suboptimal.
>
> I cleaned up config.gcc so default_gnu_indirect_function is set in a
> single place now and has the right defaults for android/uclibc/musl.
>
thanks, it looks ok to me (but i cannot approve the patch).
>> and i think the attribute syntax should be always supported
>> and this setting should only mean that ifunc use is allowed
>> in the runtime libraries.)
>
> I think that might be a reasonable thing to do but should be a
> separate patch from this change, so I have not done anything
> with that.
>
> I retested on aarch64 but I did not test any of the other platforms
> where I moved the setting of default_gnu_indirect_function, but I
> don't think I changed any defaults.
>
> Steve Ellcey
> sellcey@cavium.com
>
>
> 2017-09-05 Steve Ellcey <sellcey@cavium.com>
>
> * config.gcc: Add new case statement to set
> default_gnu_indirect_function. Remove it from x86_64-*-linux*,
> i[34567]86-*, powerpc*-*-linux*spe*, powerpc*-*-linux*, s390-*-linux*,
> s390x-*-linux* case statements. Added aarch64 to the list of
> supported architectures.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][aarch64] Enable ifunc resolver attribute by default
2017-09-05 17:09 ` Steve Ellcey
2017-09-05 17:20 ` Szabolcs Nagy
@ 2017-09-21 12:37 ` Joseph Myers
2017-09-22 18:06 ` Steve Ellcey
1 sibling, 1 reply; 6+ messages in thread
From: Joseph Myers @ 2017-09-21 12:37 UTC (permalink / raw)
To: Steve Ellcey
Cc: Szabolcs Nagy, gcc-patches, nd, richard.earnshaw,
james.greenhalgh, marcus.shawcroft, davem, ebotcazou, nickc,
ramana.radhakrishnan, kyrylo.tkachov
[-- Attachment #1: Type: text/plain, Size: 1345 bytes --]
On Tue, 5 Sep 2017, Steve Ellcey wrote:
> 2017-09-05  Steve Ellcey  <sellcey@cavium.com>
>
> Â Â Â Â Â Â Â Â * config.gcc: Add new case statement to set
>         default_gnu_indirect_function.  Remove it from x86_64-*-linux*,
> Â Â Â Â Â Â Â Â i[34567]86-*, powerpc*-*-linux*spe*, powerpc*-*-linux*, s390-*-linux*,
>         s390x-*-linux* case statements.   Added aarch64 to the list of
> Â Â Â Â Â Â Â Â supported architectures.
This is OK subject to AArch64 maintainer approval, or in the absence of
AArch64 maintainer objections within 24 hours.
I think we also need to add SPARC to the list of architectures here to fix
the glibc build there, but that can be done separately. It would make
sense to add ARM, since that has IFUNC support as well (but it's not
needed for the glibc build there at present, as all the multi-arch ARM
IFUNCs are defined in .S files). And as discussed I'm dubious of having
an architecture list here at all - it would be better for GCC to support
this feature for all GNU targets, so that existing GCC versions work with
new binutils when the feature is added for more architectures - but that
can also be considered separately.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][aarch64] Enable ifunc resolver attribute by default
2017-09-21 12:37 ` Joseph Myers
@ 2017-09-22 18:06 ` Steve Ellcey
0 siblings, 0 replies; 6+ messages in thread
From: Steve Ellcey @ 2017-09-22 18:06 UTC (permalink / raw)
To: Joseph Myers
Cc: Szabolcs Nagy, gcc-patches, nd, richard.earnshaw,
james.greenhalgh, marcus.shawcroft, davem, ebotcazou, nickc,
ramana.radhakrishnan, kyrylo.tkachov
On Thu, 2017-09-21 at 12:37 +0000, Joseph Myers wrote:
> On Tue, 5 Sep 2017, Steve Ellcey wrote:
>
> >
> > 2017-09-05  Steve Ellcey  <sellcey@cavium.com>
> >
> > Â Â Â Â Â Â Â Â * config.gcc: Add new case statement to set
> >         default_gnu_indirect_function.  Remove it from x86_64-*-linux*,
> > Â Â Â Â Â Â Â Â i[34567]86-*, powerpc*-*-linux*spe*, powerpc*-*-linux*, s390-*-linux*,
> >         s390x-*-linux* case statements.   Added aarch64 to the list of
> > Â Â Â Â Â Â Â Â supported architectures.
> This is OK subject to AArch64 maintainer approval, or in the absence ofÂ
> AArch64 maintainer objections within 24 hours.
Since I didn't see any objections I have checked this in.
Steve Ellcey
sellcey@cavium.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-09-22 18:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-12 15:02 [PATCH][aarch64] Enable ifunc resolver attribute by default Steve Ellcey
2017-09-04 14:40 ` Szabolcs Nagy
2017-09-05 17:09 ` Steve Ellcey
2017-09-05 17:20 ` Szabolcs Nagy
2017-09-21 12:37 ` Joseph Myers
2017-09-22 18:06 ` Steve Ellcey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).