On December 29, 2021 11:11:18 AM GMT+01:00, Jakub Jelinek <ja...@redhat.com>
wrote:
>Hi!
>
>make_forwarders_with_degenerate_phis causes a -fcompare-debug failure on the
>following testcase.
>The problem is that on:
> # iftmp.4_8 = PHI <&D.2582(6), &D.2583(4), &D.2582(7), &D.2583(5)>
>the exact DECL_UIDs are different between -g and -g0 (which is ok, with -g
>the decls can have larger gaps in between the uids), which means
>iterative_hash_expr is different and because there are 2 pairs of edges
>with matching phi arguments, the function processes them in different
>orders.
>The following patch fixes it by using the iterative_hash_expr order
>only to determine which arguments are the same, then replaces the hashes
>with the minimum dest_idx in the set of matching arguments and qsorts
>again (which makes it stable for -fcompare-debug) and only splits edges etc.
>on that stable order.
>As a small optimization, if no arguments are equal, it doesn't do the
>second qsort and continues, and if all arguments of the PHI are
>constants or SSA_NAMEs (I think that is a pretty common case for many
>PHIs), then it doesn't do the second qsort either, because in that case
>the hash values will be stable, only computed from the constant values or
>SSA_NAME_VERSIONs.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok.
Thanks,
Richard.
>2021-12-28 Jakub Jelinek <ja...@redhat.com>
>
> PR debug/103742
> * tree-ssa-dce.c (make_forwarders_with_degenerate_phis): If any phi
> argument is not CONSTANT_CLASS_P or SSA_NAME and any arguments are
> equal, change second from hash value to lowest dest_idx from the
> edges which have equal argument and resort to ensure -fcompare-debug
> stability.
>
> * g++.dg/opt/pr103742.C: New test.
>
>--- gcc/tree-ssa-dce.c.jj 2021-11-29 14:24:14.120633762 +0100
>+++ gcc/tree-ssa-dce.c 2021-12-28 17:30:19.983056815 +0100
>@@ -1671,6 +1671,7 @@ make_forwarders_with_degenerate_phis (fu
> continue;
> gphi *phi = gsi.phi ();
> auto_vec<std::pair<edge, hashval_t>, 8> args;
>+ bool need_resort = false;
> for (unsigned i = 0; i < gimple_phi_num_args (phi); ++i)
> {
> edge e = gimple_phi_arg_edge (phi, i);
>@@ -1682,12 +1683,42 @@ make_forwarders_with_degenerate_phis (fu
> if (loops_state_satisfies_p (LOOP_CLOSED_SSA)
> && loop_exit_edge_p (e->src->loop_father, e))
> continue;
>- args.safe_push (std::make_pair (e, iterative_hash_expr
>- (gimple_phi_arg_def (phi, i), 0)));
>+
>+ tree arg = gimple_phi_arg_def (phi, i);
>+ if (!CONSTANT_CLASS_P (arg) && TREE_CODE (arg) != SSA_NAME)
>+ need_resort = true;
>+ args.safe_push (std::make_pair (e, iterative_hash_expr (arg, 0)));
> }
> if (args.length () < 2)
> continue;
> args.qsort (sort_phi_args);
>+ /* The above sorting can be different between -g and -g0, as e.g. decls
>+ can have different uids (-g could have bigger gaps in between them).
>+ So, only use that to determine which args are equal, then change
>+ second from hash value to smallest dest_idx of the edges which have
>+ equal argument and sort again. If all the phi arguments are
>+ constants or SSA_NAME, there is no need for the second sort, the hash
>+ values are stable in that case. */
>+ hashval_t hash = args[0].second;
>+ args[0].second = args[0].first->dest_idx;
>+ bool any_equal = false;
>+ for (unsigned i = 1; i < args.length (); ++i)
>+ if (hash == args[i].second
>+ && operand_equal_p (PHI_ARG_DEF_FROM_EDGE (phi, args[i - 1].first),
>+ PHI_ARG_DEF_FROM_EDGE (phi, args[i].first)))
>+ {
>+ args[i].second = args[i - 1].second;
>+ any_equal = true;
>+ }
>+ else
>+ {
>+ hash = args[i].second;
>+ args[i].second = args[i].first->dest_idx;
>+ }
>+ if (!any_equal)
>+ continue;
>+ if (need_resort)
>+ args.qsort (sort_phi_args);
>
> /* From the candidates vector now verify true candidates for
> forwarders and create them. */
>@@ -1697,8 +1728,7 @@ make_forwarders_with_degenerate_phis (fu
> {
> unsigned i;
> for (i = start + 1; i < args.length (); ++i)
>- if (!operand_equal_p (PHI_ARG_DEF_FROM_EDGE (phi,
>args[start].first),
>- PHI_ARG_DEF_FROM_EDGE (phi, args[i].first)))
>+ if (args[start].second != args[i].second)
> break;
> /* args[start]..args[i-1] are equal. */
> if (start != i - 1)
>--- gcc/testsuite/g++.dg/opt/pr103742.C.jj 2021-12-28 17:30:35.788837099
>+0100
>+++ gcc/testsuite/g++.dg/opt/pr103742.C 2021-12-28 17:31:22.719184723
>+0100
>@@ -0,0 +1,36 @@
>+// PR debug/103742
>+// { dg-do compile { target c++17 } }
>+// { dg-options "-O2 -fnon-call-exceptions --param=early-inlining-insns=82
>-fcompare-debug" }
>+
>+template <typename T> T max(T a, T b) { return a >= b ? a : b; }
>+template <typename T> T abs(T);
>+template <int T, int U> struct A {
>+ long a;
>+ A(A &x) { a = x.a; }
>+ A(long);
>+ A foo(A) {
>+ if (abs(a) && a == a)
>+ a = a ? U : T;
>+ else
>+ a += a;
>+ return *this;
>+ }
>+ bool operator>=(A) { return a; }
>+};
>+struct B {};
>+struct C {
>+ A<2147483647, 0> c;
>+};
>+struct D {
>+ A<2147483647, 0> d;
>+ C e[];
>+};
>+struct E : D{} * f;
>+A<2147483647, 0> bar() {
>+ A<2147483647, 0> g = g.foo(f->d);
>+ return max(g, (A<2147483647, 0>)1);
>+}
>+E *h;
>+void baz() {
>+ h->e[0].c = bar();
>+}
>
> Jakub
>
