Discussion:
[patch] various OpenACC reduction enhancements - FE changes
Cesar Philippidis
2018-06-29 18:22:00 UTC
Permalink
Attaches are the FE changes for the OpenACC reduction enhancements. It
depends on the ME patch.

Is this patch OK for trunk? It bootstrapped / regression tested cleanly
for x86_64 with nvptx offloading.

Thanks,
Cesar
Jakub Jelinek
2018-12-04 12:57:24 UTC
Permalink
gcc/c/
* c-parser.c (c_parser_omp_variable_list): New c_omp_region_type
argument. Use it to specialize handling of OMP_CLAUSE_REDUCTION for
OpenACC.
(c_parser_omp_clause_reduction): Update call to
c_parser_omp_variable_list. Propage OpenACC errors as necessary.
(c_parser_oacc_all_clauses): Update call to
p_parser_omp_clause_reduction.
(c_parser_omp_all_clauses): Likewise.
* c-typeck.c (c_finish_omp_clauses): Emit an error on orphan OpenACC
gang reductions.
gcc/cp/
* parser.c (cp_parser_omp_var_list_no_open): New c_omp_region_type
argument. Use it to specialize handling of OMP_CLAUSE_REDUCTION for
OpenACC.
(cp_parser_omp_clause_reduction): Update call to
cp_parser_omp_variable_list. Propage OpenACC errors as necessary.
(cp_parser_oacc_all_clauses): Update call to
cp_parser_omp_clause_reduction.
(cp_parser_omp_all_clauses): Likewise.
* semantics.c (finish_omp_clauses): Emit an error on orphan OpenACC
gang reductions.
gcc/fortran/
* openmp.c (resolve_oacc_loop_blocks): Emit an error on orphan OpenACC
gang reductions.
* trans-openmp.c (gfc_omp_clause_copy_ctor): Permit reductions.
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 7a926285f3a..a6f453dae54 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -965,12 +965,13 @@ class token_pair
/* Like token_pair::require_close, except that tokens will be skipped
until the desired token is found. An error message is still produced
- if the next token is not as expected. */
+ if the next token is not as expected, unless QUIET is set. */
- void skip_until_found_close (c_parser *parser) const
+ void skip_until_found_close (c_parser *parser, bool quiet = false) const
{
c_parser_skip_until_found (parser, traits_t::close_token_type,
- traits_t::close_gmsgid, m_open_loc);
+ quiet ? NULL : traits_t::close_gmsgid,
+ m_open_loc);
}
I don't like these changes, why do you need them? C++ FE doesn't have such
changes either, and it is fine to diagnose missing ) even if there was some
earlier error. All other spots which require matching parens do it the
same. Please leave those out.

static tree
-c_parser_omp_clause_reduction (c_parser *parser, tree list)
+c_parser_omp_clause_reduction (c_parser *parser, tree list,
+ enum c_omp_region_type ort)

