diff mbox

[gomp4] dimension API

Message ID 55CB4A62.9010809@acm.org
State New
Headers show

Commit Message

Nathan Sidwell Aug. 12, 2015, 1:30 p.m. UTC
I've committed this patch to gomp4.  It reworks the oacc functiuon attribute 
dimension handling.  Rather than pass the TREE_LIST to the various hooks, I 
convert it to a regular C array of ints.  That makes life simpler for the 
consumers.  They return a 'changed' flag to indicate whether the attrribute 
should be rewritten.  That rewriting is done in a way that doesn;t presume the 
attribute is unshared (Cesar, your workaround should no longer be necessary).

Also  put in the change I mentioned earlier this morning about the default 
validate dims hook setting the dimensions to 1 on accelerators to.  I'll  revert 
Thomas's patch shortly.

nathan

Comments

Nathan Sidwell Aug. 12, 2015, 3:49 p.m. UTC | #1
On 08/12/15 09:30, Nathan Sidwell wrote:
> I've committed this patch to gomp4.  It reworks the oacc functiuon attribute
> dimension handling.  Rather than pass the TREE_LIST to the various hooks, I
> convert it to a regular C array of ints.  That makes life simpler for the
> consumers.  They return a 'changed' flag to indicate whether the attrribute
> should be rewritten.  That rewriting is done in a way that doesn;t presume the
> attribute is unshared (Cesar, your workaround should no longer be necessary).

I've discovered I'd committed a slightly earlier version of what I'd tested. 
Consequently there is some breakage.  I'll be fixing it later today, after a retest.

nathan
Thomas Schwinge Aug. 13, 2015, 6:17 a.m. UTC | #2
Hi Nathan!

On Wed, 12 Aug 2015 09:30:10 -0400, Nathan Sidwell <nathan@acm.org> wrote:
> I've committed this patch to gomp4.  [...]
> 
> Also  put in the change I mentioned earlier this morning about the default 
> validate dims hook setting the dimensions to 1 on accelerators to.  I'll  revert 
> Thomas's patch shortly.

Confirmed, that works, thanks!


Grüße,
 Thomas
diff mbox

Patch

2015-08-12  Nathan Sidwell  <nathan@codesourcery.com>

	* target.def (validate_dims): Adjust API.
	* targhooks.h (default_goacc_validate_dims): Adjust.
	* omp-low.c (replace_oacc_fn_attrib): New function.
	(set_oacc_fn_attrib): Use it.
	(oacc_xform_dim): Dims is array of ints.
	(execute_oacc_transform): Create int array of dims, adjust uses.
	(default_goacc_validate_dims): Adjust API.  Force to w everywhere.
	* doc/tm.texi: Rebuild.
	* config/nvptx/nvptx.c (nvptx_validate_dims): Adjust API.

Index: gcc/config/nvptx/nvptx.c
===================================================================
--- gcc/config/nvptx/nvptx.c	(revision 226808)
+++ gcc/config/nvptx/nvptx.c	(working copy)
@@ -3538,71 +3538,46 @@  nvptx_expand_builtin (tree exp, rtx targ
   return d->expander (exp, target, mode, ignore);
 }
 
