Re: [PR rtl-optimization/55604]: fix ICE in remove_some_program_points_and_update_live_ranges

[Vladimir Makarov <vmakarov at redhat dot com>]

On 12-12-05 2:11 PM, Steven Bosscher wrote:
> On Wed, Dec 5, 2012 at 7:47 PM, Vladimir Makarov wrote:
> > On 12-12-05 1:13 PM, Aldy Hernandez wrote:
> > > This is a division by zero ICE.
> > >
> > > In the testcase in the PR, both `n' and `lra_live_max_point' are zero. I
> > > have opted to inhibit the dump when lra_live_max_point is zero, but I can
> > > just as easily avoiding printing the percentage in this case.
> > Interesting.  I never thought that is possible.
> >
> > > Let me know what you prefer.
> > >
> > > OK for trunk?
> > Yes, it is ok if you add the testcase for GCC testsuite.
> Eh, are you sure?
>
> This means we enter remove_some_program_points_and_update_live_ranges
> with lra_live_max_point==0 (it's updated _after_ compression so at the
> point of the dump it's using the value on entry, _before_
> compression). So we're doing things like:
>
>    born = sbitmap_alloc (lra_live_max_point); // => sbitmap_alloc (0);
>    dead = sbitmap_alloc (lra_live_max_point); // idem
>    bitmap_clear (born); // clear a bitmap that doesn't really exit
>    bitmap_clear (dead); // idem.
>    max_regno = max_reg_num ();
>    for (i = FIRST_PSEUDO_REGISTER; i < (unsigned) max_regno; i++)
>      {  // walk all registers on looking for live ranges -- but there
> can't be any
>        for (r = lra_reg_info[i].live_ranges; r != NULL; r = r->next)
>          {
>            lra_assert (r->start <= r->finish);
>            bitmap_set_bit (born, r->start);
>            bitmap_set_bit (dead, r->finish);
>          }
>      }
>
> etc.
>
>
> That makes no sense...
>
> The function that triggers this, has this RTL after IRA:
>
>      1: NOTE_INSN_DELETED
>      3: NOTE_INSN_BASIC_BLOCK 2
>      2: NOTE_INSN_FUNCTION_BEG
>      5: {r60:DI=frame:DI-0x10;clobber flags:CC;}
>        REG_UNUSED flags:CC
>        REG_EQUIV frame:DI-0x10
>      6: dx:DI=r60:DI
>        REG_DEAD r60:DI
>        REG_EQUAL frame:DI-0x10
>      7: si:SI=0x5
>      8: di:DI=`*.LC0'
>      9: ax:QI=0
>     10: ax:SI=call [`printf'] argc:0
>        REG_DEAD di:DI
>        REG_DEAD si:SI
>        REG_DEAD dx:DI
>        REG_UNUSED ax:SI
>     15: ax:SI=0
>     18: use ax:SI
>
> Note how there are no pseudos at all in the function. So why run LRA
> on it at all?
The insn constraints could be not satisfied even if we have only hard 
registers.   So we need to run LRA in any case.
> How about the fix proposed below?
It could work too.  Either code is ok to me.  The testscase is very rare 
and can be achieved only for a small program.  So the code speed is not 
important.  I favor a bit your code as it does not change the code 
presented in IRA and LRA which might be unified someday.  So if you are 
ready to commit your patch before Aldy did it, please do it, Steven.


> Ciao!
> Steven
>
>
> Index: lra-lives.c
> ===================================================================
> --- lra-lives.c (revision 194226)
> +++ lra-lives.c (working copy)
> @@ -915,6 +915,7 @@ lra_create_live_ranges (bool all_p)
>     basic_block bb;
>     int i, hard_regno, max_regno = max_reg_num ();
>     int curr_point;
> +  bool have_referenced_pseudos = false;
>
>     timevar_push (TV_LRA_CREATE_LIVE_RANGES);
>
> @@ -947,10 +948,24 @@ lra_create_live_ranges (bool all_p)
>         lra_reg_info[i].call_p = false;
>   #endif
>         if (i >= FIRST_PSEUDO_REGISTER
> -         && lra_reg_info[i].nrefs != 0 && (hard_regno = reg_renumber[i]) >= 0)
> -       lra_hard_reg_usage[hard_regno] += lra_reg_info[i].freq;
> +         && lra_reg_info[i].nrefs != 0)
> +       {
> +         if ((hard_regno = reg_renumber[i]) >= 0)
> +           lra_hard_reg_usage[hard_regno] += lra_reg_info[i].freq;
> +         have_referenced_pseudos = true;
> +       }
>       }
>     lra_free_copies ();
> +
> +  /* Under some circumstances, we can have functions without pseudo
> +     registers.  For such functions, lra_live_max_point will be 0,
> +     see e.g. PR55604, and there's nothing more to do for us here.  */
> +  if (! have_referenced_pseudos)
> +    {
> +      timevar_pop (TV_LRA_CREATE_LIVE_RANGES);
> +      return;
> +    }
> +
>     pseudos_live = sparseset_alloc (max_regno);
>     pseudos_live_through_calls = sparseset_alloc (max_regno);
>     pseudos_live_through_setjumps = sparseset_alloc (max_regno);
> @@ -973,6 +988,7 @@ lra_create_live_ranges (bool all_p)
>       }
>     free (post_order_rev_cfg);
>     lra_live_max_point = curr_point;
> +  gcc_checking_assert (lra_live_max_point > 0);
>     if (lra_dump_file != NULL)
>       print_live_ranges (lra_dump_file);
>     /* Clean up. */