[PATCH] lto: Cache location_ts including BLOCKs in GIMPLE streaming [PR94311]

[Jakub Jelinek]

On Thu, Sep 03, 2020 at 12:07:36PM +0200, Richard Biener wrote:
> LGTM.

Thanks.

> Can we now remove stream_input_location_now?

There are a few remaining users, one is:
      case LTO_ert_must_not_throw:
        {
          r->type = ERT_MUST_NOT_THROW;
          r->u.must_not_throw.failure_decl = stream_read_tree (ib, data_in);
          bitpack_d bp = streamer_read_bitpack (ib);
          r->u.must_not_throw.failure_loc
           = stream_input_location_now (&bp, data_in);
        }
and the other two:
  /* Input the function start and end loci.  */
  fn->function_start_locus = stream_input_location_now (&bp, data_in);
  fn->function_end_locus = stream_input_location_now (&bp, data_in);
I know nothing about those, so have kept them as is.  I don't know if
failure_loc can or can't include blocks, what is the reason why it uses
*_now (can r be reallocated?) and similarly about the function start/end.
Will defer these to Honza or anybody else who knows LTO better.

Ok, looking some more, seems r isn't really reallocated and failure_loc
shouldn't really include blocks, judging from
    case ERT_MUST_NOT_THROW:
      new_r->u.must_not_throw.failure_loc =
        LOCATION_LOCUS (old_r->u.must_not_throw.failure_loc);
and
      this_region->u.must_not_throw.failure_loc
        = LOCATION_LOCUS (gimple_location (tp));
as the only setters of failure_loc outside of lto-streamer-in.c.
Dunno if it would be sufficient to just use normal stream_input_location
and let the apply cache after all EH regions and later all bbs are input
apply it.

> > all these != checks to non-zero?  Either by oring something into those
> > tests, or perhaps:
> >   if (ob->current_reset)
> >     {
> >       if (xloc.file == NULL)
> > 	ob->current_file = "";
> >       if (xloc.line == 0)
> > 	ob->current_line = 1;
> >       if (xloc.column == 0)
> > 	ob->current_column = 1;
> >       ob->current_reset = false;
> >     }
> > before doing those bp_pack_value calls with a comment, effectively forcing
> > all 6 != comparisons to be true?
> 
> Yeah, guess that would be cleanest.  How does UNKNOWN_LOCATION
> expand btw?  Using that as "reset" value also might work?

On the lto-streamer-out.c, for UNKNOWN_LOCATION/BUILTINS_LOCATION we just
do:
  bp_pack_int_in_range (bp, 0, RESERVED_LOCATION_COUNT,
                        loc < RESERVED_LOCATION_COUNT
                        ? loc : RESERVED_LOCATION_COUNT);
and that is it (well, for locs with blocks I do more now, but
ob->current_{file,line,column,sysp} aren't touched).

I'll test a patch.

	Jakub