diff mbox series

openmp: Call c_omp_adjust_map_clauses even for combined target [PR100902]

Message ID 20210604121800.GT7746@tucnak
State New
Headers show
Series openmp: Call c_omp_adjust_map_clauses even for combined target [PR100902] | expand

Commit Message

Jakub Jelinek June 4, 2021, 12:18 p.m. UTC 
Hi!

When looking at in_reduction support for target, I've noticed that
c_omp_adjust_map_clauses is not called for the combined target case.

The following patch fixes it, will commit once tested.

Unfortunately, there are other issues.

One is (also mentioned in the PR) that currently the pointer attachment
stuff seems to be clause ordering dependent (the standard says that clause
ordering on the same construct does not matter), the baz and qux cases
in the PR are rejected while when swapped it is accepted.
Note, the order of clauses in GCC really is treated as insignificant
initially and only later on the compiler can adjust the ordering (e.g. when
we sort map clauses based on what they refer to etc.) and in particular,
clauses from parsing is reverse of the order in user code, while
c_omp_split_clauses performed for combined/composite constructs typically
reverses that ordering, i.e. makes it follow the user code ordering.

And another one is I'm slightly afraid c_omp_adjust_map_clauses might
misbehave in templates, though haven't tried to verify it with testcases.
When processing_template_decl, the non-dependent clauses will be handled
usually the same as when not in a template, but dependent clauses aren't
processed or only limited processing is done there, and rest is deferred
till later.  From quick skimming of c_omp_adjust_map_clauses, it seems
it might not be very happy about non-processed map clauses that might
still have the TREE_LIST representation of array sections, or might
not have finalized decls or base decls etc.
So, for this I wonder if cp_parser_omp_target (and other cp/parser.c
callers of c_omp_adjust_map_clauses) shouldn't call it only 
if (!processing_template_decl) - perhaps you could add
cp_omp_adjust_map_clauses wrapper that would be
if (!processing_template_decl)
  c_omp_adjust_map_clauses (...);
- and call c_omp_adjust_map_clauses from within pt.c after the clauses
are tsubsted and finish_omp_clauses is called again.

2021-06-04  Jakub Jelinek  <jakub@redhat.com>

	PR c/100902
	* c-parser.c (c_parser_omp_target): Call c_omp_adjust_map_clauses
	even when target is combined with other constructs.

	* parser.c (cp_parser_omp_target): Call c_omp_adjust_map_clauses
	even when target is combined with other constructs.

	* c-c++-common/gomp/pr100902-1.c: New test.


	Jakub
diff mbox series

Patch

--- gcc/c/c-parser.c.jj	2021-06-04 11:15:11.616690819 +0200
+++ gcc/c/c-parser.c	2021-06-04 13:15:42.788471162 +0200
@@ -20133,6 +20133,7 @@  c_parser_omp_target (c_parser *parser, e
 	  tree stmt = make_node (OMP_TARGET);
 	  TREE_TYPE (stmt) = void_type_node;
 	  OMP_TARGET_CLAUSES (stmt) = cclauses[C_OMP_CLAUSE_SPLIT_TARGET];
+	  c_omp_adjust_map_clauses (OMP_TARGET_CLAUSES (stmt), true);
 	  OMP_TARGET_BODY (stmt) = block;
 	  OMP_TARGET_COMBINED (stmt) = 1;
 	  SET_EXPR_LOCATION (stmt, loc);
--- gcc/cp/parser.c.jj	2021-05-31 10:11:15.145978965 +0200
+++ gcc/cp/parser.c	2021-06-04 13:17:00.952392489 +0200
@@ -42233,6 +42233,7 @@  cp_parser_omp_target (cp_parser *parser,
 	  tree stmt = make_node (OMP_TARGET);
 	  TREE_TYPE (stmt) = void_type_node;
 	  OMP_TARGET_CLAUSES (stmt) = cclauses[C_OMP_CLAUSE_SPLIT_TARGET];
+	  c_omp_adjust_map_clauses (OMP_TARGET_CLAUSES (stmt), true);
 	  OMP_TARGET_BODY (stmt) = body;
 	  OMP_TARGET_COMBINED (stmt) = 1;
 	  SET_EXPR_LOCATION (stmt, pragma_tok->location);
--- gcc/testsuite/c-c++-common/gomp/pr100902-1.c.jj	2021-06-04 13:13:48.471048762 +0200
+++ gcc/testsuite/c-c++-common/gomp/pr100902-1.c	2021-06-04 13:02:07.655723606 +0200
@@ -0,0 +1,17 @@ 
+/* PR c/100902 */
+
+void
+foo (int *ptr)
+{
+  #pragma omp target map (ptr, ptr[:4])
+  #pragma omp parallel master
+  ptr[0] = 1;
+}
+
+void
+bar (int *ptr)
+{
+  #pragma omp target parallel map (ptr[:4], ptr)
+  #pragma omp master
+  ptr[0] = 1;
+}