* [PATCH] tree-ssa-dce: Fix up -fcompare-debug failures in make_forwarders_with_degenerate_phis [PR103742]
@ 2021-12-29 10:11 Jakub Jelinek
2021-12-29 19:17 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2021-12-29 10:11 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
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?
2021-12-28 Jakub Jelinek <jakub@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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] tree-ssa-dce: Fix up -fcompare-debug failures in make_forwarders_with_degenerate_phis [PR103742]
2021-12-29 10:11 [PATCH] tree-ssa-dce: Fix up -fcompare-debug failures in make_forwarders_with_degenerate_phis [PR103742] Jakub Jelinek
@ 2021-12-29 19:17 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2021-12-29 19:17 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On December 29, 2021 11:11:18 AM GMT+01:00, Jakub Jelinek <jakub@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 <jakub@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
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-12-29 19:17 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-29 10:11 [PATCH] tree-ssa-dce: Fix up -fcompare-debug failures in make_forwarders_with_degenerate_phis [PR103742] Jakub Jelinek
2021-12-29 19:17 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).