diff mbox series

[OpenMP,5.0] Allow duplicate mapping of variables

Message ID e6634bec-616c-9fb2-4289-64fad8fe0826@codesourcery.com
State New
Headers show
Series [OpenMP,5.0] Allow duplicate mapping of variables | expand

Commit Message

Chung-Lin Tang Dec. 16, 2020, 3:05 p.m. UTC
Hi Jakub,
when running sollve_vv tests (which we're trying to make work out-of-the-box),
some of the 5.0 tests which were testing map clause ordering also required
supporting mapping the same variable multiple times.

This was not allowed before, so besides the small changes needed in the
front-ends, the omp-lowering conventions for field lookup had to be adjusted.
For map clauses, we now use the whole OMP_CLAUSE expression to lookup
fields, instead of the variable DECL, which will clash and ICE when
multiple occurrences happen.

The modifications OpenACC firstprivate variables exist because they seem
to share code with normal map handling in many ways, so they were adjusted
to use the clause-as-key convention too (including the adjustments in
lower_oacc_reductions).

This has been tested for no regressions for gcc, g++, gfortran, and libgomp.
Is this okay for trunk after stage1 reopens?

Thanks,
Chung-Lin

2020-12-16  Chung-Lin Tang  <cltang@codesourcery.com>

	gcc/c/
	* c-typeck.c (c_finish_omp_clauses): Adjust to allow duplicate
	mapped variables for OpenMP.

	gcc/cp/
	* semantics.c (finish_omp_clauses):  Adjust to allow duplicate
	mapped variables for OpenMP.

	gcc/
	* omp-low.c (install_var_field): Add new 'tree key_expr = NULL_TREE'
	default parameter. Set splay-tree lookup key to key_expr instead of
	var if key_expr is non-NULL.
	(scan_sharing_clauses): Use clause tree expression as splay-tree key
	for map/to/from and OpenACC firstprivate cases when installing the
	variable field into the send/receive record type.
	(lower_oacc_reductions): Adjust to find map-clause of reduction
	variable, then create receiver-ref.
	(lower_omp_target): Adjust to lookup var field using clause expression.

	gcc/testsuite/
	* c-c++-common/gomp/clauses-2.c: Adjust testcase.
diff mbox series

Patch

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 7d58e8de342..6bf57ae2ba1 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -14776,12 +14776,12 @@  c_finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 		}
 	      else
 		bitmap_set_bit (&generic_head, DECL_UID (t));
 	    }
 	  else if (bitmap_bit_p (&map_head, DECL_UID (t))
-		   && (ort != C_ORT_OMP
-		       || !bitmap_bit_p (&map_field_head, DECL_UID (t))))
+		   && !bitmap_bit_p (&map_field_head, DECL_UID (t))
+		   && ort != C_ORT_OMP)
 	    {
 	      if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_MAP)
 		error_at (OMP_CLAUSE_LOCATION (c),
 			  "%qD appears more than once in motion clauses", t);
 	      else if (ort == C_ORT_ACC)
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 92e32c8e0ad..4159b770613 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -7643,11 +7643,12 @@  finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 		}
 	      else
 		bitmap_set_bit (&generic_head, DECL_UID (t));
 	    }
 	  else if (bitmap_bit_p (&map_head, DECL_UID (t))
-		   && !bitmap_bit_p (&map_field_head, DECL_UID (t)))
+		   && !bitmap_bit_p (&map_field_head, DECL_UID (t))
+		   && ort != C_ORT_OMP)
 	    {
 	      if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_MAP)
 		error_at (OMP_CLAUSE_LOCATION (c),
 			  "%qD appears more than once in motion clauses", t);
 	      else if (ort == C_ORT_ACC)
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index 91a5e3d1431..f8109d9752a 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -761,24 +761,31 @@  build_sender_ref (tree var, omp_context *ctx)
 
 /* Add a new field for VAR inside the structure CTX->SENDER_DECL.  If
    BASE_POINTERS_RESTRICT, declare the field with restrict.  */
 
 static void
