On Mon, 8 Jun 2015, Jan Hubicka wrote:
> > > + bp_pack_value (bp, loc == BUILTINS_LOCATION, 1);
> > > + if (loc == BUILTINS_LOCATION)
> > > + return;
> >
> > Hmm, with this and
> >
> > #define DECL_IS_BUILTIN(DECL) \
> > (LOCATION_LOCUS (DECL_SOURCE_LOCATION (DECL)) <= BUILTINS_LOCATION)
> >
> > shouldn't we rather stream all locations <= BUILTINS_LOCATION literally?
> > That is, instead of two bits stream a [0, BUILTINS_LOCATION+1] 'enum'
> > here? Btw, line-map.h has RESERVED_LOCATION_COUNT for the locations
> > that are "special" (currently two, so your patch will work in practice).
>
> Yep, i considered that. Because we have precisely two special locations (ATM)
> and UNKNOWN_LOCATION is quite common, that would waste one extra bit on that
> case.
>
> Probably not a big deal, so if you think streaming all locations up to
> BUILTINS_LOCATION literarly is more robust, i will update the patch.
Yeah, I think streaming all locations up to RESERVED_LOCATION_COUNT
literally is more robust. Thus do
bp_pack_int_in_range (bp, 0, RESERVED_LOCATION_COUNT,
loc < RESERVED_LOCATION_COUNT ? loc :
RESERVED_LOCATION_COUNT);
if (loc < RESERVED_LOCATION_COUNT)
return;
and on unpacking use the special RESERVED_LOCATION_COUNT value
to fall thru to unpacked location handling.
Richard.
> >
> > > xloc = expand_location (loc);
> > >
> > > Index: lto-streamer-in.c
> > > ===================================================================
> > > --- lto-streamer-in.c (revision 224201)
> > > +++ lto-streamer-in.c (working copy)
> > > @@ -283,6 +283,11 @@
> > > *loc = UNKNOWN_LOCATION;
> > > return;
> > > }
> > > + if (bp_unpack_value (bp, 1))
> > > + {
> > > + *loc = BUILTINS_LOCATION;
> > > + return;
> > > + }
> > > *loc = BUILTINS_LOCATION + 1;
> >
> > Btw, this assignment to *loc looks odd (I suppose it's to make
> > location caching work).
>
> *loc is set to UNKNOWN_LOCATION/BUILTINS_LOCATION for those locations that are
> not cached and all others get BUILTINS_LOCATION + 1 which quite safely
> triggers
> ICE in line_map lookup though I do not recall why. I originally used
> UNKNOWN_LOCATION for cached values but that did not work as it confused
> DECL_IS_BUILTIN.
>
> We could extend API by adding INVALID_LOCATION and set it to INT_MAX or
> something
> that would also ICE.
>
> Honza
> >
> > Richard.
> >
> > > file_change = bp_unpack_value (bp, 1);
> > >
> > >
> >
> > --
> > Richard Biener <rguent...@suse.de>
> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu,
> > Graham Norton, HRB 21284 (AG Nuernberg)
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham
Norton, HRB 21284 (AG Nuernberg)
