diff mbox

[1/5] OpenACC tile clause support, OMP_CLAUSE_TILE adjustments

Message ID 7fb21d78-1a4f-986d-6c6a-069c8acbad7a@mentor.com
State New
Headers show

Commit Message

Chung-Lin Tang Nov. 17, 2016, 9:27 a.m. UTC 
On 2016/11/11 5:38 PM, Jakub Jelinek wrote:
> Hi!

> 

> On Thu, Nov 10, 2016 at 06:44:52PM +0800, Chung-Lin Tang wrote:

> 

> Above this it is fine.

> 

>> @@ -9388,10 +9373,23 @@ gimplify_omp_for (tree *expr_p, gimple_seq *pre_p)

>>  						 (OMP_FOR_INIT (for_stmt))

>>  					       * 2);

>>      }

>> -  int collapse = 1;

>> -  c = find_omp_clause (OMP_FOR_CLAUSES (for_stmt), OMP_CLAUSE_COLLAPSE);

>> -  if (c)

>> -    collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (c));

>> +  int collapse = 0;

>> +  /* Find the first of COLLAPSE or TILE.  */

>> +  for (c = OMP_FOR_CLAUSES (for_stmt); c; c = TREE_CHAIN (c))

>> +    if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_COLLAPSE)

>> +      {

>> +	collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (c));

>> +	if (collapse == 1)

>> +	  /* Not really collapsing.  */

>> +	  collapse = 0;

>> +	break;

>> +      }

>> +    else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_TILE)

>> +      {

>> +	collapse = list_length (OMP_CLAUSE_TILE_LIST (c));

>> +	break;

>> +      }

> 

> I don't really like this, especially pretending collapse(1) or lack

> of collapse clause e.g. on OpenMP construct is collapse(0).

> I'd keep what it does, i.e. 

>   int collapse = 1;

>   c = find_omp_clause (OMP_FOR_CLAUSES (for_stmt), OMP_CLAUSE_COLLAPSE);

>   if (c)

>     collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (c));

> and in the first switch in gimplify_omp_for you can:

>     case OACC_LOOP:

>       ort = ORT_ACC;

>       c = find_omp_clause (OMP_FOR_CLAUSES (for_stmt), OMP_CLAUSE_TILE);

>       if (c)

> 	tile = list_length (OMP_CLAUSE_TILE_LIST (c));

>       break;

> and then just use tile != 0 or whatever || with collapse > 1 where needed.

> 

>   > +

>>    for (i = 0; i < TREE_VEC_LENGTH (OMP_FOR_INIT (for_stmt)); i++)

>>      {

>>        t = TREE_VEC_ELT (OMP_FOR_INIT (for_stmt), i);

>> @@ -9807,7 +9805,7 @@ gimplify_omp_for (tree *expr_p, gimple_seq *pre_p)

>>  	  OMP_CLAUSE_LINEAR_STEP (c2) = OMP_CLAUSE_LINEAR_STEP (c);

>>  	}

>>  

>> -      if ((var != decl || collapse > 1) && orig_for_stmt == for_stmt)

>> +      if ((var != decl || collapse) && orig_for_stmt == for_stmt)

>>  	{

>>  	  for (c = OMP_FOR_CLAUSES (for_stmt); c ; c = OMP_CLAUSE_CHAIN (c))

>>  	    if (((OMP_CLAUSE_CODE (c) == OMP_CLAUSE_LASTPRIVATE

> 

> Like here.

> 

>> @@ -9817,7 +9815,7 @@ gimplify_omp_for (tree *expr_p, gimple_seq *pre_p)

>>  		     && OMP_CLAUSE_LINEAR_GIMPLE_SEQ (c) == NULL))

>>  		&& OMP_CLAUSE_DECL (c) == decl)

>>  	      {

>> -		if (is_doacross && (collapse == 1 || i >= collapse))

>> +		if (is_doacross && (!collapse || i >= collapse))

>>  		  t = var;

>>  		else

>>  		  {

> 

> And not here.  You don't really have doacross loops in OpenACC, do you?

> 

> 	Jakub

> 


Hi Jakub,
I've updated the patch as you suggested.
Here's the v2 of the first patch, mainly gimplify.c adjusted.

About the ChangeLog issues, I'll make sure the final committed diffs will solve them.

Thanks,
Chung-Lin

Comments

Jakub Jelinek Nov. 18, 2016, 11:06 a.m. UTC | #1 
On Thu, Nov 17, 2016 at 05:27:14PM +0800, Chung-Lin Tang wrote:
> I've updated the patch as you suggested.

> Here's the v2 of the first patch, mainly gimplify.c adjusted.

> 

> About the ChangeLog issues, I'll make sure the final committed diffs will solve them.


Ok.

	Jakub
diff mbox

Patch

Index: tree.c
===================================================================
--- tree.c	(revision 241809)
+++ tree.c	(working copy)
@@ -327,7 +327,7 @@  unsigned const char omp_clause_num_ops[] =
   1, /* OMP_CLAUSE_NUM_GANGS  */
   1, /* OMP_CLAUSE_NUM_WORKERS  */
   1, /* OMP_CLAUSE_VECTOR_LENGTH  */
-  1, /* OMP_CLAUSE_TILE  */
+  3, /* OMP_CLAUSE_TILE  */
   2, /* OMP_CLAUSE__GRIDDIM_  */
 };
 
Index: tree.h
===================================================================
--- tree.h	(revision 241809)
+++ tree.h	(working copy)
@@ -1654,6 +1654,10 @@  extern void protected_set_expr_location (tree, loc
 
 #define OMP_CLAUSE_TILE_LIST(NODE) \
   OMP_CLAUSE_OPERAND (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_TILE), 0)
+#define OMP_CLAUSE_TILE_ITERVAR(NODE) \
+  OMP_CLAUSE_OPERAND (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_TILE), 1)
+#define OMP_CLAUSE_TILE_COUNT(NODE) \
+  OMP_CLAUSE_OPERAND (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_TILE), 2)
 
 #define OMP_CLAUSE__GRIDDIM__DIMENSION(NODE) \
   (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE__GRIDDIM_)\
