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

[Jakub Jelinek <jakub@redhat.com>]

On Tue, Oct 27, 2015 at 05:06:58PM +0300, Kirill Yukhin wrote:
> Boostrapped. Regtesting is in progress. Is it ok for trunk if pass?
> 
> gcc/
>         * cp/parser.h (cp_parser): Add simd_attr_present.
>         * cp/parser.c (cp_parser_late_return_type_opt): Handle simd_attr_present,
> 	require comman in __vector__ attribute.
>         (cp_parser_gnu_attribute_list): Ditto.
>         * 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.

gcc/cp/ and gcc/c/ have their own ChangeLog files, therefore cp/ or c/
prefixes should also never appear in the ChangeLog entries.

>         (c_finish_omp_declare_simd): Handle simd_attr_present.
>         * doc/extend.texi (simd): Document new attribute.
>         * omp-low.c (pass_omp_simd_clone::gate): If target allows - call
>         without additional conditions.
> 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.
> -      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 "

I'd write %<simd%> instead of __simd__.  __simd__ is just one of the
possible spellings of the attribute...

>  	     "function marked as a Cilk Plus SIMD-enabled function");
>        vec_free (parser->cilk_simd_fn_tokens);
>        return;
> @@ -15423,7 +15441,7 @@ c_finish_omp_declare_simd (c_parser *parser, tree fndecl, tree parms,
>    unsigned int tokens_avail = parser->tokens_avail;
>    gcc_assert (parser->tokens == &parser->tokens_buf[0]);
>    bool is_cilkplus_cilk_simd_fn = false;
> -  
> + 
>    if (flag_cilkplus && !vec_safe_is_empty (parser->cilk_simd_fn_tokens))

If you are changing this, please remove all trailing whitespace from the
empty line.

> +  if (parser->simd_attr_present
> +      && is_cilkplus_cilk_simd_fn)

This could fit on one line
  if (parser->simd_attr_present && is_cilkplus_cilk_simd_fn)
just fine.

> +      error ("SIMD-enabled function attributes"
> +	     "are allowed when attribute __simd__ is specified");

See earlier.
>  
> +  /* Attach `omp declare simd’ attribute if __simd__ is specified AND no OpenMP clauses
> +     present in decl.  */
> +  if (parser->simd_attr_present
> +      && parser->tokens_avail == 0)

See earlier.

> @@ -19363,6 +19365,18 @@ cp_parser_late_return_type_opt (cp_parser* parser, cp_declarator *declarator,
>        = cp_parser_late_parsing_omp_declare_simd (parser,
>  						 declarator->std_attributes);
>  
> +  if (parser->simd_attr_present
> +      && !declare_simd_p)

Ditto.
> +    {
> +      if (cilk_simd_fn_vector_p)
> +	error ("__simd__ attribute cannot be used in the same function"
> +	       " marked as a Cilk Plus SIMD-enabled function");

Ditto.

> 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

I wonder what the compile time effect this will have.
have (alternative is of course 

> 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" } } */

You should also test other spellings of the attribute...

	Jakub