[gomp4] signed nums are better for dimensions

[Nathan Sidwell <nathan at acm dot org>]

This patch does a number of things.
1) signed results from the DIM_SIZE and DIM_POS internal fns are better.  The 
rest of the loop machinery uses signed ints -- to show overflow is undefined. 
Returning unsigned simply caused an unsigned->signed conversion and subsequent 
confusion.

2) We really should not be getting to the expanders if there's nothing to expand 
to.  That's simply covering up lack of earlier handling.  That earlier removal 
gives optimizers more leeway.

3) Added a hook so the backend can say what the absolute dimension limits are. 
This is used by the DIM_SIZE/DIM_POS transform as a fall back.  It's main raison 
d'etre is for a min/max elision patch I've been  working on, but turned out 
useful here.

4) I'm forcing the oacc function attribute to always have dimensions.  This 
makes users of that data simpler.  However, Cesar discovered that the 
overwriting we were already doing was being broken by LTO's (reasonable) habit 
of commonizing identical tree lists.  I'll be working on correcting the 
behaviour once I have these patches flushed out.

committed to gomp4.

nathan

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

	* internal-fn.c (expand_GOACC_FORK): Use ARG_UNUSED.  Rename
	varible sensibly. Assert we're never reached if there is no
	expansion.
	(expand_GOACC_JOIN, expand_GOACC_DIM_SIZE,
	expand_GOACC_DIM_POS): likewise.
	* targhooks.h (default_goacc_dim_limit): Declare.
	* target.def (validate_dims): Expand documentation.
	(dim_limit): New hook.
	* config/nvptx/nvptx.c (nvptx_reorg): Expect attribute to always
	have dimensions.
	(nvptx_record_offload_symbol): Likewise.
	(nvptx_validate_syms): Force dimension creation and adjust.
	(nvtpx_dim_limit): Define.
	(TARGET_GOACC_DIM_LIMIT): Override.
	* omp-low.c (expand_oacc_get_num_threads): Builtins return int not
	unsigned.
	(expand_oacc_get_thread_num, oacc_init_count_vars): Likewise.
	(oacc_xform_dim): Result is signed.
	(execute_oacc_transform): Expect dims to always be present.
	(default_goacc_validate_dims): Force dimension creation.
	(default_goacc_dim_limit): New.
	* doc/tm.texi.in (TARGET_GOACC_DIM_LIMIT): Add.
	* doc/tm.texi: Rebuilt.

Index: internal-fn.c
===================================================================
--- internal-fn.c	(revision 226770)
+++ internal-fn.c	(working copy)
@@ -1965,58 +1965,62 @@ expand_GOACC_DATA_END_WITH_ARG (gcall *s
 }
 
 static void
-expand_GOACC_FORK (gcall *stmt ATTRIBUTE_UNUSED)
+expand_GOACC_FORK (gcall *ARG_UNUSED (stmt))
 {
 #ifdef HAVE_oacc_fork
-  rtx mode = expand_normal (gimple_call_arg (stmt, 0));
+  rtx dim = expand_normal (gimple_call_arg (stmt, 0));
   
-  emit_insn (gen_oacc_fork (mode));
+  emit_insn (gen_oacc_fork (dim));
+#else
+  gcc_unreachable ();
 #endif
 }
 
 static void
-expand_GOACC_JOIN (gcall *stmt ATTRIBUTE_UNUSED)
+expand_GOACC_JOIN (gcall *ARG_UNUSED (stmt))
 {
 #ifdef HAVE_oacc_join
-  rtx mode = expand_normal (gimple_call_arg (stmt, 0));
+  rtx dim = expand_normal (gimple_call_arg (stmt, 0));
   
-  emit_insn (gen_oacc_join (mode));
+  emit_insn (gen_oacc_join (dim));
+#else
+  gcc_unreachable ();
 #endif
 }
 
 static void
