* [RFC PATCH] Implementing ifunc target hook @ 2012-12-26 12:15 Alexander Ivchenko 2012-12-26 17:06 ` Joseph S. Myers 2012-12-27 22:59 ` Maxim Kuvyrkov 0 siblings, 2 replies; 16+ messages in thread From: Alexander Ivchenko @ 2012-12-26 12:15 UTC (permalink / raw) To: gcc-patches, Joseph S. Myers, H.J. Lu, pavel.v.chupin, Sergey Ostanevich, Maxim Kuvyrkov [-- Attachment #1: Type: text/plain, Size: 374 bytes --] Hi, Currently Android dynamic loader does not support indirect functions (And I don't think that it will someday). But there is no way for us to specify that for gcc, and for example, tests like gcc.dg/attr-ifunc-* are failing on android right now. The attached patch is indended to add the target hook for indicating the support of ifunc on target. Thank you, Alexander [-- Attachment #2: disable_ifunc_for_android2.patch --] [-- Type: application/octet-stream, Size: 6685 bytes --] diff --git a/ChangeLog b/ChangeLog index fc8f60b..61b733d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +2012-12-26 Alexander Ivchenko <alexander.ivchenko@intel.com> + + * gcc/doc/tm.texi.in (TARGET_HAS_IFUNC): New target hook. + * gcc/doc/tm.texi: Regenerate + * gcc/defaults.h (TARGET_HAS_IFUNC): Defining it by default + as HAVE_GNU_INDIRECT_FUNCTION. + * gcc/config/linux-android.h (TARGET_HAS_IFUNC): Disable + support of indirect functions in android. + * gcc/config/i386/i386.c (ix86_get_function_versions_dispatcher): + Using TARGET_HAS_IFUNC hook instead of HAVE_GNU_INDIRECT_FUNCTION. + * gcc/varasm.c (do_assemble_alias): Likewise. + 2012-12-20 Matthias Klose <doko@ubuntu.com> * Makefile.def (install-target-libgfortran): Depend on diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index b466a4f..2af52bf 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -29146,7 +29146,7 @@ make_name (tree decl, const char *suffix, bool make_unique) return global_var_name; } -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) /* Make a dispatcher declaration for the multi-versioned function DECL. Calls to DECL function will be replaced with calls to the dispatcher @@ -29213,7 +29213,7 @@ ix86_get_function_versions_dispatcher (void *decl) tree dispatch_decl = NULL; -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) struct cgraph_function_version_info *it_v = NULL; struct cgraph_node *dispatcher_node = NULL; struct cgraph_function_version_info *dispatcher_version_info = NULL; @@ -29263,24 +29263,33 @@ ix86_get_function_versions_dispatcher (void *decl) default_node = default_version_info->this_node; -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION - /* Right now, the dispatching is done via ifunc. */ - dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); - - dispatcher_node = cgraph_get_create_node (dispatch_decl); - gcc_assert (dispatcher_node != NULL); - dispatcher_node->dispatcher_function = 1; - dispatcher_version_info - = insert_new_cgraph_node_version (dispatcher_node); - dispatcher_version_info->next = default_version_info; - dispatcher_node->local.finalized = 1; - - /* Set the dispatcher for all the versions. */ - it_v = default_version_info; - while (it_v->next != NULL) +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) + if (TARGET_HAS_IFUNC) + { + /* Right now, the dispatching is done via ifunc. */ + dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); + + dispatcher_node = cgraph_get_create_node (dispatch_decl); + gcc_assert (dispatcher_node != NULL); + dispatcher_node->dispatcher_function = 1; + dispatcher_version_info + = insert_new_cgraph_node_version (dispatcher_node); + dispatcher_version_info->next = default_version_info; + dispatcher_node->local.finalized = 1; + + /* Set the dispatcher for all the versions. */ + it_v = default_version_info; + while (it_v->next != NULL) + { + it_v->dispatcher_resolver = dispatch_decl; + it_v = it_v->next; + } + } + else { - it_v->dispatcher_resolver = dispatch_decl; - it_v = it_v->next; + error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), + "multiversioning needs ifunc which is not supported " + "on this target"); } #else error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), diff --git a/gcc/config/linux-android.h b/gcc/config/linux-android.h index e74e261..f6f44f1 100644 --- a/gcc/config/linux-android.h +++ b/gcc/config/linux-android.h @@ -58,3 +58,6 @@ #define ANDROID_ENDFILE_SPEC \ "%{shared: crtend_so%O%s;: crtend_android%O%s}" + +#undef TARGET_HAS_IFUNC +#define TARGET_HAS_IFUNC (!TARGET_ANDROID) diff --git a/gcc/defaults.h b/gcc/defaults.h index 76909ab..2180a47 100644 --- a/gcc/defaults.h +++ b/gcc/defaults.h @@ -111,6 +111,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see #endif #endif +#ifndef TARGET_HAS_IFUNC +#define TARGET_HAS_IFUNC HAVE_GNU_INDIRECT_FUNCTION +#endif + #ifndef IFUNC_ASM_TYPE #define IFUNC_ASM_TYPE "gnu_indirect_function" #endif diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index 75aa867..1c8bc51 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -5847,6 +5847,12 @@ If @code{ASM_OUTPUT_DEF} is not available, the hook's default definition is @code{NULL}, which disables the use of section anchors altogether. @end deftypefn +@defmac TARGET_HAS_IFUNC +When this macro is nonzero, GCC will assume that target supports GNU indirect +functions. The support includes the assembler, linker and dynamic linker. The +default value of this hook is defined as for the host machine. +@end defmac + @deftypefn {Target Hook} bool TARGET_USE_ANCHORS_FOR_SYMBOL_P (const_rtx @var{x}) Return true if GCC should attempt to use anchors to access @code{SYMBOL_REF} @var{x}. You can assume @samp{SYMBOL_REF_HAS_BLOCK_INFO_P (@var{x})} and diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index 95fab18..e1646b8 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -5751,6 +5751,12 @@ If @code{ASM_OUTPUT_DEF} is not available, the hook's default definition is @code{NULL}, which disables the use of section anchors altogether. @end deftypefn +@defmac TARGET_HAS_IFUNC +When this macro is nonzero, GCC will assume that target supports GNU indirect +functions. The support includes the assembler, linker and dynamic linker. The +default value of this hook is defined as for the host machine. +@end defmac + @hook TARGET_USE_ANCHORS_FOR_SYMBOL_P Return true if GCC should attempt to use anchors to access @code{SYMBOL_REF} @var{x}. You can assume @samp{SYMBOL_REF_HAS_BLOCK_INFO_P (@var{x})} and diff --git a/gcc/varasm.c b/gcc/varasm.c index 7d083fd..14be8fb 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -5489,10 +5489,14 @@ do_assemble_alias (tree decl, tree target) } if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))) { -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION - ASM_OUTPUT_TYPE_DIRECTIVE - (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), - IFUNC_ASM_TYPE); +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) + if (TARGET_HAS_IFUNC) + ASM_OUTPUT_TYPE_DIRECTIVE + (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), + IFUNC_ASM_TYPE); + else + error_at (DECL_SOURCE_LOCATION (decl), + "ifunc is not supported on this target"); #else error_at (DECL_SOURCE_LOCATION (decl), "ifunc is not supported in this configuration"); ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH] Implementing ifunc target hook 2012-12-26 12:15 [RFC PATCH] Implementing ifunc target hook Alexander Ivchenko @ 2012-12-26 17:06 ` Joseph S. Myers 2012-12-27 22:59 ` Maxim Kuvyrkov 1 sibling, 0 replies; 16+ messages in thread From: Joseph S. Myers @ 2012-12-26 17:06 UTC (permalink / raw) To: Alexander Ivchenko Cc: gcc-patches, H.J. Lu, pavel.v.chupin, Sergey Ostanevich, Maxim Kuvyrkov On Wed, 26 Dec 2012, Alexander Ivchenko wrote: > The attached patch is indended to add the target hook for indicating > the support of ifunc on target. That's not a hook, it's a target macro. What is the rationale for this needing to be a target macro instead of a target hook? -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH] Implementing ifunc target hook 2012-12-26 12:15 [RFC PATCH] Implementing ifunc target hook Alexander Ivchenko 2012-12-26 17:06 ` Joseph S. Myers @ 2012-12-27 22:59 ` Maxim Kuvyrkov 2012-12-28 12:30 ` Alexander Ivchenko 1 sibling, 1 reply; 16+ messages in thread From: Maxim Kuvyrkov @ 2012-12-27 22:59 UTC (permalink / raw) To: Alexander Ivchenko Cc: gcc-patches, Joseph S. Myers, H.J. Lu, pavel.v.chupin, Sergey Ostanevich On 27/12/2012, at 1:15 AM, Alexander Ivchenko wrote: > Hi, > > Currently Android dynamic loader does not support indirect functions > (And I don't think that > it will someday). But there is no way for us to specify that for gcc, > and for example, tests like > gcc.dg/attr-ifunc-* are failing on android right now. > The attached patch is indended to add the target hook for indicating > the support of ifunc on target. The idea behind the patch looks OK, but implementation needs a bit of tweaking. As Joseph mentioned, you need to convert this macro into a target hook. GCC is making a gradual transition away from target macros to target hook functions, and all new hooks should be added as functions, unless there is a compelling argument to make it a macro. For an example of adding a target hook function see rev 194608 (among many others). > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -29146,7 +29146,7 @@ make_name (tree decl, const char *suffix, bool make_unique) > return global_var_name; > } > > -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION > +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) > > /* Make a dispatcher declaration for the multi-versioned function DECL. > Calls to DECL function will be replaced with calls to the dispatcher > @@ -29213,7 +29213,7 @@ ix86_get_function_versions_dispatcher (void *decl) > > tree dispatch_decl = NULL; > > -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION > +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) > struct cgraph_function_version_info *it_v = NULL; > struct cgraph_node *dispatcher_node = NULL; > struct cgraph_function_version_info *dispatcher_version_info = NULL; It seems you can move these variables inside the 'if (TARGET_HAS_IFUNC)' clause below and make the code cleaner, no? > @@ -29263,24 +29263,33 @@ ix86_get_function_versions_dispatcher (void *decl) > > default_node = default_version_info->this_node; > > -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION > - /* Right now, the dispatching is done via ifunc. */ > - dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); > - > - dispatcher_node = cgraph_get_create_node (dispatch_decl); > - gcc_assert (dispatcher_node != NULL); > - dispatcher_node->dispatcher_function = 1; > - dispatcher_version_info > - = insert_new_cgraph_node_version (dispatcher_node); > - dispatcher_version_info->next = default_version_info; > - dispatcher_node->local.finalized = 1; > - > - /* Set the dispatcher for all the versions. */ > - it_v = default_version_info; > - while (it_v->next != NULL) > +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) > + if (TARGET_HAS_IFUNC) > + { > + /* Right now, the dispatching is done via ifunc. */ > + dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); > + > + dispatcher_node = cgraph_get_create_node (dispatch_decl); > + gcc_assert (dispatcher_node != NULL); > + dispatcher_node->dispatcher_function = 1; > + dispatcher_version_info > + = insert_new_cgraph_node_version (dispatcher_node); > + dispatcher_version_info->next = default_version_info; > + dispatcher_node->local.finalized = 1; > + > + /* Set the dispatcher for all the versions. */ > + it_v = default_version_info; > + while (it_v->next != NULL) > + { > + it_v->dispatcher_resolver = dispatch_decl; > + it_v = it_v->next; > + } > + } > + else > { > - it_v->dispatcher_resolver = dispatch_decl; > - it_v = it_v->next; > + error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), > + "multiversioning needs ifunc which is not supported " > + "on this target"); > } This looks wrong. Before the patch this code would be ignored if !HAVE_GNU_INDIRECT_FUNCTION and after the patch it will produce an error. Removing the else-clause should fix this. > #else > error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), > diff --git a/gcc/config/linux-android.h b/gcc/config/linux-android.h > index e74e261..f6f44f1 100644 > --- a/gcc/config/linux-android.h > +++ b/gcc/config/linux-android.h > @@ -58,3 +58,6 @@ > > #define ANDROID_ENDFILE_SPEC \ > "%{shared: crtend_so%O%s;: crtend_android%O%s}" > + > +#undef TARGET_HAS_IFUNC > +#define TARGET_HAS_IFUNC (!TARGET_ANDROID) This initialization should be moved to targethooks.c ... > diff --git a/gcc/defaults.h b/gcc/defaults.h > index 76909ab..2180a47 100644 > --- a/gcc/defaults.h > +++ b/gcc/defaults.h > @@ -111,6 +111,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see > #endif > #endif > > +#ifndef TARGET_HAS_IFUNC > +#define TARGET_HAS_IFUNC HAVE_GNU_INDIRECT_FUNCTION > +#endif ... and this one too. > + > #ifndef IFUNC_ASM_TYPE > #define IFUNC_ASM_TYPE "gnu_indirect_function" > #endif > diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi > index 75aa867..1c8bc51 100644 > --- a/gcc/doc/tm.texi > +++ b/gcc/doc/tm.texi > @@ -5847,6 +5847,12 @@ If @code{ASM_OUTPUT_DEF} is not available, the hook's default definition > is @code{NULL}, which disables the use of section anchors altogether. > @end deftypefn > > +@defmac TARGET_HAS_IFUNC > +When this macro is nonzero, GCC will assume that target supports GNU indirect > +functions. The support includes the assembler, linker and dynamic linker. The > +default value of this hook is defined as for the host machine. > +@end defmac > + > @deftypefn {Target Hook} bool TARGET_USE_ANCHORS_FOR_SYMBOL_P (const_rtx @var{x}) > Return true if GCC should attempt to use anchors to access @code{SYMBOL_REF} > @var{x}. You can assume @samp{SYMBOL_REF_HAS_BLOCK_INFO_P (@var{x})} and > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in > index 95fab18..e1646b8 100644 > --- a/gcc/doc/tm.texi.in > +++ b/gcc/doc/tm.texi.in > @@ -5751,6 +5751,12 @@ If @code{ASM_OUTPUT_DEF} is not available, the hook's default definition > is @code{NULL}, which disables the use of section anchors altogether. > @end deftypefn > > +@defmac TARGET_HAS_IFUNC > +When this macro is nonzero, GCC will assume that target supports GNU indirect > +functions. The support includes the assembler, linker and dynamic linker. The > +default value of this hook is defined as for the host machine. > +@end defmac > + > @hook TARGET_USE_ANCHORS_FOR_SYMBOL_P > Return true if GCC should attempt to use anchors to access @code{SYMBOL_REF} > @var{x}. You can assume @samp{SYMBOL_REF_HAS_BLOCK_INFO_P (@var{x})} and > diff --git a/gcc/varasm.c b/gcc/varasm.c > index 7d083fd..14be8fb 100644 > --- a/gcc/varasm.c > +++ b/gcc/varasm.c > @@ -5489,10 +5489,14 @@ do_assemble_alias (tree decl, tree target) > } > if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))) > { > -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION > - ASM_OUTPUT_TYPE_DIRECTIVE > - (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), > - IFUNC_ASM_TYPE); > +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) > + if (TARGET_HAS_IFUNC) > + ASM_OUTPUT_TYPE_DIRECTIVE > + (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), > + IFUNC_ASM_TYPE); > + else > + error_at (DECL_SOURCE_LOCATION (decl), > + "ifunc is not supported on this target"); > #else > error_at (DECL_SOURCE_LOCATION (decl), > "ifunc is not supported in this configuration"); Please use same error message in both error cases. It seems you can just convert '#else' preprocessor clause into 'else' code clause without touching the error_at() statement beyond indentation updates. Thanks, -- Maxim Kuvyrkov CodeSourcery / Mentor Graphics ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH] Implementing ifunc target hook 2012-12-27 22:59 ` Maxim Kuvyrkov @ 2012-12-28 12:30 ` Alexander Ivchenko 2013-01-02 23:08 ` Maxim Kuvyrkov 0 siblings, 1 reply; 16+ messages in thread From: Alexander Ivchenko @ 2012-12-28 12:30 UTC (permalink / raw) To: Maxim Kuvyrkov Cc: gcc-patches, Joseph S. Myers, H.J. Lu, pavel.v.chupin, Sergey Ostanevich [-- Attachment #1: Type: text/plain, Size: 12219 bytes --] Joseph, Maxim, thank you for your input. I converted this macro into a target hook as you said. I had to add gcc/config/linux-protos.h in order to declare linux (that works for android) version of this hook - otherwise I don't know where to declare it.. >> --- a/gcc/config/i386/i386.c >> +++ b/gcc/config/i386/i386.c >> @@ -29146,7 +29146,7 @@ make_name (tree decl, const char *suffix, bool make_unique) >> return global_var_name; >> } >> >> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >> >> /* Make a dispatcher declaration for the multi-versioned function DECL. >> Calls to DECL function will be replaced with calls to the dispatcher >> @@ -29213,7 +29213,7 @@ ix86_get_function_versions_dispatcher (void *decl) >> >> tree dispatch_decl = NULL; >> >> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >> struct cgraph_function_version_info *it_v = NULL; >> struct cgraph_node *dispatcher_node = NULL; >> struct cgraph_function_version_info *dispatcher_version_info = NULL; > >It seems you can move these variables inside the 'if (TARGET_HAS_IFUNC)' clause below and make >the code cleaner, no? All variable declarations should be at the beginning of the routine. Or is it changed right now? >> @@ -29263,24 +29263,33 @@ ix86_get_function_versions_dispatcher (void *decl) >> >> default_node = default_version_info->this_node; >> >> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >> - /* Right now, the dispatching is done via ifunc. */ >> - dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); >> - >> - dispatcher_node = cgraph_get_create_node (dispatch_decl); >> - gcc_assert (dispatcher_node != NULL); >> - dispatcher_node->dispatcher_function = 1; >> - dispatcher_version_info >> - = insert_new_cgraph_node_version (dispatcher_node); >> - dispatcher_version_info->next = default_version_info; >> - dispatcher_node->local.finalized = 1; >> - >> - /* Set the dispatcher for all the versions. */ >> - it_v = default_version_info; >> - while (it_v->next != NULL) >> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >> + if (TARGET_HAS_IFUNC) >> + { >> + /* Right now, the dispatching is done via ifunc. */ >> + dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); >> + >> + dispatcher_node = cgraph_get_create_node (dispatch_decl); >> + gcc_assert (dispatcher_node != NULL); >> + dispatcher_node->dispatcher_function = 1; >> + dispatcher_version_info >> + = insert_new_cgraph_node_version (dispatcher_node); >> + dispatcher_version_info->next = default_version_info; >> + dispatcher_node->local.finalized = 1; >> + >> + /* Set the dispatcher for all the versions. */ >> + it_v = default_version_info; >> + while (it_v->next != NULL) >> + { >> + it_v->dispatcher_resolver = dispatch_decl; >> + it_v = it_v->next; >> + } >> + } >> + else >> { >> - it_v->dispatcher_resolver = dispatch_decl; >> - it_v = it_v->next; >> + error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), >> + "multiversioning needs ifunc which is not supported " >> + "on this target"); >> } > >This looks wrong. Before the patch this code would be ignored if !HAVE_GNU_INDIRECT_FUNCTION and after the patch it will produce an error. Removing the else-clause should fix this. Mmm, I believe that we want this code not to be ignored, because we want it to produces an error if the target does not support ifuncs and we are compiling something that uses ifuncs.. May be I missed something.. >Please use same error message in both error cases. It seems you can just convert '#else' >preprocessor clause into 'else' code clause without touching the error_at() statement beyond >indentation updates. Done. thank you, Alexander 2012/12/28 Maxim Kuvyrkov <maxim@codesourcery.com>: > On 27/12/2012, at 1:15 AM, Alexander Ivchenko wrote: > >> Hi, >> >> Currently Android dynamic loader does not support indirect functions >> (And I don't think that >> it will someday). But there is no way for us to specify that for gcc, >> and for example, tests like >> gcc.dg/attr-ifunc-* are failing on android right now. >> The attached patch is indended to add the target hook for indicating >> the support of ifunc on target. > > The idea behind the patch looks OK, but implementation needs a bit of tweaking. > > As Joseph mentioned, you need to convert this macro into a target hook. GCC is making a gradual transition away from target macros to target hook functions, and all new hooks should be added as functions, unless there is a compelling argument to make it a macro. For an example of adding a target hook function see rev 194608 (among many others). > >> --- a/gcc/config/i386/i386.c >> +++ b/gcc/config/i386/i386.c >> @@ -29146,7 +29146,7 @@ make_name (tree decl, const char *suffix, bool make_unique) >> return global_var_name; >> } >> >> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >> >> /* Make a dispatcher declaration for the multi-versioned function DECL. >> Calls to DECL function will be replaced with calls to the dispatcher >> @@ -29213,7 +29213,7 @@ ix86_get_function_versions_dispatcher (void *decl) >> >> tree dispatch_decl = NULL; >> >> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >> struct cgraph_function_version_info *it_v = NULL; >> struct cgraph_node *dispatcher_node = NULL; >> struct cgraph_function_version_info *dispatcher_version_info = NULL; > > It seems you can move these variables inside the 'if (TARGET_HAS_IFUNC)' clause below and make the code cleaner, no? > >> @@ -29263,24 +29263,33 @@ ix86_get_function_versions_dispatcher (void *decl) >> >> default_node = default_version_info->this_node; >> >> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >> - /* Right now, the dispatching is done via ifunc. */ >> - dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); >> - >> - dispatcher_node = cgraph_get_create_node (dispatch_decl); >> - gcc_assert (dispatcher_node != NULL); >> - dispatcher_node->dispatcher_function = 1; >> - dispatcher_version_info >> - = insert_new_cgraph_node_version (dispatcher_node); >> - dispatcher_version_info->next = default_version_info; >> - dispatcher_node->local.finalized = 1; >> - >> - /* Set the dispatcher for all the versions. */ >> - it_v = default_version_info; >> - while (it_v->next != NULL) >> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >> + if (TARGET_HAS_IFUNC) >> + { >> + /* Right now, the dispatching is done via ifunc. */ >> + dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); >> + >> + dispatcher_node = cgraph_get_create_node (dispatch_decl); >> + gcc_assert (dispatcher_node != NULL); >> + dispatcher_node->dispatcher_function = 1; >> + dispatcher_version_info >> + = insert_new_cgraph_node_version (dispatcher_node); >> + dispatcher_version_info->next = default_version_info; >> + dispatcher_node->local.finalized = 1; >> + >> + /* Set the dispatcher for all the versions. */ >> + it_v = default_version_info; >> + while (it_v->next != NULL) >> + { >> + it_v->dispatcher_resolver = dispatch_decl; >> + it_v = it_v->next; >> + } >> + } >> + else >> { >> - it_v->dispatcher_resolver = dispatch_decl; >> - it_v = it_v->next; >> + error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), >> + "multiversioning needs ifunc which is not supported " >> + "on this target"); >> } > > This looks wrong. Before the patch this code would be ignored if !HAVE_GNU_INDIRECT_FUNCTION and after the patch it will produce an error. Removing the else-clause should fix this. > >> #else >> error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), >> diff --git a/gcc/config/linux-android.h b/gcc/config/linux-android.h >> index e74e261..f6f44f1 100644 >> --- a/gcc/config/linux-android.h >> +++ b/gcc/config/linux-android.h >> @@ -58,3 +58,6 @@ >> >> #define ANDROID_ENDFILE_SPEC \ >> "%{shared: crtend_so%O%s;: crtend_android%O%s}" >> + >> +#undef TARGET_HAS_IFUNC >> +#define TARGET_HAS_IFUNC (!TARGET_ANDROID) > > This initialization should be moved to targethooks.c ... > >> diff --git a/gcc/defaults.h b/gcc/defaults.h >> index 76909ab..2180a47 100644 >> --- a/gcc/defaults.h >> +++ b/gcc/defaults.h >> @@ -111,6 +111,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >> #endif >> #endif >> >> +#ifndef TARGET_HAS_IFUNC >> +#define TARGET_HAS_IFUNC HAVE_GNU_INDIRECT_FUNCTION >> +#endif > > ... and this one too. > >> + >> #ifndef IFUNC_ASM_TYPE >> #define IFUNC_ASM_TYPE "gnu_indirect_function" >> #endif >> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi >> index 75aa867..1c8bc51 100644 >> --- a/gcc/doc/tm.texi >> +++ b/gcc/doc/tm.texi >> @@ -5847,6 +5847,12 @@ If @code{ASM_OUTPUT_DEF} is not available, the hook's default definition >> is @code{NULL}, which disables the use of section anchors altogether. >> @end deftypefn >> >> +@defmac TARGET_HAS_IFUNC >> +When this macro is nonzero, GCC will assume that target supports GNU indirect >> +functions. The support includes the assembler, linker and dynamic linker. The >> +default value of this hook is defined as for the host machine. >> +@end defmac >> + >> @deftypefn {Target Hook} bool TARGET_USE_ANCHORS_FOR_SYMBOL_P (const_rtx @var{x}) >> Return true if GCC should attempt to use anchors to access @code{SYMBOL_REF} >> @var{x}. You can assume @samp{SYMBOL_REF_HAS_BLOCK_INFO_P (@var{x})} and >> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in >> index 95fab18..e1646b8 100644 >> --- a/gcc/doc/tm.texi.in >> +++ b/gcc/doc/tm.texi.in >> @@ -5751,6 +5751,12 @@ If @code{ASM_OUTPUT_DEF} is not available, the hook's default definition >> is @code{NULL}, which disables the use of section anchors altogether. >> @end deftypefn >> >> +@defmac TARGET_HAS_IFUNC >> +When this macro is nonzero, GCC will assume that target supports GNU indirect >> +functions. The support includes the assembler, linker and dynamic linker. The >> +default value of this hook is defined as for the host machine. >> +@end defmac >> + >> @hook TARGET_USE_ANCHORS_FOR_SYMBOL_P >> Return true if GCC should attempt to use anchors to access @code{SYMBOL_REF} >> @var{x}. You can assume @samp{SYMBOL_REF_HAS_BLOCK_INFO_P (@var{x})} and >> diff --git a/gcc/varasm.c b/gcc/varasm.c >> index 7d083fd..14be8fb 100644 >> --- a/gcc/varasm.c >> +++ b/gcc/varasm.c >> @@ -5489,10 +5489,14 @@ do_assemble_alias (tree decl, tree target) >> } >> if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))) >> { >> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >> - ASM_OUTPUT_TYPE_DIRECTIVE >> - (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), >> - IFUNC_ASM_TYPE); >> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >> + if (TARGET_HAS_IFUNC) >> + ASM_OUTPUT_TYPE_DIRECTIVE >> + (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), >> + IFUNC_ASM_TYPE); >> + else >> + error_at (DECL_SOURCE_LOCATION (decl), >> + "ifunc is not supported on this target"); >> #else >> error_at (DECL_SOURCE_LOCATION (decl), >> "ifunc is not supported in this configuration"); > > Please use same error message in both error cases. It seems you can just convert '#else' preprocessor clause into 'else' code clause without touching the error_at() statement beyond indentation updates. > > Thanks, > > -- > Maxim Kuvyrkov > CodeSourcery / Mentor Graphics > [-- Attachment #2: disable_ifunc_for_android_03.patch --] [-- Type: application/octet-stream, Size: 9828 bytes --] diff --git a/ChangeLog b/ChangeLog index fc8f60b..23c0484 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,20 @@ +2012-12-28 Alexander Ivchenko <alexander.ivchenko@intel.com> + + * gcc/target.def (TARGET_HAS_IFUNC): New target hook. + * gcc/doc/tm.texi.in (TARGET_HAS_IFUNC): New. + * gcc/doc/tm.texi: Regenerate. + * gcc/targhooks.h (default_have_ifunc): New. + * gcc/targhooks.c (default_have_ifunc): Ditto. + * gcc/config/linux-protos.h: New file. + * gcc/config.gcc: Added include of linux-protos.h. + * gcc/config/linux.h (TARGET_HAS_IFUNC): Using version of this + hook for linux which disables support of indirect functions in + android. + * gcc/config/host-linux.c (linux_has_ifunc): New. + * gcc/config/i386/i386.c (ix86_get_function_versions_dispatcher): + Using TARGET_HAS_IFUNC hook instead of HAVE_GNU_INDIRECT_FUNCTION. + * gcc/varasm.c (do_assemble_alias): Likewise. + 2012-12-20 Matthias Klose <doko@ubuntu.com> * Makefile.def (install-target-libgfortran): Depend on diff --git a/gcc/config.gcc b/gcc/config.gcc index db56819..e535218 100644 --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -637,6 +637,11 @@ case ${target} in native_system_header_dir=/include ;; esac + case $target in + *linux*) + tm_p_file="${tm_p_file} linux-protos.h" + ;; + esac # glibc / uclibc / bionic switch. # uclibc and bionic aren't usable for GNU/Hurd and neither for GNU/k*BSD. case $target in diff --git a/gcc/config/host-linux.c b/gcc/config/host-linux.c index b535758..c72db79 100644 --- a/gcc/config/host-linux.c +++ b/gcc/config/host-linux.c @@ -22,7 +22,7 @@ #include "coretypes.h" #include "hosthooks.h" #include "hosthooks-def.h" - +#include "tm.h" /* Linux has a feature called exec-shield-randomize that perturbs the address of non-fixed mapped segments by a (relatively) small amount. @@ -222,5 +222,13 @@ linux_gt_pch_use_address (void *base, size_t size, int fd, size_t offset) return 1; } +/* Android does not support GNU indirect functions. */ + +bool +linux_has_ifunc (void) +{ + return TARGET_ANDROID ? false : HAVE_GNU_INDIRECT_FUNCTION; +} + \f const struct host_hooks host_hooks = HOST_HOOKS_INITIALIZER; diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index b466a4f..c422fd9 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -29146,7 +29146,7 @@ make_name (tree decl, const char *suffix, bool make_unique) return global_var_name; } -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) /* Make a dispatcher declaration for the multi-versioned function DECL. Calls to DECL function will be replaced with calls to the dispatcher @@ -29213,7 +29213,7 @@ ix86_get_function_versions_dispatcher (void *decl) tree dispatch_decl = NULL; -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) struct cgraph_function_version_info *it_v = NULL; struct cgraph_node *dispatcher_node = NULL; struct cgraph_function_version_info *dispatcher_version_info = NULL; @@ -29263,24 +29263,33 @@ ix86_get_function_versions_dispatcher (void *decl) default_node = default_version_info->this_node; -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION - /* Right now, the dispatching is done via ifunc. */ - dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); - - dispatcher_node = cgraph_get_create_node (dispatch_decl); - gcc_assert (dispatcher_node != NULL); - dispatcher_node->dispatcher_function = 1; - dispatcher_version_info - = insert_new_cgraph_node_version (dispatcher_node); - dispatcher_version_info->next = default_version_info; - dispatcher_node->local.finalized = 1; - - /* Set the dispatcher for all the versions. */ - it_v = default_version_info; - while (it_v->next != NULL) +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) + if (targetm.has_ifunc ()) + { + /* Right now, the dispatching is done via ifunc. */ + dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); + + dispatcher_node = cgraph_get_create_node (dispatch_decl); + gcc_assert (dispatcher_node != NULL); + dispatcher_node->dispatcher_function = 1; + dispatcher_version_info + = insert_new_cgraph_node_version (dispatcher_node); + dispatcher_version_info->next = default_version_info; + dispatcher_node->local.finalized = 1; + + /* Set the dispatcher for all the versions. */ + it_v = default_version_info; + while (it_v->next != NULL) + { + it_v->dispatcher_resolver = dispatch_decl; + it_v = it_v->next; + } + } + else { - it_v->dispatcher_resolver = dispatch_decl; - it_v = it_v->next; + error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), + "multiversioning needs ifunc which is not supported " + "on this target"); } #else error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), diff --git a/gcc/config/linux-protos.h b/gcc/config/linux-protos.h new file mode 100644 index 0000000..ea3d77d --- /dev/null +++ b/gcc/config/linux-protos.h @@ -0,0 +1,21 @@ +/* Prototypes. + Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2010, 2011, 2012 + Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +<http://www.gnu.org/licenses/>. */ + +extern bool linux_has_ifunc (void); diff --git a/gcc/config/linux.h b/gcc/config/linux.h index fb459e6..bdfbc0d 100644 --- a/gcc/config/linux.h +++ b/gcc/config/linux.h @@ -108,3 +108,6 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see /* Whether we have Bionic libc runtime */ #undef TARGET_HAS_BIONIC #define TARGET_HAS_BIONIC (OPTION_BIONIC) + +#undef TARGET_HAS_IFUNC +#define TARGET_HAS_IFUNC linux_has_ifunc diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index 75aa867..2d7121f 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -11343,3 +11343,9 @@ memory model bits are allowed. @deftypevr {Target Hook} {unsigned char} TARGET_ATOMIC_TEST_AND_SET_TRUEVAL This value should be set if the result written by @code{atomic_test_and_set} is not exactly 1, i.e. the @code{bool} @code{true}. @end deftypevr + +@deftypefn {Target Hook} bool TARGET_HAS_IFUNC (void) +It returns true if the target supports GNU indirect functions. +The support includes the assembler, linker and dynamic linker. +The default value of this hook is defined as for the host machine. +@end deftypefn diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index 95fab18..40dba77 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -11179,3 +11179,5 @@ memory model bits are allowed. @end deftypefn @hook TARGET_ATOMIC_TEST_AND_SET_TRUEVAL + +@hook TARGET_HAS_IFUNC diff --git a/gcc/target.def b/gcc/target.def index c5bbfae..627ae2d 100644 --- a/gcc/target.def +++ b/gcc/target.def @@ -1520,6 +1520,15 @@ DEFHOOK bool, (const_rtx x), default_use_anchors_for_symbol_p) +/* True if target supports indirect functions. */ +DEFHOOK +(has_ifunc, + "It returns true if the target supports GNU indirect functions.\n\ +The support includes the assembler, linker and dynamic linker.\n\ +The default value of this hook is defined as for the host machine.", + bool, (void), + default_have_ifunc) + /* True if it is OK to do sibling call optimization for the specified call expression EXP. DECL will be the called function, or NULL if this is an indirect call. */ diff --git a/gcc/targhooks.c b/gcc/targhooks.c index 954cdb9..ea0f9e2 100644 --- a/gcc/targhooks.c +++ b/gcc/targhooks.c @@ -451,6 +451,14 @@ default_fixed_point_supported_p (void) return ENABLE_FIXED_POINT; } +/* True if the target supports GNU indirect functions. */ + +bool +default_have_ifunc (void) +{ + return HAVE_GNU_INDIRECT_FUNCTION; +} + /* NULL if INSN insn is valid within a low-overhead loop, otherwise returns an error message. diff --git a/gcc/targhooks.h b/gcc/targhooks.h index bd7d4bc..8622d58 100644 --- a/gcc/targhooks.h +++ b/gcc/targhooks.h @@ -73,6 +73,8 @@ extern bool targhook_float_words_big_endian (void); extern bool default_decimal_float_supported_p (void); extern bool default_fixed_point_supported_p (void); +extern bool default_have_ifunc (void); + extern const char * default_invalid_within_doloop (const_rtx); extern tree default_builtin_vectorized_function (tree, tree, tree); diff --git a/gcc/varasm.c b/gcc/varasm.c index 7d083fd..dd677a4 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -5489,14 +5489,15 @@ do_assemble_alias (tree decl, tree target) } if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))) { -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION - ASM_OUTPUT_TYPE_DIRECTIVE - (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), - IFUNC_ASM_TYPE); -#else - error_at (DECL_SOURCE_LOCATION (decl), - "ifunc is not supported in this configuration"); +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) + if (targetm.has_ifunc ()) + ASM_OUTPUT_TYPE_DIRECTIVE + (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), + IFUNC_ASM_TYPE); + else #endif + error_at (DECL_SOURCE_LOCATION (decl), + "ifunc is not supported on this target"); } # ifdef ASM_OUTPUT_DEF_FROM_DECLS ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH] Implementing ifunc target hook 2012-12-28 12:30 ` Alexander Ivchenko @ 2013-01-02 23:08 ` Maxim Kuvyrkov 2013-01-09 11:24 ` Alexander Ivchenko 0 siblings, 1 reply; 16+ messages in thread From: Maxim Kuvyrkov @ 2013-01-02 23:08 UTC (permalink / raw) To: Alexander Ivchenko Cc: GCC Patches, Joseph S. Myers, H.J. Lu, Pavel Chupin, Sergey Ostanevich On 29/12/2012, at 1:30 AM, Alexander Ivchenko wrote: > Joseph, Maxim, thank you for your input. I converted this macro into > a target hook as you said. I had to add gcc/config/linux-protos.h in order > to declare linux (that works for android) version of this hook - otherwise > I don't know where to declare it.. See notes below on this. Once you added a new target hook you can call it via targetm.<hook>(), so references to TARGET_HAS_IFUNC should be replaced with calls to targetm.has_ifunc_p(). Sorry, but it will take another iteration or two to make this patch just right. GCC config machinery is tricky, and placing things in the right places is non-trivial. > >>> --- a/gcc/config/i386/i386.c >>> +++ b/gcc/config/i386/i386.c >>> @@ -29146,7 +29146,7 @@ make_name (tree decl, const char *suffix, bool make_unique) >>> return global_var_name; >>> } >>> >>> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >>> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >>> >>> /* Make a dispatcher declaration for the multi-versioned function DECL. >>> Calls to DECL function will be replaced with calls to the dispatcher >>> @@ -29213,7 +29213,7 @@ ix86_get_function_versions_dispatcher (void *decl) >>> >>> tree dispatch_decl = NULL; >>> >>> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >>> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >>> struct cgraph_function_version_info *it_v = NULL; >>> struct cgraph_node *dispatcher_node = NULL; >>> struct cgraph_function_version_info *dispatcher_version_info = NULL; >> >> It seems you can move these variables inside the 'if (TARGET_HAS_IFUNC)' clause below and make >the code cleaner, no? > > All variable declarations should be at the beginning of the routine. > Or is it changed right now? The variable declarations should be at the beginning of a block. Since you are adding a new block below (under if (TARGET_HAS_IFUNC) { <block> }), and these variables are used only within that block, it would be cleaner to move them there. > >>> @@ -29263,24 +29263,33 @@ ix86_get_function_versions_dispatcher (void *decl) >>> >>> default_node = default_version_info->this_node; >>> >>> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >>> - /* Right now, the dispatching is done via ifunc. */ >>> - dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); >>> - >>> - dispatcher_node = cgraph_get_create_node (dispatch_decl); >>> - gcc_assert (dispatcher_node != NULL); >>> - dispatcher_node->dispatcher_function = 1; >>> - dispatcher_version_info >>> - = insert_new_cgraph_node_version (dispatcher_node); >>> - dispatcher_version_info->next = default_version_info; >>> - dispatcher_node->local.finalized = 1; >>> - >>> - /* Set the dispatcher for all the versions. */ >>> - it_v = default_version_info; >>> - while (it_v->next != NULL) >>> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >>> + if (TARGET_HAS_IFUNC) >>> + { >>> + /* Right now, the dispatching is done via ifunc. */ >>> + dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); >>> + >>> + dispatcher_node = cgraph_get_create_node (dispatch_decl); >>> + gcc_assert (dispatcher_node != NULL); >>> + dispatcher_node->dispatcher_function = 1; >>> + dispatcher_version_info >>> + = insert_new_cgraph_node_version (dispatcher_node); >>> + dispatcher_version_info->next = default_version_info; >>> + dispatcher_node->local.finalized = 1; >>> + >>> + /* Set the dispatcher for all the versions. */ >>> + it_v = default_version_info; >>> + while (it_v->next != NULL) >>> + { >>> + it_v->dispatcher_resolver = dispatch_decl; >>> + it_v = it_v->next; >>> + } >>> + } >>> + else >>> { >>> - it_v->dispatcher_resolver = dispatch_decl; >>> - it_v = it_v->next; >>> + error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), >>> + "multiversioning needs ifunc which is not supported " >>> + "on this target"); >>> } >> >> This looks wrong. Before the patch this code would be ignored if !HAVE_GNU_INDIRECT_FUNCTION and after the patch it will produce an error. Removing the else-clause should fix this. > > Mmm, I believe that we want this code not to be ignored, because we > want it to produces an error > if the target does not support ifuncs and we are compiling something > that uses ifuncs.. May be I > missed something.. You are right, I read the patch wrong. > index db56819..e535218 100644 > --- a/gcc/config.gcc > +++ b/gcc/config.gcc > @@ -637,6 +637,11 @@ case ${target} in > native_system_header_dir=/include > ;; > esac > + case $target in > + *linux*) > + tm_p_file="${tm_p_file} linux-protos.h" > + ;; > + esac > # glibc / uclibc / bionic switch. > # uclibc and bionic aren't usable for GNU/Hurd and neither for GNU/k*BSD. > case $target in Should not be necessary. Put the hook override into config/linux-android.h: #undef TARGET_HAS_IFUNC_P #define TARGET_HAS_IFUNC_P linux_android_has_ifunc_p > diff --git a/gcc/config/host-linux.c b/gcc/config/host-linux.c > index b535758..c72db79 100644 > --- a/gcc/config/host-linux.c > +++ b/gcc/config/host-linux.c > ... > > +/* Android does not support GNU indirect functions. */ > + > +bool > +linux_has_ifunc (void) > +{ > + return TARGET_ANDROID ? false : HAVE_GNU_INDIRECT_FUNCTION; > +} > + OK, so you defined new *target* hook has_ifunc[_p]. Placing one of the hook's implementation is into a host-specific file seems like a strange idea, but maybe I'm missing something. It seems you want to add a new file config/linux-android.c (with implementation for symbols used in config/linux-android.h) and add it Android-flavoured linux targets in config.gcc: # Add Android userspace support to Linux targets. case $target in *linux*) tm_file="$tm_file linux-android.h" extra_options="$extra_options linux-android.opt" + extra_objs="$extra_objs linux-android.o" ;; esac > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index b466a4f..c422fd9 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -29146,7 +29146,7 @@ make_name (tree decl, const char *suffix, bool make_unique) > return global_var_name; > } > > -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION > +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) > > /* Make a dispatcher declaration for the multi-versioned function DECL. > Calls to DECL function will be replaced with calls to the dispatcher > @@ -29213,7 +29213,7 @@ ix86_get_function_versions_dispatcher (void *decl) > > tree dispatch_decl = NULL; > > -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION > +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) > struct cgraph_function_version_info *it_v = NULL; > struct cgraph_node *dispatcher_node = NULL; > struct cgraph_function_version_info *dispatcher_version_info = NULL; > @@ -29263,24 +29263,33 @@ ix86_get_function_versions_dispatcher (void *decl) > > default_node = default_version_info->this_node; > > -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION > - /* Right now, the dispatching is done via ifunc. */ > - dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); > - > - dispatcher_node = cgraph_get_create_node (dispatch_decl); > - gcc_assert (dispatcher_node != NULL); > - dispatcher_node->dispatcher_function = 1; > - dispatcher_version_info > - = insert_new_cgraph_node_version (dispatcher_node); > - dispatcher_version_info->next = default_version_info; > - dispatcher_node->local.finalized = 1; > - > - /* Set the dispatcher for all the versions. */ > - it_v = default_version_info; > - while (it_v->next != NULL) > +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) > + if (targetm.has_ifunc ()) > + { > + /* Right now, the dispatching is done via ifunc. */ > + dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); > + > + dispatcher_node = cgraph_get_create_node (dispatch_decl); > + gcc_assert (dispatcher_node != NULL); > + dispatcher_node->dispatcher_function = 1; > + dispatcher_version_info > + = insert_new_cgraph_node_version (dispatcher_node); > + dispatcher_version_info->next = default_version_info; > + dispatcher_node->local.finalized = 1; > + > + /* Set the dispatcher for all the versions. */ > + it_v = default_version_info; > + while (it_v->next != NULL) > + { > + it_v->dispatcher_resolver = dispatch_decl; > + it_v = it_v->next; > + } > + } > + else > { > - it_v->dispatcher_resolver = dispatch_decl; > - it_v = it_v->next; > + error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), > + "multiversioning needs ifunc which is not supported " > + "on this target"); > } > #else > error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), Would you please use same trick for arranging #if-else-#endif as you did for varasm.c ? > diff --git a/gcc/config/linux-protos.h b/gcc/config/linux-protos.h > new file mode 100644 > index 0000000..ea3d77d > --- /dev/null > +++ b/gcc/config/linux-protos.h > @@ -0,0 +1,21 @@ > +/* Prototypes. > + Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2010, 2011, 2012 > + Free Software Foundation, Inc. > + > +This file is part of GCC. > + > +GCC is free software; you can redistribute it and/or modify > +it under the terms of the GNU General Public License as published by > +the Free Software Foundation; either version 3, or (at your option) > +any later version. > + > +GCC is distributed in the hope that it will be useful, > +but WITHOUT ANY WARRANTY; without even the implied warranty of > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +GNU General Public License for more details. > + > +You should have received a copy of the GNU General Public License > +along with GCC; see the file COPYING3. If not see > +<http://www.gnu.org/licenses/>. */ > + > +extern bool linux_has_ifunc (void); Config/linux-android.h is a better place for this declaration. > diff --git a/gcc/config/linux.h b/gcc/config/linux.h > index fb459e6..bdfbc0d 100644 > --- a/gcc/config/linux.h > +++ b/gcc/config/linux.h > @@ -108,3 +108,6 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see > /* Whether we have Bionic libc runtime */ > #undef TARGET_HAS_BIONIC > #define TARGET_HAS_BIONIC (OPTION_BIONIC) > + > +#undef TARGET_HAS_IFUNC > +#define TARGET_HAS_IFUNC linux_has_ifunc As noted above, please move the hook override to linux-android.h . The override should be effective only for Linux targets that support Android, otherwise compilation may fail due to undefined TARGET_ANDROID used in linux_has_ifunc. > diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi > index 75aa867..2d7121f 100644 > --- a/gcc/doc/tm.texi > +++ b/gcc/doc/tm.texi > @@ -11343,3 +11343,9 @@ memory model bits are allowed. > @deftypevr {Target Hook} {unsigned char} TARGET_ATOMIC_TEST_AND_SET_TRUEVAL > This value should be set if the result written by @code{atomic_test_and_set} is not exactly 1, i.e. the @code{bool} @code{true}. > @end deftypevr > + > +@deftypefn {Target Hook} bool TARGET_HAS_IFUNC (void) > +It returns true if the target supports GNU indirect functions. > +The support includes the assembler, linker and dynamic linker. > +The default value of this hook is defined as for the host machine. > +@end deftypefn > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in > index 95fab18..40dba77 100644 > --- a/gcc/doc/tm.texi.in > +++ b/gcc/doc/tm.texi.in > @@ -11179,3 +11179,5 @@ memory model bits are allowed. > @end deftypefn > > @hook TARGET_ATOMIC_TEST_AND_SET_TRUEVAL > + > +@hook TARGET_HAS_IFUNC > diff --git a/gcc/target.def b/gcc/target.def > index c5bbfae..627ae2d 100644 > --- a/gcc/target.def > +++ b/gcc/target.def > @@ -1520,6 +1520,15 @@ DEFHOOK > bool, (const_rtx x), > default_use_anchors_for_symbol_p) > > +/* True if target supports indirect functions. */ > +DEFHOOK > +(has_ifunc, The convention is to add "_p" for functions that behave like boolean predicates, i.e., "has_ifunc_p". > + "It returns true if the target supports GNU indirect functions.\n\ > +The support includes the assembler, linker and dynamic linker.\n\ > +The default value of this hook is defined as for the host machine.", Are your sure the last sentence is correct? It seems the default value for this hook is based on which libc is being used. Maybe it would be more accurate to say "The default value of this hook is based on target's libc."? > + bool, (void), > + default_have_ifunc) > + > /* True if it is OK to do sibling call optimization for the specified > call expression EXP. DECL will be the called function, or NULL if > this is an indirect call. */ > diff --git a/gcc/targhooks.c b/gcc/targhooks.c > index 954cdb9..ea0f9e2 100644 > --- a/gcc/targhooks.c > +++ b/gcc/targhooks.c > @@ -451,6 +451,14 @@ default_fixed_point_supported_p (void) > return ENABLE_FIXED_POINT; > } > > +/* True if the target supports GNU indirect functions. */ > + > +bool > +default_have_ifunc (void) > +{ > + return HAVE_GNU_INDIRECT_FUNCTION; > +} > + > /* NULL if INSN insn is valid within a low-overhead loop, otherwise returns > an error message. > > diff --git a/gcc/targhooks.h b/gcc/targhooks.h > index bd7d4bc..8622d58 100644 > --- a/gcc/targhooks.h > +++ b/gcc/targhooks.h > @@ -73,6 +73,8 @@ extern bool targhook_float_words_big_endian (void); > extern bool default_decimal_float_supported_p (void); > extern bool default_fixed_point_supported_p (void); > > +extern bool default_have_ifunc (void); > + > extern const char * default_invalid_within_doloop (const_rtx); > > extern tree default_builtin_vectorized_function (tree, tree, tree); > diff --git a/gcc/varasm.c b/gcc/varasm.c > index 7d083fd..dd677a4 100644 > --- a/gcc/varasm.c > +++ b/gcc/varasm.c > @@ -5489,14 +5489,15 @@ do_assemble_alias (tree decl, tree target) > } > if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))) > { > -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION > - ASM_OUTPUT_TYPE_DIRECTIVE > - (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), > - IFUNC_ASM_TYPE); > -#else > - error_at (DECL_SOURCE_LOCATION (decl), > - "ifunc is not supported in this configuration"); > +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) > + if (targetm.has_ifunc ()) > + ASM_OUTPUT_TYPE_DIRECTIVE > + (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), > + IFUNC_ASM_TYPE); > + else > #endif > + error_at (DECL_SOURCE_LOCATION (decl), > + "ifunc is not supported on this target"); > } I like how you arranged #if-else-#endif to have a single error path. Thank you, -- Maxim Kuvyrkov ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH] Implementing ifunc target hook 2013-01-02 23:08 ` Maxim Kuvyrkov @ 2013-01-09 11:24 ` Alexander Ivchenko 2013-01-10 23:24 ` Maxim Kuvyrkov 0 siblings, 1 reply; 16+ messages in thread From: Alexander Ivchenko @ 2013-01-09 11:24 UTC (permalink / raw) To: Maxim Kuvyrkov; +Cc: GCC Patches, Joseph S. Myers, H.J. Lu [-- Attachment #1: Type: text/plain, Size: 21089 bytes --] Hello Maxim, >> >>>> --- a/gcc/config/i386/i386.c >>>> +++ b/gcc/config/i386/i386.c >>>> @@ -29146,7 +29146,7 @@ make_name (tree decl, const char *suffix, bool make_unique) >>>> return global_var_name; >>>> } >>>> >>>> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >>>> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >>>> >>>> /* Make a dispatcher declaration for the multi-versioned function DECL. >>>> Calls to DECL function will be replaced with calls to the dispatcher >>>> @@ -29213,7 +29213,7 @@ ix86_get_function_versions_dispatcher (void *decl) >>>> >>>> tree dispatch_decl = NULL; >>>> >>>> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >>>> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >>>> struct cgraph_function_version_info *it_v = NULL; >>>> struct cgraph_node *dispatcher_node = NULL; >>>> struct cgraph_function_version_info *dispatcher_version_info = NULL; >>> >>> It seems you can move these variables inside the 'if (TARGET_HAS_IFUNC)' clause below and make >the code cleaner, no? >> >> All variable declarations should be at the beginning of the routine. >> Or is it changed right now? > >The variable declarations should be at the beginning of a block. Since you are adding a new block below (under if (TARGET_HAS_IFUNC) { <block> }), and these variables are used only within that block, it would be cleaner to move them there. Done, thank you. >> diff --git a/gcc/config/host-linux.c b/gcc/config/host-linux.c >> index b535758..c72db79 100644 >> --- a/gcc/config/host-linux.c >> +++ b/gcc/config/host-linux.c >> .... >> >> +/* Android does not support GNU indirect functions. */ >> + >> +bool >> +linux_has_ifunc (void) >> +{ >> + return TARGET_ANDROID ? false : HAVE_GNU_INDIRECT_FUNCTION; >> +} >> + > >OK, so you defined new *target* hook has_ifunc[_p]. Placing one of the hook's implementation is into a host-specific file seems like a strange idea, but maybe I'm missing something. > >It seems you want to add a new file config/linux-android.c (with implementation for symbols used in config/linux-android.h) and add it Android-flavoured linux targets in config.gcc: > # Add Android userspace support to Linux targets. . case $target in > *linux*) > tm_file="$tm_file linux-android.h" > extra_options="$extra_options linux-android.opt" >+ extra_objs="$extra_objs linux-android.o" > ;; > esac > I agree, that would be much better, I added gcc/config/linux-android.c and also gcc/config/t-linux-android for the rule, thanks. >> diff --git a/gcc/config/linux-protos.h b/gcc/config/linux-protos.h >> new file mode 100644 >> index 0000000..ea3d77d >> --- /dev/null >> +++ b/gcc/config/linux-protos.h >> @@ -0,0 +1,21 @@ >> +/* Prototypes. >> + Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2010, 2011, 2012 >> + Free Software Foundation, Inc. >> + >> +This file is part of GCC. >> + >> +GCC is free software; you can redistribute it and/or modify >> +it under the terms of the GNU General Public License as published by >> +the Free Software Foundation; either version 3, or (at your option) >> +any later version. >> + >> +GCC is distributed in the hope that it will be useful, >> +but WITHOUT ANY WARRANTY; without even the implied warranty of >> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> +GNU General Public License for more details. >> + >> +You should have received a copy of the GNU General Public License >> +along with GCC; see the file COPYING3. If not see >> +<http://www.gnu.org/licenses/>. */ >> + >> +extern bool linux_has_ifunc (void); > >Config/linux-android.h is a better place for this declaration. That wouldn't help, I got the following error: In file included from ../../.././gcc/tm.h:24:0, from [..]/src/gcc/libgcc/generic-morestack-thread.c:29: [..]/src/gcc/libgcc/../gcc/config/linux-android.h:62:1: error: unknown type name ‘bool’ extern bool linux_android_has_ifunc_p (void); ^ Anyway, linux-protos.h looks to me as a good thing to have (e.g. for libc_has_function hook, that is supposed to be commited in a near future) for declaring linux (and Android) specific versions of hooks.. >> +/* True if target supports indirect functions. */ >> +DEFHOOK >> +(has_ifunc, > >The convention is to add "_p" for functions that behave like boolean predicates, i.e., "has_ifunc_p". Done. >> + "It returns true if the target supports GNU indirect functions.\n\ >> +The support includes the assembler, linker and dynamic linker.\n\ >> +The default value of this hook is defined as for the host machine.", > >Are your sure the last sentence is correct? It seems the default value for this hook is based on which libc is being used. Maybe it would be more accurate to say "The default value of this hook is based on target's libc."? Well yes, you are right that the default value depends on version of libc, but this version is checked on the configure time for the host machine (HAVE_GNU_INDIRECT_FUNCTION), so may be the correct sentence would be "The default value of this hook is based on host's libc."? thanks, Alexander 2013/1/3 Maxim Kuvyrkov <maxim.kuvyrkov@gmail.com>: > On 29/12/2012, at 1:30 AM, Alexander Ivchenko wrote: > >> Joseph, Maxim, thank you for your input. I converted this macro into >> a target hook as you said. I had to add gcc/config/linux-protos.h in order >> to declare linux (that works for android) version of this hook - otherwise >> I don't know where to declare it.. > > See notes below on this. Once you added a new target hook you can call it via targetm.<hook>(), so references to TARGET_HAS_IFUNC should be replaced with calls to targetm.has_ifunc_p(). > > Sorry, but it will take another iteration or two to make this patch just right. GCC config machinery is tricky, and placing things in the right places is non-trivial. > >> >>>> --- a/gcc/config/i386/i386.c >>>> +++ b/gcc/config/i386/i386.c >>>> @@ -29146,7 +29146,7 @@ make_name (tree decl, const char *suffix, bool make_unique) >>>> return global_var_name; >>>> } >>>> >>>> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >>>> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >>>> >>>> /* Make a dispatcher declaration for the multi-versioned function DECL. >>>> Calls to DECL function will be replaced with calls to the dispatcher >>>> @@ -29213,7 +29213,7 @@ ix86_get_function_versions_dispatcher (void *decl) >>>> >>>> tree dispatch_decl = NULL; >>>> >>>> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >>>> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >>>> struct cgraph_function_version_info *it_v = NULL; >>>> struct cgraph_node *dispatcher_node = NULL; >>>> struct cgraph_function_version_info *dispatcher_version_info = NULL; >>> >>> It seems you can move these variables inside the 'if (TARGET_HAS_IFUNC)' clause below and make >the code cleaner, no? >> >> All variable declarations should be at the beginning of the routine. >> Or is it changed right now? > > The variable declarations should be at the beginning of a block. Since you are adding a new block below (under if (TARGET_HAS_IFUNC) { <block> }), and these variables are used only within that block, it would be cleaner to move them there. > >> >>>> @@ -29263,24 +29263,33 @@ ix86_get_function_versions_dispatcher (void *decl) >>>> >>>> default_node = default_version_info->this_node; >>>> >>>> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >>>> - /* Right now, the dispatching is done via ifunc. */ >>>> - dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); >>>> - >>>> - dispatcher_node = cgraph_get_create_node (dispatch_decl); >>>> - gcc_assert (dispatcher_node != NULL); >>>> - dispatcher_node->dispatcher_function = 1; >>>> - dispatcher_version_info >>>> - = insert_new_cgraph_node_version (dispatcher_node); >>>> - dispatcher_version_info->next = default_version_info; >>>> - dispatcher_node->local.finalized = 1; >>>> - >>>> - /* Set the dispatcher for all the versions. */ >>>> - it_v = default_version_info; >>>> - while (it_v->next != NULL) >>>> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >>>> + if (TARGET_HAS_IFUNC) >>>> + { >>>> + /* Right now, the dispatching is done via ifunc. */ >>>> + dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); >>>> + >>>> + dispatcher_node = cgraph_get_create_node (dispatch_decl); >>>> + gcc_assert (dispatcher_node != NULL); >>>> + dispatcher_node->dispatcher_function = 1; >>>> + dispatcher_version_info >>>> + = insert_new_cgraph_node_version (dispatcher_node); >>>> + dispatcher_version_info->next = default_version_info; >>>> + dispatcher_node->local.finalized = 1; >>>> + >>>> + /* Set the dispatcher for all the versions. */ >>>> + it_v = default_version_info; >>>> + while (it_v->next != NULL) >>>> + { >>>> + it_v->dispatcher_resolver = dispatch_decl; >>>> + it_v = it_v->next; >>>> + } >>>> + } >>>> + else >>>> { >>>> - it_v->dispatcher_resolver = dispatch_decl; >>>> - it_v = it_v->next; >>>> + error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), >>>> + "multiversioning needs ifunc which is not supported " >>>> + "on this target"); >>>> } >>> >>> This looks wrong. Before the patch this code would be ignored if !HAVE_GNU_INDIRECT_FUNCTION and after the patch it will produce an error. Removing the else-clause should fix this. >> >> Mmm, I believe that we want this code not to be ignored, because we >> want it to produces an error >> if the target does not support ifuncs and we are compiling something >> that uses ifuncs.. May be I >> missed something.. > > You are right, I read the patch wrong. > >> index db56819..e535218 100644 >> --- a/gcc/config.gcc >> +++ b/gcc/config.gcc >> @@ -637,6 +637,11 @@ case ${target} in >> native_system_header_dir=/include >> ;; >> esac >> + case $target in >> + *linux*) >> + tm_p_file="${tm_p_file} linux-protos.h" >> + ;; >> + esac >> # glibc / uclibc / bionic switch. >> # uclibc and bionic aren't usable for GNU/Hurd and neither for GNU/k*BSD. >> case $target in > > Should not be necessary. Put the hook override into config/linux-android.h: > #undef TARGET_HAS_IFUNC_P > #define TARGET_HAS_IFUNC_P linux_android_has_ifunc_p > >> diff --git a/gcc/config/host-linux.c b/gcc/config/host-linux.c >> index b535758..c72db79 100644 >> --- a/gcc/config/host-linux.c >> +++ b/gcc/config/host-linux.c >> > ... >> >> +/* Android does not support GNU indirect functions. */ >> + >> +bool >> +linux_has_ifunc (void) >> +{ >> + return TARGET_ANDROID ? false : HAVE_GNU_INDIRECT_FUNCTION; >> +} >> + > > OK, so you defined new *target* hook has_ifunc[_p]. Placing one of the hook's implementation is into a host-specific file seems like a strange idea, but maybe I'm missing something. > > It seems you want to add a new file config/linux-android.c (with implementation for symbols used in config/linux-android.h) and add it Android-flavoured linux targets in config.gcc: > # Add Android userspace support to Linux targets. > case $target in > *linux*) > tm_file="$tm_file linux-android.h" > extra_options="$extra_options linux-android.opt" > + extra_objs="$extra_objs linux-android.o" > ;; > esac > > >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >> index b466a4f..c422fd9 100644 >> --- a/gcc/config/i386/i386.c >> +++ b/gcc/config/i386/i386.c >> @@ -29146,7 +29146,7 @@ make_name (tree decl, const char *suffix, bool make_unique) >> return global_var_name; >> } >> >> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >> >> /* Make a dispatcher declaration for the multi-versioned function DECL. >> Calls to DECL function will be replaced with calls to the dispatcher >> @@ -29213,7 +29213,7 @@ ix86_get_function_versions_dispatcher (void *decl) >> >> tree dispatch_decl = NULL; >> >> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >> struct cgraph_function_version_info *it_v = NULL; >> struct cgraph_node *dispatcher_node = NULL; >> struct cgraph_function_version_info *dispatcher_version_info = NULL; >> @@ -29263,24 +29263,33 @@ ix86_get_function_versions_dispatcher (void *decl) >> >> default_node = default_version_info->this_node; >> >> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >> - /* Right now, the dispatching is done via ifunc. */ >> - dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); >> - >> - dispatcher_node = cgraph_get_create_node (dispatch_decl); >> - gcc_assert (dispatcher_node != NULL); >> - dispatcher_node->dispatcher_function = 1; >> - dispatcher_version_info >> - = insert_new_cgraph_node_version (dispatcher_node); >> - dispatcher_version_info->next = default_version_info; >> - dispatcher_node->local.finalized = 1; >> - >> - /* Set the dispatcher for all the versions. */ >> - it_v = default_version_info; >> - while (it_v->next != NULL) >> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >> + if (targetm.has_ifunc ()) >> + { >> + /* Right now, the dispatching is done via ifunc. */ >> + dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); >> + >> + dispatcher_node = cgraph_get_create_node (dispatch_decl); >> + gcc_assert (dispatcher_node != NULL); >> + dispatcher_node->dispatcher_function = 1; >> + dispatcher_version_info >> + = insert_new_cgraph_node_version (dispatcher_node); >> + dispatcher_version_info->next = default_version_info; >> + dispatcher_node->local.finalized = 1; >> + >> + /* Set the dispatcher for all the versions. */ >> + it_v = default_version_info; >> + while (it_v->next != NULL) >> + { >> + it_v->dispatcher_resolver = dispatch_decl; >> + it_v = it_v->next; >> + } >> + } >> + else >> { >> - it_v->dispatcher_resolver = dispatch_decl; >> - it_v = it_v->next; >> + error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), >> + "multiversioning needs ifunc which is not supported " >> + "on this target"); >> } >> #else >> error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), > > Would you please use same trick for arranging #if-else-#endif as you did for varasm.c ? > >> diff --git a/gcc/config/linux-protos.h b/gcc/config/linux-protos.h >> new file mode 100644 >> index 0000000..ea3d77d >> --- /dev/null >> +++ b/gcc/config/linux-protos.h >> @@ -0,0 +1,21 @@ >> +/* Prototypes. >> + Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2010, 2011, 2012 >> + Free Software Foundation, Inc. >> + >> +This file is part of GCC. >> + >> +GCC is free software; you can redistribute it and/or modify >> +it under the terms of the GNU General Public License as published by >> +the Free Software Foundation; either version 3, or (at your option) >> +any later version. >> + >> +GCC is distributed in the hope that it will be useful, >> +but WITHOUT ANY WARRANTY; without even the implied warranty of >> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> +GNU General Public License for more details. >> + >> +You should have received a copy of the GNU General Public License >> +along with GCC; see the file COPYING3. If not see >> +<http://www.gnu.org/licenses/>. */ >> + >> +extern bool linux_has_ifunc (void); > > Config/linux-android.h is a better place for this declaration. > >> diff --git a/gcc/config/linux.h b/gcc/config/linux.h >> index fb459e6..bdfbc0d 100644 >> --- a/gcc/config/linux.h >> +++ b/gcc/config/linux.h >> @@ -108,3 +108,6 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >> /* Whether we have Bionic libc runtime */ >> #undef TARGET_HAS_BIONIC >> #define TARGET_HAS_BIONIC (OPTION_BIONIC) >> + >> +#undef TARGET_HAS_IFUNC >> +#define TARGET_HAS_IFUNC linux_has_ifunc > > As noted above, please move the hook override to linux-android.h . The override should be effective only for Linux targets that support Android, otherwise compilation may fail due to undefined TARGET_ANDROID used in linux_has_ifunc. > > >> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi >> index 75aa867..2d7121f 100644 >> --- a/gcc/doc/tm.texi >> +++ b/gcc/doc/tm.texi >> @@ -11343,3 +11343,9 @@ memory model bits are allowed. >> @deftypevr {Target Hook} {unsigned char} TARGET_ATOMIC_TEST_AND_SET_TRUEVAL >> This value should be set if the result written by @code{atomic_test_and_set} is not exactly 1, i.e. the @code{bool} @code{true}. >> @end deftypevr >> + >> +@deftypefn {Target Hook} bool TARGET_HAS_IFUNC (void) >> +It returns true if the target supports GNU indirect functions. >> +The support includes the assembler, linker and dynamic linker. >> +The default value of this hook is defined as for the host machine. >> +@end deftypefn >> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in >> index 95fab18..40dba77 100644 >> --- a/gcc/doc/tm.texi.in >> +++ b/gcc/doc/tm.texi.in >> @@ -11179,3 +11179,5 @@ memory model bits are allowed. >> @end deftypefn >> >> @hook TARGET_ATOMIC_TEST_AND_SET_TRUEVAL >> + >> +@hook TARGET_HAS_IFUNC >> diff --git a/gcc/target.def b/gcc/target.def >> index c5bbfae..627ae2d 100644 >> --- a/gcc/target.def >> +++ b/gcc/target.def >> @@ -1520,6 +1520,15 @@ DEFHOOK >> bool, (const_rtx x), >> default_use_anchors_for_symbol_p) >> >> +/* True if target supports indirect functions. */ >> +DEFHOOK >> +(has_ifunc, > > The convention is to add "_p" for functions that behave like boolean predicates, i.e., "has_ifunc_p". > >> + "It returns true if the target supports GNU indirect functions.\n\ >> +The support includes the assembler, linker and dynamic linker.\n\ >> +The default value of this hook is defined as for the host machine.", > > Are your sure the last sentence is correct? It seems the default value for this hook is based on which libc is being used. Maybe it would be more accurate to say "The default value of this hook is based on target's libc."? > >> + bool, (void), >> + default_have_ifunc) >> + >> /* True if it is OK to do sibling call optimization for the specified >> call expression EXP. DECL will be the called function, or NULL if >> this is an indirect call. */ >> diff --git a/gcc/targhooks.c b/gcc/targhooks.c >> index 954cdb9..ea0f9e2 100644 >> --- a/gcc/targhooks.c >> +++ b/gcc/targhooks.c >> @@ -451,6 +451,14 @@ default_fixed_point_supported_p (void) >> return ENABLE_FIXED_POINT; >> } >> >> +/* True if the target supports GNU indirect functions. */ >> + >> +bool >> +default_have_ifunc (void) >> +{ >> + return HAVE_GNU_INDIRECT_FUNCTION; >> +} >> + >> /* NULL if INSN insn is valid within a low-overhead loop, otherwise returns >> an error message. >> >> diff --git a/gcc/targhooks.h b/gcc/targhooks.h >> index bd7d4bc..8622d58 100644 >> --- a/gcc/targhooks.h >> +++ b/gcc/targhooks.h >> @@ -73,6 +73,8 @@ extern bool targhook_float_words_big_endian (void); >> extern bool default_decimal_float_supported_p (void); >> extern bool default_fixed_point_supported_p (void); >> >> +extern bool default_have_ifunc (void); >> + >> extern const char * default_invalid_within_doloop (const_rtx); >> >> extern tree default_builtin_vectorized_function (tree, tree, tree); >> diff --git a/gcc/varasm.c b/gcc/varasm.c >> index 7d083fd..dd677a4 100644 >> --- a/gcc/varasm.c >> +++ b/gcc/varasm.c >> @@ -5489,14 +5489,15 @@ do_assemble_alias (tree decl, tree target) >> } >> if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))) >> { >> -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION >> - ASM_OUTPUT_TYPE_DIRECTIVE >> - (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), >> - IFUNC_ASM_TYPE); >> -#else >> - error_at (DECL_SOURCE_LOCATION (decl), >> - "ifunc is not supported in this configuration"); >> +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) >> + if (targetm.has_ifunc ()) >> + ASM_OUTPUT_TYPE_DIRECTIVE >> + (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), >> + IFUNC_ASM_TYPE); >> + else >> #endif >> + error_at (DECL_SOURCE_LOCATION (decl), >> + "ifunc is not supported on this target"); >> } > > I like how you arranged #if-else-#endif to have a single error path. > > Thank you, > > -- > Maxim Kuvyrkov > > > > [-- Attachment #2: disable_ifunc_for_android_05.patch --] [-- Type: application/octet-stream, Size: 12771 bytes --] diff --git a/gcc/ChangeLog b/gcc/ChangeLog index d625bb1..abce372 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,21 @@ +2012-12-28 Alexander Ivchenko <alexander.ivchenko@intel.com> + + * gcc/target.def (TARGET_HAS_IFUNC_P): New target hook. + * gcc/doc/tm.texi.in (TARGET_HAS_IFUNC_P): New. + * gcc/doc/tm.texi: Regenerate. + * gcc/targhooks.h (default_have_ifunc_p): New. + * gcc/targhooks.c (default_have_ifunc_p): Ditto. + * gcc/config/linux-protos.h: New file. + * gcc/config/linux-androids.h (TARGET_HAS_IFUNC_P): Using version of + this hook for linux which disables support of indirect functions in + android. + * gcc/config/linux-android.c: New file. + * gcc/config/t-linux-android.c: Ditto. + * gcc/config.gcc: Added new object file linux-android.o. + * gcc/config/i386/i386.c (ix86_get_function_versions_dispatcher): + Using TARGET_HAS_IFUNC hook instead of HAVE_GNU_INDIRECT_FUNCTION. + * gcc/varasm.c (do_assemble_alias): Likewise. + 2012-12-22 Jan Hubicka <jh@suse.cz> PR lto/54728 diff --git a/gcc/config.gcc b/gcc/config.gcc index db56819..bbf65c2 100644 --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -637,6 +637,11 @@ case ${target} in native_system_header_dir=/include ;; esac + case $target in + *linux*) + tm_p_file="${tm_p_file} linux-protos.h" + ;; + esac # glibc / uclibc / bionic switch. # uclibc and bionic aren't usable for GNU/Hurd and neither for GNU/k*BSD. case $target in @@ -662,8 +667,10 @@ case ${target} in # Add Android userspace support to Linux targets. case $target in *linux*) + tmake_file="${tmake_file} t-linux-android" tm_file="$tm_file linux-android.h" extra_options="$extra_options linux-android.opt" + extra_objs="$extra_objs linux-android.o" ;; esac # Enable compilation for Android by default for *android* targets. diff --git a/gcc/config/host-linux.c b/gcc/config/host-linux.c index b535758..ae5d211 100644 --- a/gcc/config/host-linux.c +++ b/gcc/config/host-linux.c @@ -23,7 +23,6 @@ #include "hosthooks.h" #include "hosthooks-def.h" - /* Linux has a feature called exec-shield-randomize that perturbs the address of non-fixed mapped segments by a (relatively) small amount. The feature is intended to make it harder to attack the system with diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index b466a4f..98e2998 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -29146,7 +29146,7 @@ make_name (tree decl, const char *suffix, bool make_unique) return global_var_name; } -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) /* Make a dispatcher declaration for the multi-versioned function DECL. Calls to DECL function will be replaced with calls to the dispatcher @@ -29213,12 +29213,6 @@ ix86_get_function_versions_dispatcher (void *decl) tree dispatch_decl = NULL; -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION - struct cgraph_function_version_info *it_v = NULL; - struct cgraph_node *dispatcher_node = NULL; - struct cgraph_function_version_info *dispatcher_version_info = NULL; -#endif - struct cgraph_function_version_info *default_version_info = NULL; gcc_assert (fn != NULL && DECL_FUNCTION_VERSIONED (fn)); @@ -29263,30 +29257,40 @@ ix86_get_function_versions_dispatcher (void *decl) default_node = default_version_info->this_node; -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION - /* Right now, the dispatching is done via ifunc. */ - dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); - - dispatcher_node = cgraph_get_create_node (dispatch_decl); - gcc_assert (dispatcher_node != NULL); - dispatcher_node->dispatcher_function = 1; - dispatcher_version_info - = insert_new_cgraph_node_version (dispatcher_node); - dispatcher_version_info->next = default_version_info; - dispatcher_node->local.finalized = 1; - - /* Set the dispatcher for all the versions. */ - it_v = default_version_info; - while (it_v->next != NULL) +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) + if (targetm.has_ifunc_p ()) { - it_v->dispatcher_resolver = dispatch_decl; - it_v = it_v->next; + struct cgraph_function_version_info *it_v = NULL; + struct cgraph_node *dispatcher_node = NULL; + struct cgraph_function_version_info *dispatcher_version_info = NULL; + + /* Right now, the dispatching is done via ifunc. */ + dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); + + dispatcher_node = cgraph_get_create_node (dispatch_decl); + gcc_assert (dispatcher_node != NULL); + dispatcher_node->dispatcher_function = 1; + dispatcher_version_info + = insert_new_cgraph_node_version (dispatcher_node); + dispatcher_version_info->next = default_version_info; + dispatcher_node->local.finalized = 1; + + /* Set the dispatcher for all the versions. */ + it_v = default_version_info; + while (it_v->next != NULL) + { + it_v->dispatcher_resolver = dispatch_decl; + it_v = it_v->next; + } } -#else - error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), - "multiversioning needs ifunc which is not supported " - "in this configuration"); + else #endif + { + error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), + "multiversioning needs ifunc which is not supported " + "on this target"); + } + return dispatch_decl; } diff --git a/gcc/config/linux-android.c b/gcc/config/linux-android.c new file mode 100644 index 0000000..47cc50a --- /dev/null +++ b/gcc/config/linux-android.c @@ -0,0 +1,33 @@ +/* Functions for Linux Android as target machine for GNU C compiler. + Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2010, 2011, + 2012, 2013. + Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +<http://www.gnu.org/licenses/>. */ + +#include "config.h" +#include "system.h" +#include "coretypes.h" +#include "tm.h" + +/* Android does not support GNU indirect functions. */ + +bool +linux_android_has_ifunc_p (void) +{ + return TARGET_ANDROID ? false : HAVE_GNU_INDIRECT_FUNCTION; +} diff --git a/gcc/config/linux-android.h b/gcc/config/linux-android.h index e74e261..d6a2bb8 100644 --- a/gcc/config/linux-android.h +++ b/gcc/config/linux-android.h @@ -58,3 +58,6 @@ #define ANDROID_ENDFILE_SPEC \ "%{shared: crtend_so%O%s;: crtend_android%O%s}" + +#undef TARGET_HAS_IFUNC_P +#define TARGET_HAS_IFUNC_P linux_android_has_ifunc_p diff --git a/gcc/config/linux-protos.h b/gcc/config/linux-protos.h new file mode 100644 index 0000000..aae1d28 --- /dev/null +++ b/gcc/config/linux-protos.h @@ -0,0 +1,22 @@ +/* Prototypes. + Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2010, 2011, + 2012, 2013 + Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +<http://www.gnu.org/licenses/>. */ + +extern bool linux_android_has_ifunc_p (void); diff --git a/gcc/config/t-linux-android b/gcc/config/t-linux-android new file mode 100644 index 0000000..6f9b033 --- /dev/null +++ b/gcc/config/t-linux-android @@ -0,0 +1,23 @@ +# Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2013 +# Free Software Foundation, Inc. +# +# This file is part of GCC. +# +# GCC is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3, or (at your option) +# any later version. +# +# GCC is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with GCC; see the file COPYING3. If not see +# <http://www.gnu.org/licenses/>. + +linux-android.o: $(srcdir)/config/linux-android.c $(CONFIG_H) $(SYSTEM_H) \ + coretypes.h $(TM_H) $(TM_P_H) + $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \ + $(srcdir)/config/linux-android.c diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index 75aa867..48cc507 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -11343,3 +11343,9 @@ memory model bits are allowed. @deftypevr {Target Hook} {unsigned char} TARGET_ATOMIC_TEST_AND_SET_TRUEVAL This value should be set if the result written by @code{atomic_test_and_set} is not exactly 1, i.e. the @code{bool} @code{true}. @end deftypevr + +@deftypefn {Target Hook} bool TARGET_HAS_IFUNC_P (void) +It returns true if the target supports GNU indirect functions. +The support includes the assembler, linker and dynamic linker. +The default value of this hook is based on host's libc. +@end deftypefn diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index 95fab18..432cdcf 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -11179,3 +11179,5 @@ memory model bits are allowed. @end deftypefn @hook TARGET_ATOMIC_TEST_AND_SET_TRUEVAL + +@hook TARGET_HAS_IFUNC_P diff --git a/gcc/target.def b/gcc/target.def index c5bbfae..f8df671 100644 --- a/gcc/target.def +++ b/gcc/target.def @@ -1520,6 +1520,15 @@ DEFHOOK bool, (const_rtx x), default_use_anchors_for_symbol_p) +/* True if target supports indirect functions. */ +DEFHOOK +(has_ifunc_p, + "It returns true if the target supports GNU indirect functions.\n\ +The support includes the assembler, linker and dynamic linker.\n\ +The default value of this hook is based on host's libc.", + bool, (void), + default_have_ifunc_p) + /* True if it is OK to do sibling call optimization for the specified call expression EXP. DECL will be the called function, or NULL if this is an indirect call. */ diff --git a/gcc/targhooks.c b/gcc/targhooks.c index 954cdb9..2a8caec 100644 --- a/gcc/targhooks.c +++ b/gcc/targhooks.c @@ -451,6 +451,14 @@ default_fixed_point_supported_p (void) return ENABLE_FIXED_POINT; } +/* True if the target supports GNU indirect functions. */ + +bool +default_have_ifunc_p (void) +{ + return HAVE_GNU_INDIRECT_FUNCTION; +} + /* NULL if INSN insn is valid within a low-overhead loop, otherwise returns an error message. diff --git a/gcc/targhooks.h b/gcc/targhooks.h index bd7d4bc..6cb99ba 100644 --- a/gcc/targhooks.h +++ b/gcc/targhooks.h @@ -73,6 +73,8 @@ extern bool targhook_float_words_big_endian (void); extern bool default_decimal_float_supported_p (void); extern bool default_fixed_point_supported_p (void); +extern bool default_have_ifunc_p (void); + extern const char * default_invalid_within_doloop (const_rtx); extern tree default_builtin_vectorized_function (tree, tree, tree); diff --git a/gcc/varasm.c b/gcc/varasm.c index 7d083fd..d9b7bb7 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -5489,14 +5489,15 @@ do_assemble_alias (tree decl, tree target) } if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))) { -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION - ASM_OUTPUT_TYPE_DIRECTIVE - (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), - IFUNC_ASM_TYPE); -#else - error_at (DECL_SOURCE_LOCATION (decl), - "ifunc is not supported in this configuration"); +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) + if (targetm.has_ifunc_p ()) + ASM_OUTPUT_TYPE_DIRECTIVE + (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), + IFUNC_ASM_TYPE); + else #endif + error_at (DECL_SOURCE_LOCATION (decl), + "ifunc is not supported on this target"); } # ifdef ASM_OUTPUT_DEF_FROM_DECLS ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH] Implementing ifunc target hook 2013-01-09 11:24 ` Alexander Ivchenko @ 2013-01-10 23:24 ` Maxim Kuvyrkov 2013-01-14 15:55 ` Alexander Ivchenko 0 siblings, 1 reply; 16+ messages in thread From: Maxim Kuvyrkov @ 2013-01-10 23:24 UTC (permalink / raw) To: Alexander Ivchenko; +Cc: GCC Patches, Joseph S. Myers, H.J. Lu On 10/01/2013, at 12:24 AM, Alexander Ivchenko wrote: >> Config/linux-android.h is a better place for this declaration. I was wrong here. Config/linux-android.h is not a "re-includable" header, and is not fit for function declarations. > > That wouldn't help, I got the following error: > > In file included from ../../.././gcc/tm.h:24:0, > from [..]/src/gcc/libgcc/generic-morestack-thread.c:29: > [..]/src/gcc/libgcc/../gcc/config/linux-android.h:62:1: error: unknown > type name ‘bool’ > extern bool linux_android_has_ifunc_p (void); > ^ > > Anyway, linux-protos.h looks to me as a good thing to have (e.g. for > libc_has_function hook, that is > supposed to be commited in a near future) for declaring linux (and > Android) specific versions of hooks.. OK, I agree. In theory linux_android_has_ifunc_p could've been placed into linux-android-protos.h, but having a separate file (from linux-protos.h) just for 1-2 declarations is not justified. >>> + "It returns true if the target supports GNU indirect functions.\n\ >>> +The support includes the assembler, linker and dynamic linker.\n\ >>> +The default value of this hook is defined as for the host machine.", >> >> Are your sure the last sentence is correct? It seems the default value for this hook is based on which libc is being used. Maybe it would be more accurate to say "The default value of this hook is based on target's libc."? > > Well yes, you are right that the default value depends on version of > libc, but this version > is checked on the configure time for the host machine > (HAVE_GNU_INDIRECT_FUNCTION), No. And even if that was the case that would be a bug. HAVE_GNU_INDIRECT_FUNCTION is set based on default_gnu_indirect_function variable defined in config.gcc (unless overridden with a configure argument). This variable is set to true for targets that support IFUNCs irrespective of host and host's libc. > --- a/gcc/config.gcc > +++ b/gcc/config.gcc > @@ -637,6 +637,11 @@ case ${target} in > native_system_header_dir=/include > ;; > esac > + case $target in > + *linux*) > + tm_p_file="${tm_p_file} linux-protos.h" > + ;; > + esac > # glibc / uclibc / bionic switch. > # uclibc and bionic aren't usable for GNU/Hurd and neither for GNU/k*BSD. > case $target in > @@ -662,8 +667,10 @@ case ${target} in > # Add Android userspace support to Linux targets. > case $target in > *linux*) > + tmake_file="${tmake_file} t-linux-android" > tm_file="$tm_file linux-android.h" > extra_options="$extra_options linux-android.opt" > + extra_objs="$extra_objs linux-android.o" > ;; > esac > # Enable compilation for Android by default for *android* targets. > Also scan config.gcc for any other occurences of linux-android.h and add same changes there. I believe only arm*-*-linux-* is affected. > --- a/gcc/config/host-linux.c > +++ b/gcc/config/host-linux.c > @@ -23,7 +23,6 @@ > #include "hosthooks.h" > #include "hosthooks-def.h" > > - > /* Linux has a feature called exec-shield-randomize that perturbs the > address of non-fixed mapped segments by a (relatively) small amount. > The feature is intended to make it harder to attack the system with Don't change formatting unless you are editing the code around it. > diff --git a/gcc/config/linux-android.c b/gcc/config/linux-android.c > new file mode 100644 > index 0000000..47cc50a > --- /dev/null > +++ b/gcc/config/linux-android.c ... > +You should have received a copy of the GNU General Public License > +along with GCC; see the file COPYING3. If not see > +<http://www.gnu.org/licenses/>. */ > + > +#include "config.h" > +#include "system.h" > +#include "coretypes.h" > +#include "tm.h" Add '#include "linux-protos.h"'. Remember to update t-linux-android. > + > +/* Android does not support GNU indirect functions. */ > + > +bool > +linux_android_has_ifunc_p (void) > +{ > + return TARGET_ANDROID ? false : HAVE_GNU_INDIRECT_FUNCTION; > +} > Please send one last version for the final review, primarily to double check your changes to arm*-linux-* to avoid breaking it. Thanks! -- Maxim Kuvyrkov ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH] Implementing ifunc target hook 2013-01-10 23:24 ` Maxim Kuvyrkov @ 2013-01-14 15:55 ` Alexander Ivchenko 2013-01-15 4:13 ` Maxim Kuvyrkov 0 siblings, 1 reply; 16+ messages in thread From: Alexander Ivchenko @ 2013-01-14 15:55 UTC (permalink / raw) To: Maxim Kuvyrkov; +Cc: GCC Patches, Joseph S. Myers, H.J. Lu [-- Attachment #1: Type: text/plain, Size: 4666 bytes --] thank you very much for your review! I fixed the arm build and all other issues that you raised. the patch is attached. Bootstrap and tested on x86-64 linux Alexander 2013/1/11 Maxim Kuvyrkov <maxim.kuvyrkov@gmail.com>: > On 10/01/2013, at 12:24 AM, Alexander Ivchenko wrote: > >>> Config/linux-android.h is a better place for this declaration. > > I was wrong here. Config/linux-android.h is not a "re-includable" header, and is not fit for function declarations. > >> >> That wouldn't help, I got the following error: >> >> In file included from ../../.././gcc/tm.h:24:0, >> from [..]/src/gcc/libgcc/generic-morestack-thread.c:29: >> [..]/src/gcc/libgcc/../gcc/config/linux-android.h:62:1: error: unknown >> type name ‘bool’ >> extern bool linux_android_has_ifunc_p (void); >> ^ >> >> Anyway, linux-protos.h looks to me as a good thing to have (e.g. for >> libc_has_function hook, that is >> supposed to be commited in a near future) for declaring linux (and >> Android) specific versions of hooks.. > > OK, I agree. In theory linux_android_has_ifunc_p could've been placed into linux-android-protos.h, but having a separate file (from linux-protos.h) just for 1-2 declarations is not justified. > >>>> + "It returns true if the target supports GNU indirect functions.\n\ >>>> +The support includes the assembler, linker and dynamic linker.\n\ >>>> +The default value of this hook is defined as for the host machine.", >>> >>> Are your sure the last sentence is correct? It seems the default value for this hook is based on which libc is being used. Maybe it would be more accurate to say "The default value of this hook is based on target's libc."? >> >> Well yes, you are right that the default value depends on version of >> libc, but this version >> is checked on the configure time for the host machine >> (HAVE_GNU_INDIRECT_FUNCTION), > > No. And even if that was the case that would be a bug. > > HAVE_GNU_INDIRECT_FUNCTION is set based on default_gnu_indirect_function variable defined in config.gcc (unless overridden with a configure argument). This variable is set to true for targets that support IFUNCs irrespective of host and host's libc. > >> --- a/gcc/config.gcc >> +++ b/gcc/config.gcc >> @@ -637,6 +637,11 @@ case ${target} in >> native_system_header_dir=/include >> ;; >> esac >> + case $target in >> + *linux*) >> + tm_p_file="${tm_p_file} linux-protos.h" >> + ;; >> + esac >> # glibc / uclibc / bionic switch. >> # uclibc and bionic aren't usable for GNU/Hurd and neither for GNU/k*BSD. >> case $target in >> @@ -662,8 +667,10 @@ case ${target} in >> # Add Android userspace support to Linux targets. >> case $target in >> *linux*) >> + tmake_file="${tmake_file} t-linux-android" >> tm_file="$tm_file linux-android.h" >> extra_options="$extra_options linux-android.opt" >> + extra_objs="$extra_objs linux-android.o" >> ;; >> esac >> # Enable compilation for Android by default for *android* targets. >> > > Also scan config.gcc for any other occurences of linux-android.h and add same changes there. I believe only arm*-*-linux-* is affected. > > >> --- a/gcc/config/host-linux.c >> +++ b/gcc/config/host-linux.c >> @@ -23,7 +23,6 @@ >> #include "hosthooks.h" >> #include "hosthooks-def.h" >> >> - >> /* Linux has a feature called exec-shield-randomize that perturbs the >> address of non-fixed mapped segments by a (relatively) small amount. >> The feature is intended to make it harder to attack the system with > > Don't change formatting unless you are editing the code around it. > >> diff --git a/gcc/config/linux-android.c b/gcc/config/linux-android.c >> new file mode 100644 >> index 0000000..47cc50a >> --- /dev/null >> +++ b/gcc/config/linux-android.c > ... >> +You should have received a copy of the GNU General Public License >> +along with GCC; see the file COPYING3. If not see >> +<http://www.gnu.org/licenses/>. */ >> + >> +#include "config.h" >> +#include "system.h" >> +#include "coretypes.h" >> +#include "tm.h" > > Add '#include "linux-protos.h"'. Remember to update t-linux-android. > >> + >> +/* Android does not support GNU indirect functions. */ >> + >> +bool >> +linux_android_has_ifunc_p (void) >> +{ >> + return TARGET_ANDROID ? false : HAVE_GNU_INDIRECT_FUNCTION; >> +} >> > > > Please send one last version for the final review, primarily to double check your changes to arm*-linux-* to avoid breaking it. > > Thanks! > > -- > Maxim Kuvyrkov > > > > > [-- Attachment #2: disable_ifunc_for_android_06.patch --] [-- Type: application/octet-stream, Size: 12825 bytes --] diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 64df290..3d71474 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,21 @@ +2013-01-14 Alexander Ivchenko <alexander.ivchenko@intel.com> + + * gcc/target.def (TARGET_HAS_IFUNC_P): New target hook. + * gcc/doc/tm.texi.in (TARGET_HAS_IFUNC_P): New. + * gcc/doc/tm.texi: Regenerate. + * gcc/targhooks.h (default_have_ifunc_p): New. + * gcc/targhooks.c (default_have_ifunc_p): Ditto. + * gcc/config/linux-protos.h: New file. + * gcc/config/linux-androids.h (TARGET_HAS_IFUNC_P): Using version of + this hook for linux which disables support of indirect functions in + android. + * gcc/config/linux-android.c: New file. + * gcc/config/t-linux-android.c: Ditto. + * gcc/config.gcc: Added new object file linux-android.o. + * gcc/config/i386/i386.c (ix86_get_function_versions_dispatcher): + Using TARGET_HAS_IFUNC hook instead of HAVE_GNU_INDIRECT_FUNCTION. + * gcc/varasm.c (do_assemble_alias): Likewise. + 2013-01-14 Georg-Johann Lay <avr@gjlay.de> * config/avr/avr-stdint.h: Remove trailing blanks. diff --git a/gcc/config.gcc b/gcc/config.gcc index 7deac33..26602d2 100644 --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -636,6 +636,11 @@ case ${target} in native_system_header_dir=/include ;; esac + case $target in + *linux*) + tm_p_file="${tm_p_file} linux-protos.h" + ;; + esac # glibc / uclibc / bionic switch. # uclibc and bionic aren't usable for GNU/Hurd and neither for GNU/k*BSD. case $target in @@ -661,8 +666,10 @@ case ${target} in # Add Android userspace support to Linux targets. case $target in *linux*) + tmake_file="${tmake_file} t-linux-android" tm_file="$tm_file linux-android.h" extra_options="$extra_options linux-android.opt" + extra_objs="$extra_objs linux-android.o" ;; esac # Enable compilation for Android by default for *android* targets. @@ -863,7 +870,9 @@ arm*-*-netbsdelf*) tmake_file="${tmake_file} arm/t-arm" ;; arm*-*-linux-*) # ARM GNU/Linux with ELF + tmake_file="${tmake_file} t-linux-android" tm_file="dbxelf.h elfos.h gnu-user.h linux.h linux-android.h glibc-stdint.h arm/elf.h arm/linux-gas.h arm/linux-elf.h" + extra_objs="$extra_objs linux-android.o" case $target in arm*b-*-linux*) tm_defines="${tm_defines} TARGET_BIG_ENDIAN_DEFAULT=1" diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 4f778c1..1cf4f94 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -29142,7 +29142,7 @@ make_name (tree decl, const char *suffix, bool make_unique) return global_var_name; } -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) /* Make a dispatcher declaration for the multi-versioned function DECL. Calls to DECL function will be replaced with calls to the dispatcher @@ -29209,12 +29209,6 @@ ix86_get_function_versions_dispatcher (void *decl) tree dispatch_decl = NULL; -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION - struct cgraph_function_version_info *it_v = NULL; - struct cgraph_node *dispatcher_node = NULL; - struct cgraph_function_version_info *dispatcher_version_info = NULL; -#endif - struct cgraph_function_version_info *default_version_info = NULL; gcc_assert (fn != NULL && DECL_FUNCTION_VERSIONED (fn)); @@ -29259,30 +29253,40 @@ ix86_get_function_versions_dispatcher (void *decl) default_node = default_version_info->this_node; -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION - /* Right now, the dispatching is done via ifunc. */ - dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); - - dispatcher_node = cgraph_get_create_node (dispatch_decl); - gcc_assert (dispatcher_node != NULL); - dispatcher_node->dispatcher_function = 1; - dispatcher_version_info - = insert_new_cgraph_node_version (dispatcher_node); - dispatcher_version_info->next = default_version_info; - dispatcher_node->local.finalized = 1; - - /* Set the dispatcher for all the versions. */ - it_v = default_version_info; - while (it_v != NULL) +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) + if (targetm.has_ifunc_p ()) { - it_v->dispatcher_resolver = dispatch_decl; - it_v = it_v->next; + struct cgraph_function_version_info *it_v = NULL; + struct cgraph_node *dispatcher_node = NULL; + struct cgraph_function_version_info *dispatcher_version_info = NULL; + + /* Right now, the dispatching is done via ifunc. */ + dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); + + dispatcher_node = cgraph_get_create_node (dispatch_decl); + gcc_assert (dispatcher_node != NULL); + dispatcher_node->dispatcher_function = 1; + dispatcher_version_info + = insert_new_cgraph_node_version (dispatcher_node); + dispatcher_version_info->next = default_version_info; + dispatcher_node->local.finalized = 1; + + /* Set the dispatcher for all the versions. */ + it_v = default_version_info; + while (it_v != NULL) + { + it_v->dispatcher_resolver = dispatch_decl; + it_v = it_v->next; + } } -#else - error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), - "multiversioning needs ifunc which is not supported " - "in this configuration"); + else #endif + { + error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), + "multiversioning needs ifunc which is not supported " + "on this target"); + } + return dispatch_decl; } diff --git a/gcc/config/linux-android.c b/gcc/config/linux-android.c new file mode 100644 index 0000000..f3d82a5 --- /dev/null +++ b/gcc/config/linux-android.c @@ -0,0 +1,34 @@ +/* Functions for Linux Android as target machine for GNU C compiler. + Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2010, 2011, + 2012, 2013. + Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +<http://www.gnu.org/licenses/>. */ + +#include "config.h" +#include "system.h" +#include "coretypes.h" +#include "tm.h" +#include "linux-protos.h" + +/* Android does not support GNU indirect functions. */ + +bool +linux_android_has_ifunc_p (void) +{ + return TARGET_ANDROID ? false : HAVE_GNU_INDIRECT_FUNCTION; +} diff --git a/gcc/config/linux-android.h b/gcc/config/linux-android.h index 2c87c84..831a19c 100644 --- a/gcc/config/linux-android.h +++ b/gcc/config/linux-android.h @@ -57,3 +57,6 @@ #define ANDROID_ENDFILE_SPEC \ "%{shared: crtend_so%O%s;: crtend_android%O%s}" + +#undef TARGET_HAS_IFUNC_P +#define TARGET_HAS_IFUNC_P linux_android_has_ifunc_p diff --git a/gcc/config/linux-protos.h b/gcc/config/linux-protos.h new file mode 100644 index 0000000..aae1d28 --- /dev/null +++ b/gcc/config/linux-protos.h @@ -0,0 +1,22 @@ +/* Prototypes. + Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2010, 2011, + 2012, 2013 + Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +<http://www.gnu.org/licenses/>. */ + +extern bool linux_android_has_ifunc_p (void); diff --git a/gcc/config/t-linux-android b/gcc/config/t-linux-android new file mode 100644 index 0000000..6f9b033 --- /dev/null +++ b/gcc/config/t-linux-android @@ -0,0 +1,23 @@ +# Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2013 +# Free Software Foundation, Inc. +# +# This file is part of GCC. +# +# GCC is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3, or (at your option) +# any later version. +# +# GCC is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with GCC; see the file COPYING3. If not see +# <http://www.gnu.org/licenses/>. + +linux-android.o: $(srcdir)/config/linux-android.c $(CONFIG_H) $(SYSTEM_H) \ + coretypes.h $(TM_H) $(TM_P_H) + $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \ + $(srcdir)/config/linux-android.c diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index 9d6f6bc..18e464c 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -11346,3 +11346,9 @@ memory model bits are allowed. @deftypevr {Target Hook} {unsigned char} TARGET_ATOMIC_TEST_AND_SET_TRUEVAL This value should be set if the result written by @code{atomic_test_and_set} is not exactly 1, i.e. the @code{bool} @code{true}. @end deftypevr + +@deftypefn {Target Hook} bool TARGET_HAS_IFUNC_P (void) +It returns true if the target supports GNU indirect functions. +The support includes the assembler, linker and dynamic linker. +The default value of this hook is based on target's libc. +@end deftypefn diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index 3668882..e1d6cff 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -11182,3 +11182,5 @@ memory model bits are allowed. @end deftypefn @hook TARGET_ATOMIC_TEST_AND_SET_TRUEVAL + +@hook TARGET_HAS_IFUNC_P diff --git a/gcc/target.def b/gcc/target.def index 8627923..c257d4f 100644 --- a/gcc/target.def +++ b/gcc/target.def @@ -1518,6 +1518,15 @@ DEFHOOK bool, (const_rtx x), default_use_anchors_for_symbol_p) +/* True if target supports indirect functions. */ +DEFHOOK +(has_ifunc_p, + "It returns true if the target supports GNU indirect functions.\n\ +The support includes the assembler, linker and dynamic linker.\n\ +The default value of this hook is based on target's libc.", + bool, (void), + default_have_ifunc_p) + /* True if it is OK to do sibling call optimization for the specified call expression EXP. DECL will be the called function, or NULL if this is an indirect call. */ diff --git a/gcc/targhooks.c b/gcc/targhooks.c index 6c12a4a..e5ce81f 100644 --- a/gcc/targhooks.c +++ b/gcc/targhooks.c @@ -450,6 +450,14 @@ default_fixed_point_supported_p (void) return ENABLE_FIXED_POINT; } +/* True if the target supports GNU indirect functions. */ + +bool +default_have_ifunc_p (void) +{ + return HAVE_GNU_INDIRECT_FUNCTION; +} + /* NULL if INSN insn is valid within a low-overhead loop, otherwise returns an error message. diff --git a/gcc/targhooks.h b/gcc/targhooks.h index d23b352..4d11768 100644 --- a/gcc/targhooks.h +++ b/gcc/targhooks.h @@ -72,6 +72,8 @@ extern bool targhook_float_words_big_endian (void); extern bool default_decimal_float_supported_p (void); extern bool default_fixed_point_supported_p (void); +extern bool default_have_ifunc_p (void); + extern const char * default_invalid_within_doloop (const_rtx); extern tree default_builtin_vectorized_function (tree, tree, tree); diff --git a/gcc/varasm.c b/gcc/varasm.c index 0b1894d..4f3a8db 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -5489,14 +5489,15 @@ do_assemble_alias (tree decl, tree target) } if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))) { -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION - ASM_OUTPUT_TYPE_DIRECTIVE - (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), - IFUNC_ASM_TYPE); -#else - error_at (DECL_SOURCE_LOCATION (decl), - "ifunc is not supported in this configuration"); +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) + if (targetm.has_ifunc_p ()) + ASM_OUTPUT_TYPE_DIRECTIVE + (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), + IFUNC_ASM_TYPE); + else #endif + error_at (DECL_SOURCE_LOCATION (decl), + "ifunc is not supported on this target"); } # ifdef ASM_OUTPUT_DEF_FROM_DECLS ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH] Implementing ifunc target hook 2013-01-14 15:55 ` Alexander Ivchenko @ 2013-01-15 4:13 ` Maxim Kuvyrkov 2013-03-26 15:15 ` Alexander Ivchenko 0 siblings, 1 reply; 16+ messages in thread From: Maxim Kuvyrkov @ 2013-01-15 4:13 UTC (permalink / raw) To: Alexander Ivchenko; +Cc: GCC Patches, Joseph S. Myers, H.J. Lu On 15/01/2013, at 4:55 AM, Alexander Ivchenko wrote: > thank you very much for your review! > > I fixed the arm build and all other issues that you raised. > > the patch is attached. Bootstrap and tested on x86-64 linux The patch is OK with the cleanups mentioned below (no need to resubmit for review). Unfortunately, you will have to wait for Stage 1 to commit your patch. > --- a/gcc/config.gcc > +++ b/gcc/config.gcc > @@ -636,6 +636,11 @@ case ${target} in > native_system_header_dir=/include > ;; > esac > + case $target in > + *linux*) > + tm_p_file="${tm_p_file} linux-protos.h" > + ;; > + esac > # glibc / uclibc / bionic switch. > # uclibc and bionic aren't usable for GNU/Hurd and neither for GNU/k*BSD. > case $target in Can we merge this above hunk into subsequent "case $target" statement ... > @@ -661,8 +666,10 @@ case ${target} in > # Add Android userspace support to Linux targets. > case $target in > *linux*) > + tmake_file="${tmake_file} t-linux-android" > tm_file="$tm_file linux-android.h" > extra_options="$extra_options linux-android.opt" > + extra_objs="$extra_objs linux-android.o" > ;; > esac ... here? > # Enable compilation for Android by default for *android* targets. > @@ -863,7 +870,9 @@ arm*-*-netbsdelf*) > tmake_file="${tmake_file} arm/t-arm" > ;; > arm*-*-linux-*) # ARM GNU/Linux with ELF > + tmake_file="${tmake_file} t-linux-android" Merge this with tmake_file= setting a couple of lines down. Put t-linux-android last on the line. > tm_file="dbxelf.h elfos.h gnu-user.h linux.h linux-android.h glibc-stdint.h arm/elf.h arm/linux-gas.h arm/linux-elf.h" > + extra_objs="$extra_objs linux-android.o" Please push extra_objs= setting a couple of lines down so that addition of t-linux-android and linux-android.o are side-by-side. > case $target in > arm*b-*-linux*) > tm_defines="${tm_defines} TARGET_BIG_ENDIAN_DEFAULT=1" > diff --git a/gcc/config/linux-android.c b/gcc/config/linux-android.c > new file mode 100644 > index 0000000..f3d82a5 > --- /dev/null > +++ b/gcc/config/linux-android.c > @@ -0,0 +1,34 @@ > +/* Functions for Linux Android as target machine for GNU C compiler. > + Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2010, 2011, > + 2012, 2013. Should be "Copyright (C) 2013." The copyright dates start with the year in which a file was added. Also, for any file that your changes touch please add 2013 to the list of copyright years. This is an annoying chore that committers have to do at the beginning of each year. > diff --git a/gcc/config/linux-protos.h b/gcc/config/linux-protos.h > new file mode 100644 > index 0000000..aae1d28 > --- /dev/null > +++ b/gcc/config/linux-protos.h > @@ -0,0 +1,22 @@ > +/* Prototypes. > + Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2010, 2011, > + 2012, 2013 "Copyright (C) 2013." > --- /dev/null > +++ b/gcc/config/t-linux-android > @@ -0,0 +1,23 @@ > +# Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2013 "Copyright (C) 2013." Thanks! -- Maxim Kuvyrkov ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH] Implementing ifunc target hook 2013-01-15 4:13 ` Maxim Kuvyrkov @ 2013-03-26 15:15 ` Alexander Ivchenko 2013-03-26 20:59 ` Maxim Kuvyrkov 0 siblings, 1 reply; 16+ messages in thread From: Alexander Ivchenko @ 2013-03-26 15:15 UTC (permalink / raw) To: Maxim Kuvyrkov, GCC Patches [-- Attachment #1: Type: text/plain, Size: 5017 bytes --] Hi, Since almost three months have passed I feel that I need to recheck the patch before commiting it. I fixed what Maxim mentioned and also I fixed: diff --git a/gcc/configure b/gcc/configure old mode 100755 new mode 100644 index eac96cd..928693a --- a/gcc/configure +++ b/gcc/configure @@ -22055,11 +22055,14 @@ else enable_gnu_indirect_function="$default_gnu_indirect_function" fi -if test x$enable_gnu_indirect_function = xyes; then -$as_echo "#define HAVE_GNU_INDIRECT_FUNCTION 1" >>confdefs.h +gif=`if test $enable_gnu_indirect_function == yes; then echo 1; else echo 0; fi` + +cat >>confdefs.h <<_ACEOF +#define HAVE_GNU_INDIRECT_FUNCTION $gif +_ACEOF + -fi if test $in_tree_ld != yes ; then ld_ver=`$gcc_cv_ld --version 2>/dev/null | sed 1q` diff --git a/gcc/configure.ac b/gcc/configure.ac index 40a1af7..51d334c 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -2299,10 +2299,11 @@ AC_ARG_ENABLE(gnu-indirect-function, Valid choices are 'yes' and 'no'.]) ;; esac], [enable_gnu_indirect_function="$default_gnu_indirect_function"]) -if test x$enable_gnu_indirect_function = xyes; then - AC_DEFINE(HAVE_GNU_INDIRECT_FUNCTION, 1, - [Define if your system supports gnu indirect functions.]) -fi + +gif=`if test $enable_gnu_indirect_function == yes; then echo 1; else echo 0; fi` +AC_DEFINE_UNQUOTED(HAVE_GNU_INDIRECT_FUNCTION, $gif, +[Define if your system supports gnu indirect functions.]) + HAVE_GNU_INDIRECT_FUNCTION was not defined on targets that don't have the support of IFUNC and the build of compiler could be broken. Now we define HAVE_GNU_INDIRECT_FUNCTION as 0 in those cases. ok for trunk? thanks, Alexander 2013/1/15 Maxim Kuvyrkov <maxim.kuvyrkov@gmail.com>: > On 15/01/2013, at 4:55 AM, Alexander Ivchenko wrote: > >> thank you very much for your review! >> >> I fixed the arm build and all other issues that you raised. >> >> the patch is attached. Bootstrap and tested on x86-64 linux > > > The patch is OK with the cleanups mentioned below (no need to resubmit for review). Unfortunately, you will have to wait for Stage 1 to commit your patch. > > >> --- a/gcc/config.gcc >> +++ b/gcc/config.gcc >> @@ -636,6 +636,11 @@ case ${target} in >> native_system_header_dir=/include >> ;; >> esac >> + case $target in >> + *linux*) >> + tm_p_file="${tm_p_file} linux-protos.h" >> + ;; >> + esac >> # glibc / uclibc / bionic switch. >> # uclibc and bionic aren't usable for GNU/Hurd and neither for GNU/k*BSD. >> case $target in > > Can we merge this above hunk into subsequent "case $target" statement ... > >> @@ -661,8 +666,10 @@ case ${target} in >> # Add Android userspace support to Linux targets. >> case $target in >> *linux*) >> + tmake_file="${tmake_file} t-linux-android" >> tm_file="$tm_file linux-android.h" >> extra_options="$extra_options linux-android.opt" >> + extra_objs="$extra_objs linux-android.o" >> ;; >> esac > > ... here? > >> # Enable compilation for Android by default for *android* targets. >> @@ -863,7 +870,9 @@ arm*-*-netbsdelf*) >> tmake_file="${tmake_file} arm/t-arm" >> ;; >> arm*-*-linux-*) # ARM GNU/Linux with ELF >> + tmake_file="${tmake_file} t-linux-android" > > Merge this with tmake_file= setting a couple of lines down. Put t-linux-android last on the line. > >> tm_file="dbxelf.h elfos.h gnu-user.h linux.h linux-android.h glibc-stdint.h arm/elf.h arm/linux-gas.h arm/linux-elf.h" >> + extra_objs="$extra_objs linux-android.o" > > Please push extra_objs= setting a couple of lines down so that addition of t-linux-android and linux-android.o are side-by-side. > >> case $target in >> arm*b-*-linux*) >> tm_defines="${tm_defines} TARGET_BIG_ENDIAN_DEFAULT=1" >> diff --git a/gcc/config/linux-android.c b/gcc/config/linux-android.c >> new file mode 100644 >> index 0000000..f3d82a5 >> --- /dev/null >> +++ b/gcc/config/linux-android.c >> @@ -0,0 +1,34 @@ >> +/* Functions for Linux Android as target machine for GNU C compiler. >> + Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2010, 2011, >> + 2012, 2013. > > Should be "Copyright (C) 2013." The copyright dates start with the year in which a file was added. > > Also, for any file that your changes touch please add 2013 to the list of copyright years. This is an annoying chore that committers have to do at the beginning of each year. > >> diff --git a/gcc/config/linux-protos.h b/gcc/config/linux-protos.h >> new file mode 100644 >> index 0000000..aae1d28 >> --- /dev/null >> +++ b/gcc/config/linux-protos.h >> @@ -0,0 +1,22 @@ >> +/* Prototypes. >> + Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2010, 2011, >> + 2012, 2013 > > "Copyright (C) 2013." > >> --- /dev/null >> +++ b/gcc/config/t-linux-android >> @@ -0,0 +1,23 @@ >> +# Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2013 > > "Copyright (C) 2013." > > Thanks! > > -- > Maxim Kuvyrkov > [-- Attachment #2: disable_ifunc_for_android.patch --] [-- Type: application/octet-stream, Size: 13946 bytes --] diff --git a/gcc/ChangeLog b/gcc/ChangeLog index dd61f15..c1ec26f 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,21 @@ +2013-03-26 Alexander Ivchenko <alexander.ivchenko@intel.com> + + * gcc/target.def (TARGET_HAS_IFUNC_P): New target hook. + * gcc/doc/tm.texi.in (TARGET_HAS_IFUNC_P): New. + * gcc/doc/tm.texi: Regenerate. + * gcc/targhooks.h (default_have_ifunc_p): New. + * gcc/targhooks.c (default_have_ifunc_p): Ditto. + * gcc/config/linux-protos.h: New file. + * gcc/config/linux-androids.h (TARGET_HAS_IFUNC_P): Using version of + this hook for linux which disables support of indirect functions in + android. + * gcc/config/linux-android.c: New file. + * gcc/config/t-linux-android.c: Ditto. + * gcc/config.gcc: Added new object file linux-android.o. + * gcc/config/i386/i386.c (ix86_get_function_versions_dispatcher): + Using TARGET_HAS_IFUNC hook instead of HAVE_GNU_INDIRECT_FUNCTION. + * gcc/varasm.c (do_assemble_alias): Likewise. + 2013-03-26 Richard Biener <rguenther@suse.de> * emit-rtl.c (set_mem_attributes_minus_bitpos): Remove diff --git a/gcc/config.gcc b/gcc/config.gcc index 1a0be50..11af65f 100644 --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -664,8 +664,11 @@ case ${target} in # Add Android userspace support to Linux targets. case $target in *linux*) + tm_p_file="${tm_p_file} linux-protos.h" + tmake_file="${tmake_file} t-linux-android" tm_file="$tm_file linux-android.h" extra_options="$extra_options linux-android.opt" + extra_objs="$extra_objs linux-android.o" ;; esac # Enable compilation for Android by default for *android* targets. @@ -875,8 +878,9 @@ arm*-*-linux-*) # ARM GNU/Linux with ELF tm_defines="${tm_defines} TARGET_BIG_ENDIAN_DEFAULT=1" ;; esac - tmake_file="${tmake_file} arm/t-arm arm/t-arm-elf arm/t-bpabi arm/t-linux-eabi" - tm_file="$tm_file arm/bpabi.h arm/linux-eabi.h arm/aout.h vxworks-dummy.h arm/arm.h" + tmake_file="${tmake_file} arm/t-arm arm/t-arm-elf arm/t-bpabi arm/t-linux-eabi t-linux-android" + tm_file="$tm_file arm/bpabi.h arm/linux-eabi.h arm/aout.h arm/arm.h" + extra_objs="$extra_objs linux-android.o" # Define multilib configuration for arm-linux-androideabi. case ${target} in *-androideabi) diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index b835c5d..030183c 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -29206,7 +29206,7 @@ make_name (tree decl, const char *suffix, bool make_unique) return global_var_name; } -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) /* Make a dispatcher declaration for the multi-versioned function DECL. Calls to DECL function will be replaced with calls to the dispatcher @@ -29277,12 +29277,6 @@ ix86_get_function_versions_dispatcher (void *decl) tree dispatch_decl = NULL; -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION - struct cgraph_function_version_info *it_v = NULL; - struct cgraph_node *dispatcher_node = NULL; - struct cgraph_function_version_info *dispatcher_version_info = NULL; -#endif - struct cgraph_function_version_info *default_version_info = NULL; gcc_assert (fn != NULL && DECL_FUNCTION_VERSIONED (fn)); @@ -29327,30 +29321,40 @@ ix86_get_function_versions_dispatcher (void *decl) default_node = default_version_info->this_node; -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION - /* Right now, the dispatching is done via ifunc. */ - dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); - - dispatcher_node = cgraph_get_create_node (dispatch_decl); - gcc_assert (dispatcher_node != NULL); - dispatcher_node->dispatcher_function = 1; - dispatcher_version_info - = insert_new_cgraph_node_version (dispatcher_node); - dispatcher_version_info->next = default_version_info; - dispatcher_node->local.finalized = 1; - - /* Set the dispatcher for all the versions. */ - it_v = default_version_info; - while (it_v != NULL) +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) + if (targetm.has_ifunc_p ()) { - it_v->dispatcher_resolver = dispatch_decl; - it_v = it_v->next; + struct cgraph_function_version_info *it_v = NULL; + struct cgraph_node *dispatcher_node = NULL; + struct cgraph_function_version_info *dispatcher_version_info = NULL; + + /* Right now, the dispatching is done via ifunc. */ + dispatch_decl = make_dispatcher_decl (default_node->symbol.decl); + + dispatcher_node = cgraph_get_create_node (dispatch_decl); + gcc_assert (dispatcher_node != NULL); + dispatcher_node->dispatcher_function = 1; + dispatcher_version_info + = insert_new_cgraph_node_version (dispatcher_node); + dispatcher_version_info->next = default_version_info; + dispatcher_node->local.finalized = 1; + + /* Set the dispatcher for all the versions. */ + it_v = default_version_info; + while (it_v != NULL) + { + it_v->dispatcher_resolver = dispatch_decl; + it_v = it_v->next; + } } -#else - error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), - "multiversioning needs ifunc which is not supported " - "in this configuration"); + else #endif + { + error_at (DECL_SOURCE_LOCATION (default_node->symbol.decl), + "multiversioning needs ifunc which is not supported " + "on this target"); + } + return dispatch_decl; } diff --git a/gcc/config/linux-android.c b/gcc/config/linux-android.c new file mode 100644 index 0000000..d6e47a7 --- /dev/null +++ b/gcc/config/linux-android.c @@ -0,0 +1,33 @@ +/* Functions for Linux Android as target machine for GNU C compiler. + Copyright (C) 2013. + Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +<http://www.gnu.org/licenses/>. */ + +#include "config.h" +#include "system.h" +#include "coretypes.h" +#include "tm.h" +#include "linux-protos.h" + +/* Android does not support GNU indirect functions. */ + +bool +linux_android_has_ifunc_p (void) +{ + return TARGET_ANDROID ? false : HAVE_GNU_INDIRECT_FUNCTION; +} diff --git a/gcc/config/linux-android.h b/gcc/config/linux-android.h index 2c87c84..831a19c 100644 --- a/gcc/config/linux-android.h +++ b/gcc/config/linux-android.h @@ -57,3 +57,6 @@ #define ANDROID_ENDFILE_SPEC \ "%{shared: crtend_so%O%s;: crtend_android%O%s}" + +#undef TARGET_HAS_IFUNC_P +#define TARGET_HAS_IFUNC_P linux_android_has_ifunc_p diff --git a/gcc/config/linux-protos.h b/gcc/config/linux-protos.h new file mode 100644 index 0000000..3f926e5 --- /dev/null +++ b/gcc/config/linux-protos.h @@ -0,0 +1,21 @@ +/* Prototypes. + Copyright (C) 2013. + Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +<http://www.gnu.org/licenses/>. */ + +extern bool linux_android_has_ifunc_p (void); diff --git a/gcc/config/t-linux-android b/gcc/config/t-linux-android new file mode 100644 index 0000000..6f9b033 --- /dev/null +++ b/gcc/config/t-linux-android @@ -0,0 +1,23 @@ +# Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2013 +# Free Software Foundation, Inc. +# +# This file is part of GCC. +# +# GCC is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3, or (at your option) +# any later version. +# +# GCC is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with GCC; see the file COPYING3. If not see +# <http://www.gnu.org/licenses/>. + +linux-android.o: $(srcdir)/config/linux-android.c $(CONFIG_H) $(SYSTEM_H) \ + coretypes.h $(TM_H) $(TM_P_H) + $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \ + $(srcdir)/config/linux-android.c diff --git a/gcc/configure b/gcc/configure old mode 100755 new mode 100644 index eac96cd..928693a --- a/gcc/configure +++ b/gcc/configure @@ -22055,11 +22055,14 @@ else enable_gnu_indirect_function="$default_gnu_indirect_function" fi -if test x$enable_gnu_indirect_function = xyes; then -$as_echo "#define HAVE_GNU_INDIRECT_FUNCTION 1" >>confdefs.h +gif=`if test $enable_gnu_indirect_function == yes; then echo 1; else echo 0; fi` + +cat >>confdefs.h <<_ACEOF +#define HAVE_GNU_INDIRECT_FUNCTION $gif +_ACEOF + -fi if test $in_tree_ld != yes ; then ld_ver=`$gcc_cv_ld --version 2>/dev/null | sed 1q` diff --git a/gcc/configure.ac b/gcc/configure.ac index 40a1af7..51d334c 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -2299,10 +2299,11 @@ AC_ARG_ENABLE(gnu-indirect-function, Valid choices are 'yes' and 'no'.]) ;; esac], [enable_gnu_indirect_function="$default_gnu_indirect_function"]) -if test x$enable_gnu_indirect_function = xyes; then - AC_DEFINE(HAVE_GNU_INDIRECT_FUNCTION, 1, - [Define if your system supports gnu indirect functions.]) -fi + +gif=`if test $enable_gnu_indirect_function == yes; then echo 1; else echo 0; fi` +AC_DEFINE_UNQUOTED(HAVE_GNU_INDIRECT_FUNCTION, $gif, +[Define if your system supports gnu indirect functions.]) + changequote(,)dnl if test $in_tree_ld != yes ; then diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index cbbc82d..9f78ae4 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -11341,3 +11341,9 @@ memory model bits are allowed. @deftypevr {Target Hook} {unsigned char} TARGET_ATOMIC_TEST_AND_SET_TRUEVAL This value should be set if the result written by @code{atomic_test_and_set} is not exactly 1, i.e. the @code{bool} @code{true}. @end deftypevr + +@deftypefn {Target Hook} bool TARGET_HAS_IFUNC_P (void) +It returns true if the target supports GNU indirect functions. +The support includes the assembler, linker and dynamic linker. +The default value of this hook is based on target's libc. +@end deftypefn diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index dfba947..b67df84 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -11177,3 +11177,5 @@ memory model bits are allowed. @end deftypefn @hook TARGET_ATOMIC_TEST_AND_SET_TRUEVAL + +@hook TARGET_HAS_IFUNC_P diff --git a/gcc/target.def b/gcc/target.def index 831cad8..3eba3b8 100644 --- a/gcc/target.def +++ b/gcc/target.def @@ -1518,6 +1518,15 @@ DEFHOOK bool, (const_rtx x), default_use_anchors_for_symbol_p) +/* True if target supports indirect functions. */ +DEFHOOK +(has_ifunc_p, + "It returns true if the target supports GNU indirect functions.\n\ +The support includes the assembler, linker and dynamic linker.\n\ +The default value of this hook is based on target's libc.", + bool, (void), + default_have_ifunc_p) + /* True if it is OK to do sibling call optimization for the specified call expression EXP. DECL will be the called function, or NULL if this is an indirect call. */ diff --git a/gcc/targhooks.c b/gcc/targhooks.c index 6c12a4a..e5ce81f 100644 --- a/gcc/targhooks.c +++ b/gcc/targhooks.c @@ -450,6 +450,14 @@ default_fixed_point_supported_p (void) return ENABLE_FIXED_POINT; } +/* True if the target supports GNU indirect functions. */ + +bool +default_have_ifunc_p (void) +{ + return HAVE_GNU_INDIRECT_FUNCTION; +} + /* NULL if INSN insn is valid within a low-overhead loop, otherwise returns an error message. diff --git a/gcc/targhooks.h b/gcc/targhooks.h index d23b352..4d11768 100644 --- a/gcc/targhooks.h +++ b/gcc/targhooks.h @@ -72,6 +72,8 @@ extern bool targhook_float_words_big_endian (void); extern bool default_decimal_float_supported_p (void); extern bool default_fixed_point_supported_p (void); +extern bool default_have_ifunc_p (void); + extern const char * default_invalid_within_doloop (const_rtx); extern tree default_builtin_vectorized_function (tree, tree, tree); diff --git a/gcc/varasm.c b/gcc/varasm.c index 6648103..2532d80 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -5489,14 +5489,15 @@ do_assemble_alias (tree decl, tree target) } if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))) { -#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) && HAVE_GNU_INDIRECT_FUNCTION - ASM_OUTPUT_TYPE_DIRECTIVE - (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), - IFUNC_ASM_TYPE); -#else - error_at (DECL_SOURCE_LOCATION (decl), - "ifunc is not supported in this configuration"); +#if defined (ASM_OUTPUT_TYPE_DIRECTIVE) + if (targetm.has_ifunc_p ()) + ASM_OUTPUT_TYPE_DIRECTIVE + (asm_out_file, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), + IFUNC_ASM_TYPE); + else #endif + error_at (DECL_SOURCE_LOCATION (decl), + "ifunc is not supported on this target"); } # ifdef ASM_OUTPUT_DEF_FROM_DECLS ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH] Implementing ifunc target hook 2013-03-26 15:15 ` Alexander Ivchenko @ 2013-03-26 20:59 ` Maxim Kuvyrkov 2013-03-27 9:56 ` Kirill Yukhin 0 siblings, 1 reply; 16+ messages in thread From: Maxim Kuvyrkov @ 2013-03-26 20:59 UTC (permalink / raw) To: Alexander Ivchenko; +Cc: GCC Patches On 27/03/2013, at 4:14 AM, Alexander Ivchenko wrote: > Hi, > > Since almost three months have passed I feel that I need to recheck the patch > before commiting it. I fixed what Maxim mentioned and also I fixed: The patch is OK with 2 changes: 1. s/default_have_ifunc_p/default_has_ifunc_p/ The new target hook is called "has_ifunc_p", so "has" in the name of its default implementation is more appropriate. > > diff --git a/gcc/configure b/gcc/configure > old mode 100755 > new mode 100644 > index eac96cd..928693a > --- a/gcc/configure > +++ b/gcc/configure > @@ -22055,11 +22055,14 @@ else > enable_gnu_indirect_function="$default_gnu_indirect_function" > fi > > -if test x$enable_gnu_indirect_function = xyes; then > > -$as_echo "#define HAVE_GNU_INDIRECT_FUNCTION 1" >>confdefs.h > +gif=`if test $enable_gnu_indirect_function == yes; then echo 1; else > echo 0; fi` 2. gif=`if test x$enable_gnu_indirect_function = xyes; then echo 1; else echo 0; fi` Note that canonical equality operator of 'test' is "=", not "==". The 'x' before the variable is a good practice to handle empty definitions of shell variables (`if test = yes;` will produce an error). Oh, and in the changelog you have a typo "linux-androids.h" -> "linux-android.h". Otherwise OK. Thanks, -- Maxim Kuvyrkov KugelWorks ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH] Implementing ifunc target hook 2013-03-26 20:59 ` Maxim Kuvyrkov @ 2013-03-27 9:56 ` Kirill Yukhin 2013-04-02 13:49 ` Jakub Jelinek 0 siblings, 1 reply; 16+ messages in thread From: Kirill Yukhin @ 2013-03-27 9:56 UTC (permalink / raw) To: Maxim Kuvyrkov; +Cc: Alexander Ivchenko, GCC Patches > > Otherwise OK. > > Thanks, Hi, chacked into trunk: http://gcc.gnu.org/ml/gcc-cvs/2013-03/msg00785.html Thanks, K ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH] Implementing ifunc target hook 2013-03-27 9:56 ` Kirill Yukhin @ 2013-04-02 13:49 ` Jakub Jelinek 2013-04-02 14:54 ` Alexander Ivchenko 0 siblings, 1 reply; 16+ messages in thread From: Jakub Jelinek @ 2013-04-02 13:49 UTC (permalink / raw) To: Kirill Yukhin; +Cc: Maxim Kuvyrkov, Alexander Ivchenko, GCC Patches On Wed, Mar 27, 2013 at 01:56:48PM +0400, Kirill Yukhin wrote: > > > > Otherwise OK. > > > > Thanks, > > Hi, chacked into trunk: http://gcc.gnu.org/ml/gcc-cvs/2013-03/msg00785.html This leads to: ../../gcc/config/t-linux-android:22: warning: overriding recipe for target `linux-android.o' ../../gcc/config/t-linux-android:22: warning: ignoring old recipe for target `linux-android.o' for arm*-linux* target (cross in my case). t-linux-android is listed twice. Jakub ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH] Implementing ifunc target hook 2013-04-02 13:49 ` Jakub Jelinek @ 2013-04-02 14:54 ` Alexander Ivchenko 2013-04-02 15:04 ` Jakub Jelinek 0 siblings, 1 reply; 16+ messages in thread From: Alexander Ivchenko @ 2013-04-02 14:54 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Kirill Yukhin, Maxim Kuvyrkov, GCC Patches Yep.. we missed that: t-linux-android was added here: # Add Android userspace support to Linux targets. case $target in *linux*) tm_p_file="${tm_p_file} linux-protos.h" tmake_file="${tmake_file} t-linux-android" tm_file="$tm_file linux-android.h" extra_options="$extra_options linux-android.opt" extra_objs="$extra_objs linux-android.o" ;; esac and here: arm*-*-linux-*) # ARM GNU/Linux with ELF tm_file="dbxelf.h elfos.h gnu-user.h linux.h linux-android.h glibc-stdint.h arm/elf.h arm/linux-gas.h arm/linux-elf.h" case $target in arm*b-*-linux*) tm_defines="${tm_defines} TARGET_BIG_ENDIAN_DEFAULT=1" ;; esac tmake_file="${tmake_file} arm/t-arm arm/t-arm-elf arm/t-bpabi arm/t-linux-eabi t-linux-android" tm_file="$tm_file arm/bpabi.h arm/linux-eabi.h arm/aout.h arm/arm.h" extra_objs="$extra_objs linux-android.o" # Define multilib configuration for arm-linux-androideabi. case ${target} in *-androideabi) tmake_file="$tmake_file arm/t-linux-androideabi" ;; esac I deleted the second encounter: diff --git a/gcc/ChangeLog b/gcc/ChangeLog index d18c6e9..0e1d5e4 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,7 @@ +2013-04-02 Alexander Ivchenko <alexander.ivchenko@intel.com> + + * config.gcc (arm*-*-linux-*): Remove duplicate t-linux-android. + 2013-04-02 Richard Biener <rguenther@suse.de> PR tree-optimization/56778 diff --git a/gcc/config.gcc b/gcc/config.gcc index e51db91..44ed190 100644 --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -878,7 +878,7 @@ arm*-*-linux-*) # ARM GNU/Linux with ELF tm_defines="${tm_defines} TARGET_BIG_ENDIAN_DEFAULT=1" ;; esac - tmake_file="${tmake_file} arm/t-arm arm/t-arm-elf arm/t-bpabi arm/t-linux-eabi t-linux-android" + tmake_file="${tmake_file} arm/t-arm arm/t-arm-elf arm/t-bpabi arm/t-linux-eabi" tm_file="$tm_file arm/bpabi.h arm/linux-eabi.h arm/aout.h arm/arm.h" extra_objs="$extra_objs linux-android.o" # Define multilib configuration for arm-linux-androideabi. is it ok? thanks, Alexander 2013/4/2 Jakub Jelinek <jakub@redhat.com>: > On Wed, Mar 27, 2013 at 01:56:48PM +0400, Kirill Yukhin wrote: >> > >> > Otherwise OK. >> > >> > Thanks, >> >> Hi, chacked into trunk: http://gcc.gnu.org/ml/gcc-cvs/2013-03/msg00785.html > > This leads to: > ../../gcc/config/t-linux-android:22: warning: overriding recipe for target `linux-android.o' > ../../gcc/config/t-linux-android:22: warning: ignoring old recipe for target `linux-android.o' > for arm*-linux* target (cross in my case). t-linux-android is listed twice. > > Jakub ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH] Implementing ifunc target hook 2013-04-02 14:54 ` Alexander Ivchenko @ 2013-04-02 15:04 ` Jakub Jelinek 2013-04-02 15:06 ` Kirill Yukhin 0 siblings, 1 reply; 16+ messages in thread From: Jakub Jelinek @ 2013-04-02 15:04 UTC (permalink / raw) To: Alexander Ivchenko; +Cc: Kirill Yukhin, Maxim Kuvyrkov, GCC Patches On Tue, Apr 02, 2013 at 06:24:06PM +0400, Alexander Ivchenko wrote: > --- a/gcc/ChangeLog > +++ b/gcc/ChangeLog > @@ -1,3 +1,7 @@ > +2013-04-02 Alexander Ivchenko <alexander.ivchenko@intel.com> > + > + * config.gcc (arm*-*-linux-*): Remove duplicate t-linux-android. > + > 2013-04-02 Richard Biener <rguenther@suse.de> > > PR tree-optimization/56778 > diff --git a/gcc/config.gcc b/gcc/config.gcc > index e51db91..44ed190 100644 > --- a/gcc/config.gcc > +++ b/gcc/config.gcc > @@ -878,7 +878,7 @@ arm*-*-linux-*) # ARM GNU/Linux with ELF > tm_defines="${tm_defines} TARGET_BIG_ENDIAN_DEFAULT=1" > ;; > esac > - tmake_file="${tmake_file} arm/t-arm arm/t-arm-elf arm/t-bpabi > arm/t-linux-eabi t-linux-android" > + tmake_file="${tmake_file} arm/t-arm arm/t-arm-elf arm/t-bpabi > arm/t-linux-eabi" > tm_file="$tm_file arm/bpabi.h arm/linux-eabi.h arm/aout.h arm/arm.h" > extra_objs="$extra_objs linux-android.o" > # Define multilib configuration for arm-linux-androideabi. > > > is it ok? Yes. Jakub ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH] Implementing ifunc target hook 2013-04-02 15:04 ` Jakub Jelinek @ 2013-04-02 15:06 ` Kirill Yukhin 0 siblings, 0 replies; 16+ messages in thread From: Kirill Yukhin @ 2013-04-02 15:06 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Alexander Ivchenko, Maxim Kuvyrkov, GCC Patches Hi, >> is it ok? > > Yes. Checked into trunk: http://gcc.gnu.org/ml/gcc-cvs/2013-04/msg00066.html Thanks, K ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2013-04-02 14:33 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-12-26 12:15 [RFC PATCH] Implementing ifunc target hook Alexander Ivchenko 2012-12-26 17:06 ` Joseph S. Myers 2012-12-27 22:59 ` Maxim Kuvyrkov 2012-12-28 12:30 ` Alexander Ivchenko 2013-01-02 23:08 ` Maxim Kuvyrkov 2013-01-09 11:24 ` Alexander Ivchenko 2013-01-10 23:24 ` Maxim Kuvyrkov 2013-01-14 15:55 ` Alexander Ivchenko 2013-01-15 4:13 ` Maxim Kuvyrkov 2013-03-26 15:15 ` Alexander Ivchenko 2013-03-26 20:59 ` Maxim Kuvyrkov 2013-03-27 9:56 ` Kirill Yukhin 2013-04-02 13:49 ` Jakub Jelinek 2013-04-02 14:54 ` Alexander Ivchenko 2013-04-02 15:04 ` Jakub Jelinek 2013-04-02 15:06 ` Kirill Yukhin
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).