Re: PATCH: Add ifunc attribute

[Richard Guenther <richard dot guenther at gmail dot com>]

On Fri, Jun 12, 2009 at 5:40 PM, H.J. Lu<hongjiu.lu@intel.com> wrote:
> Hi,
>
> This patch adds ifunc attribute for STT_GNU_IFUNC as specified in
> ifunc.txt at
>
> http://groups.google.com/group/generic-abi/files
>
> Any comments?

Why does the attribute have an argument?  I see that you get around
all the issues of wrong optimizations when seeing the function body,
but at least the documentation needs to be more explicit about
this subtle difference.

Richard.

> Thanks.
>
>
> H.J.
> ---
> gcc/
>
> 2009-06-12 ?H.J. Lu ?<hongjiu.lu@intel.com>
>
> ? ? ? ?* c-common.c (handle_ifunc_attribute): New.
> ? ? ? ?(c_common_attribute_table): Add "ifunc".
>
> ? ? ? ?* tree.c (decl_assembler_name): Handle ifunc attribute.
>
> ? ? ? ?* config/elfos.h (ASM_DECLARE_FUNCTION_NAME): Output
> ? ? ? ?"gnu_indirect_function" instead of "function" for ifunc
> ? ? ? ?attribute.
>
> ? ? ? ?* doc/extend.texi: Document ifunc attribute.
>
> gcc/testsuite/
>
> 2009-06-12 ?H.J. Lu ?<hongjiu.lu@intel.com>
>
> ? ? ? ?* gcc.dg/ifunc-1.c: New.
>
> --- gcc/c-common.c.ifunc ? ? ? ?2009-06-10 02:58:33.000000000 -0700
> +++ gcc/c-common.c ? ? ?2009-06-12 08:21:01.000000000 -0700
> @@ -529,6 +529,7 @@ static tree handle_type_generic_attribut
> ?static tree handle_alloc_size_attribute (tree *, tree, tree, int, bool *);
> ?static tree handle_target_attribute (tree *, tree, tree, int, bool *);
> ?static tree handle_optimize_attribute (tree *, tree, tree, int, bool *);
> +static tree handle_ifunc_attribute (tree *, tree, tree, int, bool *);
>
> ?static void check_function_nonnull (tree, int, tree *);
> ?static void check_nonnull_arg (void *, tree, unsigned HOST_WIDE_INT);
> @@ -817,6 +818,8 @@ const struct attribute_spec c_common_att
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?handle_target_attribute },
> ? { "optimize", ? ? ? ? ? ? ? 1, -1, true, false, false,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?handle_optimize_attribute },
> + ?{ "ifunc", ? ? ? ? ? ? ? ? ?1, 1, true, ?false, false,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? handle_ifunc_attribute },
> ? { NULL, ? ? ? ? ? ? ? ? ? ? 0, 0, false, false, false, NULL }
> ?};
>
> @@ -6659,6 +6662,53 @@ handle_alias_attribute (tree *node, tree
> ? return NULL_TREE;
> ?}
>
> +/* Handle an "ifunc" attribute; arguments as in
> + ? struct attribute_spec.handler. ?*/
> +
> +static tree
> +handle_ifunc_attribute (tree *node, tree name, tree args,
> + ? ? ? ? ? ? ? ? ? ? ? int ARG_UNUSED (flags), bool *no_add_attrs)
> +{
> + ?tree decl = *node;
> +
> + ?if (TREE_CODE (decl) != FUNCTION_DECL)
> + ? ?{
> + ? ? ?warning (OPT_Wattributes, "%qE attribute ignored", name);
> + ? ? ?*no_add_attrs = true;
> + ? ?}
> + ?else if (!TREE_PUBLIC (decl))
> + ? ?{
> + ? ? ?error ("static indirec function %q+D", decl);
> + ? ? ?*no_add_attrs = true;
> + ? ?}
> +
> + ?/* Note that the very first time we process a nested declaration,
> + ? ? decl_function_context will not be set. ?Indeed, *would* never
> + ? ? be set except for the DECL_INITIAL/DECL_EXTERNAL frobbery that
> + ? ? we do below. ?After such frobbery, pushdecl would set the context.
> + ? ? In any case, this is never what we want. ?*/
> + ?else if (decl_function_context (decl) == 0
> + ? ? ? ? ?&& current_function_decl == NULL)
> + ? ?{
> + ? ? ?if (TREE_CODE (TREE_VALUE (args)) != STRING_CST)
> + ? ? ? {
> + ? ? ? ? error ("ifunc argument not a string");
> + ? ? ? ? *no_add_attrs = true;
> + ? ? ? ? return NULL_TREE;
> + ? ? ? }
> +
> + ? ? ?/* Mark it as used. ?*/
> + ? ? ?TREE_USED (decl) = 1;
> + ? ?}
> + ?else
> + ? ?{
> + ? ? ?warning (OPT_Wattributes, "%qE attribute ignored", name);
> + ? ? ?*no_add_attrs = true;
> + ? ?}
> +
> + ?return NULL_TREE;
> +}
> +
> ?/* Handle a "weakref" attribute; arguments as in struct
> ? ?attribute_spec.handler. ?*/
>
> --- gcc/config/elfos.h.ifunc ? ?2009-04-13 16:12:15.000000000 -0700
> +++ gcc/config/elfos.h ?2009-06-10 15:42:03.000000000 -0700
> @@ -280,7 +280,11 @@ see the files COPYING3 and COPYING.RUNTI
> ?#define ASM_DECLARE_FUNCTION_NAME(FILE, NAME, DECL) ? ? ? ? ? ?\
> ? do ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ? ? { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> - ? ? ?ASM_OUTPUT_TYPE_DIRECTIVE (FILE, NAME, "function"); ? ? ?\
> + ? ? ?if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (DECL))) ?\
> + ? ? ? ASM_OUTPUT_TYPE_DIRECTIVE (FILE, NAME, ? ? ? ? ? ? ? ? ?\
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"gnu_indirect_function"); ? ?\
> + ? ? ?else ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? ASM_OUTPUT_TYPE_DIRECTIVE (FILE, NAME, "function"); ? ? \
> ? ? ? ASM_DECLARE_RESULT (FILE, DECL_RESULT (DECL)); ? ? ? ? ? \
> ? ? ? ASM_OUTPUT_LABEL (FILE, NAME); ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ? ? } ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> --- gcc/doc/extend.texi.ifunc ? 2009-06-10 02:58:10.000000000 -0700
> +++ gcc/doc/extend.texi 2009-06-12 08:18:03.000000000 -0700
> @@ -1874,6 +1874,25 @@ is not defined in the same translation u
>
> ?Not all target machines support this attribute.
>
> +@item ifunc ("@var{function}")
> +@cindex @code{ifunc} attribute
> +The @code{ifunc} attribute only applies to global function definition,
> +which causes the definition to be emitted as an indirec function. ?For
> +instance,
> +
> +@smallexample
> +void* f_ifunc () __attribute__ ((ifunc ("f")));
> +@end smallexample
> +
> +defines @samp{f_ifunc} to be an indirect function for @samp{f}. This
> +attribute is ignored if @samp{f_ifunc} is undefined in the same
> +translation unit.
> +
> +See @code{STT_GNU_IFUNC} specified in @file{ifunc.txt} at
> +@uref{http://groups.google.com/group/generic-abi/files}.
> +
> +Not all target machines support this attribute.
> +
> ?@item aligned (@var{alignment})
> ?@cindex @code{aligned} attribute
> ?This attribute specifies a minimum alignment for the function,
> --- gcc/testsuite/gcc.dg/ifunc-1.c.ifunc ? ? ? ?2009-06-10 16:03:27.000000000 -0700
> +++ gcc/testsuite/gcc.dg/ifunc-1.c ? ? ?2009-06-10 16:13:36.000000000 -0700
> @@ -0,0 +1,18 @@
> +/* { dg-do compile { target i?86-*-linux* x86_64-*-linux* } } */
> +/* { dg-options "-w" } */
> +
> +static int
> +zero (int x)
> +{
> + ?return 0;
> +}
> +
> +void *
> +__attribute__ ((ifunc ("foo")))
> +foo_ifunc (void)
> +{
> + ?return zero;
> +}
> +
> +/* { dg-final { scan-assembler-not "foo_ifunc" } } */
> +/* { dg-final { scan-assembler ".type\[ ? ? ? ?\]\+foo, .gnu_indirect_function" } } */
> --- gcc/tree.c.ifunc ? ?2009-06-10 15:31:10.000000000 -0700
> +++ gcc/tree.c ?2009-06-12 08:35:44.000000000 -0700
> @@ -347,7 +347,19 @@ tree
> ?decl_assembler_name (tree decl)
> ?{
> ? if (!DECL_ASSEMBLER_NAME_SET_P (decl))
> - ? ?lang_hooks.set_decl_assembler_name (decl);
> + ? ?{
> + ? ? ?tree attr;
> +
> + ? ? ?if (TREE_CODE (decl) == FUNCTION_DECL
> + ? ? ? ? && (attr = lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))))
> + ? ? ? {
> + ? ? ? ? attr = TREE_VALUE (TREE_VALUE (attr));
> + ? ? ? ? attr = get_identifier (TREE_STRING_POINTER (attr));
> + ? ? ? ? SET_DECL_ASSEMBLER_NAME (decl, attr);
> + ? ? ? }
> + ? ? ?else
> + ? ? ? lang_hooks.set_decl_assembler_name (decl);
> + ? ?}
> ? return DECL_WITH_VIS_CHECK (decl)->decl_with_vis.assembler_name;
> ?}
>
>