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
