Patchwork [gomp4] Some progress on #pragma omp simd

login
register
mail settings
Submitter Aldy Hernandez
Date June 18, 2013, 12:59 a.m.
Message ID <51BFB0E3.4060502@redhat.com>
Download mbox | patch
Permalink /patch/252091/
State New
Headers show

Comments

Aldy Hernandez - June 18, 2013, 12:59 a.m.
On 06/17/13 12:23, Richard Henderson wrote:
> On 06/17/2013 10:13 AM, Aldy Hernandez wrote:
>> -	  data.simduid = tree_low_cst (gimple_call_arg (stmt, 0), 1);
>> +	  data.simduid = gimple_call_arg (stmt, 0);
>
> Doesn't this copy the ADDR_EXPR from the call into simduid?
>
>>   simduid_to_vf::hash (const value_type *p)
>>   {
>> -  return p->simduid;
>> +  return htab_hash_pointer (p->simduid);
>
> ... at which point this bit is meaningless since all ADDR_EXPRs must of course
> have different pointers.
>
> I think we should validate the DECL_P extracted from the call_arg, and store
> that.  The hash should use DECL_UID to minimize hash variation due to memory
> layout.

As discussed on IRC.  Attached are these changes you requested, plus 
changing OMP_CLAUSE__SIMDUID__UID to OMP_CLAUSE__SIMDUID__DECL.

I will tackle the dot named builtins in the next iteration.

BTW, this patch bootstraps with no regressions.  I also manually 
inspected the gimple generated by the test below, and made sure that 
inlining func() into both foo() and bar() have different temporaries. 
Without this patch, the same constant was used incorrectly as arguments 
to __builtin_GOMP.simd_vf and __builtin_GOMP.simd_lane.

How does this look?
Aldy

#define N 1000

static inline int func (int *p)
{
   int x = 0, i;
#pragma simd reduction (+:x)
   for (i = 0; i < 1000; i++)
     x += p[i];
   return x;
}

int array[55555];
int dork[5555];
foo()
{
   return func(array);
}

bar()
{
   return func(dork) + 666;
}
Jakub Jelinek - June 18, 2013, 7:09 a.m.
On Mon, Jun 17, 2013 at 07:59:15PM -0500, Aldy Hernandez wrote:
> As discussed on IRC.  Attached are these changes you requested, plus
> changing OMP_CLAUSE__SIMDUID__UID to OMP_CLAUSE__SIMDUID__DECL.
> 
> I will tackle the dot named builtins in the next iteration.

Thanks.

> --- a/gcc/builtin-types.def
> +++ b/gcc/builtin-types.def
> @@ -227,6 +227,7 @@ DEF_FUNCTION_TYPE_1 (BT_FN_DFLOAT128_DFLOAT128, BT_DFLOAT128, BT_DFLOAT128)
>  DEF_FUNCTION_TYPE_1 (BT_FN_VOID_VPTR, BT_VOID, BT_VOLATILE_PTR)
>  DEF_FUNCTION_TYPE_1 (BT_FN_VOID_PTRPTR, BT_VOID, BT_PTR_PTR)
>  DEF_FUNCTION_TYPE_1 (BT_FN_UINT_UINT, BT_UINT, BT_UINT)
> +DEF_FUNCTION_TYPE_1 (BT_FN_UINT_PTR, BT_UINT, BT_PTR)
>  DEF_FUNCTION_TYPE_1 (BT_FN_ULONG_ULONG, BT_ULONG, BT_ULONG)
>  DEF_FUNCTION_TYPE_1 (BT_FN_ULONGLONG_ULONGLONG, BT_ULONGLONG, BT_ULONGLONG)
>  DEF_FUNCTION_TYPE_1 (BT_FN_UINT16_UINT16, BT_UINT16, BT_UINT16)

You can avoid this by using say unsigned_type_node as the type of the magic
decl rather than pointer type.  Though, with internal functions this will
not be needed anyway.

> diff --git a/gcc/cfgloop.h b/gcc/cfgloop.h
> index 6cc9a6c..41677bc 100644
> --- a/gcc/cfgloop.h
> +++ b/gcc/cfgloop.h
> @@ -176,7 +176,7 @@ struct GTY ((chain_next ("%h.next"))) loop {
>  
>    /* For SIMD loops, this is a unique identifier of the loop, referenced
>       by __builtin_GOMP.simd_vf and __builtin_GOMP.simd_lane builtins.  */
> -  unsigned int simduid;
> +  tree simduid;
>  
>    /* True if we should try harder to vectorize this loop.  */
>    bool force_vect;

Please move simduid after force_vect, so that it is better packed.

	Jakub

Patch

diff --git a/gcc/ChangeLog.gomp b/gcc/ChangeLog.gomp
index 7f9151d..0ed1b2c 100644
--- a/gcc/ChangeLog.gomp
+++ b/gcc/ChangeLog.gomp
@@ -1,3 +1,24 @@ 
+2013-06-17  Aldy Hernandez  <aldyh@redhat.com>
+
+	* builtin-types.def (BT_FN_UINT_PTR): New.
+	* omp-builtins.def (BUILT_IN_GOMP_SIMD_LANE): Use it.
+	(BUILT_IN_GOMP_SIMD_VF): Same.
+	* cfgloop.h (struct loop): Change type of simduid to tree.
+	* omp-low.c (lower_rec_input_clauses): Adapt to use simduid as a
+	tree.
+	(expand_omp_simd): Same.
+	* tree-data-ref.c (get_references_in_stmt): Same.
+	* tree-vect-data-refs.c (vect_analyze_data_refs): Same.
+	* tree-vectorizer.c (struct simduid_to_vf): Change type of simduid
+	to tree.
+	(simduid_to_vf::hash): Hash pointer.
+	(adjust_simduid_builtins): Add comment.
+	Use simduid as tree.
+	* tree-pretty-print.c (dump_omp_clause): Rename
+	OMP_CLAUSE__SIMDUID__UID to OMP_CLAUSE__SIMDUID__DECL.
+	* tree.h (OMP_CLAUSE__SIMDUID__DECL): Rename from
+	OMP_CLAUSE__SIMDUID__UID.
+
 2013-06-14  Jakub Jelinek  <jakub@redhat.com>
 
 	* gimple-pretty-print.c (dump_gimple_omp_for): Don't handle
diff --git a/gcc/builtin-types.def b/gcc/builtin-types.def
index 4c866f2..171fdb7 100644
--- a/gcc/builtin-types.def
+++ b/gcc/builtin-types.def
@@ -227,6 +227,7 @@  DEF_FUNCTION_TYPE_1 (BT_FN_DFLOAT128_DFLOAT128, BT_DFLOAT128, BT_DFLOAT128)
 DEF_FUNCTION_TYPE_1 (BT_FN_VOID_VPTR, BT_VOID, BT_VOLATILE_PTR)
 DEF_FUNCTION_TYPE_1 (BT_FN_VOID_PTRPTR, BT_VOID, BT_PTR_PTR)
 DEF_FUNCTION_TYPE_1 (BT_FN_UINT_UINT, BT_UINT, BT_UINT)
+DEF_FUNCTION_TYPE_1 (BT_FN_UINT_PTR, BT_UINT, BT_PTR)
 DEF_FUNCTION_TYPE_1 (BT_FN_ULONG_ULONG, BT_ULONG, BT_ULONG)
 DEF_FUNCTION_TYPE_1 (BT_FN_ULONGLONG_ULONGLONG, BT_ULONGLONG, BT_ULONGLONG)
 DEF_FUNCTION_TYPE_1 (BT_FN_UINT16_UINT16, BT_UINT16, BT_UINT16)
diff --git a/gcc/cfgloop.h b/gcc/cfgloop.h
index 6cc9a6c..41677bc 100644
--- a/gcc/cfgloop.h
+++ b/gcc/cfgloop.h
@@ -176,7 +176,7 @@  struct GTY ((chain_next ("%h.next"))) loop {
 
   /* For SIMD loops, this is a unique identifier of the loop, referenced
      by __builtin_GOMP.simd_vf and __builtin_GOMP.simd_lane builtins.  */
-  unsigned int simduid;
+  tree simduid;
 
   /* True if we should try harder to vectorize this loop.  */
   bool force_vect;
diff --git a/gcc/omp-builtins.def b/gcc/omp-builtins.def
index 8ad2113..ddbe2c1 100644
--- a/gcc/omp-builtins.def
+++ b/gcc/omp-builtins.def
@@ -220,6 +220,6 @@  DEF_GOMP_BUILTIN (BUILT_IN_GOMP_SINGLE_COPY_END, "GOMP_single_copy_end",
 		  BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST)
 
 DEF_GOMP_BUILTIN (BUILT_IN_GOMP_SIMD_LANE, "GOMP.simd_lane",
-		  BT_FN_UINT_UINT, ATTR_NOVOPS_NOTHROW_LEAF_LIST)
+		  BT_FN_UINT_PTR, ATTR_NOVOPS_NOTHROW_LEAF_LIST)
 DEF_GOMP_BUILTIN (BUILT_IN_GOMP_SIMD_VF, "GOMP.simd_vf",
-		  BT_FN_UINT_UINT, ATTR_CONST_NOTHROW_LEAF_LIST)
+		  BT_FN_UINT_PTR, ATTR_CONST_NOTHROW_LEAF_LIST)
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index a9e2758..731c6d9 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -2497,7 +2497,6 @@  lower_rec_input_clauses (tree clauses, gimple_seq *ilist, gimple_seq *dlist,
   bool copyin_by_ref = false;
   bool lastprivate_firstprivate = false;
   int pass;
-  static int simd_uid;
   bool is_simd = (gimple_code (ctx->stmt) == GIMPLE_OMP_FOR
 		  && gimple_omp_for_kind (ctx->stmt) == GF_OMP_FOR_KIND_SIMD);
   int max_vf = 0;
@@ -2887,15 +2886,15 @@  lower_rec_input_clauses (tree clauses, gimple_seq *ilist, gimple_seq *dlist,
 
   if (lane)
     {
-      tree uid_cst = build_int_cst (unsigned_type_node, ++simd_uid);
+      tree uid = create_tmp_var (ptr_type_node, "simduid");
       gimple g
 	= gimple_build_call (builtin_decl_explicit (BUILT_IN_GOMP_SIMD_LANE), 1,
-			     uid_cst);
+			     uid);
       gimple_call_set_lhs (g, lane);
       gimple_stmt_iterator gsi = gsi_start_1 (gimple_omp_body_ptr (ctx->stmt));
       gsi_insert_before_without_update (&gsi, g, GSI_SAME_STMT);
       c = build_omp_clause (UNKNOWN_LOCATION, OMP_CLAUSE__SIMDUID_);
-      OMP_CLAUSE__SIMDUID__UID (c) = uid_cst;
+      OMP_CLAUSE__SIMDUID__DECL (c) = uid;
       OMP_CLAUSE_CHAIN (c) = gimple_omp_for_clauses (ctx->stmt);
       gimple_omp_for_set_clauses (ctx->stmt, c);
       for (int i = 0; i < 2; i++)
@@ -2903,7 +2902,7 @@  lower_rec_input_clauses (tree clauses, gimple_seq *ilist, gimple_seq *dlist,
 	  {
 	    tree vf = create_tmp_var (unsigned_type_node, NULL);
 	    tree fndecl = builtin_decl_explicit (BUILT_IN_GOMP_SIMD_VF);
-	    g = gimple_build_call (fndecl, 1, uid_cst);
+	    g = gimple_build_call (fndecl, 1, uid);
 	    gimple_call_set_lhs (g, vf);
 	    gimple_seq *seq = i == 0 ? ilist : dlist;
 	    gimple_seq_add_stmt (seq, g);
@@ -5661,7 +5660,7 @@  expand_omp_simd (struct omp_region *region, struct omp_for_data *fd)
 	}
       if (simduid)
 	{
-	  loop->simduid = tree_low_cst (OMP_CLAUSE__SIMDUID__UID (simduid), 1);
+	  loop->simduid = OMP_CLAUSE__SIMDUID__DECL (simduid);
 	  cfun->has_simduid_loops = true;
 	}
       /* If not -fno-tree-vectorize, hint that we want to vectorize
diff --git a/gcc/tree-data-ref.c b/gcc/tree-data-ref.c
index 52658ef..a577406 100644
--- a/gcc/tree-data-ref.c
+++ b/gcc/tree-data-ref.c
@@ -4335,17 +4335,17 @@  get_references_in_stmt (gimple stmt, vec<data_ref_loc, va_stack> *references)
       && !(gimple_call_flags (stmt) & ECF_CONST))
     {
       /* Allow __builtin_GOMP.simd_lane in their own loops.  */
-      if (!gimple_call_builtin_p (stmt, BUILT_IN_GOMP_SIMD_LANE))
-	clobbers_memory = true;
-      else
+      if (gimple_call_builtin_p (stmt, BUILT_IN_GOMP_SIMD_LANE))
 	{
 	  struct loop *loop = gimple_bb (stmt)->loop_father;
 	  tree uid = gimple_call_arg (stmt, 0);
+	  gcc_assert (TREE_CODE (uid) == SSA_NAME);
 	  if (loop == NULL
-	      || !host_integerp (uid, 1)
-	      || loop->simduid != tree_low_cst (uid, 1))
+	      || loop->simduid != SSA_NAME_VAR (uid))
 	    clobbers_memory = true;
 	}
+      else
+	clobbers_memory = true;
     }
   else if (stmt_code == GIMPLE_ASM
 	   && (gimple_asm_volatile_p (stmt) || gimple_vuse (stmt)))
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index e67e48d..f759b0d 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -594,7 +594,7 @@  dump_omp_clause (pretty_printer *buffer, tree clause, int spc, int flags)
 
     case OMP_CLAUSE__SIMDUID_:
       pp_string (buffer, "_simduid_(");
-      dump_generic_node (buffer, OMP_CLAUSE__SIMDUID__UID (clause),
+      dump_generic_node (buffer, OMP_CLAUSE__SIMDUID__DECL (clause),
 			 spc, flags, false);
       pp_character (buffer, ')');
       break;
diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
index 884d369..e833416 100644
--- a/gcc/tree-vect-data-refs.c
+++ b/gcc/tree-vect-data-refs.c
@@ -2939,20 +2939,22 @@  vect_analyze_data_refs (loop_vec_info loop_vinfo,
 			      gimple def = SSA_NAME_DEF_STMT (off);
 			      tree reft = TREE_TYPE (DR_REF (newdr));
 			      if (gimple_call_builtin_p (def,
-						BUILT_IN_GOMP_SIMD_LANE)
-				  && host_integerp (gimple_call_arg (def, 0),
-						    1)
-				  && (unsigned)
-				     tree_low_cst (gimple_call_arg (def, 0), 1)
-				     == loop->simduid
-				  /* For now.  */
-				  && tree_int_cst_equal (TYPE_SIZE_UNIT (reft),
-							 step))
+						BUILT_IN_GOMP_SIMD_LANE))
 				{
-				  DR_OFFSET (newdr) = ssize_int (0);
-				  DR_STEP (newdr) = step;
-				  dr = newdr;
-				  simd_lane_access = true;
+				  tree arg = gimple_call_arg (def, 0);
+				  gcc_assert (TREE_CODE (arg) == SSA_NAME);
+				  arg = SSA_NAME_VAR (arg);
+				  if (arg == loop->simduid
+				      /* For now.  */
+				      && tree_int_cst_equal
+				           (TYPE_SIZE_UNIT (reft),
+					    step))
+				    {
+				      DR_OFFSET (newdr) = ssize_int (0);
+				      DR_STEP (newdr) = step;
+				      dr = newdr;
+				      simd_lane_access = true;
+				    }
 				}
 			    }
 			}
