Re: [PATCH] OpenACC for C++ front end

2014-11-21 Thread Jakub Jelinek
On Thu, Nov 20, 2014 at 05:33:57PM -0600, James Norris wrote:
> >>+ t = OMP_CLAUSE_ASYNC_EXPR (c);
> >>+ if (t == error_mark_node)
> >>+   remove = true;
> >>+ else if (!type_dependent_expression_p (t)
> >>+  && !INTEGRAL_TYPE_P (TREE_TYPE (t)))
> >>+   {
> >>+ error ("% expression must be integral");
> >You have OMP_CLAUSE_LOCATION (c) which you could use for error_at.
> 
> I followed the convention that was used elsewhere in the function
> at using error ().

Perhaps it would be better to change even those other spots in the function.
But that can be certainly done as a follow-up patch.

> Thank you for taking the time to review!
> 
> OK to commit after middle end has been accepted?

Yes, thanks.

Jakub


Re: [PATCH] OpenACC for C front end

2014-11-21 Thread Jakub Jelinek
On Thu, Nov 20, 2014 at 05:50:33PM -0600, James Norris wrote:
> >>+   case 'h':
> >>+ if (!strcmp ("host", p))
> >>+   result = PRAGMA_OMP_CLAUSE_SELF;
> >>+ break;
> >Shouldn't this be PRAGMA_OMP_CLAUSE_HOST (PRAGMA_OACC_CLAUSE_HOST)
> >instead?  It is _HOST in the C++ patch, are there no C tests with
> >that clause covering it?
> 
> The "host" clause is a synonym for the "self" clause. The initial
> C++ patch did not treat "host" as a synonym and has amended
> accordingly.

Can you add a comment mentioning that (for casual readers)?

> There was a mistake in naming the function:
> c_parser_omp_clause_vector_length.
> Once it was renamed to: c_parser_oacc_clause_vector_length, diff was able to
> keep track.

Great.

> OK to commit after middle end is accepted?

Ok, thanks.

Jakub


Re: [PATCH] OpenACC for C front end

2014-11-20 Thread James Norris

Hi!

On 11/13/2014 09:04 AM, Jakub Jelinek wrote:

On Wed, Nov 05, 2014 at 03:39:44PM -0600, James Norris wrote:

* c-typeck.c (c_finish_oacc_parallel, c_finish_oacc_kernels,
c_finish_oacc_data): New functions.
(handle_omp_array_sections, c_finish_omp_clauses):

Handle should be on the above line, no need to wrap too early.


Fixed.




@@ -9763,6 +9830,10 @@ c_parser_omp_clause_name (c_parser *parser)
  else if (!strcmp ("from", p))
result = PRAGMA_OMP_CLAUSE_FROM;
  break;
+   case 'h':
+ if (!strcmp ("host", p))
+   result = PRAGMA_OMP_CLAUSE_SELF;
+ break;

Shouldn't this be PRAGMA_OMP_CLAUSE_HOST (PRAGMA_OACC_CLAUSE_HOST)
instead?  It is _HOST in the C++ patch, are there no C tests with
that clause covering it?


The "host" clause is a synonym for the "self" clause. The initial
C++ patch did not treat "host" as a synonym and has amended
accordingly.


+  tree v = TREE_PURPOSE (t);
+
+  /* FIXME diagnostics: Ideally we should keep individual
+locations for all the variables in the var list to make the
+following errors more precise.  Perhaps
+c_parser_omp_var_list_parens() should construct a list of
+locations to go along with the var list.  */

Like in C++ patch, please avoid the comment, file a PR instead,
or just queue the work for GCC 6.


Will submit a PR.


+  /* Attempt to statically determine when the number isn't positive.  */
+  c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t,
+  build_int_cst (TREE_TYPE (t), 0));

build_int_cst not aligned below expr_loc.


Fixed.


