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]

[tuples] OMP_CLAUSE_REDUCTION_{INIT,MERGE}


Hi folks!

I ran into a snafu when converting the OMP reduction merge and init
code.  We currently gimplify in-place into
OMP_CLAUSE_REDUCTION_{INIT,MERGE}, and then extract these bits 
we lower OMP in omp-low.c.  This approach doesn't work with tuples,
because we can't gimplify in place.

I thought of three distinct ways of handling this:

1. Bloat `struct tree_omp_clause' to contain sequences for both init and
   merge in tuple form.  Unfortunately, this will increase the size of
   tree OMP clauses by 2 pointers for all cases.

2. Convert all of the OMP clause infrastructure to use tuples.  This
   seemed like overkill, as the only clauses that needed gimplification
   for omp-low was the reduction clause.  (There is also NUM_THREADS,
   and SCHEDULE which required gimplification, but we only need an
   rvalue for these, the rest of the gimplification can go in pre_p).

3. Use an on-the-side pointer map to store these gimplifications until
   omp-low.

Diego preferred #2, but IMO that turned out to be over engineering for
just one case.  I preferred #3, which I actually implemented, but it
was a 230+ line patch to do cleanly, which also seemed like overkill.

So, I ran some tests on openmp code, including our testsuite, and found
that clauses per function were less than a dozen in 99% of the cases.
In severely constructed cases, we had 30-40 clauses in a function.
Consequently, I'm not feeling at all guilty to bloat tree_omp_clause by
8-16 bytes.

Sorry for the long explanation to say that I took approach #1.  I will
wait a day for folks to comment, to see if I missed something incredibly
obvious, and there is a prettier way of doing this.  If not, I'll just
commit it.

Aldy

	* tree.h (OMP_CLAUSE_REDUCTION_GIMPLE_INIT): New.
	(OMP_CLAUSE_REDUCTION_GIMPLE_MERGE): New.
	(struct tree_omp_clause): Add gimple_reduction_{init,merge} fields.
	* gimplify.c (gimplify_scan_omp_clauses): Gimplify reduction
	init/merge fields into separate sequences.
	(gimplify_and_add): Use gimplify_expr directly.
	* omp-low.c (lower_rec_input_clauses): Extract reduction info from
	gimple tuples in structure.
	(lower_reduction_clauses): Same.

Index: tree.h
===================================================================
--- tree.h	(revision 131962)
+++ tree.h	(working copy)
@@ -1776,6 +1776,10 @@ struct tree_constructor GTY(())
   OMP_CLAUSE_OPERAND (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_REDUCTION), 1)
 #define OMP_CLAUSE_REDUCTION_MERGE(NODE) \
   OMP_CLAUSE_OPERAND (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_REDUCTION), 2)
+#define OMP_CLAUSE_REDUCTION_GIMPLE_INIT(NODE) \
+  (OMP_CLAUSE_CHECK (NODE))->omp_clause.gimple_reduction_init
+#define OMP_CLAUSE_REDUCTION_GIMPLE_MERGE(NODE) \
+  (OMP_CLAUSE_CHECK (NODE))->omp_clause.gimple_reduction_merge
 #define OMP_CLAUSE_REDUCTION_PLACEHOLDER(NODE) \
   OMP_CLAUSE_OPERAND (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_REDUCTION), 3)
 
@@ -1933,6 +1937,12 @@ struct tree_omp_clause GTY(())
     enum omp_clause_schedule_kind schedule_kind;
     enum tree_code                reduction_code;
   } GTY ((skip)) subcode;
+
+  /* The gimplification of OMP_CLAUSE_REDUCTION_{INIT,MERGE} for omp-low's
+     usage.  */
+  gimple_seq gimple_reduction_init;
+  gimple_seq gimple_reduction_merge;
+
   tree GTY ((length ("omp_clause_num_ops[OMP_CLAUSE_CODE ((tree)&%h)]"))) ops[1];
 };
 
Index: omp-low.c
===================================================================
--- omp-low.c	(revision 131962)
+++ omp-low.c	(working copy)
@@ -1814,7 +1814,9 @@ lower_rec_input_clauses (tree clauses, t
 	    case OMP_CLAUSE_REDUCTION:
 	      if (OMP_CLAUSE_REDUCTION_PLACEHOLDER (c))
 		{
-		  gimplify_and_add (OMP_CLAUSE_REDUCTION_INIT (c), ilist);
+		  gimple_seq_append (ilist,
+		      		     OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c));
+
 		  OMP_CLAUSE_REDUCTION_INIT (c) = NULL;
 		}
 	      else
@@ -1977,7 +1979,7 @@ lower_reduction_clauses (tree clauses, t
 	    ref = build_fold_addr_expr (ref);
 	  SET_DECL_VALUE_EXPR (placeholder, ref);
 	  DECL_HAS_VALUE_EXPR_P (placeholder) = 1;
-	  gimplify_and_add (OMP_CLAUSE_REDUCTION_MERGE (c), &sub_list);
+	  gimple_seq_append (&sub_list, OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c));
 	  OMP_CLAUSE_REDUCTION_MERGE (c) = NULL;
 	  OMP_CLAUSE_REDUCTION_PLACEHOLDER (c) = NULL;
 	}
Index: gimplify.c
===================================================================
--- gimplify.c	(revision 131963)
+++ gimplify.c	(working copy)
@@ -350,7 +350,7 @@ append_to_statement_list_force (tree t, 
 void
 gimplify_and_add (tree t, gimple_seq seq)
 {
-  gimplify_stmt (&t, seq);
+  gimplify_expr (&t, seq, NULL, is_gimple_stmt, fb_none);
 }
 
 /* Gimplify statement T, and return the first tuple in the sequence of
@@ -5112,16 +5112,20 @@ gimplify_scan_omp_clauses (tree *list_p,
 				GOVD_LOCAL | GOVD_SEEN);
 	      gimplify_omp_ctxp = ctx;
 	      push_gimplify_context ();
-	      gimplify_stmt (&OMP_CLAUSE_REDUCTION_INIT (c), pre_p);
-	      /* FIXME tuples.  */
-#if 0
-	      pop_gimplify_context (OMP_CLAUSE_REDUCTION_INIT (c));
+
+	      OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c) = gimple_seq_alloc ();
+	      OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c) = gimple_seq_alloc ();
+
+	      gimplify_and_add (OMP_CLAUSE_REDUCTION_INIT (c),
+		  		OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c));
+	      pop_gimplify_context
+		(gimple_seq_first (OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c)));
 	      push_gimplify_context ();
-	      gimplify_stmt (&OMP_CLAUSE_REDUCTION_MERGE (c), pre_p);
-	      pop_gimplify_context (OMP_CLAUSE_REDUCTION_MERGE (c));
-#else
-              gimple_unreachable ();
-#endif
+	      gimplify_and_add (OMP_CLAUSE_REDUCTION_MERGE (c),
+		  		OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c));
+	      pop_gimplify_context 
+		(gimple_seq_first (OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c)));
+
 	      gimplify_omp_ctxp = outer_ctx;
 	    }
 	  if (notice_outer)


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