Patchwork Fix parloops -fcompare-debug failure (PR tree-optimization/46969)

login
register
mail settings
Submitter Jakub Jelinek
Date Dec. 16, 2010, 9:49 p.m.
Message ID <20101216214916.GA2198@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/75804/
State New
Headers show

Comments

Jakub Jelinek - Dec. 16, 2010, 9:49 p.m.
Hi!

The reduction_list hash table in parloops is hashed using htab_hash_pointer
on the gimple stmt and later on htab_traversed with code generation in the
callback, which not only can cause code differences with -g/-g0, but also
with AS randomization could result in different generated code.

Unfortunately, the pass changes the result SSA_NAME of the PHI, so we
can't use its SSA_NAME_VERSION all the time as the hash value, and during
gather_scalar_reductions we can't use gimple_uid, as it is used by the
vectorizer.  So, this patch starts using SSA_NAME_VERSION, at the end
of gather_scalar_reductions sets gimple_uid of the PHIs to the corresponding
SSA_NAME_VERSION and afterwards in reduction_phi uses gimple_uid.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2010-12-16  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/46969
	* tree-parloops.c (struct reduction_info): Add reduc_version.
	(reduction_info_hash): Return reduc_version field.
	(reduction_phi): Set reduc_version to gimple_uid (phi).
	(build_new_reduction): Set reduc_version to SSA_NAME_VERSION of
	phi result.
	(set_reduc_phi_uids): New function.
	(gather_scalar_reductions): Call it at the end through htab_traverse.

	* gcc.dg/autopar/pr46969.c: New test.


	Jakub
