Re: [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le

["Gabriel F. T. Gomes" <gabriel at inconstante dot eti dot br>]

Hi, Joseph,

Thanks for your detailed reply and explanations...

On Thu, 24 May 2018, Joseph Myers wrote:

>Now, the set of such __nldbl_* ABIs for such functions for 
>-mlong-double-64 is a plausible starting point for identifying relevant 
>ABIs.  But it's only a starting point: there are functions that are 
>missing __nldbl_* ABIs but should have them, and there are __nldbl_* ABIs 
>for which the need is questionable.
>
>For the former, as previously discussed, the printf-like argp.h, err.h and 
>error.h functions, at least, are missing __nldbl_* variants for 
>-mlong-double-64.  For reasons explained below, I think fixing this will 
>be needed as part of the preparation for adding IEEE 128-bit long double 
>variants of those functions.
>
>For the latter, for example, your patch 4 adds public ABIs for 
>__ieee128___printf_fp, __ieee128__IO_vfprintf and __ieee128_vfprintf.  
>There is certainly no need for __ieee128__IO_vfprintf, since _IO_vfprintf 
>is not declared in any installed header since we stopped installing 
>libio.h.  But even if _IO_vfprintf were a public API, it *still* wouldn't 
>need __ieee128__IO_vfprintf, because the semantics of _IO_vfprintf are 
>exactly those of vfprintf, so just having __ieee128_vfprintf suffices. 

Yes, indeed.  And you have already explained that before, so, sorry about
that (I could try to explain myself saying that I induced myself to error,
because I was thinking of additions to the ABI for the *IBM* format, but
now I finally understood that even in that case there would be no need for
new *_IO_* symbols.  Really sorry).

>And __printf_fp isn't declared in any installed header, a strong 
>indication that it is not a public API or ABI and so no export of 
>__ieee128___printf_fp is needed either.  (The comment in 
>stdio-common/Versions on __printf_fp says "functions used in other 
>libraries", but as far as I can tell it's *not* in fact now used in any 
>other glibc library - and in any case, being used in another glibc library 
>would only justify an export at GLIBC_PRIVATE under current practice.)

Yes.  Sorry about this, too.

>I think doing things that way - shared long double redirection headers, 
>with only the definitions of macros such as __LDBL_REDIR_DECL being 
>different in the new IEEE 128-bit case - is clearly much cleaner than 
>having a separate set of redirection headers for the new case.  We've had 
>enough problems in the past with feature test macro tests in *-ldbl.h 
>getting out of sync with those in the including headers, we don't want to 
>add a third place that also needs feature test macro handling kept in 
>sync.
>
>(Of course that would mean you can't have redirections for some but not 
>all functions in the installed headers as a transitional state, because 
>such redirections would automatically be present for all or none of the 
>relevant functions.)

OK.  I'll probably keep this header (stdio-ieee128.h) in a *local branch*,
because it allows us to progress in smaller sets of functions.  In future
patch submissions (before all functions are ready), I'll not send it, but
I could make it available if someone wants to test the functions. Then,
when all functions are ready, we'll send a patch to do as you suggested
(i.e.: one which adjusts bits/stdio-ldbl.h with no extra headers).

>Now for (3), how the __ieee128_* for printf / scanf / strfmon functions 
>are implemented.  As previously discussed, the approach used for many of 
>the __nldbl_* functions, of setting a TLS variable __no_long_double, is 
>not a good one to emulate.  But that does not necessarily mean the two 
>sets of functions should be implemented in different ways.  The key 
>question to answer is: is the best way to implement __ieee128_* the same 
>as or different from the best way to implement __nldbl_*?  If they are the 
>same, changing how __nldbl_* are implemented may well be good preparation 
>for implementing __ieee128_*.
>
>Specifically, Zack sent a patch series to move away from __no_long_double 
>to explicit flags passed to internal functions.  So if there's some reason 
>why you think such explicit flags are not the best approach for 
>implementing __ieee128_*, but are the best approach for __nldbl_* you need 
>to explain why you think that's the case.  Otherwise, if explicit flags 
>would be appropriate for __ieee128_*, it would seem to make sense to work 
>on getting Zack's patches into glibc (for example, updating them for your 
>review comments and reposting them, if he hasn't had time to update them 
>himself) as preparation for __ieee128_* being able to use such flags.  
>(I'm presuming you do think the explicit flags are the right approach for 
>__nldbl_* since you didn't mention otherwise in your reviews of some of 
>Zack's patches.)

You presumed correctly, indeed.  I think that Zack's proposal is the right
thing to have in the end.  However, I also expected that rebuilding these
files with -mabi=ieeelongdouble had the advantage of being potentially
easier to get right, because I wouldn't need to touch anything from the
current implementation.  Besides that, I also expect that it will be
faster to get done, because it only adds files to the build, and since this
is useful for POWER9 users, getting it done sooner than later is something
that we need.

Taking this "timing" perspective into account and also that there seems to
be other problems that would need fixing before Zack's proposal can be
fully complete [1], would it be OK to have these functions added as new
implementations in a first step?  Afterwards, with "timing" concerns out
of the way, we could work together (if Zack is OK with that) on the
cleanup (for nldbl and ieee128-on-powerpc).

[1] https://sourceware.org/ml/libc-alpha/2018-03/msg00554.html

>I note that, for example, __printf_fp *already* supports _Float128.  So it 
>really ought not be necessary to build a second copy of it at all, if you 
>can make vfprintf for IEEE 128-bit long double (however implemented) pass 
>appropriate data structures for the existing __printf_fp to behave as 
>required.

For the same reason (getting the new format available sooner than later),
I thought of starting with a copy of vfprintf (adjusted to use a copy of
__printf_fp).