* [PATCH] ipa-fnsummary: Fix ICE with switch predicates [PR96130]
@ 2020-07-10 16:05 Jakub Jelinek
2020-07-13 15:58 ` Martin Jambor
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2020-07-10 16:05 UTC (permalink / raw)
To: Richard Biener, Jan Hubicka, Martin Jambor; +Cc: gcc-patches, Feng Xue OS
Hi!
The following testcase ICEs since r10-3199.
There is a switch with default label, where the controlling expression has
range just 0..7 and there are case labels for all those 8 values, but
nothing has yet optimized away the default.
Since r10-3199, set_switch_stmt_execution_predicate sets the switch to
default label's edge's predicate to a false predicate and then
compute_bb_predicates propagates the predicates through the cfg, but false
predicates aren't really added. The caller of compute_bb_predicates
in one place handles NULL bb->aux as false predicate:
if (fbi.info)
{
if (bb->aux)
bb_predicate = *(predicate *) bb->aux;
else
bb_predicate = false;
}
else
bb_predicate = true;
but then in two further spots that the patch below is changing
it assumes bb->aux must be non-NULL. Those two spots are guarded by a
condition that is only true if fbi.info is non-NULL, so I think the right
fix is to treat NULL aux as false predicate in those spots too.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/10.2?
2020-07-10 Jakub Jelinek <jakub@redhat.com>
PR ipa/96130
* ipa-fnsummary.c (analyze_function_body): Treat NULL bb->aux
as false predicate.
* gcc.dg/torture/pr96130.c: New test.
--- gcc/ipa-fnsummary.c.jj 2020-04-05 00:27:26.000000000 +0200
+++ gcc/ipa-fnsummary.c 2020-07-10 16:12:59.155168850 +0200
@@ -2766,7 +2766,10 @@ analyze_function_body (struct cgraph_nod
edge ex;
unsigned int j;
class tree_niter_desc niter_desc;
- bb_predicate = *(predicate *) loop->header->aux;
+ if (loop->header->aux)
+ bb_predicate = *(predicate *) loop->header->aux;
+ else
+ bb_predicate = false;
exits = get_loop_exit_edges (loop);
FOR_EACH_VEC_ELT (exits, j, ex)
@@ -2799,7 +2802,10 @@ analyze_function_body (struct cgraph_nod
for (unsigned i = 0; i < loop->num_nodes; i++)
{
gimple_stmt_iterator gsi;
- bb_predicate = *(predicate *) body[i]->aux;
+ if (body[i]->aux)
+ bb_predicate = *(predicate *) body[i]->aux;
+ else
+ bb_predicate = false;
for (gsi = gsi_start_bb (body[i]); !gsi_end_p (gsi);
gsi_next (&gsi))
{
--- gcc/testsuite/gcc.dg/torture/pr96130.c.jj 2020-07-10 16:15:28.794082127 +0200
+++ gcc/testsuite/gcc.dg/torture/pr96130.c 2020-07-10 16:14:49.273633241 +0200
@@ -0,0 +1,26 @@
+/* PR ipa/96130 */
+/* { dg-do compile } */
+
+struct S { unsigned j : 3; };
+int k, l, m;
+
+void
+foo (struct S x)
+{
+ while (l != 5)
+ switch (x.j)
+ {
+ case 1:
+ case 3:
+ case 4:
+ case 6:
+ case 2:
+ case 5:
+ l = m;
+ case 7:
+ case 0:
+ k = 0;
+ default:
+ break;
+ }
+}
Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] ipa-fnsummary: Fix ICE with switch predicates [PR96130]
2020-07-10 16:05 [PATCH] ipa-fnsummary: Fix ICE with switch predicates [PR96130] Jakub Jelinek
@ 2020-07-13 15:58 ` Martin Jambor
0 siblings, 0 replies; 2+ messages in thread
From: Martin Jambor @ 2020-07-13 15:58 UTC (permalink / raw)
To: Jakub Jelinek, Richard Biener, Jan Hubicka; +Cc: gcc-patches, Feng Xue OS
On Fri, Jul 10 2020, Jakub Jelinek wrote:
> Hi!
>
> The following testcase ICEs since r10-3199.
> There is a switch with default label, where the controlling expression has
> range just 0..7 and there are case labels for all those 8 values, but
> nothing has yet optimized away the default.
> Since r10-3199, set_switch_stmt_execution_predicate sets the switch to
> default label's edge's predicate to a false predicate and then
> compute_bb_predicates propagates the predicates through the cfg, but false
> predicates aren't really added. The caller of compute_bb_predicates
> in one place handles NULL bb->aux as false predicate:
> if (fbi.info)
> {
> if (bb->aux)
> bb_predicate = *(predicate *) bb->aux;
> else
> bb_predicate = false;
> }
> else
> bb_predicate = true;
> but then in two further spots that the patch below is changing
> it assumes bb->aux must be non-NULL. Those two spots are guarded by a
> condition that is only true if fbi.info is non-NULL, so I think the right
> fix is to treat NULL aux as false predicate in those spots too.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/10.2?
>
The above is correct and the patch is OK. OTOH, simply putting continue
statement in both of the two places when the predicate has not been
created (or if it is false) would also work and might avoid some
unnecessary work - the code below always computes a predicate only to
and it with the BB predicate. But hopefully we do not have too many
unreachable loops after early optimizations and I can do this change
afterwards as a follow-up.
Thanks a lot for looking into this.
Martin
> 2020-07-10 Jakub Jelinek <jakub@redhat.com>
>
> PR ipa/96130
> * ipa-fnsummary.c (analyze_function_body): Treat NULL bb->aux
> as false predicate.
>
> * gcc.dg/torture/pr96130.c: New test.
>
> --- gcc/ipa-fnsummary.c.jj 2020-04-05 00:27:26.000000000 +0200
> +++ gcc/ipa-fnsummary.c 2020-07-10 16:12:59.155168850 +0200
> @@ -2766,7 +2766,10 @@ analyze_function_body (struct cgraph_nod
> edge ex;
> unsigned int j;
> class tree_niter_desc niter_desc;
> - bb_predicate = *(predicate *) loop->header->aux;
> + if (loop->header->aux)
> + bb_predicate = *(predicate *) loop->header->aux;
> + else
> + bb_predicate = false;
>
> exits = get_loop_exit_edges (loop);
> FOR_EACH_VEC_ELT (exits, j, ex)
> @@ -2799,7 +2802,10 @@ analyze_function_body (struct cgraph_nod
> for (unsigned i = 0; i < loop->num_nodes; i++)
> {
> gimple_stmt_iterator gsi;
> - bb_predicate = *(predicate *) body[i]->aux;
> + if (body[i]->aux)
> + bb_predicate = *(predicate *) body[i]->aux;
> + else
> + bb_predicate = false;
> for (gsi = gsi_start_bb (body[i]); !gsi_end_p (gsi);
> gsi_next (&gsi))
> {
> --- gcc/testsuite/gcc.dg/torture/pr96130.c.jj 2020-07-10 16:15:28.794082127 +0200
> +++ gcc/testsuite/gcc.dg/torture/pr96130.c 2020-07-10 16:14:49.273633241 +0200
> @@ -0,0 +1,26 @@
> +/* PR ipa/96130 */
> +/* { dg-do compile } */
> +
> +struct S { unsigned j : 3; };
> +int k, l, m;
> +
> +void
> +foo (struct S x)
> +{
> + while (l != 5)
> + switch (x.j)
> + {
> + case 1:
> + case 3:
> + case 4:
> + case 6:
> + case 2:
> + case 5:
> + l = m;
> + case 7:
> + case 0:
> + k = 0;
> + default:
> + break;
> + }
> +}
>
> Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-07-13 15:58 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-10 16:05 [PATCH] ipa-fnsummary: Fix ICE with switch predicates [PR96130] Jakub Jelinek
2020-07-13 15:58 ` Martin Jambor
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).