diff --git a/gcc/tree-vectorizer.c b/gcc/tree-vectorizer.c
index a0bca03..6f9b894 100644
--- a/gcc/tree-vectorizer.c
+++ b/gcc/tree-vectorizer.c
@@ -101,6 +101,9 @@  simduid_to_vf::equal (const value_type *p1, const value_type *p2)
   return p1->simduid == p2->simduid;
 }
 
+/* Expand BUILT_IN_GOMP_SIMD_LANE and BUILT_IN_GOMP_SIMD_VF into their
+   corresponding constants.  */
+
 static void
 adjust_simduid_builtins (hash_table <simduid_to_vf> &htab)
 {
@@ -121,9 +124,11 @@  adjust_simduid_builtins (hash_table <simduid_to_vf> &htab)
 	    is_lane = true;
 	  else if (!gimple_call_builtin_p (stmt, BUILT_IN_GOMP_SIMD_VF))
 	    continue;
-	  gcc_assert (host_integerp (gimple_call_arg (stmt, 0), 1));
+	  tree arg = gimple_call_arg (stmt, 0);
+	  gcc_assert (arg != NULL_TREE);
+	  gcc_assert (TREE_CODE (arg) == SSA_NAME);
 	  simduid_to_vf *p = NULL, data;
-	  data.simduid = tree_low_cst (gimple_call_arg (stmt, 0), 1);
+	  data.simduid = DECL_UID (SSA_NAME_VAR (arg));
 	  if (htab.is_created ())
 	    p = htab.find (&data);
 	  if (p)
@@ -223,7 +228,7 @@  vectorize_loops (void)
 	    simduid_to_vf *simduid_to_vf_data = XNEW (simduid_to_vf);
 	    if (!simduid_to_vf_htab.is_created ())
 	      simduid_to_vf_htab.create (15);
-	    simduid_to_vf_data->simduid = loop->simduid;
+	    simduid_to_vf_data->simduid = DECL_UID (loop->simduid);
 	    simduid_to_vf_data->vf = loop_vinfo->vectorization_factor;
 	    *simduid_to_vf_htab.find_slot (simduid_to_vf_data, INSERT)
 	      = simduid_to_vf_data;
diff --git a/gcc/tree.h b/gcc/tree.h
index 0a7774a..d825606 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -2001,7 +2001,7 @@  extern void protected_set_expr_location (tree, location_t);
 #define OMP_CLAUSE_SIMDLEN_EXPR(NODE) \
   OMP_CLAUSE_OPERAND (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_SIMDLEN), 0)
 
-#define OMP_CLAUSE__SIMDUID__UID(NODE) \
+#define OMP_CLAUSE__SIMDUID__DECL(NODE) \
   OMP_CLAUSE_OPERAND (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE__SIMDUID_), 0)
 
 enum omp_clause_schedule_kind