Re: [PATCH, VECTOR ABI] Add __attribute__((__simd__)) to GCC.

[Kirill Yukhin <kirill dot yukhin at gmail dot com>]

Hello Jakub,
I've fixed all long lines, thanks!
I've also fixed max_len for "simd" attribute.

Tests are fixed w/ scan for SIMD-mangled routines, routines
made `extern'.

ChangeLog entry was updated.
gcc/
        * omp-low.c (pass_omp_simd_clone::gate): If target allows - call
        without additional conditions.
	* doc/extend.texi (@item simd): New.
gcc/c-family/
	* c-common.c (handle_simd_attribute): New.
	(struct attribute_spec): Add entry for "simd".
	(handle_simd_attribute): New
gcc/c/
	* c-parser.c (c_finish_omp_declare_simd): Look for
	"simd" attribute as well. Update error message.
gcc/cp/
	* parser.c (cp_parser_late_parsing_cilk_simd_fn_info): Look for
	"simd" attribute as well. Update error message.
gcc/testsuite/
        * c-c++-common/attr-simd.c: New test.
        * c-c++-common/attr-simd-2.c: New test.
        * c-c++-common/attr-simd-3.c: New test.

Bootstrapped and regtested.

If no more objections - I'll check it into main trunk next ww.

> Are you going to update VectorABI.txt based on the latest changes in
> the Intel ABI document?  I mean especially using L, R or U for linear
> %val(), %ref() or %uval() (if references), not using s for linear with
> uniform parameter stride, but instead using ls, Ls, Rs or Us for those?

Yes, we'll update it.

--
Thanks, K

On 10 Nov 09:58, Jakub Jelinek wrote:
> On Tue, Nov 10, 2015 at 11:44:18AM +0300, Kirill Yukhin wrote:
> >  					       bool *);
> > +static tree handle_simd_attribute (tree *, tree, tree, int, bool *);
> >  static tree handle_omp_declare_target_attribute (tree *, tree, tree, int,
> >  						 bool *);
> >  static tree handle_designated_init_attribute (tree *, tree, tree, int, bool *);
> > @@ -818,6 +819,8 @@ const struct attribute_spec c_common_attribute_table[] =
> >  			      handle_omp_declare_simd_attribute, false },
> >    { "cilk simd function",     0, -1, true,  false, false,
> >  			      handle_omp_declare_simd_attribute, false },
> > +  { "simd",		      0, -1, true,  false, false,
> > +			      handle_simd_attribute, false },
> 
> Why the -1 in there?  I thought the simd attribute has exactly zero
> arguments, so 0, 0.
> 
> > +static tree
> > +handle_simd_attribute (tree *node, tree name, tree ARG_UNUSED (args),
> > +		       int ARG_UNUSED (flags), bool *no_add_attrs)
> 
> As per recent discussion, please leave ARG_UNUSED (args) etc. out, just
> use unnamed arguments.
> > +{
> > +  if (TREE_CODE (*node) == FUNCTION_DECL)
> > +    {
> > +      if (lookup_attribute ("cilk simd function", DECL_ATTRIBUTES (*node)) != NULL)
> 
> Too long line.
> > +	{
> > +	  error_at (DECL_SOURCE_LOCATION (*node),
> > +		    "%<__simd__%> attribute cannot be "
> > +		    "used in the same function marked as a Cilk Plus SIMD-enabled function");
> 
> Too long line.  You should just move "use in the same " one line above.
> 
> > +	  *no_add_attrs = true;
> > +	}
> > +      else
> > +	{
> > +	  DECL_ATTRIBUTES (*node)
> > +	    = tree_cons (get_identifier ("omp declare simd"),
> > +			 NULL_TREE, DECL_ATTRIBUTES (*node));	  
> > +	}
> 
> Please avoid {}s around single statement in the body.
> >      {
> > -      error ("%<#pragma omp declare simd%> cannot be used in the same "
> > +      error ("%<#pragma omp declare simd%> or %<simd%> attribute cannot be used in the same "
> >  	     "function marked as a Cilk Plus SIMD-enabled function");
> 
> Too long line.
> 
> > +      if (lookup_attribute ("simd", DECL_ATTRIBUTES (fndecl)) != NULL)
> > +	{
> > +	  error_at (DECL_SOURCE_LOCATION (fndecl),
> > +		    "%<__simd__%> attribute cannot be "
> > +		    "used in the same function marked as a Cilk Plus SIMD-enabled function");
> 
> Too long line, see above.
> 
> >        c = build_tree_list (get_identifier ("omp declare simd"), c);
> >        TREE_CHAIN (c) = DECL_ATTRIBUTES (fndecl);
> >        DECL_ATTRIBUTES (fndecl) = c;
> > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> > index 7555bf3..f3831b9 100644
> > --- a/gcc/cp/parser.c
> > +++ b/gcc/cp/parser.c
> > @@ -34534,10 +34534,11 @@ cp_parser_late_parsing_cilk_simd_fn_info (cp_parser *parser, tree attrs)
> >    cp_omp_declare_simd_data *info = parser->cilk_simd_fn_info;
> >    int ii = 0;
> >  
> > -  if (parser->omp_declare_simd != NULL)
> > +  if (parser->omp_declare_simd != NULL
> > +      || lookup_attribute ("simd", attrs))
> >      {
> > -      error ("%<#pragma omp declare simd%> cannot be used in the same function"
> > -	     " marked as a Cilk Plus SIMD-enabled function");
> > +      error ("%<#pragma omp declare simd%> of %<simd%> attribute cannot be used "
> > +	     "in the same function marked as a Cilk Plus SIMD-enabled function");
> 
> Too long lines.
> 
> >        XDELETE (parser->cilk_simd_fn_info);
> >        parser->cilk_simd_fn_info = NULL;
> >        return attrs;
> > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > index fdb1547..32994a2 100644
> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -3066,6 +3066,21 @@ This function attribute make a stack protection of the function if
> >  flags @option{fstack-protector} or @option{fstack-protector-strong}
> >  or @option{fstack-protector-explicit} are set.
> >  
> > +@item simd
> > +@cindex @code{simd} function attribute.
> > +This attribute enables creation of one or more function versions that
> > +can process multiple arguments using SIMD instructions from a
> > +single invocation.  Specifying this attribute allows compiler to
> > +assume that such a versions are available at link time (provided
> > +in the same or another translation unit).  Generated versions are
> > +target dependent and described in corresponding Vector ABI document.  For
> > +x86_64 target this document can be found
> > +@w{@uref{https://sourceware.org/glibc/wiki/libmvec?action=AttachFile&do=view&target=VectorABI.txt,here}}.
> 
> Are you going to update VectorABI.txt based on the latest changes in
> the Intel ABI document?  I mean especially using L, R or U for linear
> %val(), %ref() or %uval() (if references), not using s for linear with
> uniform parameter stride, but instead using ls, Ls, Rs or Us for those?
> 
> > diff --git a/gcc/testsuite/c-c++-common/attr-simd.c b/gcc/testsuite/c-c++-common/attr-simd.c
> > new file mode 100644
> > index 0000000..6fd757a
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/attr-simd.c
> > @@ -0,0 +1,17 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-fdump-tree-optimized" } */
> > +
> > +__attribute__((__simd__))
> > +static int simd_attr (void)
> > +{
> > +  return 0;
> > +}
> > +
> > +__attribute__((simd))
> > +static int simd_attr2 (void)
> > +{
> > +  return 0;
> > +}
> > +
> > +/* { dg-final { scan-tree-dump "simd_attr\[ \\t\]simdclone|vector" "optimized" } } */
> > +/* { dg-final { scan-tree-dump "simd_attr2\[ \\t\]simdclone|vector" "optimized" } } */
> 
> You should add a few scan-assembler lines for x86_64/i?86 (see some
> declare-simd*.{c,C} tests) to test mangling.  Of course not when the
> function is static...
> 
> Otherwise LGTM.
> 
> 	Jakub


diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 6e2ce0a..ed9db8af 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -387,6 +387,7 @@ static tree handle_warn_unused_attribute (tree *, tree, tree, int, bool *);
 static tree handle_returns_nonnull_attribute (tree *, tree, tree, int, bool *);
 static tree handle_omp_declare_simd_attribute (tree *, tree, tree, int,
 					       bool *);
+static tree handle_simd_attribute (tree *, tree, tree, int, bool *);
 static tree handle_omp_declare_target_attribute (tree *, tree, tree, int,
 						 bool *);
 static tree handle_designated_init_attribute (tree *, tree, tree, int, bool *);
@@ -817,6 +818,8 @@ const struct attribute_spec c_common_attribute_table[] =
 			      handle_omp_declare_simd_attribute, false },
   { "cilk simd function",     0, -1, true,  false, false,
 			      handle_omp_declare_simd_attribute, false },
+  { "simd",		      0, 0, true,  false, false,
+			      handle_simd_attribute, false },
   { "omp declare target",     0, 0, true, false, false,
 			      handle_omp_declare_target_attribute, false },
   { "alloc_align",	      1, 1, false, true, true,
@@ -9013,6 +9016,35 @@ handle_omp_declare_simd_attribute (tree *, tree, tree, int, bool *)
   return NULL_TREE;
 }
 
+/* Handle an "simd" attribute.  */
+
+static tree
+handle_simd_attribute (tree *node, tree name, tree, int , bool *no_add_attrs)
+{
+  if (TREE_CODE (*node) == FUNCTION_DECL)
+    {
+      if (lookup_attribute ("cilk simd function",
+			    DECL_ATTRIBUTES (*node)) != NULL)
+	{
+	  error_at (DECL_SOURCE_LOCATION (*node),
+		    "%<__simd__%> attribute cannot be used in the same "
+		    "function marked as a Cilk Plus SIMD-enabled function");
+	  *no_add_attrs = true;
+	}
+      else
+	DECL_ATTRIBUTES (*node)
+	  = tree_cons (get_identifier ("omp declare simd"),
+		       NULL_TREE, DECL_ATTRIBUTES (*node));
+    }
+  else
+    {
+      warning (OPT_Wattributes, "%qE attribute ignored", name);
+      *no_add_attrs = true;
+    }
+
+  return NULL_TREE;
+}
+
 /* Handle an "omp declare target" attribute; arguments as in
    struct attribute_spec.handler.  */
 
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index c01d651..b33be9d 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -16055,10 +16055,13 @@ c_finish_omp_declare_simd (c_parser *parser, tree fndecl, tree parms,
 			   vec<c_token> clauses)
 {
   if (flag_cilkplus
-      && clauses.exists () && !vec_safe_is_empty (parser->cilk_simd_fn_tokens))
+      && (clauses.exists ()
+	  || lookup_attribute ("simd", DECL_ATTRIBUTES (fndecl)))
+      && !vec_safe_is_empty (parser->cilk_simd_fn_tokens))
     {
-      error ("%<#pragma omp declare simd%> cannot be used in the same "
-	     "function marked as a Cilk Plus SIMD-enabled function");
+      error ("%<#pragma omp declare simd%> or %<simd%> attribute cannot be "
+	     "used in the same function marked as a Cilk Plus SIMD-enabled "
+	     "function");
       vec_free (parser->cilk_simd_fn_tokens);
       return;
     }
@@ -16096,6 +16099,16 @@ c_finish_omp_declare_simd (c_parser *parser, tree fndecl, tree parms,
       parser->tokens = parser->cilk_simd_fn_tokens->address ();
       parser->tokens_avail = vec_safe_length (parser->cilk_simd_fn_tokens);
       is_cilkplus_cilk_simd_fn = true;
+
+      if (lookup_attribute ("simd", DECL_ATTRIBUTES (fndecl)) != NULL)
+	{
+	  error_at (DECL_SOURCE_LOCATION (fndecl),
+		    "%<__simd__%> attribute cannot be used in the same "
+		    "function marked as a Cilk Plus SIMD-enabled function");
+	  vec_free (parser->cilk_simd_fn_tokens);
+	  return;
+	}
+
     }
   else
     {
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 0c918ecf..2386c60 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -35042,10 +35042,12 @@ cp_parser_late_parsing_cilk_simd_fn_info (cp_parser *parser, tree attrs)
   cp_omp_declare_simd_data *info = parser->cilk_simd_fn_info;
   int ii = 0;
 
-  if (parser->omp_declare_simd != NULL)
+  if (parser->omp_declare_simd != NULL
+      || lookup_attribute ("simd", attrs))
     {
-      error ("%<#pragma omp declare simd%> cannot be used in the same function"
-	     " marked as a Cilk Plus SIMD-enabled function");
+      error ("%<#pragma omp declare simd%> of %<simd%> attribute cannot be "
+	     "used in the same function marked as a Cilk Plus SIMD-enabled "
+	     " function");
       XDELETE (parser->cilk_simd_fn_info);
       parser->cilk_simd_fn_info = NULL;
       return attrs;
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index aab6bad..e973047 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3143,6 +3143,21 @@ one compiled with @option{-msse4.1} and another with @option{-mavx}.
 At the function call it will create resolver @code{ifunc}, that will
 dynamically call a clone suitable for current architecture.
 
+@item simd
+@cindex @code{simd} function attribute.
+This attribute enables creation of one or more function versions that
+can process multiple arguments using SIMD instructions from a
+single invocation.  Specifying this attribute allows compiler to
+assume that such a versions are available at link time (provided
+in the same or another translation unit).  Generated versions are
+target dependent and described in corresponding Vector ABI document.  For
+x86_64 target this document can be found
+@w{@uref{https://sourceware.org/glibc/wiki/libmvec?action=AttachFile&do=view&target=VectorABI.txt,here}}.
+It is prohibited to use the attribute along with Cilk Plus's @code{vector}
+attribute. If the attribute is specified and @code{#pragma omp declare simd}
+presented on a declaration and @code{-fopenmp} or @code{-fopenmp-simd}
+switch is specified, then the attribute is ignored.
+
 @item target (@var{options})
 @cindex @code{target} function attribute
 Multiple target back ends implement the @code{target} attribute
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index f7584de..15e8737 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -18405,10 +18405,7 @@ public:
 bool
 pass_omp_simd_clone::gate (function *)
 {
-  return ((flag_openmp || flag_openmp_simd
-	   || flag_cilkplus
-	   || (in_lto_p && !flag_wpa))
-	  && (targetm.simd_clone.compute_vecsize_and_simdlen != NULL));
+  return targetm.simd_clone.compute_vecsize_and_simdlen != NULL;
 }
 
 } // anon namespace
diff --git a/gcc/testsuite/c-c++-common/attr-simd-2.c b/gcc/testsuite/c-c++-common/attr-simd-2.c
new file mode 100644
index 0000000..bc91ccf
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-simd-2.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-optimized -fopenmp-simd" } */
+
+#pragma omp declare simd
+__attribute__((__simd__))
+extern
+int simd_attr (void)
+{
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump "omp declare simd" "optimized" } } */
+/* { dg-final { scan-assembler-times "_ZGVbN4_(?:_Z9)?simd_attr(?:v)?:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVbM4_(?:_Z9)?simd_attr(?:v)?:" 1 { target { i?86-*-* x86_64-*-* } } } } */
diff --git a/gcc/testsuite/c-c++-common/attr-simd-3.c b/gcc/testsuite/c-c++-common/attr-simd-3.c
new file mode 100644
index 0000000..2bbdf04
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-simd-3.c
@@ -0,0 +1,5 @@
+/* { dg-do compile } */
+/* { dg-options "-fcilkplus" } */
+/* { dg-prune-output "undeclared here \\(not in a function\\)|\[^\n\r\]* was not declared in this scope" } */
+
+void f () __attribute__((__simd__, __vector__)); /* { dg-error "in the same function marked as a Cilk Plus" } */
diff --git a/gcc/testsuite/c-c++-common/attr-simd.c b/gcc/testsuite/c-c++-common/attr-simd.c
new file mode 100644
index 0000000..b4eda34
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-simd.c
@@ -0,0 +1,32 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-optimized" } */
+
+__attribute__((__simd__))
+extern
+int simd_attr (void)
+{
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump "simd_attr\[ \\t\]simdclone|vector" "optimized" } } */
+/* { dg-final { scan-assembler-times "_ZGVbN4_(?:_Z9)?simd_attr(?:v)?:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVbM4_(?:_Z9)?simd_attr(?:v)?:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVcN4_(?:_Z9)?simd_attr(?:v)?:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVcM4_(?:_Z9)?simd_attr(?:v)?:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVdN8_(?:_Z9)?simd_attr(?:v)?:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVdM8_(?:_Z9)?simd_attr(?:v)?:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+
+__attribute__((simd))
+extern
+int simd_attr2 (void)
+{
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump "simd_attr2\[ \\t\]simdclone|vector" "optimized" } } */
+/* { dg-final { scan-assembler-times "_ZGVbN4_(?:_Z10)?simd_attr2(?:v)?:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVbM4_(?:_Z10)?simd_attr2(?:v)?:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVcN4_(?:_Z10)?simd_attr2(?:v)?:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVcM4_(?:_Z10)?simd_attr2(?:v)?:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVdN8_(?:_Z10)?simd_attr2(?:v)?:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVdM8_(?:_Z10)?simd_attr2(?:v)?:" 1 { target { i?86-*-* x86_64-*-* } } } } */