[v4] Prepare vfprintf to use __printf_fp/__printf_fphex with float128 arg
diff mbox series

Message ID 20190225133946.17585-1-gabriel@inconstante.eti.br
State New
Headers show
Series
  • [v4] Prepare vfprintf to use __printf_fp/__printf_fphex with float128 arg
Related show

Commit Message

Gabriel F. T. Gomes Feb. 25, 2019, 1:39 p.m. UTC
Changes since v3:

  - Update after the commit 2d9837c1fbf4 ('Set behavior of sprintf-like
    functions with overlapping source and destination').

Changes since v2:

  - Fixed style error in `do { ... } while (0)' blocks.
  - Zero-initialize args_value[cnt] with memset, rather than relying on
    the `.pa_long_double' member being the largest of the members.

Changes since v1:

  - Updated to the revised and integrated patches for __ldbl_is_dbl
    removal, i.e.: the patches in the following thread:
    <https://sourceware.org/ml/libc-alpha/2018-12/msg00186.html>.
    - Added description for the PRINTF_LDBL_USES_FLOAT128 macro.
    - Removed the LDBL_USES_FLOAT128 macro.
  - Added `do { } while (0)' to the PARSE_FLOAT_VA_ARG_EXTENDED,
    PARSE_FLOAT_VA_ARG, and SETUP_FLOAT128_INFO macros.  Appended
    expansions with `;', accordingly.

-- 8< --
On powerpc64le, long double can currently take two formats: the same as
double (-mlong-double-64) or IBM Extended Precision (default with
-mlong-double-128 or explicitly with -mabi=ibmlongdouble).  The internal
implementation of printf-like functions is aware of these possibilities
and properly parses floating-point values from the variable arguments,
before making calls to __printf_fp and __printf_fphex.  These functions
are also aware of the format possibilities and know how to convert both
formats to string.

When library support for TS 18661-3 was added to glibc, __printf_fp and
__printf_fphex were extended with support for an additional type
(__float128/_Float128) with a different format (binary128).  Now that
powerpc64le is getting support for its third long double format, and
taking into account that this format is the same as the format of
__float128/_Float128, this patch extends __vfprintf_internal to properly
call __printf_fp and __printf_fphex with this new format.

Tested for powerpc64le (with additional patches to actually enable the
use of these preparations) and for x86_64.

	* libio/libioP.h (PRINTF_LDBL_USES_FLOAT128): New macro to be
	used as a mask for the mode argument of __vfprintf_internal.
	* stdio-common/printf-parse.h (printf_arg): New union member:
	pa_float128.
	* stdio-common/vfprintf-internal.c
	(PARSE_FLOAT_VA_ARG_EXTENDED): New macro.
	(PARSE_FLOAT_VA_ARG): Likewise.
	(SETUP_FLOAT128_INFO): Likewise.
	(process_arg): Use PARSE_FLOAT_VA_ARG_EXTENDED and
	SETUP_FLOAT128_INFO.
	[__HAVE_FLOAT128_UNLIKE_LDBL] (printf_positional): Write
	floating-point value to the new union member, pa_float128.
	(printf_positional): Zero-initialize args_value[cnt] with memset.
---
 libio/libioP.h                   | 20 ++++++++---
 stdio-common/printf-parse.h      |  3 ++
 stdio-common/vfprintf-internal.c | 73 +++++++++++++++++++++++++++++++++-------
 3 files changed, 79 insertions(+), 17 deletions(-)

Comments

Gabriel F. T. Gomes April 10, 2019, 8:52 p.m. UTC | #1
Ping.  This patch [1] is pending review.

Testing this patch depends on the subsequent patches that create new
functions, e.g. __printfieee128, that actually use these changes, as
well as it depends on the subsequent patch that enables the ldbl
redirections (e.g. from printf to __printfieee128 in bits/stdio-ldbl.h).

However, such redirections can only be turned on when the functions for
all other redirections (i.e. those in *-ldbl.h headers) are ready.
Thus, I uploaded all of them to my personal branch [2], in the hopes
that someone willing to understand how this patch is used, *and tested*,
can do it easier (I haven't simply submitted all the patches from my
branch to the mailing list, because I think some of them still need a
little work from me before review).

[1] https://sourceware.org/ml/libc-alpha/2019-02/msg00582.html
[2] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/gabriel/powerpc-ieee128-printscan

On Mon, Feb 25 2019, Gabriel F. T. Gomes wrote:
> Changes since v3:
> 
>   - Update after the commit 2d9837c1fbf4 ('Set behavior of sprintf-like
>     functions with overlapping source and destination').
> 
> Changes since v2:
> 
>   - Fixed style error in `do { ... } while (0)' blocks.
>   - Zero-initialize args_value[cnt] with memset, rather than relying on
>     the `.pa_long_double' member being the largest of the members.
> 
> Changes since v1:
> 
>   - Updated to the revised and integrated patches for __ldbl_is_dbl
>     removal, i.e.: the patches in the following thread:
>     <https://sourceware.org/ml/libc-alpha/2018-12/msg00186.html>.
>     - Added description for the PRINTF_LDBL_USES_FLOAT128 macro.
>     - Removed the LDBL_USES_FLOAT128 macro.
>   - Added `do { } while (0)' to the PARSE_FLOAT_VA_ARG_EXTENDED,
>     PARSE_FLOAT_VA_ARG, and SETUP_FLOAT128_INFO macros.  Appended
>     expansions with `;', accordingly.
Gabriel F. T. Gomes April 25, 2019, 1:59 a.m. UTC | #2
Ping.  This patch is pending review.

On Wed, Apr 10 2019, Gabriel F. T. Gomes wrote:
> Ping.  This patch [1] is pending review.
> 
> Testing this patch depends on the subsequent patches that create new
> functions, e.g. __printfieee128, that actually use these changes, as
> well as it depends on the subsequent patch that enables the ldbl
> redirections (e.g. from printf to __printfieee128 in bits/stdio-ldbl.h).
> 
> However, such redirections can only be turned on when the functions for
> all other redirections (i.e. those in *-ldbl.h headers) are ready.
> Thus, I uploaded all of them to my personal branch [2], in the hopes
> that someone willing to understand how this patch is used, *and tested*,
> can do it easier (I haven't simply submitted all the patches from my
> branch to the mailing list, because I think some of them still need a
> little work from me before review).
> 
> [1] https://sourceware.org/ml/libc-alpha/2019-02/msg00582.html
> [2] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/gabriel/powerpc-ieee128-printscan
> 
> On Mon, Feb 25 2019, Gabriel F. T. Gomes wrote:
> > Changes since v3:
> > 
> >   - Update after the commit 2d9837c1fbf4 ('Set behavior of sprintf-like
> >     functions with overlapping source and destination').
> > 
> > Changes since v2:
> > 
> >   - Fixed style error in `do { ... } while (0)' blocks.
> >   - Zero-initialize args_value[cnt] with memset, rather than relying on
> >     the `.pa_long_double' member being the largest of the members.
> > 
> > Changes since v1:
> > 
> >   - Updated to the revised and integrated patches for __ldbl_is_dbl
> >     removal, i.e.: the patches in the following thread:
> >     <https://sourceware.org/ml/libc-alpha/2018-12/msg00186.html>.
> >     - Added description for the PRINTF_LDBL_USES_FLOAT128 macro.
> >     - Removed the LDBL_USES_FLOAT128 macro.
> >   - Added `do { } while (0)' to the PARSE_FLOAT_VA_ARG_EXTENDED,
> >     PARSE_FLOAT_VA_ARG, and SETUP_FLOAT128_INFO macros.  Appended
> >     expansions with `;', accordingly.
Gabriel F. T. Gomes May 3, 2019, 2:05 a.m. UTC | #3
Ping.  This patch is pending review.
https://sourceware.org/ml/libc-alpha/2019-02/msg00582.html

On Wed, Apr 24 2019, Gabriel F. T. Gomes wrote:
> Ping.  This patch is pending review.
> 
> On Wed, Apr 10 2019, Gabriel F. T. Gomes wrote:
> > Ping.  This patch [1] is pending review.
> > 
> > Testing this patch depends on the subsequent patches that create new
> > functions, e.g. __printfieee128, that actually use these changes, as
> > well as it depends on the subsequent patch that enables the ldbl
> > redirections (e.g. from printf to __printfieee128 in bits/stdio-ldbl.h).
> > 
> > However, such redirections can only be turned on when the functions for
> > all other redirections (i.e. those in *-ldbl.h headers) are ready.
> > Thus, I uploaded all of them to my personal branch [2], in the hopes
> > that someone willing to understand how this patch is used, *and tested*,
> > can do it easier (I haven't simply submitted all the patches from my
> > branch to the mailing list, because I think some of them still need a
> > little work from me before review).
> > 
> > [1] https://sourceware.org/ml/libc-alpha/2019-02/msg00582.html
> > [2] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/gabriel/powerpc-ieee128-printscan
> > 
> > On Mon, Feb 25 2019, Gabriel F. T. Gomes wrote:
> > > Changes since v3:
> > > 
> > >   - Update after the commit 2d9837c1fbf4 ('Set behavior of sprintf-like
> > >     functions with overlapping source and destination').
> > > 
> > > Changes since v2:
> > > 
> > >   - Fixed style error in `do { ... } while (0)' blocks.
> > >   - Zero-initialize args_value[cnt] with memset, rather than relying on
> > >     the `.pa_long_double' member being the largest of the members.
> > > 
> > > Changes since v1:
> > > 
> > >   - Updated to the revised and integrated patches for __ldbl_is_dbl
> > >     removal, i.e.: the patches in the following thread:
> > >     <https://sourceware.org/ml/libc-alpha/2018-12/msg00186.html>.
> > >     - Added description for the PRINTF_LDBL_USES_FLOAT128 macro.
> > >     - Removed the LDBL_USES_FLOAT128 macro.
> > >   - Added `do { } while (0)' to the PARSE_FLOAT_VA_ARG_EXTENDED,
> > >     PARSE_FLOAT_VA_ARG, and SETUP_FLOAT128_INFO macros.  Appended
> > >     expansions with `;', accordingly.
Adhemerval Zanella May 9, 2019, 8:29 p.m. UTC | #4
On 25/02/2019 10:39, Gabriel F. T. Gomes wrote:
> Changes since v3:
> 
>   - Update after the commit 2d9837c1fbf4 ('Set behavior of sprintf-like
>     functions with overlapping source and destination').
> 
> Changes since v2:
> 
>   - Fixed style error in `do { ... } while (0)' blocks.
>   - Zero-initialize args_value[cnt] with memset, rather than relying on
>     the `.pa_long_double' member being the largest of the members.
> 
> Changes since v1:
> 
>   - Updated to the revised and integrated patches for __ldbl_is_dbl
>     removal, i.e.: the patches in the following thread:
>     <https://sourceware.org/ml/libc-alpha/2018-12/msg00186.html>.
>     - Added description for the PRINTF_LDBL_USES_FLOAT128 macro.
>     - Removed the LDBL_USES_FLOAT128 macro.
>   - Added `do { } while (0)' to the PARSE_FLOAT_VA_ARG_EXTENDED,
>     PARSE_FLOAT_VA_ARG, and SETUP_FLOAT128_INFO macros.  Appended
>     expansions with `;', accordingly.
> 
> -- 8< --
> On powerpc64le, long double can currently take two formats: the same as
> double (-mlong-double-64) or IBM Extended Precision (default with
> -mlong-double-128 or explicitly with -mabi=ibmlongdouble).  The internal
> implementation of printf-like functions is aware of these possibilities
> and properly parses floating-point values from the variable arguments,
> before making calls to __printf_fp and __printf_fphex.  These functions
> are also aware of the format possibilities and know how to convert both
> formats to string.
> 
> When library support for TS 18661-3 was added to glibc, __printf_fp and
> __printf_fphex were extended with support for an additional type
> (__float128/_Float128) with a different format (binary128).  Now that
> powerpc64le is getting support for its third long double format, and
> taking into account that this format is the same as the format of
> __float128/_Float128, this patch extends __vfprintf_internal to properly
> call __printf_fp and __printf_fphex with this new format.
> 
> Tested for powerpc64le (with additional patches to actually enable the
> use of these preparations) and for x86_64.

It at least shows no issue when applied against master, although current 
tests not stress the code itself. And it seems that the code would only be 
enabled for powerpc with -mabi=ieeelongdouble, so I would if it would be 
better to actually push this patch along with powerpc64 -mabi=ieeelongdouble
patchset.

> 
> 	* libio/libioP.h (PRINTF_LDBL_USES_FLOAT128): New macro to be
> 	used as a mask for the mode argument of __vfprintf_internal.
> 	* stdio-common/printf-parse.h (printf_arg): New union member:
> 	pa_float128.
> 	* stdio-common/vfprintf-internal.c
> 	(PARSE_FLOAT_VA_ARG_EXTENDED): New macro.
> 	(PARSE_FLOAT_VA_ARG): Likewise.
> 	(SETUP_FLOAT128_INFO): Likewise.
> 	(process_arg): Use PARSE_FLOAT_VA_ARG_EXTENDED and
> 	SETUP_FLOAT128_INFO.
> 	[__HAVE_FLOAT128_UNLIKE_LDBL] (printf_positional): Write
> 	floating-point value to the new union member, pa_float128.
> 	(printf_positional): Zero-initialize args_value[cnt] with memset.
> ---
>  libio/libioP.h                   | 20 ++++++++---
>  stdio-common/printf-parse.h      |  3 ++
>  stdio-common/vfprintf-internal.c | 73 +++++++++++++++++++++++++++++++++-------
>  3 files changed, 79 insertions(+), 17 deletions(-)
> 
> diff --git a/libio/libioP.h b/libio/libioP.h
> index 7bdec86a62..eb86e07ad6 100644
> --- a/libio/libioP.h
> +++ b/libio/libioP.h
> @@ -708,10 +708,22 @@ extern int __vswprintf_internal (wchar_t *string, size_t maxlen,
>     defined to 1 or 2.  Otherwise, such checks are ignored.
>  
>     PRINTF_CHK indicates, to the internal function being called, that the
> -   call is originated from one of the __*printf_chk functions.  */
> -#define PRINTF_LDBL_IS_DBL 0x0001
> -#define PRINTF_FORTIFY     0x0002
> -#define PRINTF_CHK	   0x0004
> +   call is originated from one of the __*printf_chk functions.
> +
> +   PRINTF_LDBL_USES_FLOAT128 is used on platforms where the long double
> +   format used to be different from the IEC 60559 double format *and*
> +   also different from the Quadruple 128-bits IEC 60559 format (such as
> +   the IBM Extended Precision format on powerpc or the 80-bits IEC 60559
> +   format on x86), but was later converted to the Quadruple 128-bits IEC
> +   60559 format, which is the same format that the _Float128 always has
> +   (hence the `USES_FLOAT128' suffix in the name of the flag).  When set
> +   to one, this macros indicates that long double values are to be

s/macros/macro

> +   handled as having this new format.  Otherwise, they should be handled
> +   as the previous format on that platform.  */
> +#define PRINTF_LDBL_IS_DBL		0x0001
> +#define PRINTF_FORTIFY			0x0002
> +#define PRINTF_CHK			0x0004
> +#define PRINTF_LDBL_USES_FLOAT128	0x0008
>  
>  extern size_t _IO_getline (FILE *,char *, size_t, int, int);
>  libc_hidden_proto (_IO_getline)
> diff --git a/stdio-common/printf-parse.h b/stdio-common/printf-parse.h
> index 397508638d..00f3280b8a 100644
> --- a/stdio-common/printf-parse.h
> +++ b/stdio-common/printf-parse.h
> @@ -57,6 +57,9 @@ union printf_arg
>      unsigned long long int pa_u_long_long_int;
>      double pa_double;
>      long double pa_long_double;
> +#if __HAVE_FLOAT128_UNLIKE_LDBL
> +    _Float128 pa_float128;
> +#endif
>      const char *pa_string;
>      const wchar_t *pa_wstring;
>      void *pa_pointer;
> diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c
> index ead2b04cb9..d9dca78fc6 100644
> --- a/stdio-common/vfprintf-internal.c
> +++ b/stdio-common/vfprintf-internal.c
> @@ -68,6 +68,57 @@
>      } while (0)
>  #define UNBUFFERED_P(S) ((S)->_flags & _IO_UNBUFFERED)
>  
> +#if __HAVE_FLOAT128_UNLIKE_LDBL
> +# define PARSE_FLOAT_VA_ARG_EXTENDED(INFO)				      \
> +  do									      \
> +    {									      \
> +      if (is_long_double						      \
> +	  && (mode_flags & PRINTF_LDBL_USES_FLOAT128) != 0)		      \
> +	{								      \
> +	  INFO.is_binary128 = 1;					      \
> +	  the_arg.pa_float128 = va_arg (ap, _Float128);			      \
> +	}								      \
> +      else								      \
> +	{								      \
> +	  PARSE_FLOAT_VA_ARG (INFO);					      \
> +	}								      \
> +    }									      \
> +  while (0)
> +#else
> +# define PARSE_FLOAT_VA_ARG_EXTENDED(INFO)				      \
> +  PARSE_FLOAT_VA_ARG (INFO);
> +#endif

Do we really to continue use the macro-style to extend this code? I usually
think is is more readable to just use static inline function instead, 
something as:

---
static void
parse_float_va_arg_extended (struct printf_info *info,
                             union printf_arg *the_arg, int is_long_double,
                             unsigned int mode_flags, va_list *ap)
{
#if __HAVE_FLOAT128_UNLIKE_LDBL
  if (is_long_double && (mode_flags & PRINTF_LDBL_USES_FLOAT128) != 0)
    {
      info->is_binary128 = 1;
      the_arg->pa_float128 = va_arg (*ap, _Float128);
    }
  else
#endif
    {
      info->is_binary128 = 0;
      if (is_long_double)
        the_arg->pa_long_double = va_arg (*ap, long double);
      else
        the_arg->pa_double = va_arg (*ap, double);
    }
}
---

> +
> +#define PARSE_FLOAT_VA_ARG(INFO)					      \
> +  do									      \
> +    {									      \
> +      INFO.is_binary128 = 0;						      \
> +      if (is_long_double)						      \
> +	the_arg.pa_long_double = va_arg (ap, long double);		      \
> +      else								      \
> +	the_arg.pa_double = va_arg (ap, double);			      \
> +    }									      \
> +  while (0)
> +
> +#if __HAVE_FLOAT128_UNLIKE_LDBL
> +# define SETUP_FLOAT128_INFO(INFO)					      \
> +  do									      \
> +    {									      \
> +      if ((mode_flags & PRINTF_LDBL_USES_FLOAT128) != 0)		      \
> +	INFO.is_binary128 = is_long_double;				      \
> +      else								      \
> +	INFO.is_binary128 = 0;						      \
> +    }									      \
> +  while (0)
> +#else
> +# define SETUP_FLOAT128_INFO(INFO)					      \
> +  do									      \
> +    {									      \
> +      INFO.is_binary128 = 0;						      \
> +    }									      \
> +  while (0)
> +#endif
> +

Same as before.

>  #define done_add(val) \
>    do {									      \
>      unsigned int _val = val;						      \
> @@ -771,10 +822,7 @@ static const uint8_t jump_table[] =
>  					.wide = sizeof (CHAR_T) != 1,	      \
>  					.is_binary128 = 0};		      \
>  									      \
> -	    if (is_long_double)						      \
> -	      the_arg.pa_long_double = va_arg (ap, long double);	      \
> -	    else							      \
> -	      the_arg.pa_double = va_arg (ap, double);			      \
> +	    PARSE_FLOAT_VA_ARG_EXTENDED (info);				      \
>  	    ptr = (const void *) &the_arg;				      \
>  									      \
>  	    function_done = __printf_fp (s, &info, &ptr);		      \
> @@ -787,8 +835,7 @@ static const uint8_t jump_table[] =
>  		fspec->data_arg_type = PA_DOUBLE;			      \
>  		fspec->info.is_long_double = 0;				      \
>  	      }								      \
> -	    /* Not supported by *printf functions.  */			      \
> -	    fspec->info.is_binary128 = 0;				      \
> +	    SETUP_FLOAT128_INFO (fspec->info);				      \
>  									      \
>  	    function_done = __printf_fp (s, &fspec->info, &ptr);	      \
>  	  }								      \
> @@ -831,10 +878,7 @@ static const uint8_t jump_table[] =
>  					.wide = sizeof (CHAR_T) != 1,	      \
>  					.is_binary128 = 0};		      \
>  									      \
> -	    if (is_long_double)						      \
> -	      the_arg.pa_long_double = va_arg (ap, long double);	      \
> -	    else							      \
> -	      the_arg.pa_double = va_arg (ap, double);			      \
> +	    PARSE_FLOAT_VA_ARG_EXTENDED (info);				      \
>  	    ptr = (const void *) &the_arg;				      \
>  									      \
>  	    function_done = __printf_fphex (s, &info, &ptr);		      \
> @@ -844,8 +888,7 @@ static const uint8_t jump_table[] =
>  	    ptr = (const void *) &args_value[fspec->data_arg];		      \
>  	    if (__glibc_unlikely ((mode_flags & PRINTF_LDBL_IS_DBL) != 0))    \
>  	      fspec->info.is_long_double = 0;				      \
> -	    /* Not supported by *printf functions.  */			      \
> -	    fspec->info.is_binary128 = 0;				      \
> +	    SETUP_FLOAT128_INFO (fspec->info);				      \
>  									      \
>  	    function_done = __printf_fphex (s, &fspec->info, &ptr);	      \
>  	  }								      \
> @@ -1869,6 +1912,10 @@ printf_positional (FILE *s, const CHAR_T *format, int readonly_format,
>  	    args_value[cnt].pa_double = va_arg (*ap_savep, double);
>  	    args_type[cnt] &= ~PA_FLAG_LONG_DOUBLE;
>  	  }
> +#if __HAVE_FLOAT128_UNLIKE_LDBL
> +	else if ((mode_flags & PRINTF_LDBL_USES_FLOAT128) != 0)
> +	  args_value[cnt].pa_float128 = va_arg (*ap_savep, _Float128);
> +#endif
>  	else
>  	  args_value[cnt].pa_long_double = va_arg (*ap_savep, long double);
>  	break;
> @@ -1887,7 +1934,7 @@ printf_positional (FILE *s, const CHAR_T *format, int readonly_format,
>  	      (args_value[cnt].pa_user, ap_savep);
>  	  }
>  	else
> -	  args_value[cnt].pa_long_double = 0.0;
> +	  memset (&args_value[cnt], 0, sizeof (args_value[cnt]));
>  	break;
>        case -1:
>  	/* Error case.  Not all parameters appear in N$ format
>
Gabriel F. T. Gomes May 16, 2019, 9:36 p.m. UTC | #5
On Thu, May 09 2019, Adhemerval Zanella wrote:
> 
> On 25/02/2019 10:39, Gabriel F. T. Gomes wrote:
> > 
> > Tested for powerpc64le (with additional patches to actually enable the
> > use of these preparations) and for x86_64.
> 
> It at least shows no issue when applied against master, although current 
> tests not stress the code itself. And it seems that the code would only be 
> enabled for powerpc with -mabi=ieeelongdouble, so I would if it would be 
> better to actually push this patch along with powerpc64 -mabi=ieeelongdouble
> patchset.

Well, I intentionally write small patches that are valid for master, so
they can be applied sooner than later.  I have written many patches this
way to avoid the burden of carrying patches on personal branches.

I also provided a link to such branch where the changes in this patch
get tested for many of the functions added by subsequent patches.

I was hoping that these things combined would provide a good
justification for acceptance.  With that in mind, do you have an
objection to this patch?

> > +   PRINTF_LDBL_USES_FLOAT128 is used on platforms where the long double
> > +   format used to be different from the IEC 60559 double format *and*
> > +   also different from the Quadruple 128-bits IEC 60559 format (such as
> > +   the IBM Extended Precision format on powerpc or the 80-bits IEC 60559
> > +   format on x86), but was later converted to the Quadruple 128-bits IEC
> > +   60559 format, which is the same format that the _Float128 always has
> > +   (hence the `USES_FLOAT128' suffix in the name of the flag).  When set
> > +   to one, this macros indicates that long double values are to be
> 
> s/macros/macro

Thanks.  Fixed locally.

> > +#if __HAVE_FLOAT128_UNLIKE_LDBL
> > +# define PARSE_FLOAT_VA_ARG_EXTENDED(INFO)				      \
> > [...]
> 
> Do we really to continue use the macro-style to extend this code? I usually
> think is is more readable to just use static inline function instead, 
> something as:

Sounds good to me.  I'll send a new version.

> static void
> parse_float_va_arg_extended (struct printf_info *info,
>                              union printf_arg *the_arg, int is_long_double,
>                              unsigned int mode_flags, va_list *ap)

Should I also add an __always_inline attribute?

> > +#define PARSE_FLOAT_VA_ARG(INFO)					      \
> > [...]
> 
> Same as before.

OK.

Patch
diff mbox series

diff --git a/libio/libioP.h b/libio/libioP.h
index 7bdec86a62..eb86e07ad6 100644
--- a/libio/libioP.h
+++ b/libio/libioP.h
@@ -708,10 +708,22 @@  extern int __vswprintf_internal (wchar_t *string, size_t maxlen,
    defined to 1 or 2.  Otherwise, such checks are ignored.
 
    PRINTF_CHK indicates, to the internal function being called, that the
-   call is originated from one of the __*printf_chk functions.  */
-#define PRINTF_LDBL_IS_DBL 0x0001
-#define PRINTF_FORTIFY     0x0002
-#define PRINTF_CHK	   0x0004
+   call is originated from one of the __*printf_chk functions.
+
+   PRINTF_LDBL_USES_FLOAT128 is used on platforms where the long double
+   format used to be different from the IEC 60559 double format *and*
+   also different from the Quadruple 128-bits IEC 60559 format (such as
+   the IBM Extended Precision format on powerpc or the 80-bits IEC 60559
+   format on x86), but was later converted to the Quadruple 128-bits IEC
+   60559 format, which is the same format that the _Float128 always has
+   (hence the `USES_FLOAT128' suffix in the name of the flag).  When set
+   to one, this macros indicates that long double values are to be
+   handled as having this new format.  Otherwise, they should be handled
+   as the previous format on that platform.  */
+#define PRINTF_LDBL_IS_DBL		0x0001
+#define PRINTF_FORTIFY			0x0002
+#define PRINTF_CHK			0x0004
+#define PRINTF_LDBL_USES_FLOAT128	0x0008
 
 extern size_t _IO_getline (FILE *,char *, size_t, int, int);
 libc_hidden_proto (_IO_getline)
diff --git a/stdio-common/printf-parse.h b/stdio-common/printf-parse.h
index 397508638d..00f3280b8a 100644
--- a/stdio-common/printf-parse.h
+++ b/stdio-common/printf-parse.h
@@ -57,6 +57,9 @@  union printf_arg
     unsigned long long int pa_u_long_long_int;
     double pa_double;
     long double pa_long_double;
+#if __HAVE_FLOAT128_UNLIKE_LDBL
+    _Float128 pa_float128;
+#endif
     const char *pa_string;
     const wchar_t *pa_wstring;
     void *pa_pointer;
diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c
index ead2b04cb9..d9dca78fc6 100644
--- a/stdio-common/vfprintf-internal.c
+++ b/stdio-common/vfprintf-internal.c
@@ -68,6 +68,57 @@ 
     } while (0)
 #define UNBUFFERED_P(S) ((S)->_flags & _IO_UNBUFFERED)
 
+#if __HAVE_FLOAT128_UNLIKE_LDBL
+# define PARSE_FLOAT_VA_ARG_EXTENDED(INFO)				      \
+  do									      \
+    {									      \
+      if (is_long_double						      \
+	  && (mode_flags & PRINTF_LDBL_USES_FLOAT128) != 0)		      \
+	{								      \
+	  INFO.is_binary128 = 1;					      \
+	  the_arg.pa_float128 = va_arg (ap, _Float128);			      \
+	}								      \
+      else								      \
+	{								      \
+	  PARSE_FLOAT_VA_ARG (INFO);					      \
+	}								      \
+    }									      \
+  while (0)
+#else
+# define PARSE_FLOAT_VA_ARG_EXTENDED(INFO)				      \
+  PARSE_FLOAT_VA_ARG (INFO);
+#endif
+
+#define PARSE_FLOAT_VA_ARG(INFO)					      \
+  do									      \
+    {									      \
+      INFO.is_binary128 = 0;						      \
+      if (is_long_double)						      \
+	the_arg.pa_long_double = va_arg (ap, long double);		      \
+      else								      \
+	the_arg.pa_double = va_arg (ap, double);			      \
+    }									      \
+  while (0)
+
+#if __HAVE_FLOAT128_UNLIKE_LDBL
+# define SETUP_FLOAT128_INFO(INFO)					      \
+  do									      \
+    {									      \
+      if ((mode_flags & PRINTF_LDBL_USES_FLOAT128) != 0)		      \
+	INFO.is_binary128 = is_long_double;				      \
+      else								      \
+	INFO.is_binary128 = 0;						      \
+    }									      \
+  while (0)
+#else
+# define SETUP_FLOAT128_INFO(INFO)					      \
+  do									      \
+    {									      \
+      INFO.is_binary128 = 0;						      \
+    }									      \
+  while (0)
+#endif
+
 #define done_add(val) \
   do {									      \
     unsigned int _val = val;						      \
@@ -771,10 +822,7 @@  static const uint8_t jump_table[] =
 					.wide = sizeof (CHAR_T) != 1,	      \
 					.is_binary128 = 0};		      \
 									      \
-	    if (is_long_double)						      \
-	      the_arg.pa_long_double = va_arg (ap, long double);	      \
-	    else							      \
-	      the_arg.pa_double = va_arg (ap, double);			      \
+	    PARSE_FLOAT_VA_ARG_EXTENDED (info);				      \
 	    ptr = (const void *) &the_arg;				      \
 									      \
 	    function_done = __printf_fp (s, &info, &ptr);		      \
@@ -787,8 +835,7 @@  static const uint8_t jump_table[] =
 		fspec->data_arg_type = PA_DOUBLE;			      \
 		fspec->info.is_long_double = 0;				      \
 	      }								      \
-	    /* Not supported by *printf functions.  */			      \
-	    fspec->info.is_binary128 = 0;				      \
+	    SETUP_FLOAT128_INFO (fspec->info);				      \
 									      \
 	    function_done = __printf_fp (s, &fspec->info, &ptr);	      \
 	  }								      \
@@ -831,10 +878,7 @@  static const uint8_t jump_table[] =
 					.wide = sizeof (CHAR_T) != 1,	      \
 					.is_binary128 = 0};		      \
 									      \
-	    if (is_long_double)						      \
-	      the_arg.pa_long_double = va_arg (ap, long double);	      \
-	    else							      \
-	      the_arg.pa_double = va_arg (ap, double);			      \
+	    PARSE_FLOAT_VA_ARG_EXTENDED (info);				      \
 	    ptr = (const void *) &the_arg;				      \
 									      \
 	    function_done = __printf_fphex (s, &info, &ptr);		      \
@@ -844,8 +888,7 @@  static const uint8_t jump_table[] =
 	    ptr = (const void *) &args_value[fspec->data_arg];		      \
 	    if (__glibc_unlikely ((mode_flags & PRINTF_LDBL_IS_DBL) != 0))    \
 	      fspec->info.is_long_double = 0;				      \
-	    /* Not supported by *printf functions.  */			      \
-	    fspec->info.is_binary128 = 0;				      \
+	    SETUP_FLOAT128_INFO (fspec->info);				      \
 									      \
 	    function_done = __printf_fphex (s, &fspec->info, &ptr);	      \
 	  }								      \
@@ -1869,6 +1912,10 @@  printf_positional (FILE *s, const CHAR_T *format, int readonly_format,
 	    args_value[cnt].pa_double = va_arg (*ap_savep, double);
 	    args_type[cnt] &= ~PA_FLAG_LONG_DOUBLE;
 	  }
+#if __HAVE_FLOAT128_UNLIKE_LDBL
+	else if ((mode_flags & PRINTF_LDBL_USES_FLOAT128) != 0)
+	  args_value[cnt].pa_float128 = va_arg (*ap_savep, _Float128);
+#endif
 	else
 	  args_value[cnt].pa_long_double = va_arg (*ap_savep, long double);
 	break;
@@ -1887,7 +1934,7 @@  printf_positional (FILE *s, const CHAR_T *format, int readonly_format,
 	      (args_value[cnt].pa_user, ap_savep);
 	  }
 	else
-	  args_value[cnt].pa_long_double = 0.0;
+	  memset (&args_value[cnt], 0, sizeof (args_value[cnt]));
 	break;
       case -1:
 	/* Error case.  Not all parameters appear in N$ format