-/* Validate compute dimensions, fill in defaults.  */
-
-static tree
-nvptx_validate_dims (tree decl, tree dims)
-{
-  tree adims[GOMP_DIM_MAX];
-  unsigned ix;
-  tree *pos_ptr;
-
-  for (ix = 0, pos_ptr = &dims; ix != GOMP_DIM_MAX;
-       ix++, pos_ptr = &TREE_CHAIN (*pos_ptr))
-    {
-      if (!*pos_ptr)
-	*pos_ptr = tree_cons (NULL_TREE, NULL_TREE, NULL_TREE);
-      
-      adims[ix] = TREE_VALUE (*pos_ptr);
-    }
-
-  /* Define vector size for known hardware.  */
+/* Define vector size for known hardware.  */
 #define PTX_VECTOR_LENGTH 32
 #define PTX_WORKER_LENGTH 32
 
+/* Validate compute dimensions, fill in non-unity defaults.  */
+
+static bool
+nvptx_validate_dims (tree decl, int dims[])
+{
+  bool changed = false;
+
   /* If the worker size is not 1, the vector size must be 32.  If
      the vector  size is not 1, it must be 32.  */
-  if ((adims[GOMP_DIM_WORKER]
-       && TREE_INT_CST_LOW (adims[GOMP_DIM_WORKER]) != 1)
-      || (adims[GOMP_DIM_VECTOR]
-	  && TREE_INT_CST_LOW (adims[GOMP_DIM_VECTOR]) != 1))
+  if ((dims[GOMP_DIM_WORKER] > 1 || dims[GOMP_DIM_WORKER] == 0)
+      || (dims[GOMP_DIM_VECTOR] > 1 || dims[GOMP_DIM_VECTOR] == 0))
     {
-      if (!adims[GOMP_DIM_VECTOR]
-	  || TREE_INT_CST_LOW (adims[GOMP_DIM_VECTOR]) != PTX_VECTOR_LENGTH)
+      if (dims[GOMP_DIM_VECTOR] != PTX_VECTOR_LENGTH)
 	{
-	  tree use = build_int_cst (integer_type_node, PTX_VECTOR_LENGTH);
-	  if (adims[GOMP_DIM_VECTOR])
+	  if (dims[GOMP_DIM_VECTOR] >= 0)
 	    warning_at (DECL_SOURCE_LOCATION (decl), 0,
-			TREE_INT_CST_LOW (adims[GOMP_DIM_VECTOR])
-			? "using vector_length (%E), ignoring %E"
-			: "using vector_length (%E), ignoring runtime setting",
-			use, adims[GOMP_DIM_VECTOR]);
-	  adims[GOMP_DIM_VECTOR] = use;
+			dims[GOMP_DIM_VECTOR]
+			? "using vector_length (%d), ignoring %d"
+			: "using vector_length (%d), ignoring runtime setting",
+			PTX_VECTOR_LENGTH, dims[GOMP_DIM_VECTOR]);
+	  dims[GOMP_DIM_VECTOR] = PTX_VECTOR_LENGTH;
+	  changed = true;
 	}
     }
 
   /* Check the num workers is not too large.  */
-  if (adims[GOMP_DIM_WORKER]
-      && TREE_INT_CST_LOW (adims[GOMP_DIM_WORKER]) > PTX_WORKER_LENGTH)
+  if (dims[GOMP_DIM_WORKER] > PTX_WORKER_LENGTH)
     {
-      tree use = build_int_cst (integer_type_node, PTX_WORKER_LENGTH);
       warning_at (DECL_SOURCE_LOCATION (decl), 0,
-		  "using num_workers (%E), ignoring %E",
-		  use, adims[GOMP_DIM_WORKER]);
-      adims[GOMP_DIM_WORKER] = use;
+		  "using num_workers (%d), ignoring %d",
+		  PTX_WORKER_LENGTH, dims[GOMP_DIM_WORKER]);
+      dims[GOMP_DIM_WORKER] = PTX_WORKER_LENGTH;
+      changed = true;
     }
 
-  /* Set defaults.  */
-  for (ix = 0; ix != GOMP_DIM_MAX; ix++)
-    if (!adims[ix])
-      adims[ix] = integer_one_node;
-
-  /* Write results.  */
-  tree pos;
-  for (ix = 0, pos = dims; ix != GOMP_DIM_MAX; ix++, pos = TREE_CHAIN (pos))
-    TREE_VALUE (pos) = adims[ix];
-
-  return dims;
+  return changed;
 }
 
 /* Return maximum dimension size, or zero for unbounded.  */
Index: gcc/doc/tm.texi
===================================================================
--- gcc/doc/tm.texi	(revision 226808)
+++ gcc/doc/tm.texi	(working copy)
@@ -5740,11 +5740,12 @@  usable.  In that case, the smaller the n
 to use it.
 @end deftypefn
 
