Re: [PATCH] Fix parloops -fcompare-debug failure (PR tree-optimization/46969)

[Richard Guenther <richard dot guenther at gmail dot com>]

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
>