diff mbox series

ipa-fnsummary: Fix ICE with switch predicates [PR96130]

Message ID 20200710160529.GP2363@tucnak
State New
Headers show
Series ipa-fnsummary: Fix ICE with switch predicates [PR96130] | expand

Commit Message

Jakub Jelinek July 10, 2020, 4:05 p.m. UTC
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.


	Jakub

Comments

Martin Jambor July 13, 2020, 3:58 p.m. UTC | #1
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
diff mbox series

Patch

--- 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;
+      }
+}