Re: [PATCH] Attribute noclone

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

2009/7/17 Martin Jambor <mjambor@suse.cz>:
> Hi,
>
> since the ?attribute noinline does not prevent ?ipa-cp cloning from
> taking place, there ?has been a need to provide ?a new attribute to
> do just that. ? The following patch introduces it, ?adds a testcase
> to verify ?it works, amends ?a testcase which ?will need it ?in the
> future and documents it.
>
> Bootstrapped and tested on x86_64-linux and with make pdf.
>
> OK for trunk?

Hm.  Why not simply make the noinline attribute prevent cloning as well?
I think that is what code using the noinline attribute would expect
(the body shouldn't be duplicated).

Thanks,
Richard.

> Thanks,
>
> Martin
>
> 2009-07-17 ?Martin Jambor ?<mjambor@suse.cz>
>
> ? ? ? ?* c-common.c (c_common_attribute_table): New element for noclone.
> ? ? ? ?(handle_noclone_attribute): New function. Forward-declare.
> ? ? ? ?* ipa-cp.c (ipcp_initialize_node_lattices): Query for noclone attribute.
> ? ? ? ?* doc/extend.texi (Labels as Values): Document need for noclone.
> ? ? ? ?(Function Attributes): Document noclone attribute.
> ? ? ? ?(Return Address): Mention potential need for the caller a function
> ? ? ? ?using __builtin_return_address to have noinline and noclone
> ? ? ? ?attributes.
>
> ? ? ? ?* testsuite/gcc.c-torture/execute/pr17377.c: Add noclone attribute to
> ? ? ? ?function y.
> ? ? ? ?* testsuite/gcc.dg/ipa/noclone-1.c: New test.
>
>
> Index: icln/gcc/c-common.c
> ===================================================================
> --- icln.orig/gcc/c-common.c
> +++ icln/gcc/c-common.c
> @@ -482,6 +482,7 @@ static tree handle_noreturn_attribute (t
> ?static tree handle_hot_attribute (tree *, tree, tree, int, bool *);
> ?static tree handle_cold_attribute (tree *, tree, tree, int, bool *);
> ?static tree handle_noinline_attribute (tree *, tree, tree, int, bool *);
> +static tree handle_noclone_attribute (tree *, tree, tree, int, bool *);
> ?static tree handle_always_inline_attribute (tree *, tree, tree, int,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?bool *);
> ?static tree handle_gnu_inline_attribute (tree *, tree, tree, int, bool *);
> @@ -731,6 +732,8 @@ const struct attribute_spec c_common_att
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?handle_noreturn_attribute },
> ? { "noinline", ? ? ? ? ? ? ? 0, 0, true, ?false, false,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?handle_noinline_attribute },
> + ?{ "noclone", ? ? ? ? ? ? ? ?0, 0, true, ?false, false,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? handle_noclone_attribute },
> ? { "always_inline", ? ? ? ? ?0, 0, true, ?false, false,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?handle_always_inline_attribute },
> ? { "gnu_inline", ? ? ? ? ? ? 0, 0, true, ?false, false,
> @@ -5903,6 +5906,23 @@ handle_noinline_attribute (tree *node, t
> ? ? {
> ? ? ? warning (OPT_Wattributes, "%qE attribute ignored", name);
> ? ? ? *no_add_attrs = true;
> + ? ?}
> +
> + ?return NULL_TREE;
> +}
> +
> +/* Handle a "noclone" attribute; arguments as in
> + ? struct attribute_spec.handler. ?*/
> +
> +static tree
> +handle_noclone_attribute (tree *node, tree name,
> + ? ? ? ? ? ? ? ? ? ? ? ? tree ARG_UNUSED (args),
> + ? ? ? ? ? ? ? ? ? ? ? ? int ARG_UNUSED (flags), bool *no_add_attrs)
> +{
> + ?if (TREE_CODE (*node) != FUNCTION_DECL)
> + ? ?{
> + ? ? ?warning (OPT_Wattributes, "%qE attribute ignored", name);
> + ? ? ?*no_add_attrs = true;
> ? ? }
>
> ? return NULL_TREE;
> Index: icln/gcc/ipa-cp.c
> ===================================================================
> --- icln.orig/gcc/ipa-cp.c
> +++ icln/gcc/ipa-cp.c
> @@ -495,7 +495,8 @@ ipcp_initialize_node_lattices (struct cg
> ? struct ipa_node_params *info = IPA_NODE_REF (node);
> ? enum ipa_lattice_type type;
>
> - ?if (ipa_is_called_with_var_arguments (info))
> + ?if (ipa_is_called_with_var_arguments (info)
> + ? ? ?|| lookup_attribute ("noclone", DECL_ATTRIBUTES (node->decl)))
> ? ? type = IPA_BOTTOM;
> ? else if (!node->needed)
> ? ? type = IPA_TOP;
> Index: icln/gcc/doc/extend.texi
> ===================================================================
> --- icln.orig/gcc/doc/extend.texi
> +++ icln/gcc/doc/extend.texi
> @@ -372,11 +372,12 @@ This is more friendly to code living in
> ?the number of dynamic relocations that are needed, and by consequence,
> ?allows the data to be read-only.
>
> -The @code{&&foo} expressions for the same label might have different values
> -if the containing function is inlined or cloned. ?If a program relies on
> -them being always the same, @code{__attribute__((__noinline__))} should
> -be used to prevent inlining. ?If @code{&&foo} is used
> -in a static variable initializer, inlining is forbidden.
> +The @code{&&foo} expressions for the same label might have different
> +values if the containing function is inlined or cloned. ?If a program
> +relies on them being always the same,
> +@code{__attribute__((__noinline__,__noclone__))} should be used to
> +prevent inlining and cloning. ?If @code{&&foo} is used in a static
> +variable initializer, inlining and cloning is forbidden.
>
> ?@node Nested Functions
> ?@section Nested Functions
> @@ -1882,19 +1883,18 @@ attributes when making a declaration. ?T
> ?attribute specification inside double parentheses. ?The following
> ?attributes are currently defined for functions on all targets:
> ?@code{aligned}, @code{alloc_size}, @code{noreturn},
> -@code{returns_twice}, @code{noinline}, @code{always_inline},
> -@code{flatten}, @code{pure}, @code{const}, @code{nothrow},
> -@code{sentinel}, @code{format}, @code{format_arg},
> +@code{returns_twice}, @code{noinline}, @code{noclone},
> +@code{always_inline}, @code{flatten}, @code{pure}, @code{const},
> +@code{nothrow}, @code{sentinel}, @code{format}, @code{format_arg},
> ?@code{no_instrument_function}, @code{section}, @code{constructor},
> ?@code{destructor}, @code{used}, @code{unused}, @code{deprecated},
> ?@code{weak}, @code{malloc}, @code{alias}, @code{warn_unused_result},
> ?@code{nonnull}, @code{gnu_inline}, @code{externally_visible},
> -@code{hot}, @code{cold}, @code{artificial}, @code{error}
> -and @code{warning}.
> -Several other attributes are defined for functions on particular
> -target systems. ?Other attributes, including @code{section} are
> -supported for variables declarations (@pxref{Variable Attributes}) and
> -for types (@pxref{Type Attributes}).
> +@code{hot}, @code{cold}, @code{artificial}, @code{error} and
> +@code{warning}. ?Several other attributes are defined for functions on
> +particular target systems. ?Other attributes, including @code{section}
> +are supported for variables declarations (@pxref{Variable Attributes})
> +and for types (@pxref{Type Attributes}).
>
> ?You may also specify attributes with @samp{__} preceding and following
> ?each keyword. ?This allows you to use them in header files without
> @@ -2719,6 +2719,13 @@ asm ("");
> ?(@pxref{Extended Asm}) in the called function, to serve as a special
> ?side-effect.
>
> +@item noclone
> +@cindex @code{noclone} function attribute
> +This function attribute prevents a function from being considered for
> +cloning - a mechanism which produces specialized copies of functions
> +and which is (currently) performed by interprocedural constant
> +propagation.
> +
> ?@item nonnull (@var{arg-index}, @dots{})
> ?@cindex @code{nonnull} function attribute
> ?The @code{nonnull} attribute specifies that some function parameters should
> @@ -5799,14 +5806,18 @@ These functions may be used to get infor
> ?function.
>
> ?@deftypefn {Built-in Function} {void *} __builtin_return_address (unsigned int @var{level})
> -This function returns the return address of the current function, or of
> -one of its callers. ?The @var{level} argument is number of frames to
> -scan up the call stack. ?A value of @code{0} yields the return address
> -of the current function, a value of @code{1} yields the return address
> -of the caller of the current function, and so forth. ?When inlining
> -the expected behavior is that the function will return the address of
> -the function that will be returned to. ?To work around this behavior use
> -the @code{noinline} function attribute.
> +This function returns the return address of the current function, or
> +of one of its callers. ?The @var{level} argument is number of frames
> +to scan up the call stack. ?A value of @code{0} yields the return
> +address of the current function, a value of @code{1} yields the return
> +address of the caller of the current function, and so forth. ?When
> +inlining the expected behavior is that the function will return the
> +address of the function that will be returned to. ?To work around this
> +behavior use the @code{noinline} function attribute. ?When a caller of
> +a function calling this built-in has been inlined or cloned, the
> +address might differ even when this is not apprent from the source
> +code. ?To prevent this from happening, use both the @code{noinline}
> +and @code{noclone} function attributes on the caller.
>
> ?The @var{level} argument must be a constant integer.
>
> Index: icln/gcc/testsuite/gcc.c-torture/execute/pr17377.c
> ===================================================================
> --- icln.orig/gcc/testsuite/gcc.c-torture/execute/pr17377.c
> +++ icln/gcc/testsuite/gcc.c-torture/execute/pr17377.c
> @@ -26,7 +26,7 @@ f (int i)
>
> ?int x;
>
> -void *y (int i) __attribute__ ((__noinline__));
> +void *y (int i) __attribute__ ((__noinline__,__noclone__));
> ?void *
> ?y (int i)
> ?{
> Index: icln/gcc/testsuite/gcc.dg/ipa/noclone-1.c
> ===================================================================
> --- /dev/null
> +++ icln/gcc/testsuite/gcc.dg/ipa/noclone-1.c
> @@ -0,0 +1,29 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -fipa-cp -fipa-cp-clone -fdump-ipa-cp -fno-early-inlining" ?} */
> +
> +int global_1, global_2;
> +
> +__attribute__((__noclone__)) int g (int b, int c)
> + {
> + ?global_1 = b;
> + ?global_2 = c;
> +}
> +
> +__attribute__((__noclone__)) int f (int a)
> +{
> + ?/* Second parameter of g gets different values. ?*/
> + ?if (a > 0)
> + ? ?g (a, 3);
> + ?else
> + ? ?g (a, 5);
> +}
> +
> +int main ()
> +{
> + ?f (7);
> + ?return 0;
> +}
> +
> +
> +/* { dg-final { scan-ipa-dump-times "versioned function" 0 "cp" ?} } */
> +/* { dg-final { cleanup-ipa-dump "cp" } } */
>
>