[gomp4] Re: [1/3] OpenACC reductions

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi Nathan!

On Mon, 2 Nov 2015 11:18:37 -0500, Nathan Sidwell <nathan@acm.org> wrote:
> This is the core execution bits of OpenACC reductions.

> One thing not handled by this patch are reductions of variables of reference 
> type.  We have an implementation on gomp4 branch [...]

Trying to keep the existing code on gomp-4_0-branch alive, I merged your
trunk r229767 into gomp-4_0-branch in r229835.  To avoid regressions in
libgomp reduction execution tests, I had to apply one hack; please have a
look.  For your easier review, here is the merge commit in two variants,
first displayed as a three-way diff by Git's --cc option:

commit 2b76127eebddb59d45e5f068324e14efe77bb05c
Merge: bed2efe 641a0fa
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Fri Nov 6 09:33:40 2015 +0000

    svn merge -r 229764:229767 svn+ssh://gcc.gnu.org/svn/gcc/trunk
    
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@229835 138bc75d-0d04-0410-961f-82ee72b054a4


 gcc/ChangeLog   | 28 +++++++++++++++++++++++++++-
 gcc/omp-low.c   | 58 ++++++++++++++++++++++++++++++++++++++-------------------
 gcc/targhooks.h |  2 +-
 3 files changed, 67 insertions(+), 21 deletions(-)