-install_var_field (tree var, bool by_ref, int mask, omp_context *ctx)
+install_var_field (tree var, bool by_ref, int mask, omp_context *ctx,
+		   tree key_expr = NULL_TREE)
 {
   tree field, type, sfield = NULL_TREE;
   splay_tree_key key = (splay_tree_key) var;
 
-  if ((mask & 16) != 0)
-    {
-      key = (splay_tree_key) &DECL_NAME (var);
-      gcc_checking_assert (key != (splay_tree_key) var);
-    }
-  if ((mask & 8) != 0)
+  if (key_expr)
+    /* Allow user to explicitly set the expression used as the key.  */
+    key = (splay_tree_key) key_expr;
+  else
     {
-      key = (splay_tree_key) &DECL_UID (var);
-      gcc_checking_assert (key != (splay_tree_key) var);
+      if ((mask & 16) != 0)
+	{
+	  key = (splay_tree_key) &DECL_NAME (var);
+	  gcc_checking_assert (key != (splay_tree_key) var);
+	}
+      if ((mask & 8) != 0)
+	{
+	  key = (splay_tree_key) &DECL_UID (var);
+	  gcc_checking_assert (key != (splay_tree_key) var);
+	}
     }
   gcc_assert ((mask & 1) == 0
 	      || !splay_tree_lookup (ctx->field_map, key));
   gcc_assert ((mask & 2) == 0 || !ctx->sfield_map
 	      || !splay_tree_lookup (ctx->sfield_map, key));
@@ -1306,11 +1313,18 @@  scan_sharing_clauses (tree clauses, omp_context *ctx)
 	  if ((OMP_CLAUSE_CODE (c) == OMP_CLAUSE_FIRSTPRIVATE
 	       || OMP_CLAUSE_CODE (c) == OMP_CLAUSE_IS_DEVICE_PTR)
 	      && is_gimple_omp_offloaded (ctx->stmt))
 	    {
 	      if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_FIRSTPRIVATE)
-		install_var_field (decl, !omp_is_reference (decl), 3, ctx);
+		{
+		  /* OpenACC firstprivate clauses are later processed with same
+		     code path as map clauses in lower_omp_target, so follow
+		     the same convention of using the whole clause expression
+		     as splay-tree key.  */
+		  tree k = (is_gimple_omp_oacc (ctx->stmt) ? c : NULL_TREE);
+		  install_var_field (decl, !omp_is_reference (decl), 3, ctx, k);
+		}
 	      else if (TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
 		install_var_field (decl, true, 3, ctx);
 	      else
 		install_var_field (decl, false, 3, ctx);
 	    }
@@ -1517,23 +1531,23 @@  scan_sharing_clauses (tree clauses, omp_context *ctx)
 		{
 		  tree decl2 = DECL_VALUE_EXPR (decl);
 		  gcc_assert (TREE_CODE (decl2) == INDIRECT_REF);
 		  decl2 = TREE_OPERAND (decl2, 0);
 		  gcc_assert (DECL_P (decl2));
-		  install_var_field (decl2, true, 3, ctx);
+		  install_var_field (decl2, true, 3, ctx, c);
 		  install_var_local (decl2, ctx);
 		  install_var_local (decl, ctx);
 		}
 	      else
 		{
 		  if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
 		      && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_POINTER
 		      && !OMP_CLAUSE_MAP_ZERO_BIAS_ARRAY_SECTION (c)
 		      && TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
-		    install_var_field (decl, true, 7, ctx);
+		    install_var_field (decl, true, 7, ctx, c);
 		  else
-		    install_var_field (decl, true, 3, ctx);
+		    install_var_field (decl, true, 3, ctx, c);
 		  if (is_gimple_omp_offloaded (ctx->stmt)
 		      && !OMP_CLAUSE_MAP_IN_REDUCTION (c))
 		    install_var_local (decl, ctx);
 		}
 	    }
@@ -1563,11 +1577,11 @@  scan_sharing_clauses (tree clauses, omp_context *ctx)
 		  tree field
 		    = build_decl (OMP_CLAUSE_LOCATION (c),
 				  FIELD_DECL, NULL_TREE, ptr_type_node);
 		  SET_DECL_ALIGN (field, TYPE_ALIGN (ptr_type_node));
 		  insert_field_into_struct (ctx->record_type, field);
-		  splay_tree_insert (ctx->field_map, (splay_tree_key) decl,
+		  splay_tree_insert (ctx->field_map, (splay_tree_key) c,
 				     (splay_tree_value) field);
 		}
 	    }
 	  break;
 
@@ -7019,10 +7033,11 @@  lower_oacc_reductions (location_t loc, tree clauses, tree level, bool inner,
 	gcc_checking_assert (!is_oacc_kernels (ctx));
 	/* Likewise, on OpenACC 'kernels' decomposed parts.  */
 	gcc_checking_assert (!is_oacc_kernels_decomposed_part (ctx));
 
 	tree orig = OMP_CLAUSE_DECL (c);
+	tree orig_clause;
 	tree var = maybe_lookup_decl (orig, ctx);
 	tree ref_to_res = NULL_TREE;
 	tree incoming, outgoing, v1, v2, v3;
 	bool is_private = false;
 
@@ -7089,14 +7104,24 @@  lower_oacc_reductions (location_t loc, tree clauses, tree level, bool inner,
 	      }
 
 	  do_lookup:
 	    /* This is the outermost construct with this reduction,
 	       see if there's a mapping for it.  */
-	    if (gimple_code (outer->stmt) == GIMPLE_OMP_TARGET
-		&& maybe_lookup_field (orig, outer) && !is_private)
+	    orig_clause = NULL_TREE;
+	    if (gimple_code (outer->stmt) == GIMPLE_OMP_TARGET)
+	      for (tree cls = gimple_omp_target_clauses (outer->stmt);
+		   cls; cls = OMP_CLAUSE_CHAIN (cls))
+		if (OMP_CLAUSE_CODE (cls) == OMP_CLAUSE_MAP
+		    && orig == OMP_CLAUSE_DECL (cls)
+		    && maybe_lookup_field (cls, outer))
+		  {
+		    orig_clause = cls;
+		    break;
+		  }
+	    if (orig_clause != NULL_TREE && !is_private)
 	      {
-		ref_to_res = build_receiver_ref (orig, false, outer);
+		ref_to_res = build_receiver_ref (orig_clause, false, outer);
 		if (omp_is_reference (orig))
 		  ref_to_res = build_simple_mem_ref (ref_to_res);
 
 		tree type = TREE_TYPE (var);
 		if (POINTER_TYPE_P (type))
@@ -11907,19 +11932,19 @@  lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx)
 	    gcc_assert (maybe_lookup_field (c, ctx));
 	    map_cnt++;
 	    continue;
 	  }
 
