Hi!

Thanks to Joseph and you for your review!

On Fri, 9 Oct 2015 08:26:37 -0400, Nathan Sidwell <nathan@acm.org> wrote:
> On 10/08/15 12:39, Thomas Schwinge wrote:
> > Some bits extracted out of gomp-4_0-branch, and some other bits
> > rewritten; here is a patch to support OpenACC Combined Directives in C,
> > C++.  (The Fortran front end already does support these.)
> >
> > As far as I know, Jakub is not available at this time, so maybe the C
> > (Joseph) and C++ (Jason, Nathan) front end maintainers could please
> > review this, instead of him?  (The front end changes as well as the few
> > other cleanup changes should all be straight forward.)  OK for trunk once
> > bootstrap tested?

Assuming the following review comments resolved, does this constitute
approval for the whole patch?  (Will of course address any later review
comments, as usual.)

> > --- gcc/cp/parser.c
> > +++ gcc/cp/parser.c
> > @@ -33132,69 +33132,64 @@ cp_parser_oacc_enter_exit_data (cp_parser *parser, cp_token *pragma_tok,
> 
> > -/* OpenACC 2.0:
> >      # pragma acc loop oacc-loop-clause[optseq] new-line
> >        structured-block  */
> >
> >   #define OACC_LOOP_CLAUSE_MASK						\
> >   	( (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_COLLAPSE)		\
> > -	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_REDUCTION))
> > +	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_REDUCTION) )
> >
> >   static tree
> > -cp_parser_oacc_loop (cp_parser *parser, cp_token *pragma_tok)
> > +cp_parser_oacc_loop (cp_parser *parser, cp_token *pragma_tok, char *p_name,
> > +		     omp_clause_mask mask, tree *cclauses)
> >
> 
> Needs documentation.
> 
> 
> >   static tree
> > -cp_parser_oacc_parallel (cp_parser *parser, cp_token *pragma_tok)
> > +cp_parser_oacc_kernels_parallel (cp_parser *parser, cp_token *pragma_tok,
> > +				 char *p_name)
> 
> Likewise,

You mean the cp_parser_oacc_loop and cp_parser_oacc_kernels_parallel
functions need documentation?  I agree it's a bit terse, but documenting
these by just listing the accepted parsing tokens "# pragma acc loop"
etc., followed by the *_CLAUSE_MASKs is what's done for the other
OpenACC/OpenMP directives in the C/C++ front ends.  So, I don't see a
reason to be different for these two?

> > --- gcc/cp/semantics.c
> > +++ gcc/cp/semantics.c
> > @@ -6124,8 +6124,17 @@ finish_omp_structured_block (tree block)
> >     return do_poplevel (block);
> >   }
> >
> > +/* Similarly, except force the retention of the BLOCK.  */
> 
> similar to what?
> 
> > +
> > +tree
> > +begin_omp_parallel (void)

I just moved begin_omp_parallel up a little bit in the file (where it
originally resided); the "similarly" applies to the function defined just
before.


Grüße,
 Thomas