Patchwork RFC: simd enabled functions (omp declare simd / elementals)

login
register
mail settings
Submitter Jakub Jelinek
Date Nov. 1, 2013, 4:50 p.m.
Message ID <20131101165052.GE27813@tucnak.zalov.cz>
Download mbox | patch
Permalink /patch/287873/
State New
Headers show

Comments

Jakub Jelinek - Nov. 1, 2013, 4:50 p.m.
On Fri, Nov 01, 2013 at 01:35:35PM +0100, Jakub Jelinek wrote:
> So, IMHO much better would be to have an enum simd_clone_arg_type which
> would be
> enum simd_clone_arg_type
> {
>   SIMD_CLONE_ARG_TYPE_VECTOR,
>   SIMD_CLONE_ARG_TYPE_UNIFORM,
>   SIMD_CLONE_ARG_TYPE_LINEAR_CONSTANT_STEP,
>   SIMD_CLONE_ARG_TYPE_LINEAR_VARIABLE_STEP
> };
> drop uniform bitfield, change linear_stride_num to
> say union { unsigned HOST_WIDE_INT linear_constant_step; int linear_step_argno; };
> or similar.

I've committed this to gomp-4_0-branch as follow-up to your patch:

2013-11-01  Jakub Jelinek  <jakub@redhat.com>

	* cgraph.h (enum linear_stride_type): Remove.
	(enum simd_clone_arg_type): New.
	(struct simd_clone_arg): Remove linear_stride, linear_stride_num
	and uniform fields.  Add arg_type and linear_step.
	* omp-low.c (simd_clone_struct_copy): Formatting.
	(simd_clone_struct_alloc): Likewise.  Use size_t.
	(simd_clone_clauses_extract, simd_clone_compute_base_data_type,
	simd_clone_adjust_argument_types): Adjust for struct simd_clone_arg
	changes.
	(simd_clone_mangle): Likewise.  Handle negative linear step.


	Jakub

Patch

--- gcc/cgraph.h.jj	2013-11-01 17:11:42.000000000 +0100
+++ gcc/cgraph.h	2013-11-01 17:24:59.472995514 +0100
@@ -250,10 +250,12 @@  struct GTY(()) cgraph_clone_info
   bitmap combined_args_to_skip;
 };
 
-enum linear_stride_type {
-  LINEAR_STRIDE_NO,
-  LINEAR_STRIDE_YES_CONSTANT,
-  LINEAR_STRIDE_YES_VARIABLE
+enum simd_clone_arg_type
+{
+  SIMD_CLONE_ARG_TYPE_VECTOR,
+  SIMD_CLONE_ARG_TYPE_UNIFORM,
+  SIMD_CLONE_ARG_TYPE_LINEAR_CONSTANT_STEP,
+  SIMD_CLONE_ARG_TYPE_LINEAR_VARIABLE_STEP
 };
 
 /* Function arguments in the original function of a SIMD clone.
@@ -282,28 +284,17 @@  struct GTY(()) simd_clone_arg {
   tree simd_array;
 
   /* A SIMD clone's argument can be either linear (constant or
-     variable), uniform, or vector.  If the argument is neither linear
-     or uniform, the default is vector.  */
+     variable), uniform, or vector.  */
+  enum simd_clone_arg_type arg_type;
 
-  /* If the linear stride is a constant, `linear_stride' is
-     LINEAR_STRIDE_YES_CONSTANT, and `linear_stride_num' holds
-     the numeric stride.
-
-     If the linear stride is variable, `linear_stride' is
-     LINEAR_STRIDE_YES_VARIABLE, and `linear_stride_num' contains
-     the function argument containing the stride (as an index into the
-     function arguments starting at 0).
-
-     Otherwise, `linear_stride' is LINEAR_STRIDE_NO and
-     `linear_stride_num' is unused.  */
-  enum linear_stride_type linear_stride;
-  unsigned HOST_WIDE_INT linear_stride_num;
+  /* For arg_type SIMD_CLONE_ARG_TYPE_LINEAR_CONSTANT_STEP this is
+     the constant linear step, if arg_type is
+     SIMD_CLONE_ARG_TYPE_LINEAR_VARIABLE_STEP, this is index of
+     the uniform argument holding the step, otherwise 0.  */
+  HOST_WIDE_INT linear_step;
 
   /* Variable alignment if available, otherwise 0.  */
   unsigned int alignment;
-
-  /* True if variable is uniform.  */
-  unsigned int uniform : 1;
 };
 
 /* Specific data for a SIMD function clone.  */
--- gcc/omp-low.c.jj	2013-11-01 17:11:42.000000000 +0100
+++ gcc/omp-low.c	2013-11-01 17:41:26.635904034 +0100
@@ -10561,8 +10561,8 @@  static struct simd_clone *
 simd_clone_struct_alloc (int nargs)
 {
   struct simd_clone *clone_info;
-  int len = sizeof (struct simd_clone)
-    + nargs * sizeof (struct simd_clone_arg);
+  size_t len = (sizeof (struct simd_clone)
+		+ nargs * sizeof (struct simd_clone_arg));
   clone_info = ggc_alloc_cleared_simd_clone_stat (len PASS_MEM_STAT);
   return clone_info;
 }
