Hi Nathan!

On Mon, 2 Nov 2015 11:18:37 -0500, Nathan Sidwell <nat...@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

Reply via email to