RE: [PATCH 1/4] gensupport: Centralize machine-generator output management
Tamar Christina <[email protected]> Wed, 5 Aug 2026 15:45:15 +0000
| Newsgroups | gmane.comp.gcc.patches |
|---|---|
| Message-ID | <VI0PR08MB10392BC550F09309C2F861CCEFFD32@VI0PR08MB10392.eurprd08.prod.outlook.com> |
> -----Original Message----- > From: Robin Dapp <[email protected]> > Sent: 05 August 2026 16:22 > To: [email protected]; [email protected] > Cc: Tamar Christina <[email protected]>; > [email protected]; [email protected] > Subject: Re: [PATCH 1/4] gensupport: Centralize machine-generator output > management > > > From: Kyrylo Tkachov <[email protected]> > > > > genemit and genrecog use the same size-based output selection, but each > > generator owns its file names, opens files during option parsing, and closes > > them separately. Adding more partitioned generators would copy this logic. > > > > Put the common output record and lifecycle helpers in gensupport. The > helpers > > validate names, delay file opening until option parsing is complete, select > > only partition outputs by current size, and close every output with a useful > > diagnostic. Fixed outputs, such as headers, use the same record but do not > > participate in selection. > > > > Convert genemit and genrecog to the common interface. Also remove the > unused > > output index and disabled round-robin code from choose_output, and > remove a > > genrecog selection whose result is discarded. Diagnose missing or repeated > > genrecog -H options, remove its fixed header-name limit, and stop printing > > parsed output options to standard output. > > This mostly reads like a more verbose changelog and doesn't really give > a rationale. For example: While it might be ok to rip out the non-ftell > approach, why did you do it? Or rather, if it's unnecessary, why not > remove its origin in genmatch as well? Granted, back when introducing > this to gensupport, I didn't really verify if we need it but simply > copied. To my knowledge nobody ever used this particular de-feature. > But IMHO now would be the time to check :) FWIW, the original reason for having this in genmatch was because we didn't know how the ftell version would work out in practice so added a round-robin approach. It was also useful to have somewhat deterministic output for testing. But that's less of a concern since you can just change the split count to 1. ftell is however not guaranteed to be fast, the C standard makes no performance guarantees on this. However more I/O managers when you call ftell on a file you've open do not perform a seek. It can return the pointer from their internal buffers. ftell can also fail on non-seekable streams, (pipes, sockets, etc). on some implementations ftell can even call lseek which may trigger a kernel operation rather than being fully user-mode like glibc normally is with buffered streams. So while ftell is likely to be cheap, and likely to work, we left a workaround for when it doesn't. This is of course separate from the question of whether the workaround is actually needed :) Hopefully that clarifies the original intent. Thanks, Tamar > > -- > Regards > Robin