On Sat, Oct 24, 2015 at 02:11:41PM -0700, Cesar Philippidis wrote:
> @@ -29582,6 +29592,144 @@ cp_parser_oacc_data_clause_deviceptr (cp_parser 
> *parser, tree list)
>    return list;
>  }
>  
> +/* OpenACC 2.0:
> +   auto
> +   independent
> +   nohost
> +   seq */
> +
> +static tree
> +cp_parser_oacc_simple_clause (cp_parser *ARG_UNUSED (parser),
> +                           enum omp_clause_code code,
> +                           tree list, location_t location)
> +{
> +  check_no_duplicate_clause (list, code, omp_clause_code_name[code], 
> location);
> +  tree c = build_omp_clause (location, code);
> +  OMP_CLAUSE_CHAIN (c) = list;
> +  return c;

Here the PARSER argument is unconditionally unused, I'd use what is used
elsewhere, i.e. cp_parser * /* parser */,

> +           idx = 1;
> +           if (ops[idx] != NULL)
> +             {
> +               cp_parser_error (parser, "too many %<static%> arguements");

Typo, arguments.

> --- a/gcc/cp/semantics.c
> +++ b/gcc/cp/semantics.c
> @@ -5911,6 +5911,31 @@ finish_omp_clauses (tree clauses, bool allow_fields, 
> bool declare_simd)
>           bitmap_set_bit (&firstprivate_head, DECL_UID (t));
>         goto handle_field_decl;
>  
> +     case OMP_CLAUSE_GANG:
> +     case OMP_CLAUSE_VECTOR:
> +     case OMP_CLAUSE_WORKER:
> +       /* Operand 0 is the num: or length: argument.  */
> +       t = OMP_CLAUSE_OPERAND (c, 0);
> +       if (t == NULL_TREE)
> +         break;
> +
> +       if (!processing_template_decl)
> +         t = fold_build_cleanup_point_expr (TREE_TYPE (t), t);
> +       OMP_CLAUSE_OPERAND (c, 0) = t;
> +
> +       if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_GANG)
> +         break;

I think it would be better to do the Operand 1 stuff first for
case OMP_CLAUSE_GANG: only, and then have /* FALLTHRU */ into
case OMP_CLAUSE_{VECTOR,WORKER}: which would handle the first argument.

You should add testing that the operand has INTEGRAL_TYPE_P type
(except that for processing_template_decl it can be
type_dependent_expression_p instead of INTEGRAL_TYPE_P).

Also, the if (t == NULL_TREE) stuff looks fishy, because e.g. right now
if you have OMP_CLAUSE_GANG gang (static: expr) or similar,
you wouldn't wrap the expr into cleanup point.
So, instead it should be
  if (t)
    {
      if (t == error_mark_node)
        remove = true;
      else if (!type_dependent_expression_p (t)
                   && !INTEGRAL_TYPE_P (TREE_TYPE (t)))
        {
          error_at (OMP_CLAUSE_LOCATION (c), ...);
          remove = true;
        }
      else
        {
          t = mark_rvalue_use (t);
          if (!processing_template_decl)
            t = fold_build_cleanup_point_expr (TREE_TYPE (t), t);
          OMP_CLAUSE_OPERAND (c, 0) = t;
        }
    }
or so.  Also, can the expressions be arbitrary integers, or just
non-negative, or positive?  If it is INTEGER_CST, that is something that
could be checked here too.

>         else if (!type_dependent_expression_p (t)
>                  && !INTEGRAL_TYPE_P (TREE_TYPE (t)))
>           {
> -           error ("num_threads expression must be integral");
> +          switch (OMP_CLAUSE_CODE (c))
> +             {
> +             case OMP_CLAUSE_NUM_TASKS:
> +               error ("%<num_tasks%> expression must be integral"); break;
> +             case OMP_CLAUSE_NUM_TEAMS:
> +               error ("%<num_teams%> expression must be integral"); break;
> +             case OMP_CLAUSE_NUM_THREADS:
> +               error ("%<num_threads%> expression must be integral"); break;
> +             case OMP_CLAUSE_NUM_GANGS:
> +               error ("%<num_gangs%> expression must be integral"); break;
> +             case OMP_CLAUSE_NUM_WORKERS:
> +               error ("%<num_workers%> expression must be integral");
> +               break;
> +             case OMP_CLAUSE_VECTOR_LENGTH:
> +               error ("%<vector_length%> expression must be integral");
> +               break;

When touching these, can you please use error_at (OMP_CLAUSE_LOCATION (c),
instead of error ( ?

> +             default:
> +               error ("invalid argument");

What invalid argument?  I'd say that is clearly gcc_unreachable (); case.

But, I think it would be better to just use
  error_at (OMP_CLAUSE_LOCATION (c), "%qs expression must be integral",
            omp_clause_code_name[c]);

> -                   warning_at (OMP_CLAUSE_LOCATION (c), 0,
> -                               "%<num_threads%> value must be positive");
> +                   switch (OMP_CLAUSE_CODE (c))
> +                     {
> +                     case OMP_CLAUSE_NUM_TASKS:
> +                       warning_at (OMP_CLAUSE_LOCATION (c), 0,
> +                                   "%<num_tasks%> value must be positive");
> +                       break;
> +                     case OMP_CLAUSE_NUM_TEAMS:
> +                       warning_at (OMP_CLAUSE_LOCATION (c), 0,
> +                                   "%<num_teams%> value must be positive");
> +                       break;
> +                     case OMP_CLAUSE_NUM_THREADS:
> +                       warning_at (OMP_CLAUSE_LOCATION (c), 0,
> +                                   "%<num_threads%> value must be"
> +                                   "positive"); break;
> +                     case OMP_CLAUSE_NUM_GANGS:
> +                       warning_at (OMP_CLAUSE_LOCATION (c), 0,
> +                                   "%<num_gangs%> value must be positive");
> +                       break;
> +                     case OMP_CLAUSE_NUM_WORKERS:
> +                       warning_at (OMP_CLAUSE_LOCATION (c), 0,
> +                                   "%<num_workers%> value must be"
> +                                   "positive"); break;
> +                     case OMP_CLAUSE_VECTOR_LENGTH:
> +                       warning_at (OMP_CLAUSE_LOCATION (c), 0,
> +                                   "%<vector_length%> value must be"
> +                                   "positive"); break;
> +                     default:
> +                       error ("invalid argument");
> +                     }

And similarly here.

        Jakub

Reply via email to