This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[committed] Fix OpenMP final scan merging with hypothetical non-commutative combiner


Hi!

I've noticed I got the order of arguments in the last UDR combine operation
of worksharing scan wrong, var2 contains either the neutral element (first
thread) or the prefix sum from the original value up to the last iteration
before what the current thread handles and rprivb array contains the partial
prefix sum just for the iterations the current thread handled, so if
hypothetically user uses some non-commutative UDR which can still be used
for prefix sum and get some reasonable result, we get this combination
wrong.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
committed to trunk.

2019-07-06  Jakub Jelinek  <jakub@redhat.com>

	* omp-low.c (omp_find_scan): Make static.
	(lower_omp_for_scan): Fix order of merge arguments in input phase of
	the second loop, var2 represents the first partial sum and so needs
	to go before rprivb[ivar].

--- gcc/omp-low.c.jj	2019-07-04 23:38:56.100210759 +0200
+++ gcc/omp-low.c	2019-07-05 13:33:13.795296606 +0200
@@ -9104,7 +9104,7 @@ lower_omp_for_lastprivate (struct omp_fo
 
 /* Callback for walk_gimple_seq.  Find #pragma omp scan statement.  */
 
-tree
+static tree
 omp_find_scan (gimple_stmt_iterator *gsi_p, bool *handled_ops_p,
 	       struct walk_stmt_info *wi)
 {
@@ -9240,8 +9240,8 @@ omp_find_scan (gimple_stmt_iterator *gsi
    for (i = 0; i < n; i = i + 1)
      {
        {
-	 // For UDRs, this is UDR merge (rprivb[ivar], var2); r = rprivb[ivar];
-	 r = rprivb[ivar] + var2;
+	 // For UDRs, this is r = var2; UDR merge (r, rprivb[ivar]);
+	 r = var2 + rprivb[ivar];
        }
        {
 	 // This is the scan phase from user code.
@@ -9394,8 +9394,6 @@ lower_omp_for_scan (gimple_seq *body_p,
 	  {
 	    tree placeholder = OMP_CLAUSE_REDUCTION_PLACEHOLDER (c);
 	    tree val = var2;
-	    if (new_vard != new_var)
-	      val = build_fold_addr_expr_loc (clause_loc, val);
 
 	    x = lang_hooks.decls.omp_clause_default_ctor
 		    (c, var2, build_outer_var_ref (var, ctx));
@@ -9420,6 +9418,9 @@ lower_omp_for_scan (gimple_seq *body_p,
 		/* Otherwise, assign to it the identity element.  */
 		gimple_seq tseq = OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c);
 		tseq = copy_gimple_seq_and_replace_locals (tseq);
+
+		if (new_vard != new_var)
+		  val = build_fold_addr_expr_loc (clause_loc, val);
 		SET_DECL_VALUE_EXPR (new_vard, val);
 		DECL_HAS_VALUE_EXPR_P (new_vard) = 1;
 		SET_DECL_VALUE_EXPR (placeholder, error_mark_node);
@@ -9469,11 +9470,19 @@ lower_omp_for_scan (gimple_seq *body_p,
 	    x = lang_hooks.decls.omp_clause_assign_op (c, x, var2);
 	    gimplify_and_add (x, &mdlist);
 
+	    x = unshare_expr (new_var);
+	    x = lang_hooks.decls.omp_clause_assign_op (c, x, var2);
+	    gimplify_and_add (x, &input2_list);
+
+	    val = rprivb_ref;
+	    if (new_vard != new_var)
+	      val = build_fold_addr_expr_loc (clause_loc, val);
+
 	    tseq = OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c);
 	    tseq = copy_gimple_seq_and_replace_locals (tseq);
 	    SET_DECL_VALUE_EXPR (new_vard, val);
 	    DECL_HAS_VALUE_EXPR_P (new_vard) = 1;
-	    SET_DECL_VALUE_EXPR (placeholder, rprivb_ref);
+	    DECL_HAS_VALUE_EXPR_P (placeholder) = 0;
 	    lower_omp (&tseq, ctx);
 	    if (y)
 	      SET_DECL_VALUE_EXPR (new_vard, y);
@@ -9482,12 +9491,11 @@ lower_omp_for_scan (gimple_seq *body_p,
 		DECL_HAS_VALUE_EXPR_P (new_vard) = 0;
 		SET_DECL_VALUE_EXPR (new_vard, NULL_TREE);
 	      }
+	    SET_DECL_VALUE_EXPR (placeholder, new_var);
+	    DECL_HAS_VALUE_EXPR_P (placeholder) = 1;
+	    lower_omp (&tseq, ctx);
 	    gimple_seq_add_seq (&input2_list, tseq);
 
-	    x = unshare_expr (new_var);
-	    x = lang_hooks.decls.omp_clause_assign_op (c, x, rprivb_ref);
-	    gimplify_and_add (x, &input2_list);
-
 	    x = build_outer_var_ref (var, ctx);
 	    x = lang_hooks.decls.omp_clause_assign_op (c, x, rpriva_ref);
 	    gimplify_and_add (x, &last_list);
@@ -9545,7 +9553,7 @@ lower_omp_for_scan (gimple_seq *body_p,
 
 	    gimplify_assign (unshare_expr (rpriva_ref), var2, &mdlist);
 
-	    x = build2 (code, TREE_TYPE (new_var), rprivb_ref, var2);
+	    x = build2 (code, TREE_TYPE (new_var), var2, rprivb_ref);
 	    gimplify_assign (new_var, x, &input2_list);
 
 	    gimplify_assign (build_outer_var_ref (var, ctx), rpriva_ref,

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]