Note, the signature is now different, it is ok to replace is_omp argument
with enum c_omp_region_type if you wish.
{
location_t clause_loc = c_parser_peek_token (parser)->location;
+ bool seen_error = false;
+
matching_parens parens;
if (parens.require_open (parser))
{
@@ -12855,7 +12876,13 @@ c_parser_omp_clause_reduction (c_parser *parser, tree list)
tree nl, c;
nl = c_parser_omp_variable_list (parser, clause_loc,
- OMP_CLAUSE_REDUCTION, list);
+ OMP_CLAUSE_REDUCTION, list, ort);
+ if (c_parser_peek_token (parser)->type != CPP_CLOSE_PAREN)
+ {
+ seen_error = true;
+ goto cleanup;
+ }
+
for (c = nl; c != list; c = OMP_CLAUSE_CHAIN (c))
{
tree d = OMP_CLAUSE_DECL (c), type;
@@ -12891,7 +12918,8 @@ c_parser_omp_clause_reduction (c_parser *parser, tree list)
list = nl;
}
- parens.skip_until_found_close (parser);
+ parens.skip_until_found_close (parser, seen_error);
}
return list;
}
And the above hunks as well.
@@ -13998,7 +14026,7 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask,
c_name = "private";
break;
- clauses = c_parser_omp_clause_reduction (parser, clauses);
+ clauses = c_parser_omp_clause_reduction (parser, clauses, C_ORT_ACC);
c_name = "reduction";
break;
@@ -14157,7 +14185,7 @@ c_parser_omp_all_clauses (c_parser *parser, omp_clause_mask mask,
c_name = "private";
break;
- clauses = c_parser_omp_clause_reduction (parser, clauses);
+ clauses = c_parser_omp_clause_reduction (parser, clauses, C_ORT_OMP);
c_name = "reduction";
break;
Note, there are now also the IN_REDUCTION/TASK_REDUCTION clause cases that
need adjustment.
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 90ae306c99a..944db3fa8be 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -13087,6 +13087,14 @@ c_finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
goto check_dup_generic;
+ if (ort == C_ORT_ACC && oacc_get_fn_attrib (current_function_decl)
+ && omp_find_clause (clauses, OMP_CLAUSE_GANG))
This is expensive if there are many clauses, we want to avoid O(n^2)
behavior. For C we only have one loop, so just remember in some variable
whether there are reduction clause(s) that would conflict with gang, and
in another one whether gang clause has been seen, then deal with it at the
end if both the bools are true.
+ {
+ error_at (OMP_CLAUSE_LOCATION (c),
+ "gang reduction on an orphan loop");
+ remove = true;
+ break;
+ }
need_implicitly_determined = true;
t = OMP_CLAUSE_DECL (c);
if (TREE_CODE (t) == TREE_LIST)
@@ -31668,6 +31669,21 @@ cp_parser_omp_var_list_no_open (cp_parser *parser, enum omp_clause_code kind,
/* FALLTHROUGH. */
+ if (kind == OMP_CLAUSE_REDUCTION && ort == C_ORT_ACC)
+ {
+ switch (cp_lexer_peek_token (parser->lexer)->type)
+ {
+ error ("invalid reduction variable");
+ decl = error_mark_node;
+ goto skip_comma;
+ default:;
+ break;
+ }
+ }
Any reason for the above (ditto in C), rather than just adding
&& ort != C_ORT_ACC to the while loop condition for CPP_OPEN_SQUARE?
(, . or * after id-expression is like any other unhandled characters...
while (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_SQUARE))
{
tree low_bound = NULL_TREE, length = NULL_TREE;
@@ -33868,7 +33885,7 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask,
c_name = "private";
break;
- clauses = cp_parser_omp_clause_reduction (parser, clauses);
+ clauses = cp_parser_omp_clause_reduction (parser, clauses, C_ORT_ACC);
c_name = "reduction";
break;
@@ -34055,7 +34072,7 @@ cp_parser_omp_all_clauses (cp_parser *parser, omp_clause_mask mask,
c_name = "private";
break;
- clauses = cp_parser_omp_clause_reduction (parser, clauses);
+ clauses = cp_parser_omp_clause_reduction (parser, clauses, C_ORT_OMP);
c_name = "reduction";
break;
Again, needs adjustement for IN_REDUCTION/TASK_REDUCTION.
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index c779137da45..177acdd9cc4 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -5875,6 +5875,14 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
field_ok = ((ort & C_ORT_OMP_DECLARE_SIMD) == C_ORT_OMP);
goto check_dup_generic;
+ if (ort == C_ORT_ACC && oacc_get_fn_attrib (current_function_decl)
+ && omp_find_clause (clauses, OMP_CLAUSE_GANG))
+ {
+ error_at (OMP_CLAUSE_LOCATION (c),
+ "gang reduction on an orphan loop");
+ remove = true;
+ break;
+ }
field_ok = ((ort & C_ORT_OMP_DECLARE_SIMD) == C_ORT_OMP);
t = OMP_CLAUSE_DECL (c);
if (TREE_CODE (t) == TREE_LIST)
In C++ finish_omp_clauses there are 2 loops, so you can easily just remember
if OMP_CLAUSE_GANG has been seen in the first loop and diagnose this in the
second loop only.

Jakub

Loading...