On Fri Jul 3, 2026 at 4:47 PM CEST, 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... > > -Lewis
Ok, this is looks reasonable. As possible (future) improvement, would it not be better to output one hash per symtab_node? Not sure whether locations can appear somewhere without being an undividable part of symtab_node/toplevel_node. Thanks, Michal