-expand_GOACC_DIM_SIZE (gcall *stmt)
+expand_GOACC_DIM_SIZE (gcall *ARG_UNUSED (stmt))
 {
+#ifdef HAVE_oacc_dim_size
   tree lhs = gimple_call_lhs (stmt);
 
   if (!lhs)
     return;
   
   rtx target = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
-#ifdef HAVE_oacc_dim_size
-  rtx val = expand_expr (gimple_call_arg (stmt, 0), NULL_RTX,
+  rtx dim = expand_expr (gimple_call_arg (stmt, 0), NULL_RTX,
 			 VOIDmode, EXPAND_NORMAL);
-  emit_insn (gen_oacc_dim_size (target, val));
+  emit_insn (gen_oacc_dim_size (target, dim));
 #else
-  emit_move_insn (target, const1_rtx);
+  gcc_unreachable ();
 #endif
 }
 
 static void
-expand_GOACC_DIM_POS (gcall *stmt)
+expand_GOACC_DIM_POS (gcall *ARG_UNUSED (stmt))
 {
+#ifdef HAVE_oacc_dim_pos
   tree lhs = gimple_call_lhs (stmt);
 
   if (!lhs)
     return;
   
   rtx target = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
-#ifdef HAVE_oacc_dim_pos
-  rtx val = expand_expr (gimple_call_arg (stmt, 0), NULL_RTX,
+  rtx dim = expand_expr (gimple_call_arg (stmt, 0), NULL_RTX,
 			 VOIDmode, EXPAND_NORMAL);
-  emit_insn (gen_oacc_dim_pos (target, val));
+  emit_insn (gen_oacc_dim_pos (target, dim));
 #else
-  emit_move_insn (target, const0_rtx);
+  gcc_unreachable ();
 #endif
 }
 
Index: targhooks.h
===================================================================
--- targhooks.h	(revision 226770)
+++ targhooks.h	(working copy)
@@ -108,6 +108,7 @@ extern void default_finish_cost (void *,
 extern void default_destroy_cost_data (void *);
 
 extern tree default_goacc_validate_dims (tree, tree);
+extern unsigned default_goacc_dim_limit (unsigned);
 extern bool default_goacc_fork_join (bool, gimple_stmt_iterator *, gimple);
 
 /* These are here, and not in hooks.[ch], because not all users of
Index: target.def
===================================================================
--- target.def	(revision 226770)
+++ target.def	(working copy)
@@ -1648,11 +1648,19 @@ 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.  Diagnostics should be issued as appropriate.",
+and may be overwritten.  If the dimension list is NULL, one should be\n\
+created.  Diagnostics should be issued as appropriate.",
 tree, (tree, tree),
 default_goacc_validate_dims)
 
 DEFHOOK
+(dim_limit,
+"This hook should return the maximum size of a particular dimension,\n\
+or zero if unbounded.",
+unsigned, (unsigned),
+default_goacc_dim_limit)
+
+DEFHOOK
 (fork_join,
 "This hook should convert IFN_GOACC_FORK and IFN_GOACC_JOIN function\n\
 calls to target-specific gimple.  It is executed during the oacc_xform\n\
Index: config/nvptx/nvptx.c
===================================================================
--- config/nvptx/nvptx.c	(revision 226770)
+++ config/nvptx/nvptx.c	(working copy)
@@ -3051,30 +3051,23 @@ nvptx_reorg (void)
   tree attr = get_oacc_fn_attrib (current_function_decl);
   if (attr)
     {
+      /* If we determined this mask before RTL expansion, we could
+	 elide emission of some levels of forks and joins.  */
       unsigned mask = 0;
       tree dims = TREE_VALUE (attr);
       unsigned ix;
 
-      for (ix = 0; ix != GOMP_DIM_MAX; ix++)
+      for (ix = 0; ix != GOMP_DIM_MAX; ix++, dims = TREE_CHAIN (dims))
 	{
-	  unsigned HOST_WIDE_INT dim = 0;
+	  HOST_WIDE_INT size = TREE_INT_CST_LOW (TREE_VALUE (dims));
 
-	  if (dims)
-	    {
-	      tree cst = TREE_VALUE (dims);
-
-	      dim = TREE_INT_CST_LOW (cst);
-	      dims = TREE_CHAIN (dims);
-	    }
-	  if (dim != 1)
+	  if (size > 1 || (!size && !TREE_PURPOSE (dims)))
 	    mask |= GOMP_DIM_MASK (ix);
 	}
       /* If there is worker neutering, there must be vector
-	 neutering.  Otherwise the hardware will fail.  This really
-	 should be dealt with earlier because it indicates faulty
-	 logic in determining launch dimensions.  */
-      if (mask & GOMP_DIM_MASK (GOMP_DIM_WORKER))
-	mask |= GOMP_DIM_MASK (GOMP_DIM_VECTOR);
+	 neutering.  Otherwise the hardware will fail.  */
+      gcc_assert (!(mask & GOMP_DIM_MASK (GOMP_DIM_WORKER))
+		  || (mask & GOMP_DIM_MASK (GOMP_DIM_VECTOR)));
 
       parallel *pars = nvptx_discover_pars (&bb_insn_map);
       nvptx_process_pars (pars);
@@ -3175,15 +3168,10 @@ nvptx_record_offload_symbol (tree decl)
 
 	for (ix = 0; ix != GOMP_DIM_MAX; ix++, dims = TREE_CHAIN (dims))
 	  {
-	    tree cst = TREE_VALUE (dims);
+	    HOST_WIDE_INT size = TREE_INT_CST_LOW (TREE_VALUE (dims));
 
-	    /* When device_type support is added an earlier pass
-	       should have massaged the attribute to be
-	       ptx-specific.  */
-	    gcc_assert (TREE_CODE (cst) == INTEGER_CST);
-
-	    unsigned HOST_WIDE_INT dim = TREE_INT_CST_LOW (cst);
-	    fprintf (asm_out_file, ", " HOST_WIDE_INT_PRINT_HEX, dim);
+	    gcc_assert (!TREE_PURPOSE (dims));
+	    fprintf (asm_out_file, ", " HOST_WIDE_INT_PRINT_HEX, size);
 	  }
 
 	fprintf (asm_out_file, "\n");
@@ -3537,18 +3525,23 @@ nvptx_validate_dims (tree decl, tree dim
 {
   tree adims[GOMP_DIM_MAX];
   unsigned ix;
-  tree pos = dims;
+  tree *pos_ptr;
 
-  for (ix = 0; ix != GOMP_DIM_MAX; ix++)
+  for (ix = 0, pos_ptr = &dims; ix != GOMP_DIM_MAX;
+       ix++, pos_ptr = &TREE_CHAIN (*pos_ptr))
     {
-      adims[ix] = TREE_VALUE (pos);
-      pos = TREE_CHAIN (pos);
+      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 PTX_VECTOR_LENGTH 32
 #define PTX_WORKER_LENGTH 32
+
   /* If the worker size is not 1, the vector size must be 32.  If
-     the vector size is not 1, it must be 32.  */
+     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]
@@ -3585,12 +3578,31 @@ nvptx_validate_dims (tree decl, tree dim
       adims[ix] = integer_one_node;
 
   /* Write results.  */
-  pos = dims;
-  for (ix = 0; ix != GOMP_DIM_MAX; ix++, pos = TREE_CHAIN (pos))
+  tree pos;
+  for (ix = 0, pos = dims; ix != GOMP_DIM_MAX; ix++, pos = TREE_CHAIN (pos))
     TREE_VALUE (pos) = adims[ix];
-  
+
   return dims;
 }
+
+/* Return maximum dimension size, or zero for unbounded.  */
+
+static unsigned
+nvptx_dim_limit (unsigned axis)
+{
+  switch (axis)
+    {
+    case GOMP_DIM_WORKER:
+      return PTX_WORKER_LENGTH;
+      break;
+    case GOMP_DIM_VECTOR:
+      return  PTX_VECTOR_LENGTH;
+      break;
+    default: break;
+    }
+  return 0;
+}
+
 
 #undef TARGET_OPTION_OVERRIDE
 #define TARGET_OPTION_OVERRIDE nvptx_option_override
@@ -3689,6 +3701,9 @@ nvptx_validate_dims (tree decl, tree dim
 #undef TARGET_GOACC_VALIDATE_DIMS
 #define TARGET_GOACC_VALIDATE_DIMS nvptx_validate_dims
 
+#undef TARGET_GOACC_DIM_LIMIT
+#define TARGET_GOACC_DIM_LIMIT nvptx_dim_limit
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-nvptx.h"
Index: omp-low.c
===================================================================
--- omp-low.c	(revision 226770)
+++ omp-low.c	(working copy)
@@ -4681,13 +4681,13 @@ expand_oacc_get_num_threads (gimple_seq
   for (ix = GOMP_DIM_GANG; ix != GOMP_DIM_MAX; ix++)
     if (GOMP_DIM_MASK(ix) & gwv_bits)
       {
-	tree arg = build_int_cst (unsigned_type_node, ix);
-	tree count = create_tmp_var (unsigned_type_node);
+	tree arg = build_int_cst (integer_type_node, ix);
+	tree count = create_tmp_var (integer_type_node);
 	gimple call = gimple_build_call_internal (IFN_GOACC_DIM_SIZE, 1, arg);
 	
 	gimple_call_set_lhs (call, count);
 	gimple_seq_add_stmt (seq, call);
-	res = fold_build2 (MULT_EXPR, unsigned_type_node, res, count);
+	res = fold_build2 (MULT_EXPR, integer_type_node, res, count);
       }
   
   return res;
@@ -4713,30 +4713,30 @@ expand_oacc_get_thread_num (gimple_seq *
 	  {
 	    /* We had an outer index, so scale that by the size of
 	       this dimension.  */
-	    tree n = create_tmp_var (unsigned_type_node);
+	    tree n = create_tmp_var (integer_type_node);
 	    gimple call
 	      = gimple_build_call_internal (IFN_GOACC_DIM_SIZE, 1, arg);
 	    
 	    gimple_call_set_lhs (call, n);
 	    gimple_seq_add_stmt (seq, call);
-	    res = fold_build2 (MULT_EXPR, unsigned_type_node, res, n);
+	    res = fold_build2 (MULT_EXPR, integer_type_node, res, n);
 	  }
 
 	/* Determine index in this dimension.  */
-	tree id = create_tmp_var (unsigned_type_node);
+	tree id = create_tmp_var (integer_type_node);
 	gimple call = gimple_build_call_internal (IFN_GOACC_DIM_POS, 1, arg);
 	
 	gimple_call_set_lhs (call, id);
 	gimple_seq_add_stmt (seq, call);
 	if (res)
-	  res = fold_build2 (PLUS_EXPR, unsigned_type_node, res, id);
+	  res = fold_build2 (PLUS_EXPR, integer_type_node, res, id);
 	else
 	  res = id;
       }
 
   if (res == NULL_TREE)
-    res = build_int_cst (unsigned_type_node, 0);
-			 
+    res = build_int_cst (integer_type_node, 0);
+
   return res;
 }
 
@@ -11662,8 +11662,8 @@ oacc_init_count_vars (omp_context *ctx,
     {
       tree arg = build_int_cst (unsigned_type_node, GOMP_DIM_WORKER);
       
-      worker_var = create_tmp_var (unsigned_type_node, ".worker");
-      worker_count = create_tmp_var (unsigned_type_node, ".workercount");
+      worker_var = create_tmp_var (integer_type_node, ".worker");
+      worker_count = create_tmp_var (integer_type_node, ".workercount");
       
       gimple call1 = gimple_build_call_internal (IFN_GOACC_DIM_POS, 1, arg);
       gimple_call_set_lhs (call1, worker_var);
@@ -11675,8 +11675,8 @@ oacc_init_count_vars (omp_context *ctx,
     }
   else
     {
-      worker_var = build_int_cst (unsigned_type_node, 0);
-      worker_count = build_int_cst (unsigned_type_node, 1);
+      worker_var = build_int_cst (integer_type_node, 0);
+      worker_count = build_int_cst (integer_type_node, 1);
     }
   
   ctx->worker_var = worker_var;
@@ -14582,17 +14582,19 @@ oacc_xform_dim (gimple_stmt_iterator *gs
 {
   tree arg = gimple_call_arg (stmt, 0);
   unsigned axis = (unsigned)TREE_INT_CST_LOW (arg);
+
   while (axis--)
     dims = TREE_CHAIN (dims);
-  unsigned size = TREE_INT_CST_LOW (TREE_VALUE (dims));
+  int size = TREE_INT_CST_LOW (TREE_VALUE (dims));
 
-  if (size == 0)
+  if (!size)
     /* Dimension size is dynamic.  */
     return;
+  
   if (is_pos)
     {
       if (size != 1)
-	/* Size is more than 1.  */
+	/* Size is more than 1, so POS might be non-zero.  */
 	return;
       size = 0;
     }
@@ -14600,7 +14602,7 @@ oacc_xform_dim (gimple_stmt_iterator *gs
   /* Replace the internal call with a constant.  */
   tree lhs = gimple_call_lhs (stmt);
   gimple g = gimple_build_assign
-    (lhs, build_int_cst (unsigned_type_node, size));
+    (lhs, build_int_cst (integer_type_node, size));
   gsi_replace (gsi, g, false);
 }
 
@@ -14619,8 +14621,7 @@ execute_oacc_transform ()
     return 0;
 
   tree dims = TREE_VALUE (attrs);
-  if (dims)
-    dims = targetm.goacc.validate_dims (current_function_decl, dims);
+  dims = targetm.goacc.validate_dims (current_function_decl, dims);
   /* Safe to overwrite, this attribute chain is unshared.  */
   TREE_VALUE (attrs) = dims;
   
@@ -14645,13 +14646,11 @@ execute_oacc_transform ()
 		    default: break;
 
 		    case IFN_GOACC_DIM_SIZE:
-		      if (dims)
-			oacc_xform_dim (&gsi, stmt, dims, false);
+		      oacc_xform_dim (&gsi, stmt, dims, false);
 		      break;
 
 		    case IFN_GOACC_DIM_POS:
-		      if (dims)
-			oacc_xform_dim (&gsi, stmt, dims, true);
+		      oacc_xform_dim (&gsi, stmt, dims, true);
 		      break;
 
 		    case IFN_GOACC_FORK:
@@ -14681,24 +14680,42 @@ execute_oacc_transform ()
 tree
 default_goacc_validate_dims (tree ARG_UNUSED (decl), tree dims)
 {
-  tree pos = dims;
-  for (unsigned ix = GOMP_DIM_MAX; ix--; pos = TREE_CHAIN (pos))
+  tree *pos_ptr;
+  unsigned ix;
+  
+  for (ix = 0, pos_ptr = &dims;
+       ix != GOMP_DIM_MAX; ix++, pos_ptr = &TREE_CHAIN (*pos_ptr))
     {
-      tree val = TREE_VALUE (pos);
+      /* 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)
-	val = integer_one_node;
-#else
-      /* Set to 1 on the host. */
-      val = integer_one_node;
 #endif
-      TREE_VALUE (pos) =  val;
+	val = integer_one_node;
+      TREE_VALUE (*pos_ptr) = val;
     }
 
   return dims;
 }
 
+/* Default dimension bound is unknown on accelerator and 1 on host. */
+
+unsigned
+default_goacc_dim_limit (unsigned ARG_UNUSED (axis))
+{
+#ifdef ACCEL_COMPILER
+  return 0;
+#else
+  return 1;
+#endif
+}
+
 /* Default fork/join early expander.  Delete the function calls if
    there is no RTL expander.  */
 
Index: doc/tm.texi.in
===================================================================
--- doc/tm.texi.in	(revision 226770)
+++ doc/tm.texi.in	(working copy)
@@ -4247,6 +4247,8 @@ address;  but often a machine-dependent
 
 @hook TARGET_GOACC_VALIDATE_DIMS
 
+@hook TARGET_GOACC_DIM_LIMIT
+
 @hook TARGET_GOACC_FORK_JOIN
 
 @node Anchored Addresses
Index: doc/tm.texi
===================================================================
--- doc/tm.texi	(revision 226770)
+++ doc/tm.texi	(working copy)
@@ -5743,7 +5743,13 @@ to use it.
 @deftypefn {Target Hook} tree TARGET_GOACC_VALIDATE_DIMS (tree, @var{tree})
 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.  Diagnostics should be issued as appropriate.
+and may be overwritten.  If the dimension list is NULL, one should be
+created.  Diagnostics should be issued as appropriate.
+@end deftypefn
+
+@deftypefn {Target Hook} unsigned TARGET_GOACC_DIM_LIMIT (unsigned)
+This hook should return the maximum size of a particular dimension,
+or zero if unbounded.
 @end deftypefn
 
 @deftypefn {Target Hook} bool TARGET_GOACC_FORK_JOIN (bool, gimple_stmt_iterator *@var{}, @var{gimple})