+  if (CAN_HAVE_LOCATION_P (c))
+   SET_EXPR_LOCATION (c, expr_loc);
+  if (c == boolean_true_node)
+   {
+ warning_at (expr_loc, 0,
+ "% value must be positive");

This would fit perfectly on one line.


Fixed.


+  tree c, t;
+  location_t loc = c_parser_peek_token (parser)->location;
+
+  /* TODO XXX: FIX -1  (acc_async_noval).  */
+  t = build_int_cst (integer_type_node, -1);

Again, as in C++ patch, please avoid this.  Use gomp-constants.h,
or some enum, or explain what the -1 is, but avoid TODO XXX: FIX.


Fixed.


+  else
+{
+  t = c_fully_fold (t, false, NULL);
+}

Please avoid the {}s and reindent.


Fixed.




-/* OpenMP 4.0:
-   parallel
-   for
-   sections
-   taskgroup */
+  if (!INTEGRAL_TYPE_P (TREE_TYPE (t)))
+   {
+ c_parser_error (parser, "expected integer expression");
+ return list;
+   }
  
-static tree

-c_parser_omp_clause_cancelkind (c_parser *parser ATTRIBUTE_UNUSED,
-   enum omp_clause_code code, tree list)
-{
-  tree c = build_omp_clause (c_parser_peek_token (parser)->location, code);
+  /* Attempt to statically determine when the number isn't positive.  */
+  c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t,
+  build_int_cst (TREE_TYPE (t), 0));

I wonder if it wouldn't be better to just put the new OpenACC routines
into a new block of code, not stick it in between the OpenMP handling
routines, because diff apparently lost track and I'm afraid so will svn 
blame/git blame
and we'll lose history for the OpenMP changes.


There was a mistake in naming the function: 
c_parser_omp_clause_vector_length.

Once it was renamed to: c_parser_oacc_clause_vector_length, diff was able to
keep track.


+   case PRAGMA_OMP_CLAUSE_VECTOR_LENGTH:
+ clauses = c_parser_omp_clause_vector_length (parser, clauses);
+ c_name = "vector_length";
+ break;

That is OpenACC clause, right?  Shouldn't the routine be called
c_parser_oacc_clause_vector_length?


Fixed.


+   case PRAGMA_OMP_CLAUSE_WAIT:
+ clauses = c_parser_oacc_clause_wait (parser, clauses);

E.g. c_parser_oacc_clause_wait is.


Fixed.




+  clauses =  c_parser_oacc_all_clauses (parser, OACC_DATA_CLAUSE_MASK,
+   "#pragma acc data");

Too many spaces after =.


Fixed.


+/* OpenACC 2.0:
+   # pragma acc kernels oacc-kernels-clause[optseq] new-line
+ structured-block
+
+   LOC is the location of the #pragma token.

Again, what is LOC?


Fixed by removal of comment, along with other occurences.


+  clauses =  c_parser_oacc_all_clauses (parser, OACC_KERNELS_CLAUSE_MASK,
+   p_name);

See above.


Fixed.


+  c_parser_error (parser, enter
+ ? "expected % in %<#pragma acc enter data%>"
+ : "expected % in %<#pragma acc exit data%>");
+  c_parser_skip_to_pragma_eol (parser);
+  return;
+}
+
+  const char *p = IDENTIFIER_POINTER (c_parser_peek_token (parser)->value);
+  if (strcmp (p, "data") != 0)
+{
+  c_parser_error (parser, "invalid pragma");

See the C++ patch review.


Fixed.


+  if (find_omp_clause (clauses, OMP_CLAUSE_MAP) == NULL_TREE)
+{
+  error_at (loc, enter
+ 

Re: [PATCH] OpenACC for C++ front end

2014-11-20 Thread James Norris

Hi!

On 11/13/2014 07:02 AM, Jakub Jelinek wrote:

On Wed, Nov 05, 2014 at 03:37:08PM -0600, James Norris wrote:

2014-11-05  James Norris  
Cesar Philippidis  
Thomas Schwinge  
Ilmir Usmanov  

...

Please check formatting.  I see various spots with 8 spaces instead of tabs,
e.g. on
+  check_no_duplicate_clause (list, OMP_CLAUSE_VECTOR_LENGTH,
+"vector_length", location);
even the alignment is wrong, I see calls without space before (:
+  if (args == NULL || args->length() == 0)
...
+ error("% expression must be integral");
other spots where the alignment isn't right:


Fixed.


+static tree
+cp_parser_oacc_cache (cp_parser *parser,
+   cp_token *pragma_tok __attribute__((unused)))
(cp_token should be below cp_parser).  While at this,
__attribute__((unused)) should be replaced by ATTRIBUTE_UNUSED, or removing
the parameter name, or removing the parameter altogether even better.


Fixed by removing unused parameter.


For the formatting issues, you can easily look for them in the patch
(in lines starting with +), change the patch and apply interdiff to your
tree.


Thank you for the pointer to interdiff!




--- a/gcc/c-family/c-omp.c
+++ b/gcc/c-family/c-omp.c
@@ -29,8 +29,47 @@ along with GCC; see the file COPYING3.  If not see
  #include "c-pragma.h"
  #include "gimple-expr.h"
  #include "langhooks.h"
+#include "omp-low.h"

As Thomas? has said, you should include gomp-constants.h and use them in:


+t = build_int_cst (integer_type_node, -2);  /* TODO: XXX FIX -2.  */

spots like this.


Fixed.




--- a/gcc/c-family/c-pragma.c
+++ b/gcc/c-family/c-pragma.c
@@ -1180,6 +1180,16 @@ typedef struct
  static vec registered_pp_pragmas;
  
  struct omp_pragma_def { const char *name; unsigned int id; };

+static const struct omp_pragma_def oacc_pragmas[] = {
+  { "data", PRAGMA_OACC_DATA },
+  { "enter", PRAGMA_OACC_ENTER_DATA },
+  { "exit", PRAGMA_OACC_EXIT_DATA },
+  { "kernels", PRAGMA_OACC_KERNELS },
+  { "loop", PRAGMA_OACC_LOOP },
+  { "parallel", PRAGMA_OACC_PARALLEL },
+  { "update", PRAGMA_OACC_UPDATE },
+  { "wait", PRAGMA_OACC_WAIT },

I'd avoid the , after the last element.


Fixed.




@@ -1383,6 +1402,17 @@ c_invoke_pragma_handler (unsigned int id)
  void
  init_pragma (void)
  {
+  if (flag_openacc)
+{
+  const int n_oacc_pragmas
+   = sizeof (oacc_pragmas) / sizeof (*oacc_pragmas);
+  int i;
+
+  for (i = 0; i < n_oacc_pragmas; ++i)
+   cpp_register_deferred_pragma (parse_in, "acc", oacc_pragmas[i].name,
+ oacc_pragmas[i].id, true, true);
+}
+
if (flag_openmp)
  {
const int n_omp_pragmas = sizeof (omp_pragmas) / sizeof (*omp_pragmas);

Is -fopenmp -fopenacc tested not to run out of number of supported pragmas
by libcpp?


Tests will be added in a follow-up patch once the middle end has been 
accepted.





@@ -65,23 +74,30 @@ typedef enum pragma_kind {
  } pragma_kind;
  
  
-/* All clauses defined by OpenMP 2.5, 3.0, 3.1 and 4.0.

+/* All clauses defined by OpenACC 2.0, and OpenMP 2.5, 3.0, 3.1, and 4.0.
 Used internally by both C and C++ parsers.  */
  typedef enum pragma_omp_clause {
PRAGMA_OMP_CLAUSE_NONE = 0,
  
PRAGMA_OMP_CLAUSE_ALIGNED,

+  PRAGMA_OMP_CLAUSE_ASYNC,
PRAGMA_OMP_CLAUSE_COLLAPSE,
+  PRAGMA_OMP_CLAUSE_COPY,
PRAGMA_OMP_CLAUSE_COPYIN,
+  PRAGMA_OMP_CLAUSE_COPYOUT,
PRAGMA_OMP_CLAUSE_COPYPRIVATE,
+  PRAGMA_OMP_CLAUSE_CREATE,
PRAGMA_OMP_CLAUSE_DEFAULT,
+  PRAGMA_OMP_CLAUSE_DELETE,
PRAGMA_OMP_CLAUSE_DEPEND,
PRAGMA_OMP_CLAUSE_DEVICE,
+  PRAGMA_OMP_CLAUSE_DEVICEPTR,
PRAGMA_OMP_CLAUSE_DIST_SCHEDULE,
PRAGMA_OMP_CLAUSE_FINAL,
PRAGMA_OMP_CLAUSE_FIRSTPRIVATE,
PRAGMA_OMP_CLAUSE_FOR,
PRAGMA_OMP_CLAUSE_FROM,
+  PRAGMA_OMP_CLAUSE_HOST,
PRAGMA_OMP_CLAUSE_IF,
PRAGMA_OMP_CLAUSE_INBRANCH,
PRAGMA_OMP_CLAUSE_LASTPRIVATE,
@@ -90,16 +106,24 @@ typedef enum pragma_omp_clause {
PRAGMA_OMP_CLAUSE_MERGEABLE,
PRAGMA_OMP_CLAUSE_NOTINBRANCH,
PRAGMA_OMP_CLAUSE_NOWAIT,
+  PRAGMA_OMP_CLAUSE_NUM_GANGS,
PRAGMA_OMP_CLAUSE_NUM_TEAMS,
PRAGMA_OMP_CLAUSE_NUM_THREADS,
+  PRAGMA_OMP_CLAUSE_NUM_WORKERS,
PRAGMA_OMP_CLAUSE_ORDERED,
PRAGMA_OMP_CLAUSE_PARALLEL,
+  PRAGMA_OMP_CLAUSE_PRESENT,
+  PRAGMA_OMP_CLAUSE_PRESENT_OR_COPY,
+  PRAGMA_OMP_CLAUSE_PRESENT_OR_COPYIN,
+  PRAGMA_OMP_CLAUSE_PRESENT_OR_COPYOUT,
+  PRAGMA_OMP_CLAUSE_PRESENT_OR_CREATE,
PRAGMA_OMP_CLAUSE_PRIVATE,
PRAGMA_OMP_CLAUSE_PROC_BIND,
PRAGMA_OMP_CLAUSE_REDUCTION,
PRAGMA_OMP_CLAUSE_SAFELEN,
PRAGMA_OMP_CLAUSE_SCHEDULE,
PRAGMA_OMP_CLAUSE_SECTIONS,
+  PRAGMA_OMP_CLAUSE_SELF,
PRAGMA_OMP_CLAUSE_SHARED,
PRAGMA_OMP_CLAUSE_SIMDLEN,
PRAGMA_OMP_CLAUSE_TASKGROUP,
@@ -107,6 +131,8 @@ typedef enum pragma_omp_clause {
PRAGMA_OMP_CLAUSE_TO,
PRAGMA_OMP_CLAUSE_UNIFORM,
PRAGMA_OMP_CLAUSE_UNTIED,
+  PRAGMA_OMP_CLAUSE_VECTOR_LENGTH

Re: [PATCH] OpenACC for C front end

2014-11-13 Thread Joseph Myers
On Wed, 5 Nov 2014, James Norris wrote:

> Hi!
> 
> This patch represents the changes for OpenACC 2.0
> in the C front-end. At present these files will
> not compile as the changes for the middle end are
> not present.

So will things compile with the combination of this patch and the 
middle-end patch Thomas posted today?  If not, could you give a more 
detailed roadmap to the set of patches that need to be applied to current 
trunk to get this to compile?

Jakub has dealt with substantive review.  I'd add that testcases - for 
each diagnostic message in this patch, at least - should be included with 
the front-end patch.  (Execution tests can reasonably be included with the 
appropriate run-time library patch - as long as they *are* included.)

-- 
Joseph S. Myers
jos...@codesourcery.com


Re: [PATCH] OpenACC for C front end

2014-11-13 Thread Jakub Jelinek
On Wed, Nov 05, 2014 at 03:39:44PM -0600, James Norris wrote:
>   * c-typeck.c (c_finish_oacc_parallel, c_finish_oacc_kernels,
>   c_finish_oacc_data): New functions.
>   (handle_omp_array_sections, c_finish_omp_clauses):

Handle should be on the above line, no need to wrap too early.

> @@ -9763,6 +9830,10 @@ c_parser_omp_clause_name (c_parser *parser)
> else if (!strcmp ("from", p))
>   result = PRAGMA_OMP_CLAUSE_FROM;
> break;
> + case 'h':
> +   if (!strcmp ("host", p))
> + result = PRAGMA_OMP_CLAUSE_SELF;
> +   break;

Shouldn't this be PRAGMA_OMP_CLAUSE_HOST (PRAGMA_OACC_CLAUSE_HOST)
instead?  It is _HOST in the C++ patch, are there no C tests with
that clause covering it?

> +  tree v = TREE_PURPOSE (t);
> +
> +  /* FIXME diagnostics: Ideally we should keep individual
> +  locations for all the variables in the var list to make the
> +  following errors more precise.  Perhaps
> +  c_parser_omp_var_list_parens() should construct a list of
> +  locations to go along with the var list.  */

Like in C++ patch, please avoid the comment, file a PR instead,
or just queue the work for GCC 6.

> +  /* Attempt to statically determine when the number isn't positive.  */
> +  c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t,
> +build_int_cst (TREE_TYPE (t), 0));

build_int_cst not aligned below expr_loc.

> +  if (CAN_HAVE_LOCATION_P (c))
> + SET_EXPR_LOCATION (c, expr_loc);
> +  if (c == boolean_true_node)
> + {
> +   warning_at (expr_loc, 0,
> +   "% value must be positive");

This would fit perfectly on one line.

> +  tree c, t;
> +  location_t loc = c_parser_peek_token (parser)->location;
> +
> +  /* TODO XXX: FIX -1  (acc_async_noval).  */
> +  t = build_int_cst (integer_type_node, -1);

Again, as in C++ patch, please avoid this.  Use gomp-constants.h,
or some enum, or explain what the -1 is, but avoid TODO XXX: FIX.

> +  else
> +{
> +  t = c_fully_fold (t, false, NULL);
> +}

Please avoid the {}s and reindent.

> -/* OpenMP 4.0:
> -   parallel
> -   for
> -   sections
> -   taskgroup */
> +  if (!INTEGRAL_TYPE_P (TREE_TYPE (t)))
> + {
> +   c_parser_error (parser, "expected integer expression");
> +   return list;
> + }
>  
> -static tree
> -c_parser_omp_clause_cancelkind (c_parser *parser ATTRIBUTE_UNUSED,
> - enum omp_clause_code code, tree list)
> -{
> -  tree c = build_omp_clause (c_parser_peek_token (parser)->location, code);
> +  /* Attempt to statically determine when the number isn't positive.  */
> +  c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t,
> +build_int_cst (TREE_TYPE (t), 0));

I wonder if it wouldn't be better to just put the new OpenACC routines
into a new block of code, not stick it in between the OpenMP handling
routines, because diff apparently lost track and I'm afraid so will svn 
blame/git blame
and we'll lose history for the OpenMP changes.

> + case PRAGMA_OMP_CLAUSE_VECTOR_LENGTH:
> +   clauses = c_parser_omp_clause_vector_length (parser, clauses);
> +   c_name = "vector_length";
> +   break;

That is OpenACC clause, right?  Shouldn't the routine be called
c_parser_oacc_clause_vector_length?

> + case PRAGMA_OMP_CLAUSE_WAIT:
> +   clauses = c_parser_oacc_clause_wait (parser, clauses);

E.g. c_parser_oacc_clause_wait is.

> +  clauses =  c_parser_oacc_all_clauses (parser, OACC_DATA_CLAUSE_MASK,
> + "#pragma acc data");

Too many spaces after =.

> +/* OpenACC 2.0:
> +   # pragma acc kernels oacc-kernels-clause[optseq] new-line
> + structured-block
> +
> +   LOC is the location of the #pragma token.

Again, what is LOC?

> +  clauses =  c_parser_oacc_all_clauses (parser, OACC_KERNELS_CLAUSE_MASK,
> + p_name);

See above.

> +  c_parser_error (parser, enter
> +   ? "expected % in %<#pragma acc enter data%>"
> +   : "expected % in %<#pragma acc exit data%>");
> +  c_parser_skip_to_pragma_eol (parser);
> +  return;
> +}
> +
> +  const char *p = IDENTIFIER_POINTER (c_parser_peek_token (parser)->value);
> +  if (strcmp (p, "data") != 0)
> +{
> +  c_parser_error (parser, "invalid pragma");

See the C++ patch review.

> +  if (find_omp_clause (clauses, OMP_CLAUSE_MAP) == NULL_TREE)
> +{
> +  error_at (loc, enter
> + ? "%<#pragma acc enter data%> has no data movement clause"
> + : "%<#pragma acc exit data%> has no data movement clause");

Similarly (though, in this case C++ had unconditional acc enter data).

> +  clauses =  c_parser_oacc_all_clauses (parser, OACC_PARALLEL_CLAUSE_MASK,
> + p_name);

See above.

> +  if (find_omp_clause (clauses, OMP_CLAUSE_MAP) == NULL_TREE)
> +{
> +  error_at (loc,
> + 

Re: [PATCH] OpenACC for C++ front end

2014-11-13 Thread Jakub Jelinek
On Wed, Nov 05, 2014 at 03:37:08PM -0600, James Norris wrote:
> 2014-11-05  James Norris  
>   Cesar Philippidis  
>   Thomas Schwinge  
>   Ilmir Usmanov  

...

Please check formatting.  I see various spots with 8 spaces instead of tabs,
e.g. on
+  check_no_duplicate_clause (list, OMP_CLAUSE_VECTOR_LENGTH,
+"vector_length", location);
even the alignment is wrong, I see calls without space before (:
+  if (args == NULL || args->length() == 0)
...
+ error("% expression must be integral");
other spots where the alignment isn't right:
+static tree
+cp_parser_oacc_cache (cp_parser *parser,
+   cp_token *pragma_tok __attribute__((unused)))
(cp_token should be below cp_parser).  While at this,
__attribute__((unused)) should be replaced by ATTRIBUTE_UNUSED, or removing
the parameter name, or removing the parameter altogether even better.
For the formatting issues, you can easily look for them in the patch
(in lines starting with +), change the patch and apply interdiff to your
tree.

> --- a/gcc/c-family/c-omp.c
> +++ b/gcc/c-family/c-omp.c
> @@ -29,8 +29,47 @@ along with GCC; see the file COPYING3.  If not see
>  #include "c-pragma.h"
>  #include "gimple-expr.h"
>  #include "langhooks.h"
> +#include "omp-low.h"

As Thomas? has said, you should include gomp-constants.h and use them in:

> +t = build_int_cst (integer_type_node, -2);  /* TODO: XXX FIX -2.  */

spots like this.

> --- a/gcc/c-family/c-pragma.c
> +++ b/gcc/c-family/c-pragma.c
> @@ -1180,6 +1180,16 @@ typedef struct
>  static vec registered_pp_pragmas;
>  
>  struct omp_pragma_def { const char *name; unsigned int id; };
> +static const struct omp_pragma_def oacc_pragmas[] = {
> +  { "data", PRAGMA_OACC_DATA },
> +  { "enter", PRAGMA_OACC_ENTER_DATA },
> +  { "exit", PRAGMA_OACC_EXIT_DATA },
> +  { "kernels", PRAGMA_OACC_KERNELS },
> +  { "loop", PRAGMA_OACC_LOOP },
> +  { "parallel", PRAGMA_OACC_PARALLEL },
> +  { "update", PRAGMA_OACC_UPDATE },
> +  { "wait", PRAGMA_OACC_WAIT },

I'd avoid the , after the last element.

> @@ -1383,6 +1402,17 @@ c_invoke_pragma_handler (unsigned int id)
>  void
>  init_pragma (void)
>  {
> +  if (flag_openacc)
> +{
> +  const int n_oacc_pragmas
> + = sizeof (oacc_pragmas) / sizeof (*oacc_pragmas);
> +  int i;
> +
> +  for (i = 0; i < n_oacc_pragmas; ++i)
> + cpp_register_deferred_pragma (parse_in, "acc", oacc_pragmas[i].name,
> +   oacc_pragmas[i].id, true, true);
> +}
> +
>if (flag_openmp)
>  {
>const int n_omp_pragmas = sizeof (omp_pragmas) / sizeof (*omp_pragmas);

Is -fopenmp -fopenacc tested not to run out of number of supported pragmas
by libcpp?

> @@ -65,23 +74,30 @@ typedef enum pragma_kind {
>  } pragma_kind;
>  
>  
> -/* All clauses defined by OpenMP 2.5, 3.0, 3.1 and 4.0.
> +/* All clauses defined by OpenACC 2.0, and OpenMP 2.5, 3.0, 3.1, and 4.0.
> Used internally by both C and C++ parsers.  */
>  typedef enum pragma_omp_clause {
>PRAGMA_OMP_CLAUSE_NONE = 0,
>  
>PRAGMA_OMP_CLAUSE_ALIGNED,
> +  PRAGMA_OMP_CLAUSE_ASYNC,
>PRAGMA_OMP_CLAUSE_COLLAPSE,
> +  PRAGMA_OMP_CLAUSE_COPY,
>PRAGMA_OMP_CLAUSE_COPYIN,
> +  PRAGMA_OMP_CLAUSE_COPYOUT,
>PRAGMA_OMP_CLAUSE_COPYPRIVATE,
> +  PRAGMA_OMP_CLAUSE_CREATE,
>PRAGMA_OMP_CLAUSE_DEFAULT,
> +  PRAGMA_OMP_CLAUSE_DELETE,
>PRAGMA_OMP_CLAUSE_DEPEND,
>PRAGMA_OMP_CLAUSE_DEVICE,
> +  PRAGMA_OMP_CLAUSE_DEVICEPTR,
>PRAGMA_OMP_CLAUSE_DIST_SCHEDULE,
>PRAGMA_OMP_CLAUSE_FINAL,
>PRAGMA_OMP_CLAUSE_FIRSTPRIVATE,
>PRAGMA_OMP_CLAUSE_FOR,
>PRAGMA_OMP_CLAUSE_FROM,
> +  PRAGMA_OMP_CLAUSE_HOST,
>PRAGMA_OMP_CLAUSE_IF,
>PRAGMA_OMP_CLAUSE_INBRANCH,
>PRAGMA_OMP_CLAUSE_LASTPRIVATE,
> @@ -90,16 +106,24 @@ typedef enum pragma_omp_clause {
>PRAGMA_OMP_CLAUSE_MERGEABLE,
>PRAGMA_OMP_CLAUSE_NOTINBRANCH,
>PRAGMA_OMP_CLAUSE_NOWAIT,
> +  PRAGMA_OMP_CLAUSE_NUM_GANGS,
>PRAGMA_OMP_CLAUSE_NUM_TEAMS,
>PRAGMA_OMP_CLAUSE_NUM_THREADS,
> +  PRAGMA_OMP_CLAUSE_NUM_WORKERS,
>PRAGMA_OMP_CLAUSE_ORDERED,
>PRAGMA_OMP_CLAUSE_PARALLEL,
> +  PRAGMA_OMP_CLAUSE_PRESENT,
> +  PRAGMA_OMP_CLAUSE_PRESENT_OR_COPY,
> +  PRAGMA_OMP_CLAUSE_PRESENT_OR_COPYIN,
> +  PRAGMA_OMP_CLAUSE_PRESENT_OR_COPYOUT,
> +  PRAGMA_OMP_CLAUSE_PRESENT_OR_CREATE,
>PRAGMA_OMP_CLAUSE_PRIVATE,
>PRAGMA_OMP_CLAUSE_PROC_BIND,
>PRAGMA_OMP_CLAUSE_REDUCTION,
>PRAGMA_OMP_CLAUSE_SAFELEN,
>PRAGMA_OMP_CLAUSE_SCHEDULE,
>PRAGMA_OMP_CLAUSE_SECTIONS,
> +  PRAGMA_OMP_CLAUSE_SELF,
>PRAGMA_OMP_CLAUSE_SHARED,
>PRAGMA_OMP_CLAUSE_SIMDLEN,
>PRAGMA_OMP_CLAUSE_TASKGROUP,
> @@ -107,6 +131,8 @@ typedef enum pragma_omp_clause {
>PRAGMA_OMP_CLAUSE_TO,
>PRAGMA_OMP_CLAUSE_UNIFORM,
>PRAGMA_OMP_CLAUSE_UNTIED,
> +  PRAGMA_OMP_CLAUSE_VECTOR_LENGTH,
> +  PRAGMA_OMP_CLAUSE_WAIT,

Like for CILK, I'd strongly prefer if for the clauses that are
specific to OpenACC 

Re: [PATCH] OpenACC for C++ front end

2014-11-06 Thread Thomas Schwinge
Hi!

On Wed, 5 Nov 2014 21:41:02 +, Joseph Myers  wrote:
> I think the TODO: XXX FIX -2 and TODO XXX: FIX -1 comments need, at least, 
> more explanation of what the issue is and where the constants come from, 
> even if something is left until later to be fixed.

Such constants should go into include/gomp-constants.h.


Jim, for avoidance of doubt, please commit any such review changes to
gomp-4_0-branch, so that the (revised) patches that you submit and the
actual diff from the last merge of trunk into gomp-4_0-branch and
gomp-4_0-branch's current head are as similar as possible; preferably
you're submitting exactly (a relevant part of) that diff.


Grüße,
 Thomas


signature.asc
Description: PGP signature


Re: [PATCH] OpenACC for C++ front end

2014-11-05 Thread Joseph Myers
I think the TODO: XXX FIX -2 and TODO XXX: FIX -1 comments need, at least, 
more explanation of what the issue is and where the constants come from, 
even if something is left until later to be fixed.

-- 
Joseph S. Myers
jos...@codesourcery.com