-@deftypefn {Target Hook} tree TARGET_GOACC_VALIDATE_DIMS (tree, @var{tree})
+@deftypefn {Target Hook} bool TARGET_GOACC_VALIDATE_DIMS (tree, int @var{[]})
 This hook should check the launch dimensions provided.  It should fill
-in default values and verify non-defaults.  The TREE_LIST is unshared
-and may be overwritten.  If the dimension list is NULL, one should be
-created.  Diagnostics should be issued as appropriate.
+in anything that needs default to non-unity and verify non-defaults.
+Defaults are represented as -1.  Diagnostics should be issuedas 
+ppropriate.  Return true if changes have been made.  You must override
+this hook to provide dimensions larger than 1.
 @end deftypefn
 
 @deftypefn {Target Hook} unsigned TARGET_GOACC_DIM_LIMIT (unsigned)
Index: gcc/omp-low.c
===================================================================
--- gcc/omp-low.c	(revision 226808)
+++ gcc/omp-low.c	(working copy)
@@ -9314,6 +9314,16 @@  oacc_launch_pack (unsigned code, tree de
 
 #define OACC_FN_ATTRIB "oacc function"
 
+/* Replace any existing oacc fn attribute with updated dimensions.  */
+
+void
+replace_oacc_fn_attrib (tree fn, tree dims)
+{
+  /* Simply cons onto the beginning of the list.  */
+  DECL_ATTRIBUTES (fn) =
+    tree_cons (get_identifier (OACC_FN_ATTRIB), dims, DECL_ATTRIBUTES (fn));
+}
+
 static void
 set_oacc_fn_attrib (tree clauses, tree fn, vec<tree> *args)
 {
@@ -9341,9 +9351,7 @@  set_oacc_fn_attrib (tree clauses, tree f
       attr = tree_cons (NULL_TREE, dim, attr);
     }
 
-  /* Add the attributes.  */
-  DECL_ATTRIBUTES (fn) =
-    tree_cons (get_identifier (OACC_FN_ATTRIB), attr, DECL_ATTRIBUTES (fn));
+  replace_oacc_fn_attrib (fn, attr);
 
   if (non_const)
     {
@@ -14581,14 +14589,11 @@  oacc_xform_on_device (gimple_stmt_iterat
 
 static void
 oacc_xform_dim (gimple_stmt_iterator *gsi, gimple stmt,
-		tree dims, bool is_pos)
+		int dims[], bool is_pos)
 {
   tree arg = gimple_call_arg (stmt, 0);
   unsigned axis = (unsigned)TREE_INT_CST_LOW (arg);
-
-  while (axis--)
-    dims = TREE_CHAIN (dims);
-  int size = TREE_INT_CST_LOW (TREE_VALUE (dims));
+  int size = dims[axis];
 
   if (!size)
     /* Dimension size is dynamic.  */
@@ -14618,15 +14623,50 @@  execute_oacc_transform ()
 {
   basic_block bb;
   tree attrs = get_oacc_fn_attrib (current_function_decl);
+  int dims[GOMP_DIM_MAX];
   
   if (!attrs)
     /* Not an offloaded function.  */
     return 0;
 
-  tree dims = TREE_VALUE (attrs);
-  dims = targetm.goacc.validate_dims (current_function_decl, dims);
-  /* Safe to overwrite, this attribute chain is unshared.  */
-  TREE_VALUE (attrs) = dims;
+  {
+    unsigned ix;
+    tree pos = TREE_VALUE (attrs);
+
+    for (ix = 0; ix != GOMP_DIM_MAX; ix++)
+      {
+	if (!pos)
+	  dims[ix] = 0;
+	else
+	  {
+	    tree val = TREE_VALUE (pos);
+	    
+	    dims[ix] = val ? TREE_INT_CST_LOW (val) : -1;
+	    pos = TREE_CHAIN (pos);
+	  }
+      }
+
+    bool changed = targetm.goacc.validate_dims (current_function_decl, dims);
+
+    /* Default anything left undefaulted to 1.  */
+    for (ix = 0; ix != GOMP_DIM_MAX; ix++)
+      if (dims[ix] < 0)
+	{
+	  dims[ix] = 1;
+	  changed = true;
+	}
+  
+    if (changed)
+      {
+	/* Replace the attribute with new values.  */
+	pos = NULL_TREE;
+	for (ix = GOMP_DIM_MAX; ix--;)
+	  pos = tree_cons (NULL_TREE,
+			   build_int_cst (integer_type_node, dims[ix]),
+			   pos);
+	replace_oacc_fn_attrib (current_function_decl, pos);
+      }
+  }
   
   FOR_ALL_BB_FN (bb, cfun)
     {
@@ -14677,34 +14717,25 @@  execute_oacc_transform ()
   return 0;
 }
 
-/* Default launch dimension validator.  Force everything to 1 on the
-   host and default to 1 otherwise.  */
+/* Default launch dimension validator.  Force everything to 1.  A
+   backend that wants to provide larger dimensions must override this
+   hook.  */
 
-tree
-default_goacc_validate_dims (tree ARG_UNUSED (decl), tree dims)
+bool
+default_goacc_validate_dims (tree ARG_UNUSED (decl), int *ARG_UNUSED (dims))
 {
-  tree *pos_ptr;
-  unsigned ix;
-  
-  for (ix = 0, pos_ptr = &dims;
-       ix != GOMP_DIM_MAX; ix++, pos_ptr = &TREE_CHAIN (*pos_ptr))
+  bool changed = false;
+
+  for (unsigned ix = 0; ix != GOMP_DIM_MAX; ix++)
     {
-      /* Cons up a default, if the attribue list is NULL.  This
-	 happens on 'declare' routines, as theyy do not currently set
-	 the dimensions over which the routine may be active.  */
-      if (!*pos_ptr)
-	*pos_ptr = tree_cons (NULL_TREE, NULL_TREE, NULL_TREE);
-      
-      tree val = TREE_VALUE (*pos_ptr);
-      
-#ifdef ACCEL_COMPILER
-      if (!val)
-#endif
-	val = integer_one_node;
-      TREE_VALUE (*pos_ptr) = val;
+      if (dims[ix] != 1)
+	{
+	  dims[ix] = 1;
+	  changed = true;
+	}
     }
 
-  return dims;
+  return changed;
 }
 
 /* Default dimension bound is unknown on accelerator and 1 on host. */
Index: gcc/target.def
===================================================================
--- gcc/target.def	(revision 226808)
+++ gcc/target.def	(working copy)
@@ -1647,10 +1647,11 @@  HOOK_VECTOR (TARGET_GOACC, goacc)
 DEFHOOK
 (validate_dims,
 "This hook should check the launch dimensions provided.  It should fill\n\
-in default values and verify non-defaults.  The TREE_LIST is unshared\n\
-and may be overwritten.  If the dimension list is NULL, one should be\n\
-created.  Diagnostics should be issued as appropriate.",
-tree, (tree, tree),
+in anything that needs default to non-unity and verify non-defaults.\n\
+Defaults are represented as -1.  Diagnostics should be issuedas \n\
+ppropriate.  Return true if changes have been made.  You must override\n\
+this hook to provide dimensions larger than 1.",
+bool, (tree, int []),
 default_goacc_validate_dims)
 
 DEFHOOK
Index: gcc/targhooks.h
===================================================================
--- gcc/targhooks.h	(revision 226808)
+++ gcc/targhooks.h	(working copy)
@@ -107,7 +107,7 @@  extern unsigned default_add_stmt_cost (v
 extern void default_finish_cost (void *, unsigned *, unsigned *, unsigned *);
 extern void default_destroy_cost_data (void *);
 
-extern tree default_goacc_validate_dims (tree, tree);
+extern bool default_goacc_validate_dims (tree, int []);
 extern unsigned default_goacc_dim_limit (unsigned);
 extern bool default_goacc_fork_join (bool, gimple_stmt_iterator *, gimple);