On Tue, Jul 7, 2026 at 2:43 AM Lewis Hyatt <[email protected]> wrote:
>
> On Mon, Jul 6, 2026 at 3:42 AM Richard Biener
> <[email protected]> wrote:
> >
> > On Fri, Jul 3, 2026 at 4:47 PM Lewis Hyatt <[email protected]> wrote:
> > >
> > > 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...
> >
> > I think it's reasonable but I don't know either if this is an issue in 
> > practice.
> > I suppose that if the line numbers can shift (with vertical space added)
> > without affecting the location_t then we could miss emitting the correct
> > line numbers into the DWARF?
>
> I was able to reproduce such a case. If -g is omitted on the initial
> compile, but specified at WPA time, then some debug info is generated
> using the LTO location_t data that was streamed out. If an input file
> is changed in one of the limited ways that can affect line numbers
> without invalidating any of the location references, it's possible for
> the LTRANS cache to fail to recompile with a new line number, without
> this last patch I sent.
>
> I also saw this for Mach-O targets:
>
> config/rs6000/rs6000.cc-      add_compiler_branch_island (labelname, funname,
> config/rs6000/rs6000.cc:                     insn_line ((const 
> rtx_insn*)insn));
>
> which seems to establish some precedent that back end code generation
> could depend on the streamed location_t (via insn_line()). In this
> particular case, I don't think that the line number is used anymore
> (it was removed by r13-2361 "STABS: remove -gstabs and -gxcoff
> functionality"), and it was also related to debug symbols, but I
> thought it was further evidence that this issue can't be ruled out.

OK, fine by me then.

Thanks,
Richard.

>
> -Lewis
>
>
>
> -Lewis

Reply via email to