Re: [PATCH 0/5] RFC: Overhaul of diagnostics (v2)

[Richard Biener <richard dot guenther at gmail dot com>]

On Thu, Sep 24, 2015 at 2:25 AM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Wed, 2015-09-23 at 15:36 +0200, Richard Biener wrote:
>> On Wed, Sep 23, 2015 at 3:19 PM, Michael Matz <matz@suse.de> wrote:
>> > Hi,
>> >
>> > On Tue, 22 Sep 2015, David Malcolm wrote:
>> >
>> >> The drawback is that it could bloat the ad-hoc table.  Can the ad-hoc
>> >> table ever get smaller, or does it only ever get inserted into?
>> >
>> > It only ever grows.
>> >
>> >> An idea I had is that we could stash short ranges directly into the 32
>> >> bits of location_t, by offsetting the per-column-bits somewhat.
>> >
>> > It's certainly worth an experiment: let's say you restrict yourself to
>> > tokens less than 8 characters, you need an additional 3 bits (using one
>> > value, e.g. zero, as the escape value).  That leaves 20 bits for the line
>> > numbers (for the normal 8 bit columns), which might be enough for most
>> > single-file compilations.  For LTO compilation this often won't be enough.
>> >
>> >> My plan is to investigate the impact these patches have on the time and
>> >> memory consumption of the compiler,
>> >
>> > When you do so, make sure you're also measuring an LTO compilation with
>> > debug info of something big (firefox).  I know that we already had issues
>> > with the size of the linemap data in the past for these cases (probably
>> > when we added columns).
>>
>> The issue we have with LTO is that the linemap gets populated in quite
>> random order and thus we repeatedly switch files (we've mitigated this
>> somewhat for GCC 5).  We also considered dropping column info
>> (and would drop range info) as diagnostics are from optimizers only
>> with LTO and we keep locations merely for debug info.
>
> Thanks.  Presumably the mitigation you're referring to is the
> lto_location_cache class in lto-streamer-in.c?
>
> Am I right in thinking that, right now, the LTO code doesn't support
> ad-hoc locations? (presumably the block pointers only need to exist
> during optimization, which happens after the serialization)

LTO code does support ad-hoc locations but they are "restored" only
when reading function bodies and stmts (by means of COMBINE_LOCATION_DATA).

> The obvious simplification would be, as you suggest, to not bother
> storing range information with LTO, falling back to just the existing
> representation.  Then there's no need to extend LTO to serialize ad-hoc
> data; simply store the underlying locus into the bit stream.  I think
> that this happens already: lto-streamer-out.c calls expand_location and
> stores the result, so presumably any ad-hoc location_t values made by
> the v2 patches would have dropped their range data there when I ran the
> test suite.

Yep.  We only preserve BLOCKs, so if you don't add extra code to
preserve ranges they'll be "dropped".

> If it's acceptable to not bother with ranges for LTO, one way to do the
> "stashing short ranges into the location_t" idea might be for the
> bits-per-range of location_t values to be a property of the line_table
> (or possibly the line map), set up when the struct line_maps is created.
> For non-LTO it could be some tuned value (maybe from a param?); for LTO
> it could be zero, so that we have as many bits as before for line/column
> data.

That could be a possibility (likewise for column info?)

Richard.

> Hope this sounds sane
> Dave
>