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

Hi Jakub,
On 29 Oct 09:54, Jakub Jelinek wrote:
> On Wed, Oct 28, 2015 at 12:16:04PM +0300, Kirill Yukhin wrote:
> > Bootstrapped. Regtested. Is it ok for trunk?
> > 
> > 
> > gcc/
> >         * omp-low.c (pass_omp_simd_clone::gate): If target allows - call
> >         without additional conditions.
> >         * doc/extend.texi (simd): Document new attribute.
> > gcc/cp/
> >         * parser.h (cp_parser): Add simd_attr_present.
> >         * parser.c (cp_parser_late_return_type_opt): Handle simd_attr_present,
> >         require comman in __vector__ attribute.
> >         (cp_parser_gnu_attribute_list): Ditto.
> > gcc/c/
> >         * c-parser.c (c_parser): Add simd_attr_present flag.
> >         (c_parser_declaration_or_fndef): Call c_parser_declaration_or_fndef
> >         if simd_attr_present is set.
> >         (c_finish_omp_declare_simd): Handle simd_attr_present.
> 
> Actually, do you plan to eventually add some clauses/operands to the simd
> attribute, or is the plan to just say that simd attribute is
> #pragma omp declare simd
> with no clauses as if -fopenmp-simd has been enabled?
I think so/
> If you don't plan to add any clauses, I wonder whether you really need to
> add any parser changes at all, whether this couldn't be all handled in
> c-family/c-common.c - handle_simd_attribute, adding simd to the attribute
> table in there as a function decl attribute, and simply when processing it
> add
>         tree c = build_tree_list (get_identifier ("omp declare simd"), NULL_TREE);
>         TREE_CHAIN (c) = DECL_ATTRIBUTES (fndecl);
>         DECL_ATTRIBUTES (fndecl) = c;
> (after checking whether the attribute isn't already present and erroring out
> if there is "cilk simd function" attribute).
> The reason for the (admittedly ugly) parser changes for #pragma omp declare simd is
> that the clauses on the directive refer to parameters that will be declared
> later, so we need to save the tokens of the pragma and then after parsing
> the parameter declarations actually parse the clauses.  But, in the simd
> attribute case, there are no clauses, there is nothing to parse later.
I've refactored the patch.
New tests pass except one, which fails due to PR68158.
Bootstrapped and reg-tested.

Is it ok for trunk?

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: Ditto.
        * c-c++-common/attr-simd-3.c: Ditto.

> 	Jakub

--
Thanks, K

On Tue, Nov 10, 2015 at 11:44:18AM +0300, Kirill Yukhin wrote:
> gcc/
>         * omp-low.c (pass_omp_simd_clone::gate): If target allows - call
>         without additional conditions.

Please make sure CHangeLog entries are tab indented.  I would just use
a comma instead of " -" above.

>         * c-c++-common/attr-simd.c: New test.
>         * c-c++-common/attr-simd-2.c: Ditto.
>         * c-c++-common/attr-simd-3.c: Ditto.

New test is IMHO short enough that it is better to spell it
in each case.
>  					       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?

> +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 ad7c017..232dc5c 100644
> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c
> @@ -17412,10 +17412,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..e9afc11
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/attr-simd-2.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fdump-tree-optimized -fopenmp-simd" } */
> +
> +#pragma omp declare simd
> +__attribute__((__simd__))
> +static int simd_attr (void)
> +{
> +  return 0;
> +}
> +
> +/* { dg-final { scan-tree-dump "omp declare simd" "optimized" } } */
> 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..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