diff mbox

[gomp4] Some progress on #pragma omp simd

Message ID 51BF43C0.9080901@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez June 17, 2013, 5:13 p.m. UTC 
On 06/12/13 16:36, Jakub Jelinek wrote:
> On Wed, Jun 12, 2013 at 10:38:00AM -0700, Richard Henderson wrote:
>> On 06/12/2013 10:30 AM, Jakub Jelinek wrote:
>>> So the built-ins would take address of this decl, something else?
>>
>> Perhaps address, perhaps just referenced uninitialized?
>
> True, assuming no pass would actually want to change that SSA_NAME of the
> magic decl just because it is undefined (coalesce with some other undefined
> SSA_NAME or something similar).  I hope nothing does that, it would be
> problematic for the uninitialized warning pass too I bet.

Gentlemen:

Attached is a lightly tested patch rewriting the aforementioned UID to 
use the address of a dummy DECL (see the lower_rec_input_clauses fragment).

I first tried an uninitialized reference but into-SSA happens after 
lower_rec_input_clauses, and the DECL gets rewritten into SSA, while 
loop->simduid maintains the original DECL.  This causes all the equality 
checks throughout to fail.  I thought of changing the SSA pass to update 
loop->simduid as well, but that seemed kludgy.  I didn't think about it 
too hard, perhaps there's a more elegant way.

Anyway...this at least works with my contrived reduction tests for Cilk 
Plus pragma simd.

If this is what you want, I can start looking at how this behaves with 
LTO and inlining.

Aldy

Comments

Richard Henderson June 17, 2013, 5:23 p.m. UTC | #1 
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.


r~
diff mbox

Patch

diff --git a/gcc/ChangeLog.gomp b/gcc/ChangeLog.gomp
index 7f9151d..0478ab2 100644
--- a/gcc/ChangeLog.gomp
+++ b/gcc/ChangeLog.gomp
@@ -1,3 +1,20 @@ 
+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.
+
 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..84448a7 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,17 @@  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");
+      TREE_ADDRESSABLE (uid) = 1;
+      uid = build_fold_addr_expr (uid);
       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__UID (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 +2904,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 +5662,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__UID (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..63d3473 100644
--- a/gcc/tree-data-ref.c
+++ b/gcc/tree-data-ref.c
@@ -4335,17 +4335,16 @@  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);
 	  if (loop == NULL
-	      || !host_integerp (uid, 1)
-	      || loop->simduid != tree_low_cst (uid, 1))
+	      || loop->simduid != 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-vect-data-refs.c b/gcc/tree-vect-data-refs.c
index 884d369..27a215b 100644
--- a/gcc/tree-vect-data-refs.c
+++ b/gcc/tree-vect-data-refs.c
@@ -2900,7 +2900,7 @@  vect_analyze_data_refs (loop_vec_info loop_vinfo,
 	      && !TREE_THIS_VOLATILE (DR_REF (dr))
 	      && targetm.vectorize.builtin_gather != NULL;
 	  bool maybe_simd_lane_access
-	    = loop_vinfo && loop->simduid;
+	    = loop_vinfo && loop->simduid != NULL_TREE;
 
 	  /* If target supports vector gather loads, or if this might be
 	     a SIMD lane access, see if they can't be used.  */
@@ -2940,11 +2940,7 @@  vect_analyze_data_refs (loop_vec_info loop_vinfo,
 			      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
+				  && gimple_call_arg (def, 0) == loop->simduid
 				  /* For now.  */
 				  && tree_int_cst_equal (TYPE_SIZE_UNIT (reft),
 							 step))
diff --git a/gcc/tree-vectorizer.c b/gcc/tree-vectorizer.c
index a0bca03..42de09e 100644
--- a/gcc/tree-vectorizer.c
+++ b/gcc/tree-vectorizer.c
@@ -79,7 +79,7 @@  vec<vec_void_p> stmt_vec_info_vec;
 
 struct simduid_to_vf : typed_free_remove<simduid_to_vf>
 {
-  unsigned int simduid;
+  tree simduid;
   int vf;
 
   /* hash_table support.  */
@@ -92,7 +92,7 @@  struct simduid_to_vf : typed_free_remove<simduid_to_vf>
 inline hashval_t
 simduid_to_vf::hash (const value_type *p)
 {
-  return p->simduid;
+  return htab_hash_pointer (p->simduid);
 }
 
 inline int
@@ -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,9 @@  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));
+	  gcc_assert (gimple_call_arg (stmt, 0) != NULL_TREE);
 	  simduid_to_vf *p = NULL, data;
-	  data.simduid = tree_low_cst (gimple_call_arg (stmt, 0), 1);
+	  data.simduid = gimple_call_arg (stmt, 0);
 	  if (htab.is_created ())
 	    p = htab.find (&data);
 	  if (p)