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