On Thu, Jul 02, 2026 at 08:20:19AM -0400, Lewis Hyatt wrote:
> On Wed, Jul 1, 2026 at 2:54 PM Michal Jireš <[email protected]> wrote:
> >
> >
> > On Wed Jul 1, 2026 at 2:18 PM CEST, Lewis Hyatt <[email protected]> wrote:
> > > On Tue, Jun 30, 2026 at 2:38 PM Michal Jireš <[email protected]> wrote:
> > >>
> > >>
> > >> On Tue Jun 30, 2026 at 1:01 PM CEST, Lewis Hyatt <[email protected]>
> > >> wrote:
> > >> > On Tue, Jun 30, 2026 at 3:52 AM Richard Biener
> > >> > <[email protected]> wrote:
> > >> >>
> > >> >> On Tue, Jun 30, 2026 at 5:03 AM Lewis Hyatt <[email protected]> wrote:
> > >> >> >
> > >> >> > On Wed, Apr 22, 2026 at 04:29:41PM +0200, Richard Biener wrote:
> > >> >> > > On Thu, Jan 1, 2026 at 6:03 PM Lewis Hyatt <[email protected]>
> > >> >> > > wrote:
> > >> >> > > >
> > >> >> > > > After the previous changes in this series, the LTO front end
> > >> >> > > > always has an
> > >> >> > > > appropriate linemap structure for interpreting diagnostic
> > >> >> > > > pragmas, so it is
> > >> >> > > > straightforward to implement them, as is done here.
> > >> >> > > >
> > >> >> > > > The pragmas are streamed out in each linemap section; since all
> > >> >> > > > locations
> > >> >> > > > from a given linemap section will be contiguous in the
> > >> >> > > > reconstructed
> > >> >> > > > linemap, they are automatically ordered properly for the
> > >> >> > > > existing diagnostic
> > >> >> > > > pragma infrastructure to work as-is.
> > >> >> > > >
> > >> >> > > > One wrinkle is that a single function may have been streamed
> > >> >> > > > out in multiple
> > >> >> > > > sections. (For example, an inline function will be streamed out
> > >> >> > > > in all
> > >> >> > > > partitions that need it.) In this case, when merging them, LTO
> > >> >> > > > keeps only
> > >> >> > > > one of the sections, as directed by the linker resolution, so
> > >> >> > > > the diagnostic
> > >> >> > > > pragmas that will be in force (in case they were not the same
> > >> >> > > > for the
> > >> >> > > > different translation units) will be whichever were applicable
> > >> >> > > > to the
> > >> >> > > > section LTO decided to keep.
> > >> >> > >
> > >> >> > > LGTM if the rest of the series is approved.
> > >> >> > >
> > >> >> > > Thanks,
> > >> >> > > Richard.
> > >> >> > >
> > >> >> >
> > >> >> > Hi Richard-
> > >> >> >
> > >> >> > Firstly, thank you again for your time in reviewing these patches.
> > >> >> > I thought
> > >> >> > everything was finally across the finish line, but as I was
> > >> >> > reviewing the
> > >> >> > patches one more time before pushing them, I realized there is one
> > >> >> > small
> > >> >> > problem with the new approach. Could I please ask you to look at
> > >> >> > one more
> > >> >> > patch which addresses that? I attached it here as an incremental
> > >> >> > change to
> > >> >> > the rest of the series, but I would propose to squash it into the
> > >> >> > other
> > >> >> > patches before pushing.
> > >> >> >
> > >> >> > What I missed was that the LTO front end has a mode of operation for
> > >> >> > incremental linking. I had tested that my new approach works fine
> > >> >> > with
> > >> >> > "ld -r" (provided that -frandom-seed is not used to remove
> > >> >> > uniqueness from
> > >> >> > the section names), but the LTO front end version of that (which
> > >> >> > you get
> > >> >> > when using, say, "gcc -r -flto") does more than just copy the
> > >> >> > sections; it
> > >> >> > actually reads all the decls and then re-outputs a new object file
> > >> >> > with a
> > >> >> > new identifier, which contains a new decls section plus copies of
> > >> >> > the
> > >> >> > function and constructor sections. This means the linemap sections
> > >> >> > also need
> > >> >> > to get copied into the output file, and also, it means that an
> > >> >> > input file to
> > >> >> > the LTO front end could possibly contain more than one linemap,
> > >> >> > which was
> > >> >> > not something I had considered. (I had anticipated that inputs
> > >> >> > contained
> > >> >> > just one linemap, except that in LTRANS mode, there would also be
> > >> >> > one file
> > >> >> > containing all necessary linemaps copied during WPA).
> > >> >>
> > >> >> So I think there's two things now, the older -flto-linker-output=rel
> > >> >> and
> > >> >> the newer -flto-incremental.
> > >> >>
> > >> >> That said, I'm not sure about the default behavior of -flto -r and
> > >> >> would
> > >> >> suggest to add an explicit -flto-linker-output=rel here to be
> > >> >> unambiguous.
> > >> >> Did you try that with -ffat-lto-objects as well?
> > >> >>
> > >> >
> > >> > Thanks, what I have understood is:
> > >> >
> > >> > -flto-incremental is unrelated to incremental linking per se,
> > >> > that's about using a cache directory to store inputs + outputs of
> > >> > WPA+LTRANS, to avoid rerunning the LTRANS step if the partition did
> > >> > not change. I made sure that this still works the same as before my
> > >> > patches, that was one motivation for putting all the linemaps into
> > >> > their own file after WPA, to make sure a change in one partition
> > >> > doesn't needlessly invalidate the cache for a different one.
> > >>
> > >> Do I understand correctly, that LTRANS cache won't notice when location
> > >> changes while its ID remains the same?
> > >>
> > >> In LTRANS, do we use locations purely for diagnostics? = locations
> > >> cannot influnce the binary output?
> > >> And if yes, do we have it documented somewhere that locations cannot be
> > >> used in LTRANS for anything other that diagnostics?
> > >>
> > >
> > > A location can't really change without affecting the streamed object
> > > file and invalidating the cache. What ends up streamed out (and
> > > affecting the SHA1) is the map ID and the location_t offset within the
> > > map, plus any attached tree and discriminator. Any change to the line
> > > number or column number will either change the location_t or add a new
> > > map and change all map IDs.
> > >
> >
> > Physical line and column cannot change, because they are directly
> > represented by the location offset.
> >
> > But what happens with filename, to_line..?
> > Can't I change #include to different file with identical contents?
>
> Yes. In that specific case it's hard to see how recompilation would be
> necessary, but the point stands.
>
> > Or change #line?
>
> The addition of the #line directive would alter the linemap structure
> and affect the subsequent map IDs most likely, but not always.
> Thinking about it some more, if for instance you added a blank line
> with enough columns of spaces at the start of the file, it would go
> into its own map, which would not be streamed out since nothing refers
> to it, and that would change line numbering without changing the
> streamed out location IDs.
>
> > Or if I am missing something, more generally: If the relevant contents
> > of the linemap-file cannot change without affecting the cached file,
> > isn't it redundant and we do not need it? If it is needed, it must be
> > able to contain something that is not fully captured by the cached file,
> > and must not be used for the binary output.
>
> As you alluded to originally, it's really there for generating
> diagnostics, and for implementing #pragma GCC diagnostic. I understand
> you're asking if there can be certainty that it's impossible to make a
> change which does affect code generation, but which does not
> invalidate the cache because it changes the linemap sections only. If
> that could happen, then it would be a problem for -flto-incremental.
> It's a good point, and I agree with you that I have implicitly assumed
> this can't happen.
> I feel like it really shouldn't be an issue in practice, but I don't
> have a more convincing answer than that. I'm going to take a look and
> see if I can either demonstrate that it's fine, or else, adjust the
> location streaming so that something in the cached file will reflect
> it if the linemap changes in this way. Thanks!
>
The below patch (incremental to the others) resolves this issue by
outputting a hash code identifying each line map along with the location ID
when streaming out the location. I used a 32-bit hash (as provided by
inchash::hash) to minimize the space overhead; it seems like this should be
sufficient but it could be swapped for something with more bits as
well. This increases the size of the streamed LTO by approximately 1.5%,
here for instance is the size of the LTRANS inputs when compiling cc1plus:
master: 723 MB
patch v1 (sent previously): 705 MB
patch v2 (this one): 716 MB
It's still smaller than the current location streaming approach, seems
worth it to me... What do you think? Thanks...
-Lewis
gcc/lto-streamer-in.cc | 6 ++++++
gcc/lto-streamer-out.cc | 41 ++++++++++++++++++++++++++++++++++++++---
2 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/gcc/lto-streamer-in.cc b/gcc/lto-streamer-in.cc
index bcf0c025ca0..901c3212656 100644
--- a/gcc/lto-streamer-in.cc
+++ b/gcc/lto-streamer-in.cc
@@ -619,8 +619,14 @@ lto_location_cache::input_location_and_block (location_t
*dest,
const unsigned linemap_id = bp_unpack_var_len_unsigned (bp);
cur_loc_map = get_loc_map (decl_data, linemap_id + linemap_offset);
}
+ const auto prev_idx = stream_map_idx;
bp_unpack_delta (bp, stream_map_idx);
bp_unpack_delta (bp, stream_loc_offset);
+ if (stream_map_idx != prev_idx)
+ /* The hash code is only there for the benefit of -flto-incremental;
+ we don't need to do anything with it here. */
+ bp_unpack_var_len_unsigned (bp);
+
loc = get_location_from_idx (stream_map_idx, stream_loc_offset,
cur_loc_map);
if (bp_unpack_value (bp, 1))
diff --git a/gcc/lto-streamer-out.cc b/gcc/lto-streamer-out.cc
index 230670da494..722c8a6b2d4 100644
--- a/gcc/lto-streamer-out.cc
+++ b/gcc/lto-streamer-out.cc
@@ -180,6 +180,23 @@ tree_is_indexable (tree t)
namespace {
+/* Compute a 32-bit hash of the linemap details. This is currently used to
+ ensure that WPA streamed output will change whenever the linemap details
+ change, otherwise there could be potential issues with the validity of
+ the cache used to implement -flto-incremental. */
+
+hashval_t
+compute_map_hash (const line_map_ordinary *map)
+{
+ inchash::hash h;
+ h.add_int (map->sysp);
+ h.add_int (map->m_column_and_range_bits - map->m_range_bits);
+ h.add_int (map->to_line);
+ h.add_object (map->included_from);
+ h.add (map->to_file, strlen (map->to_file));
+ return h.end ();
+}
+
class location_output
{
public:
@@ -188,6 +205,7 @@ public:
{
size_t idx;
unsigned linemap_id;
+ hashval_t hash;
};
struct location_id_t
@@ -197,13 +215,23 @@ public:
};
location_id_t record_location (location_t loc);
- void register_map_id (map_id_t map_id) { orig_map_ids.safe_push (map_id); }
+
+ void register_map_id (size_t map_idx, unsigned linemap_id)
+ {
+ gcc_checking_assert (LINEMAPS_ORDINARY_USED (line_table)
+ == orig_map_ids.length () + 1);
+ const auto map = LINEMAPS_LAST_ORDINARY_MAP (line_table);
+ const hashval_t hash = compute_map_hash (map);
+ orig_map_ids.safe_push (map_id_t{map_idx, linemap_id, hash});
+ }
+
void produce_linemap_section ();
private:
struct map_data
{
size_t idx;
+ hashval_t hash;
location_t highest_location;
};
hash_map<nofree_ptr_hash<const line_map_ordinary>, map_data> map_data_map;
@@ -243,9 +271,13 @@ location_output::record_location (location_t loc)
without any range bits. */
map_data &md = map_data_map.get_or_insert (map);
if (!md.idx)
- md.idx = map_data_map.elements ();
+ {
+ md.idx = map_data_map.elements ();
+ md.hash = compute_map_hash (map);
+ }
md.highest_location = MAX (md.highest_location, loc);
loc_id.map_id.idx = md.idx;
+ loc_id.map_id.hash = md.hash;
loc_id.offset = (loc - map->start_location) >> map->m_range_bits;
return loc_id;
}
@@ -394,8 +426,11 @@ lto_output_location_1 (struct output_block *ob, struct
bitpack_d *bp,
bp_pack_var_len_unsigned (bp, loc_id.map_id.linemap_id);
ob->current_linemap_id = loc_id.map_id.linemap_id;
}
+ const bool is_new_map = (loc_id.map_id.idx != ob->current_map_idx);
bp_pack_delta (bp, loc_id.map_id.idx, ob->current_map_idx);
bp_pack_delta (bp, loc_id.offset, ob->current_loc_offset);
+ if (is_new_map)
+ bp_pack_var_len_unsigned (bp, loc_id.map_id.hash);
const unsigned discr = get_discriminator_from_loc (loc);
bp_pack_value (bp, ob->current_discr != discr, 1);
if (ob->current_discr != discr)
@@ -3035,7 +3070,7 @@ create_order_remap (lto_symtab_encoder_t encoder)
void lto_register_linemap_for_output (size_t map_idx, unsigned linemap_id)
{
- loc_output.register_map_id ({map_idx, linemap_id});
+ loc_output.register_map_id (map_idx, linemap_id);
}
/* Main entry point from the pass manager. */