Richard Guenther - Dec. 18, 2010, 8:28 p.m.
On Thu, Dec 16, 2010 at 10:49 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> The reduction_list hash table in parloops is hashed using htab_hash_pointer
> on the gimple stmt and later on htab_traversed with code generation in the
> callback, which not only can cause code differences with -g/-g0, but also
> with AS randomization could result in different generated code.
>
> Unfortunately, the pass changes the result SSA_NAME of the PHI, so we
> can't use its SSA_NAME_VERSION all the time as the hash value, and during
> gather_scalar_reductions we can't use gimple_uid, as it is used by the
> vectorizer.  So, this patch starts using SSA_NAME_VERSION, at the end
> of gather_scalar_reductions sets gimple_uid of the PHIs to the corresponding
> SSA_NAME_VERSION and afterwards in reduction_phi uses gimple_uid.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2010-12-16  Jakub Jelinek  <jakub@redhat.com>
>
>        PR tree-optimization/46969
>        * tree-parloops.c (struct reduction_info): Add reduc_version.
>        (reduction_info_hash): Return reduc_version field.
>        (reduction_phi): Set reduc_version to gimple_uid (phi).
>        (build_new_reduction): Set reduc_version to SSA_NAME_VERSION of
>        phi result.
>        (set_reduc_phi_uids): New function.
>        (gather_scalar_reductions): Call it at the end through htab_traverse.
>
>        * gcc.dg/autopar/pr46969.c: New test.
>
> --- gcc/tree-parloops.c.jj      2010-12-07 17:32:49.000000000 +0100
> +++ gcc/tree-parloops.c 2010-12-16 13:45:57.000000000 +0100
> @@ -168,6 +168,8 @@ struct reduction_info
>   gimple reduc_stmt;           /* reduction statement.  */
>   gimple reduc_phi;            /* The phi node defining the reduction.  */
>   enum tree_code reduction_code;/* code for the reduction operation.  */
> +  unsigned reduc_version;      /* SSA_NAME_VERSION of original reduc_phi
> +                                  result.  */
>   gimple keep_res;             /* The PHI_RESULT of this phi is the resulting value
>                                   of the reduction variable when existing the loop. */
>   tree initial_value;          /* The initial value of the reduction var before entering the loop.  */
> @@ -195,7 +197,7 @@ reduction_info_hash (const void *aa)
>  {
>   const struct reduction_info *a = (const struct reduction_info *) aa;
>
> -  return htab_hash_pointer (a->reduc_phi);
> +  return a->reduc_version;
>  }
>
>  static struct reduction_info *
> @@ -207,6 +209,7 @@ reduction_phi (htab_t reduction_list, gi
>     return NULL;
>
>   tmpred.reduc_phi = phi;
> +  tmpred.reduc_version = gimple_uid (phi);
>   red = (struct reduction_info *) htab_find (reduction_list, &tmpred);
>
>   return red;
> @@ -1774,11 +1777,22 @@ build_new_reduction (htab_t reduction_li
>
>   new_reduction->reduc_stmt = reduc_stmt;
>   new_reduction->reduc_phi = phi;
> +  new_reduction->reduc_version = SSA_NAME_VERSION (gimple_phi_result (phi));
>   new_reduction->reduction_code = gimple_assign_rhs_code (reduc_stmt);
>   slot = htab_find_slot (reduction_list, new_reduction, INSERT);
>   *slot = new_reduction;
>  }
>
> +/* Callback for htab_traverse.  Sets gimple_uid of reduc_phi stmts.  */
> +
> +static int
> +set_reduc_phi_uids (void **slot, void *data ATTRIBUTE_UNUSED)
> +{
> +  struct reduction_info *const red = (struct reduction_info *) *slot;
> +  gimple_set_uid (red->reduc_phi, red->reduc_version);
> +  return 1;
> +}
> +
>  /* Detect all reductions in the LOOP, insert them into REDUCTION_LIST.  */
>
>  static void
> @@ -1810,7 +1824,12 @@ gather_scalar_reductions (loop_p loop, h
>               build_new_reduction (reduction_list, reduc_stmt, phi);
>         }
>     }
> -    destroy_loop_vec_info (simple_loop_info, true);
> +  destroy_loop_vec_info (simple_loop_info, true);
> +
> +  /* As gimple_uid is used by the vectorizer in between vect_analyze_loop_form
> +     and destroy_loop_vec_info, we can set gimple_uid of reduc_phi stmts
> +     only now.  */
> +  htab_traverse (reduction_list, set_reduc_phi_uids, NULL);
>  }
>
>  /* Try to initialize NITER for code generation part.  */
> --- gcc/testsuite/gcc.dg/autopar/pr46969.c.jj   2010-12-16 14:06:42.000000000 +0100
> +++ gcc/testsuite/gcc.dg/autopar/pr46969.c      2010-12-16 14:05:42.000000000 +0100
> @@ -0,0 +1,31 @@
> +/* PR tree-optimization/46969 */
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -ftree-parallelize-loops=2 -fcompare-debug" } */
> +
> +extern void abort (void);
> +#define F(name) \
> +int                                                            \
> +name (unsigned char *x)                                                \
> +{                                                              \
> +  int i;                                                       \
> +  unsigned int c, d, e;                                                \
> +  if (x != 0)                                                  \
> +    {                                                          \
> +      for (i = 0, d = 0, e = 0xFFFFFFFF;                       \
> +          i < 64;                                              \
> +          i += (int) sizeof(unsigned int))                     \
> +        {                                                      \
> +          c = *((unsigned int *)(&x[i]));                      \
> +          d = d | c;                                           \
> +          e = e & c;                                           \
> +        }                                                      \
> +      if (!((d == e) && ((d >> 8) == (e & 0x00FFFFFF))))       \
> +        abort ();                                              \
> +    }                                                          \
> +  return 0;                                                    \
> +}
> +F (foo0) F (foo1)
> +F (foo2) F (foo3)
> +F (foo4) F (foo5)
> +F (foo6) F (foo7)
> +F (foo8) F (foo9)
>
>        Jakub
>

Patch

--- gcc/tree-parloops.c.jj	2010-12-07 17:32:49.000000000 +0100
+++ gcc/tree-parloops.c	2010-12-16 13:45:57.000000000 +0100
@@ -168,6 +168,8 @@  struct reduction_info
   gimple reduc_stmt;		/* reduction statement.  */
   gimple reduc_phi;		/* The phi node defining the reduction.  */
   enum tree_code reduction_code;/* code for the reduction operation.  */
