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