@@ -10572,8 +10572,8 @@  simd_clone_struct_alloc (int nargs)
 static inline void
 simd_clone_struct_copy (struct simd_clone *to, struct simd_clone *from)
 {
-  memcpy (to, from, sizeof (struct simd_clone)
-	  + from->nargs * sizeof (struct simd_clone_arg));
+  memcpy (to, from, (sizeof (struct simd_clone)
+		     + from->nargs * sizeof (struct simd_clone_arg)));
 }
 
 /* Given a simd clone in NEW_NODE, extract the simd specific
@@ -10637,31 +10637,27 @@  simd_clone_clauses_extract (struct cgrap
 	    int argno = TREE_INT_CST_LOW (decl);
 	    if (OMP_CLAUSE_LINEAR_VARIABLE_STRIDE (t))
 	      {
-		clone_info->args[argno].linear_stride
-		  = LINEAR_STRIDE_YES_VARIABLE;
-		clone_info->args[argno].linear_stride_num
-		  = TREE_INT_CST_LOW (step);
-		gcc_assert (!TREE_INT_CST_HIGH (step));
+		clone_info->args[argno].arg_type
+		  = SIMD_CLONE_ARG_TYPE_LINEAR_VARIABLE_STEP;
+		clone_info->args[argno].linear_step
+		  = tree_low_cst (step, 0);
+		gcc_assert (clone_info->args[argno].linear_step >= 0
+			    && clone_info->args[argno].linear_step < n);
 	      }
 	    else
 	      {
-		if (TREE_INT_CST_HIGH (step))
-		  {
-		    /* It looks like this can't really happen, since the
-		       front-ends generally issue:
-
-		       warning: integer constant is too large for its type.
-
-		       But let's assume somehow we got past all that.  */
-		    warning_at (DECL_SOURCE_LOCATION (decl), 0,
-				"ignoring large linear step");
-		  }
+		if (!host_integerp (step, 0))
+		  warning_at (OMP_CLAUSE_LOCATION (t), 0,
+			      "ignoring large linear step");
+		else if (integer_zerop (step))
+		  warning_at (OMP_CLAUSE_LOCATION (t), 0,
+			      "ignoring zero linear step");
 		else
 		  {
-		    clone_info->args[argno].linear_stride
-		      = LINEAR_STRIDE_YES_CONSTANT;
-		    clone_info->args[argno].linear_stride_num
-		      = TREE_INT_CST_LOW (step);
+		    clone_info->args[argno].arg_type
+		      = SIMD_CLONE_ARG_TYPE_LINEAR_CONSTANT_STEP;
+		    clone_info->args[argno].linear_step
+		      = tree_low_cst (step, 0);
 		  }
 	      }
 	    break;
@@ -10670,7 +10666,8 @@  simd_clone_clauses_extract (struct cgrap
 	  {
 	    tree decl = OMP_CLAUSE_DECL (t);
 	    int argno = tree_low_cst (decl, 1);
-	    clone_info->args[argno].uniform = 1;
+	    clone_info->args[argno].arg_type
+	      = SIMD_CLONE_ARG_TYPE_UNIFORM;
 	    break;
 	  }
 	case OMP_CLAUSE_ALIGNED:
@@ -10731,14 +10728,12 @@  simd_clone_compute_base_data_type (struc
     {
       argno_map map (fndecl);
       for (unsigned int i = 0; i < new_node->simdclone->nargs; ++i)
-	{
-	  struct simd_clone_arg arg = new_node->simdclone->args[i];
-	  if (!arg.uniform && arg.linear_stride == LINEAR_STRIDE_NO)
-	    {
-	      type = TREE_TYPE (map[i]);
-	      break;
-	    }
-	}
+	if (new_node->simdclone->args[i].arg_type
+	    == SIMD_CLONE_ARG_TYPE_VECTOR)
+	  {
+	    type = TREE_TYPE (map[i]);
+	    break;
+	  }
     }
 
   /* c) If the characteristic data type determined by a) or b) above
@@ -10824,20 +10819,25 @@  simd_clone_mangle (struct cgraph_node *o
     {
       struct simd_clone_arg arg = new_node->simdclone->args[n];
 
-      if (arg.uniform)
+      if (arg.arg_type == SIMD_CLONE_ARG_TYPE_UNIFORM)
 	pp_character (&pp, 'u');
-      else if (arg.linear_stride == LINEAR_STRIDE_YES_CONSTANT)
+      else if (arg.arg_type == SIMD_CLONE_ARG_TYPE_LINEAR_CONSTANT_STEP)
 	{
-	  gcc_assert (arg.linear_stride_num != 0);
+	  gcc_assert (arg.linear_step != 0);
 	  pp_character (&pp, 'l');
-	  if (arg.linear_stride_num > 1)
-	    pp_unsigned_wide_integer (&pp,
-				      arg.linear_stride_num);
+	  if (arg.linear_step > 0)
+	    pp_unsigned_wide_integer (&pp, arg.linear_step);
+	  else
+	    {
+	      pp_character (&pp, 'n');
+	      pp_unsigned_wide_integer (&pp, (-(unsigned HOST_WIDE_INT)
+					      arg.linear_step));
+	    }
 	}
-      else if (arg.linear_stride == LINEAR_STRIDE_YES_VARIABLE)
+      else if (arg.arg_type == SIMD_CLONE_ARG_TYPE_LINEAR_VARIABLE_STEP)
 	{
 	  pp_character (&pp, 's');
-	  pp_unsigned_wide_integer (&pp, arg.linear_stride_num);
+	  pp_unsigned_wide_integer (&pp, arg.linear_step);
 	}
       else
 	pp_character (&pp, 'v');
@@ -10975,8 +10975,7 @@  simd_clone_adjust_argument_types (struct
 
       node->simdclone->args[i].orig_arg = parm;
 
-      if (node->simdclone->args[i].uniform
-	  || node->simdclone->args[i].linear_stride != LINEAR_STRIDE_NO)
+      if (node->simdclone->args[i].arg_type != SIMD_CLONE_ARG_TYPE_VECTOR)
 	{
 	  /* No adjustment necessary for scalar arguments.  */
 	  adj.copy_param = 1;