-	if (!maybe_lookup_field (var, ctx))
+	if (!maybe_lookup_field (c, ctx))
 	  continue;
 
 	/* Don't remap compute constructs' reduction variables, because the
 	   intermediate result must be local to each gang.  */
 	if (offloaded && !(OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
 			   && OMP_CLAUSE_MAP_IN_REDUCTION (c)))
 	  {
-	    x = build_receiver_ref (var, true, ctx);
+	    x = build_receiver_ref (c, true, ctx);
 	    tree new_var = lookup_decl (var, ctx);
 
 	    if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
 		&& OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_POINTER
 		&& !OMP_CLAUSE_MAP_ZERO_BIAS_ARRAY_SECTION (c)
@@ -12137,11 +12162,11 @@  lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx)
 		    nc = OMP_CLAUSE_CHAIN (c);
 		    ovar = OMP_CLAUSE_DECL (nc);
 		  }
 		else
 		  {
-		    tree x = build_sender_ref (ovar, ctx);
+		    tree x = build_sender_ref (c, ctx);
 		    tree v
 		      = build_fold_addr_expr_with_type (ovar, ptr_type_node);
 		    gimplify_assign (x, v, &ilist);
 		    nc = NULL_TREE;
 		  }
@@ -12155,11 +12180,11 @@  lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx)
 		    gcc_assert (TREE_CODE (ovar2) == INDIRECT_REF);
 		    ovar2 = TREE_OPERAND (ovar2, 0);
 		    gcc_assert (DECL_P (ovar2));
 		    ovar = ovar2;
 		  }
-		if (!maybe_lookup_field (ovar, ctx)
+		if (!maybe_lookup_field (c, ctx)
 		    && !(OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
 			 && (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_ATTACH
 			     || OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_DETACH)))
 		  continue;
 	      }
@@ -12179,11 +12204,11 @@  lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx)
 		gimplify_assign (x, build_fold_addr_expr (var), &ilist);
 	      }
 	    else if (nc)
 	      {
 		var = lookup_decl_in_outer_ctx (ovar, ctx);
-		x = build_sender_ref (ovar, ctx);
+		x = build_sender_ref (c, ctx);
 
 		if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
 		    && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_POINTER
 		    && !OMP_CLAUSE_MAP_ZERO_BIAS_ARRAY_SECTION (c)
 		    && TREE_CODE (TREE_TYPE (ovar)) == ARRAY_TYPE)
@@ -12877,11 +12902,11 @@  lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx)
 		    decl2 = TREE_OPERAND (decl2, 0);
 		    gcc_assert (DECL_P (decl2));
 		    new_var = decl2;
 		    type = TREE_TYPE (new_var);
 		  }
-		x = build_receiver_ref (OMP_CLAUSE_DECL (prev), false, ctx);
+		x = build_receiver_ref (prev, false, ctx);
 		x = fold_convert_loc (clause_loc, type, x);
 		if (!integer_zerop (OMP_CLAUSE_SIZE (c)))
 		  {
 		    tree bias = OMP_CLAUSE_SIZE (c);
 		    if (DECL_P (bias))
diff --git a/gcc/testsuite/c-c++-common/gomp/clauses-2.c b/gcc/testsuite/c-c++-common/gomp/clauses-2.c
index bbc8fb4e32b..f1ce7399310 100644
--- a/gcc/testsuite/c-c++-common/gomp/clauses-2.c
+++ b/gcc/testsuite/c-c++-common/gomp/clauses-2.c
@@ -13,11 +13,11 @@  foo (int *p, int q, struct S t, int i, int j, int k, int l)
     bar (p);
   #pragma omp target map (p[0]) map (p) /* { dg-error "appears both in data and map clauses" } */
     bar (p);
   #pragma omp target map (p) , map (p[0])
     bar (p);
-  #pragma omp target map (q) map (q) /* { dg-error "appears more than once in map clauses" } */
+  #pragma omp target map (q) map (q)
     bar (&q);
   #pragma omp target map (p[0]) map (p[0]) /* { dg-error "appears more than once in data clauses" } */
     bar (p);
   #pragma omp target map (t) map (t.r)
     bar (&t.r);