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  <ja...@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

Reply via email to