diff mbox

[gomp-4_0-branch] openacc collapse clause

Message ID 53BEDFCE.1020600@codesourcery.com
State New
Headers show

Commit Message

Cesar Philippidis July 10, 2014, 6:47 p.m. UTC 
Hi Thomas,

These patch enables the collapse clause with a value greater than one.
We had partial support for the collapse clause which supported a value
of 1, because the fortran frontend always associated a collapse clause
with each openacc for loop. However in terms of actual collapse support,
a collapse value of 1 is effectively a nop.

Is this patch OK for gomp-4_0-branch?

Thanks,
Cesar

Comments

Thomas Schwinge July 11, 2014, 9:27 a.m. UTC | #1 
Hi Cesar!

On Thu, 10 Jul 2014 11:47:42 -0700, Cesar Philippidis <cesar@codesourcery.com> wrote:
> These patch enables the collapse clause with a value greater than one.

Thanks!

> Is this patch OK for gomp-4_0-branch?

OK with the following addressed:

> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -11260,6 +11260,10 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask,
>  
>        switch (c_kind)
>  	{
> +	case PRAGMA_OMP_CLAUSE_COLLAPSE:
> +	  clauses = c_parser_omp_clause_collapse (parser, clauses);
> +	  c_name = "collapse";
> +	  break;

Annotate c_parser_omp_clause_collapse that it is now used for OpenACC,
too.


>  #define OACC_LOOP_CLAUSE_MASK						\
> -	(OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_REDUCTION)
> +	( (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_COLLAPSE)		\
> +	| (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_REDUCTION))

Given the syntax generally used, add a space between the two closing
parens.


Even though we're re-using OpenMP's tested implementation, please add
some basic front end test coverage for OpenACC: reject invalid, accept
valid syntax.


Grüße,
 Thomas
diff mbox

Patch

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index d638930..6e6fe30 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -11260,6 +11260,10 @@  c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask,
 
       switch (c_kind)
 	{
+	case PRAGMA_OMP_CLAUSE_COLLAPSE:
+	  clauses = c_parser_omp_clause_collapse (parser, clauses);
+	  c_name = "collapse";
+	  break;
 	case PRAGMA_OMP_CLAUSE_COPY:
 	  clauses = c_parser_oacc_data_clause (parser, c_kind, clauses);
 	  c_name = "copy";
@@ -11702,7 +11706,8 @@  c_parser_oacc_kernels (location_t loc, c_parser *parser, char *p_name)
 */
 
 #define OACC_LOOP_CLAUSE_MASK						\
-	(OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_REDUCTION)
+	( (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_COLLAPSE)		\
+	| (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_REDUCTION))
 
 static tree
 c_parser_oacc_loop (location_t loc, c_parser *parser, char *p_name)
@@ -12306,10 +12311,7 @@  c_parser_omp_for_loop (location_t loc, c_parser *parser, enum tree_code code,
 
   for (cl = clauses; cl; cl = OMP_CLAUSE_CHAIN (cl))
     if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_COLLAPSE)
-      {
-	gcc_assert (code != OACC_LOOP);
       collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (cl));
-      }
 
   gcc_assert (collapse >= 1);
 
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index eb078b6..18d5aa7 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -381,9 +381,6 @@  extract_omp_for_data (gimple for_stmt, struct omp_for_data *fd,
       case OMP_CLAUSE_COLLAPSE:
 	if (fd->collapse > 1)
 	  {
-	    if (is_gimple_omp_oacc_specifically (for_stmt))
-	      sorry ("collapse (>1) clause not supported yet");
-
 	    collapse_iter = &OMP_CLAUSE_COLLAPSE_ITERVAR (t);
 	    collapse_count = &OMP_CLAUSE_COLLAPSE_COUNT (t);
 	  }
@@ -6684,9 +6681,6 @@  expand_omp_for_static_nochunk (struct omp_region *region,
 
   if (fd->collapse > 1)
     {
-      gcc_assert (gimple_omp_for_kind (fd->for_stmt)
-		  != GF_OMP_FOR_KIND_OACC_LOOP);
-
       int first_zero_iter = -1;
       basic_block l2_dom_bb = NULL;
 
@@ -6697,12 +6691,7 @@  expand_omp_for_static_nochunk (struct omp_region *region,
       t = NULL_TREE;
     }
   else if (gimple_omp_for_combined_into_p (fd->for_stmt))
-    {
-      gcc_assert (gimple_omp_for_kind (fd->for_stmt)
-		  != GF_OMP_FOR_KIND_OACC_LOOP);
-
     t = integer_one_node;
-    }
   else
     t = fold_binary (fd->loop.cond_code, boolean_type_node,
 		     fold_convert (type, fd->loop.n1),
@@ -6736,9 +6725,6 @@  expand_omp_for_static_nochunk (struct omp_region *region,
       ep->probability = REG_BR_PROB_BASE / 2000 - 1;
       if (gimple_in_ssa_p (cfun))
 	{
-	  gcc_assert (gimple_omp_for_kind (fd->for_stmt)
-		      != GF_OMP_FOR_KIND_OACC_LOOP);
-
 	  int dest_idx = find_edge (entry_bb, fin_bb)->dest_idx;
 	  for (gsi = gsi_start_phis (fin_bb);
 	       !gsi_end_p (gsi); gsi_next (&gsi))
@@ -6865,9 +6851,6 @@  expand_omp_for_static_nochunk (struct omp_region *region,
 
   if (gimple_omp_for_combined_p (fd->for_stmt))
     {
-      gcc_assert (gimple_omp_for_kind (fd->for_stmt)
-		  != GF_OMP_FOR_KIND_OACC_LOOP);
-
       tree clauses = gimple_code (inner_stmt) == GIMPLE_OMP_PARALLEL
 		     ? gimple_omp_parallel_clauses (inner_stmt)
 		     : gimple_omp_for_clauses (inner_stmt);
@@ -6914,12 +6897,7 @@  expand_omp_for_static_nochunk (struct omp_region *region,
       gsi_insert_after (&gsi, stmt, GSI_CONTINUE_LINKING);
     }
   if (fd->collapse > 1)
-    {
-      gcc_assert (gimple_omp_for_kind (fd->for_stmt)
-		  != GF_OMP_FOR_KIND_OACC_LOOP);
-
     expand_omp_for_init_vars (fd, &gsi, counts, inner_stmt, startvar);
-    }
 
   if (!broken_loop)
     {
@@ -6954,12 +6932,7 @@  expand_omp_for_static_nochunk (struct omp_region *region,
       gsi_remove (&gsi, true);
 
       if (fd->collapse > 1 && !gimple_omp_for_combined_p (fd->for_stmt))
-	{
-	  gcc_assert (gimple_omp_for_kind (fd->for_stmt)
-		      != GF_OMP_FOR_KIND_OACC_LOOP);
-
 	collapse_bb = extract_omp_for_update_vars (fd, cont_bb, body_bb);
-	}
     }
 
   /* Replace the GIMPLE_OMP_RETURN with a barrier, or nothing.  */
@@ -6996,9 +6969,6 @@  expand_omp_for_static_nochunk (struct omp_region *region,
 	}
       else if (fd->collapse > 1)
 	{
-	  gcc_assert (gimple_omp_for_kind (fd->for_stmt)
-		      != GF_OMP_FOR_KIND_OACC_LOOP);
-
 	  remove_edge (ep);
 	  ep = make_edge (cont_bb, collapse_bb, EDGE_TRUE_VALUE);
 	}