Fix LTO streaming of BUILTINS_LOCATION

Jan Hubicka hubicka@ucw.cz
Mon Jun 8 07:39:00 GMT 2015


> > +  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.
>   
> >    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 <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg)



More information about the Gcc-patches mailing list