+  unsigned reduc_version;	/* SSA_NAME_VERSION of original reduc_phi
+				   result.  */
   gimple keep_res;		/* The PHI_RESULT of this phi is the resulting value
 				   of the reduction variable when existing the loop. */
   tree initial_value;		/* The initial value of the reduction var before entering the loop.  */
@@ -195,7 +197,7 @@  reduction_info_hash (const void *aa)
 {
   const struct reduction_info *a = (const struct reduction_info *) aa;
 
-  return htab_hash_pointer (a->reduc_phi);
+  return a->reduc_version;
 }
 
 static struct reduction_info *
@@ -207,6 +209,7 @@  reduction_phi (htab_t reduction_list, gi
     return NULL;
 
   tmpred.reduc_phi = phi;
+  tmpred.reduc_version = gimple_uid (phi);
   red = (struct reduction_info *) htab_find (reduction_list, &tmpred);
 
   return red;
@@ -1774,11 +1777,22 @@  build_new_reduction (htab_t reduction_li
 
   new_reduction->reduc_stmt = reduc_stmt;
   new_reduction->reduc_phi = phi;
+  new_reduction->reduc_version = SSA_NAME_VERSION (gimple_phi_result (phi));
   new_reduction->reduction_code = gimple_assign_rhs_code (reduc_stmt);
   slot = htab_find_slot (reduction_list, new_reduction, INSERT);
   *slot = new_reduction;
 }
 
+/* Callback for htab_traverse.  Sets gimple_uid of reduc_phi stmts.  */
+
+static int
+set_reduc_phi_uids (void **slot, void *data ATTRIBUTE_UNUSED)
+{
+  struct reduction_info *const red = (struct reduction_info *) *slot;
+  gimple_set_uid (red->reduc_phi, red->reduc_version);
+  return 1;
+}
+
 /* Detect all reductions in the LOOP, insert them into REDUCTION_LIST.  */
 
 static void
@@ -1810,7 +1824,12 @@  gather_scalar_reductions (loop_p loop, h
               build_new_reduction (reduction_list, reduc_stmt, phi);
         }
     }
-    destroy_loop_vec_info (simple_loop_info, true);
+  destroy_loop_vec_info (simple_loop_info, true);
+
+  /* As gimple_uid is used by the vectorizer in between vect_analyze_loop_form
+     and destroy_loop_vec_info, we can set gimple_uid of reduc_phi stmts
+     only now.  */
+  htab_traverse (reduction_list, set_reduc_phi_uids, NULL);
 }
 
 /* Try to initialize NITER for code generation part.  */
--- gcc/testsuite/gcc.dg/autopar/pr46969.c.jj	2010-12-16 14:06:42.000000000 +0100
+++ gcc/testsuite/gcc.dg/autopar/pr46969.c	2010-12-16 14:05:42.000000000 +0100
@@ -0,0 +1,31 @@ 
+/* PR tree-optimization/46969 */
+/* { dg-do compile } */
+/* { dg-options "-O3 -ftree-parallelize-loops=2 -fcompare-debug" } */
+
+extern void abort (void);
+#define F(name) \
+int								\
+name (unsigned char *x)						\
+{								\
+  int i;							\
+  unsigned int c, d, e;						\
+  if (x != 0)							\
+    {								\
+      for (i = 0, d = 0, e = 0xFFFFFFFF;			\
+	   i < 64;						\
+	   i += (int) sizeof(unsigned int))			\
+        {							\
+          c = *((unsigned int *)(&x[i]));			\
+          d = d | c;						\
+          e = e & c;						\
+        }							\
+      if (!((d == e) && ((d >> 8) == (e & 0x00FFFFFF))))	\
+        abort ();						\
+    }								\
+  return 0;							\
+}
+F (foo0) F (foo1)
+F (foo2) F (foo3)
+F (foo4) F (foo5)
+F (foo6) F (foo7)
+F (foo8) F (foo9)