Hi Cesar! On Mon, 15 Sep 2014 16:35:02 -0700, Cesar Philippidis <ce...@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
pgpg3Lfp4y1ep.pgp
Description: PGP signature