Index: tree-nested.c
===================================================================
--- tree-nested.c	(revision 241809)
+++ tree-nested.c	(working copy)
@@ -1274,6 +1274,7 @@  convert_nonlocal_omp_clauses (tree *pclauses, stru
 	case OMP_CLAUSE_DEFAULT:
 	case OMP_CLAUSE_COPYIN:
 	case OMP_CLAUSE_COLLAPSE:
+	case OMP_CLAUSE_TILE:
 	case OMP_CLAUSE_UNTIED:
 	case OMP_CLAUSE_MERGEABLE:
 	case OMP_CLAUSE_PROC_BIND:
@@ -1286,8 +1287,6 @@  convert_nonlocal_omp_clauses (tree *pclauses, stru
 	case OMP_CLAUSE_AUTO:
 	  break;
 
-	  /* OpenACC tile clauses are discarded during gimplification.  */
-	case OMP_CLAUSE_TILE:
 	  /* The following clause belongs to the OpenACC cache directive, which
 	     is discarded during gimplification.  */
 	case OMP_CLAUSE__CACHE_:
@@ -1982,6 +1981,7 @@  convert_local_omp_clauses (tree *pclauses, struct
 	case OMP_CLAUSE_DEFAULT:
 	case OMP_CLAUSE_COPYIN:
 	case OMP_CLAUSE_COLLAPSE:
+	case OMP_CLAUSE_TILE:
 	case OMP_CLAUSE_UNTIED:
 	case OMP_CLAUSE_MERGEABLE:
 	case OMP_CLAUSE_PROC_BIND:
@@ -1994,8 +1994,6 @@  convert_local_omp_clauses (tree *pclauses, struct
 	case OMP_CLAUSE_AUTO:
 	  break;
 
-	  /* OpenACC tile clauses are discarded during gimplification.  */
-	case OMP_CLAUSE_TILE:
 	  /* The following clause belongs to the OpenACC cache directive, which
 	     is discarded during gimplification.  */
 	case OMP_CLAUSE__CACHE_:
Index: gimplify.c
===================================================================
--- gimplify.c	(revision 241809)
+++ gimplify.c	(working copy)
@@ -8138,20 +8138,11 @@  gimplify_scan_omp_clauses (tree *list_p, gimple_se
 	    remove = true;
 	  break;
 
-	case OMP_CLAUSE_TILE:
-	  for (tree list = OMP_CLAUSE_TILE_LIST (c); !remove && list;
-	       list = TREE_CHAIN (list))
-	    {
-	      if (gimplify_expr (&TREE_VALUE (list), pre_p, NULL,
-				 is_gimple_val, fb_rvalue) == GS_ERROR)
-		remove = true;
-	    }
-	  break;
-
 	case OMP_CLAUSE_NOWAIT:
 	case OMP_CLAUSE_ORDERED:
 	case OMP_CLAUSE_UNTIED:
 	case OMP_CLAUSE_COLLAPSE:
+	case OMP_CLAUSE_TILE:
 	case OMP_CLAUSE_AUTO:
 	case OMP_CLAUSE_SEQ:
 	case OMP_CLAUSE_INDEPENDENT:
@@ -8927,13 +8918,7 @@  gimplify_adjust_omp_clauses (gimple_seq *pre_p, gi
 	case OMP_CLAUSE_VECTOR:
 	case OMP_CLAUSE_AUTO:
 	case OMP_CLAUSE_SEQ:
-	  break;
-
 	case OMP_CLAUSE_TILE:
-	  /* We're not yet making use of the information provided by OpenACC
-	     tile clauses.  Discard these here, to simplify later middle end
-	     processing.  */
-	  remove = true;
 	  break;
 
 	default:
@@ -9388,10 +9373,13 @@  gimplify_omp_for (tree *expr_p, gimple_seq *pre_p)
 						 (OMP_FOR_INIT (for_stmt))
 					       * 2);
     }
-  int collapse = 1;
+  int collapse = 1, tile = 0;
   c = find_omp_clause (OMP_FOR_CLAUSES (for_stmt), OMP_CLAUSE_COLLAPSE);
   if (c)
     collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (c));
+  c = find_omp_clause (OMP_FOR_CLAUSES (for_stmt), OMP_CLAUSE_TILE);
+  if (c)
+    tile = list_length (OMP_CLAUSE_TILE_LIST (c));
   for (i = 0; i < TREE_VEC_LENGTH (OMP_FOR_INIT (for_stmt)); i++)
     {
       t = TREE_VEC_ELT (OMP_FOR_INIT (for_stmt), i);
@@ -9807,7 +9795,7 @@  gimplify_omp_for (tree *expr_p, gimple_seq *pre_p)
 	  OMP_CLAUSE_LINEAR_STEP (c2) = OMP_CLAUSE_LINEAR_STEP (c);
 	}
 
-      if ((var != decl || collapse > 1) && orig_for_stmt == for_stmt)
+      if ((var != decl || collapse > 1 || tile) && orig_for_stmt == for_stmt)
 	{
 	  for (c = OMP_FOR_CLAUSES (for_stmt); c ; c = OMP_CLAUSE_CHAIN (c))
 	    if (((OMP_CLAUSE_CODE (c) == OMP_CLAUSE_LASTPRIVATE