diff --cc gcc/omp-low.c
index debedb1,6a0915b..da574a9
--- gcc/omp-low.c
+++ gcc/omp-low.c
@@@ -5441,14 -5306,25 +5441,28 @@@ lower_oacc_reductions (location_t loc, 
      if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_REDUCTION)
        {
  	tree orig = OMP_CLAUSE_DECL (c);
- 	tree var = OMP_CLAUSE_REDUCTION_PRIVATE_DECL (c);
 -	tree var = maybe_lookup_decl (orig, ctx);
++	tree var;
  	tree ref_to_res = NULL_TREE;
- 	
+ 	tree incoming, outgoing;
+ 
+ 	enum tree_code rcode = OMP_CLAUSE_REDUCTION_CODE (c);
+ 	if (rcode == MINUS_EXPR)
+ 	  rcode = PLUS_EXPR;
+ 	else if (rcode == TRUTH_ANDIF_EXPR)
+ 	  rcode = BIT_AND_EXPR;
+ 	else if (rcode == TRUTH_ORIF_EXPR)
+ 	  rcode = BIT_IOR_EXPR;
+ 	tree op = build_int_cst (unsigned_type_node, rcode);
+ 
++	var = OMP_CLAUSE_REDUCTION_PRIVATE_DECL (c);
 +	if (!var)
 +	  var = maybe_lookup_decl (orig, ctx);
  	if (!var)
  	  var = orig;
+ 	gcc_assert (!is_reference (var));
  
+ 	incoming = outgoing = var;
+ 	
  	if (!inner)
  	  {
  	    /* See if an outer construct also reduces this variable.  */
@@@ -5490,24 -5365,22 +5503,31 @@@
  	       see if there's a mapping for it.  */
  	    if (gimple_code (outer->stmt) == GIMPLE_OMP_TARGET
  		&& maybe_lookup_field (orig, outer))
- 	      ref_to_res = build_receiver_ref (orig, false, outer);
+ 	      {
+ 		ref_to_res = build_receiver_ref (orig, false, outer);
+ 		if (is_reference (orig))
+ 		  ref_to_res = build_simple_mem_ref (ref_to_res);
  
+ 		outgoing = var;
+ 		incoming = omp_reduction_init_op (loc, rcode, TREE_TYPE (var));
+ 	      }
++	    /* This is enabled on trunk, but has been disabled in the merge of
++	       trunk r229767 into gomp-4_0-branch, as otherwise there were a
++	       lot of regressions in libgomp reduction execution tests.  It is
++	       unclear if the problem is in the tests themselves, or here, or
++	       elsewhere.  Given the usage of "var =
++	       OMP_CLAUSE_REDUCTION_PRIVATE_DECL (c)" on gomp-4_0-branch, maybe
++	       we have to consider that here, too, instead of "orig"?  */
++#if 0
+ 	    else
+ 	      incoming = outgoing = orig;
++#endif
+ 	      
  	  has_outer_reduction:;
  	  }
- 	gcc_assert (!is_reference (var));
+ 
  	if (!ref_to_res)
  	  ref_to_res = integer_zero_node;
- 	else if (is_reference (orig))
- 	  ref_to_res = build_simple_mem_ref (ref_to_res);
- 
- 	enum tree_code rcode = OMP_CLAUSE_REDUCTION_CODE (c);
- 	if (rcode == MINUS_EXPR)
- 	  rcode = PLUS_EXPR;
- 	else if (rcode == TRUTH_ANDIF_EXPR)
- 	  rcode = BIT_AND_EXPR;
- 	else if (rcode == TRUTH_ORIF_EXPR)
- 	  rcode = BIT_IOR_EXPR;
- 	tree op = build_int_cst (unsigned_type_node, rcode);
  
  	/* Determine position in reduction buffer, which may be used
  	   by target.  */
diff --cc gcc/targhooks.h
index f8efe47a,c34e4ae..4a4496a
--- gcc/targhooks.h
+++ gcc/targhooks.h
@@@ -109,10 -109,9 +109,10 @@@ extern void default_finish_cost (void *
  extern void default_destroy_cost_data (void *);
  
  /* OpenACC hooks.  */
- extern void default_goacc_reduction (gcall *);
  extern bool default_goacc_validate_dims (tree, int [], int);
 +extern unsigned default_goacc_dim_limit (unsigned);
  extern bool default_goacc_fork_join (gcall *, const int [], bool);
+ extern void default_goacc_reduction (gcall *);
  
  /* These are here, and not in hooks.[ch], because not all users of
     hooks.h include tm.h, and thus we don't have CUMULATIVE_ARGS.  */

..., and second, as a "plain patch" (gomp-4_0-branch before vs. after):

--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -1,3 +1,29 @@
[...]
--- gcc/omp-low.c
+++ gcc/omp-low.c
@@ -5441,14 +5441,28 @@ lower_oacc_reductions (location_t loc, tree clauses, tree level, bool inner,
     if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_REDUCTION)
       {
 	tree orig = OMP_CLAUSE_DECL (c);
-	tree var = OMP_CLAUSE_REDUCTION_PRIVATE_DECL (c);
+	tree var;
 	tree ref_to_res = NULL_TREE;
-	
+	tree incoming, outgoing;
+
+	enum tree_code rcode = OMP_CLAUSE_REDUCTION_CODE (c);
+	if (rcode == MINUS_EXPR)
+	  rcode = PLUS_EXPR;
+	else if (rcode == TRUTH_ANDIF_EXPR)
+	  rcode = BIT_AND_EXPR;
+	else if (rcode == TRUTH_ORIF_EXPR)
+	  rcode = BIT_IOR_EXPR;
+	tree op = build_int_cst (unsigned_type_node, rcode);
+
+	var = OMP_CLAUSE_REDUCTION_PRIVATE_DECL (c);
 	if (!var)
 	  var = maybe_lookup_decl (orig, ctx);
 	if (!var)
 	  var = orig;
+	gcc_assert (!is_reference (var));
 
+	incoming = outgoing = var;
+	
 	if (!inner)
 	  {
 	    /* See if an outer construct also reduces this variable.  */
@@ -5485,29 +5499,35 @@ 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))
-	      ref_to_res = build_receiver_ref (orig, false, outer);
+	      {
+		ref_to_res = build_receiver_ref (orig, false, outer);
+		if (is_reference (orig))
+		  ref_to_res = build_simple_mem_ref (ref_to_res);
 
+		outgoing = var;
+		incoming = omp_reduction_init_op (loc, rcode, TREE_TYPE (var));
+	      }
+	    /* This is enabled on trunk, but has been disabled in the merge of
+	       trunk r229767 into gomp-4_0-branch, as otherwise there were a
+	       lot of regressions in libgomp reduction execution tests.  It is
+	       unclear if the problem is in the tests themselves, or here, or
+	       elsewhere.  Given the usage of "var =
+	       OMP_CLAUSE_REDUCTION_PRIVATE_DECL (c)" on gomp-4_0-branch, maybe
+	       we have to consider that here, too, instead of "orig"?  */
+#if 0
+	    else
+	      incoming = outgoing = orig;
+#endif
+	      
 	  has_outer_reduction:;
 	  }
-	gcc_assert (!is_reference (var));
+
 	if (!ref_to_res)
 	  ref_to_res = integer_zero_node;
-	else if (is_reference (orig))
-	  ref_to_res = build_simple_mem_ref (ref_to_res);
-
-	enum tree_code rcode = OMP_CLAUSE_REDUCTION_CODE (c);
-	if (rcode == MINUS_EXPR)
-	  rcode = PLUS_EXPR;
-	else if (rcode == TRUTH_ANDIF_EXPR)
-	  rcode = BIT_AND_EXPR;
-	else if (rcode == TRUTH_ORIF_EXPR)
-	  rcode = BIT_IOR_EXPR;
-	tree op = build_int_cst (unsigned_type_node, rcode);
 
 	/* Determine position in reduction buffer, which may be used
 	   by target.  */
@@ -5533,7 +5553,7 @@ lower_oacc_reductions (location_t loc, tree clauses, tree level, bool inner,
 	  = build_call_expr_internal_loc (loc, IFN_GOACC_REDUCTION,
 					  TREE_TYPE (var), 6, setup_code,
 					  unshare_expr (ref_to_res),
-					  var, level, op, off);
+					  incoming, level, op, off);
 	tree init_call
 	  = build_call_expr_internal_loc (loc, IFN_GOACC_REDUCTION,
 					  TREE_TYPE (var), 6, init_code,
@@ -5552,7 +5572,7 @@ lower_oacc_reductions (location_t loc, tree clauses, tree level, bool inner,
 	gimplify_assign (var, setup_call, &before_fork);
 	gimplify_assign (var, init_call, &after_fork);
 	gimplify_assign (var, fini_call, &before_join);
-	gimplify_assign (var, teardown_call, &after_join);
+	gimplify_assign (outgoing, teardown_call, &after_join);
       }
 
   /* Now stitch things together.  */
@@ -19549,7 +19569,7 @@ default_goacc_fork_join (gcall *ARG_UNUSED (call),
 
 /* Default goacc.reduction early expander.
 
-   LHS-opt = IFN_RED_<foo> (RES_PTR-opt, VAR, LEVEL, OP, LID, RID)
+   LHS-opt = IFN_REDUCTION (KIND, RES_PTR, VAR, LEVEL, OP, OFFSET)
    If RES_PTR is not integer-zerop:
        SETUP - emit 'LHS = *RES_PTR', LHS = NULL
        TEARDOWN - emit '*RES_PTR = VAR'
--- gcc/targhooks.h
+++ gcc/targhooks.h
@@ -109,10 +109,10 @@ extern void default_finish_cost (void *, unsigned *, unsigned *, unsigned *);
 extern void default_destroy_cost_data (void *);
 
 /* OpenACC hooks.  */
-extern void default_goacc_reduction (gcall *);
 extern bool default_goacc_validate_dims (tree, int [], int);
 extern unsigned default_goacc_dim_limit (unsigned);
 extern bool default_goacc_fork_join (gcall *, const int [], bool);
+extern void default_goacc_reduction (gcall *);
 
 /* These are here, and not in hooks.[ch], because not all users of
    hooks.h include tm.h, and thus we don't have CUMULATIVE_ARGS.  */


GrÃÃe
 Thomas

Attachment: signature.asc
Description: PGP signature