Re: [PATCH] Reject target_clones attribute in functions that can't be cloned (PR middle-end/84723)

[Richard Biener <rguenther at suse dot de>]

On Tue, 6 Mar 2018, Jakub Jelinek wrote:

> Hi!
> 
> The following testcases show various reasons why a function definition
> with target_clones attribute can't be cloned; instead of ICEing because
> node->create_version_clone_with_body returns NULL and we dereference it,
> this patch just diagnoses those and ignores the attribute.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Richard.

> Martin Sebor said he wants to work on attribute exclusion, in that case
> the error messages in pr84723-{1,4,5}.c would be adjusted if needed and
>       const char *reason = NULL;
>       if (lookup_attribute ("noclone", DECL_ATTRIBUTES (node->decl)))
> 	reason = G_("function %q+F can never be copied "
> 		    "because it has %<noclone%> attribute");
>       else
> 	reason = copy_forbidden (DECL_STRUCT_FUNCTION (node->decl));
> could be replaced just with
>       const char *reason = copy_forbidden (DECL_STRUCT_FUNCTION (node->decl));
> but attribute exclusions can't really help for the cases where cloning
> is refused because of other reasons.
> 
> 2018-03-06  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/84723
> 	* multiple_target.c: Include tree-inline.h and intl.h.
> 	(expand_target_clones): Diagnose and fail if node->definition and
> 	!tree_versionable_function_p (node->decl).
> 
> 	* gcc.target/i386/pr84723-1.c: New test.
> 	* gcc.target/i386/pr84723-2.c: New test.
> 	* gcc.target/i386/pr84723-3.c: New test.
> 	* gcc.target/i386/pr84723-4.c: New test.
> 	* gcc.target/i386/pr84723-5.c: New test.
> 
> --- gcc/multiple_target.c.jj	2018-01-03 10:19:54.956533925 +0100
> +++ gcc/multiple_target.c	2018-03-06 11:46:09.327627941 +0100
> @@ -36,6 +36,8 @@ along with GCC; see the file COPYING3.
>  #include "pretty-print.h"
>  #include "gimple-iterator.h"
>  #include "gimple-walk.h"
> +#include "tree-inline.h"
> +#include "intl.h"
>  
>  /* Walker callback that replaces all FUNCTION_DECL of a function that's
>     going to be versioned.  */
> @@ -312,6 +314,22 @@ expand_target_clones (struct cgraph_node
>        return false;
>      }
>  
> +  if (node->definition
> +      && !tree_versionable_function_p (node->decl))
> +    {
> +      error_at (DECL_SOURCE_LOCATION (node->decl),
> +		"clones for %<target_clones%> attribute cannot be created");
> +      const char *reason = NULL;
> +      if (lookup_attribute ("noclone", DECL_ATTRIBUTES (node->decl)))
> +	reason = G_("function %q+F can never be copied "
> +		    "because it has %<noclone%> attribute");
> +      else
> +	reason = copy_forbidden (DECL_STRUCT_FUNCTION (node->decl));
> +      if (reason)
> +	inform (DECL_SOURCE_LOCATION (node->decl), reason, node->decl);
> +      return false;
> +    }
> +
>    char *attr_str = XNEWVEC (char, attr_len);
>    int attrnum = get_attr_str (arglist, attr_str);
>    char **attrs = XNEWVEC (char *, attrnum);
> --- gcc/testsuite/gcc.target/i386/pr84723-1.c.jj	2018-03-06 12:00:17.533367738 +0100
> +++ gcc/testsuite/gcc.target/i386/pr84723-1.c	2018-03-06 11:52:15.831511845 +0100
> @@ -0,0 +1,11 @@
> +/* PR middle-end/84723 */
> +/* { dg-do compile } */
> +/* { dg-require-ifunc } */
> +/* { dg-options "-O2" } */
> +
> +__attribute__((target_clones ("avx", "default")))
> +__attribute__((noclone))
> +void
> +foo (void)	/* { dg-error "clones for .target_clones. attribute cannot be created" } */
> +{		/* { dg-message "function .foo. can never be copied because it has .noclone. attribute" "" { target *-*-* } .-1 } */
> +}
> --- gcc/testsuite/gcc.target/i386/pr84723-2.c.jj	2018-03-06 12:00:21.019367631 +0100
> +++ gcc/testsuite/gcc.target/i386/pr84723-2.c	2018-03-06 11:50:30.458545226 +0100
> @@ -0,0 +1,13 @@
> +/* PR middle-end/84723 */
> +/* { dg-do compile } */
> +/* { dg-require-ifunc } */
> +/* { dg-options "-O2" } */
> +
> +__attribute__((target_clones ("avx", "default")))
> +void
> +foo (void)	/* { dg-error "clones for .target_clones. attribute cannot be created" } */
> +{		/* { dg-message "function .foo. can never be copied because it saves address of local label in a static variable" "" { target *-*-* } .-1 } */
> +  static void *p = &&lab;
> +  asm volatile ("" : "+m" (p) : : "memory");
> +lab:;
> +}
> --- gcc/testsuite/gcc.target/i386/pr84723-3.c.jj	2018-03-06 12:00:24.397367531 +0100
> +++ gcc/testsuite/gcc.target/i386/pr84723-3.c	2018-03-06 11:50:57.274536736 +0100
> @@ -0,0 +1,17 @@
> +/* PR middle-end/84723 */
> +/* { dg-do compile } */
> +/* { dg-require-ifunc } */
> +/* { dg-options "-O2" } */
> +
> +__attribute__((target_clones ("avx", "default")))
> +int
> +foo (int x)	/* { dg-error "clones for .target_clones. attribute cannot be created" } */
> +{		/* { dg-message "function .foo. can never be copied because it receives a non-local goto" "" { target *-*-* } .-1 } */
> +  __label__ lab;
> +  __attribute__((noinline)) void bar () { goto lab; }
> +  if (x == 5)
> +    bar ();
> +  x++;
> +lab:;
> +  return x;
> +}
> --- gcc/testsuite/gcc.target/i386/pr84723-4.c.jj	2018-03-06 12:00:17.533367738 +0100
> +++ gcc/testsuite/gcc.target/i386/pr84723-4.c	2018-03-06 11:52:15.831511845 +0100
> @@ -0,0 +1,11 @@
> +/* PR middle-end/84723 */
> +/* { dg-do compile } */
> +/* { dg-require-ifunc } */
> +/* { dg-options "-O2" } */
> +
> +__attribute__((target_clones ("avx", "default")))
> +__attribute__((naked))
> +void
> +foo (void)	/* { dg-error "clones for .target_clones. attribute cannot be created" } */
> +{		/* { dg-message "function .foo. can never be copied because it has .noclone. attribute" "" { target *-*-* } .-1 } */
> +}
> --- gcc/testsuite/gcc.target/i386/pr84723-5.c.jj	2018-03-06 12:00:17.533367738 +0100
> +++ gcc/testsuite/gcc.target/i386/pr84723-5.c	2018-03-06 11:52:15.831511845 +0100
> @@ -0,0 +1,11 @@
> +/* PR middle-end/84723 */
> +/* { dg-do compile } */
> +/* { dg-require-ifunc } */
> +/* { dg-options "-O2" } */
> +
> +__attribute__((target_clones ("avx", "default")))
> +__attribute__((noipa))
> +void
> +foo (void)	/* { dg-error "clones for .target_clones. attribute cannot be created" } */
> +{		/* { dg-message "function .foo. can never be copied because it has .noclone. attribute" "" { target *-*-* } .-1 } */
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)