Re: [gomp4] OpenACC routine directive

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi Cesar!

On Mon, 15 Sep 2014 16:35:02 -0700, Cesar Philippidis <cesar@codesourcery.com> wrote:
> This patch adds initial support for the OpenACC routine directive.

Thanks!

> It's
> not complete just yet because it doesn't implement any of the optional
> clauses, except for the optional function/subroutine name. As such, it
> doesn't go beyond marking functions with the "omp declare target" attribute.

Reviewing the C front end bits.

I wonder if we can integrate this better (maybe as a follow-up patch),
for example, inside an OpenMP Â#pragma omp declare target region,
current_omp_declare_target_attribute is set to non-zero, and can then
conveniently be used in gcc/c/c-decl.c:c_decl_attributes.  (Of course,
OpenACC's Â#pragma acc routine is a bit different; doesn't have a ÂendÂ
tag.)  I have not yet figured out whether we can do something similar,
for example to avoid all the c_parser_declaration_or_fndef call-sites
changes.

> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -1678,6 +1687,9 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
|  	  if (omp_declare_simd_clauses.exists ()
>  	      || !vec_safe_is_empty (parser->cilk_simd_fn_tokens))
>  	    c_finish_omp_declare_simd (parser, NULL_TREE, NULL_TREE,
>  				       omp_declare_simd_clauses);
> +	  else
> +	    c_finish_oacc_routine (parser, NULL_TREE,
> +				      oacc_routine_clauses, oacc_routine_named);
>  	  c_parser_skip_to_end_of_block_or_statement (parser);
>  	  return;
>  	}

Please be more explicit.  Using such "catch-all else statements", the
next reader of this code will wonder: why is
Â!(omp_declare_simd_clauses.exists () || !vec_safe_is_empty
(parser->cilk_simd_fn_tokens))Â the condition for calling
c_finish_oacc_routine?  (Likewise in other places.)

> +#define OACC_ROUTINE_CLAUSE_MASK					\
> +	(OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_PROC_BIND)

PRAGMA_OMP_CLAUSE_NONE (for the moment), not PRAGMA_OMP_CLAUSE_PROC_BIND.

We're missing test cases for the different syntax options that are
described in the OpenACC specification, especially the invalid ones.  For
example, should gcc/testsuite/c-c++-common/goacc/pragma_context.c be
extended?


GrÃÃe,
 Thomas

Attachment: pgpT4LD5FdxOD.pgp
Description: PGP signature