This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Remove __isinf uses that rely on signed return value
- From: Szabolcs Nagy <szabolcs dot nagy at arm dot com>
- To: Wilco Dijkstra <wdijkstr at arm dot com>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Tue, 02 Jun 2015 19:00:37 +0100
- Subject: Re: [PATCH] Remove __isinf uses that rely on signed return value
- Authentication-results: sourceware.org; auth=none
- References: <001301d09d57$c0bfdf60$423f9e20$ at com>
On 02/06/15 18:15, Wilco Dijkstra wrote:
> The printf code contains a few uses of __isinf where the sign is used - replace these with separate
> isinf and signbit. Also change __isnan into isnan.
>
i suspect it slightly changes semantics on x86 (for the better).
i386 __isnanl does not seem to handle invalid ld80 representations.
if exponent != 0 and significand>>63 == 0 then it's a "pseudo normal"
or "pseudo infinite" value that the fpu would categorize as nan, but
not __isnanl.
(passing around long doubles with invalid representation is ub,
but probably __isnanl should be consistent with fcom instruction
to be safe when it is used on random bytes).
> OK for commit?
>
> Wilco
>
> 2015-06-02 Wilco Dijkstra <wdijkstr@arm.com>
>
> * stdio-common/printf_fp.c (___printf_fp):
> Use signbit to get the sign. Use isinf/isnan macros.
> * stdio-common/printf_fphex.c (__printf_fphex): Likewise.
> * stdio-common/printf_size.c (__printf_size): Likewise.
>
> ---
> stdio-common/printf_fp.c | 17 +++++++----------
> stdio-common/printf_fphex.c | 14 +++++---------
> stdio-common/printf_size.c | 23 +++++++++++------------
> 3 files changed, 23 insertions(+), 31 deletions(-)
>
> diff --git a/stdio-common/printf_fp.c b/stdio-common/printf_fp.c
> index 575842b..11f3145 100644
> --- a/stdio-common/printf_fp.c
> +++ b/stdio-common/printf_fp.c
> @@ -332,8 +332,7 @@ ___printf_fp (FILE *fp,
> fpnum.ldbl = *(const long double *) args[0];
>
> /* Check for special values: not a number or infinity. */
> - int res;
> - if (__isnanl (fpnum.ldbl))
> + if (isnan (fpnum.ldbl))
> {
> is_neg = signbit (fpnum.ldbl);
> if (isupper (info->spec))
> @@ -347,9 +346,9 @@ ___printf_fp (FILE *fp,
> wspecial = L"nan";
> }
> }
> - else if ((res = __isinfl (fpnum.ldbl)))
> + else if (isinf (fpnum.ldbl))
> {
> - is_neg = res < 0;
> + is_neg = signbit (fpnum.ldbl);
> if (isupper (info->spec))
> {
> special = "INF";
> @@ -377,11 +376,9 @@ ___printf_fp (FILE *fp,
> fpnum.dbl = *(const double *) args[0];
>
> /* Check for special values: not a number or infinity. */
> - int res;
> - if (__isnan (fpnum.dbl))
> + if (isnan (fpnum.dbl))
> {
> - union ieee754_double u = { .d = fpnum.dbl };
> - is_neg = u.ieee.negative != 0;
> + is_neg = signbit (fpnum.dbl);
> if (isupper (info->spec))
> {
> special = "NAN";
> @@ -393,9 +390,9 @@ ___printf_fp (FILE *fp,
> wspecial = L"nan";
> }
> }
> - else if ((res = __isinf (fpnum.dbl)))
> + else if (isinf (fpnum.dbl))
> {
> - is_neg = res < 0;
> + is_neg = signbit (fpnum.dbl);
> if (isupper (info->spec))
> {
> special = "INF";
> diff --git a/stdio-common/printf_fphex.c b/stdio-common/printf_fphex.c
> index ba0639f..0627bea 100644
> --- a/stdio-common/printf_fphex.c
> +++ b/stdio-common/printf_fphex.c
> @@ -165,7 +165,7 @@ __printf_fphex (FILE *fp,
> fpnum.ldbl = *(const long double *) args[0];
>
> /* Check for special values: not a number or infinity. */
> - if (__isnanl (fpnum.ldbl))
> + if (isnan (fpnum.ldbl))
> {
> if (isupper (info->spec))
> {
> @@ -180,7 +180,7 @@ __printf_fphex (FILE *fp,
> }
> else
> {
> - if (__isinfl (fpnum.ldbl))
> + if (isinf (fpnum.ldbl))
> {
> if (isupper (info->spec))
> {
> @@ -202,9 +202,8 @@ __printf_fphex (FILE *fp,
> fpnum.dbl.d = *(const double *) args[0];
>
> /* Check for special values: not a number or infinity. */
> - if (__isnan (fpnum.dbl.d))
> + if (isnan (fpnum.dbl.d))
> {
> - negative = fpnum.dbl.ieee.negative != 0;
> if (isupper (info->spec))
> {
> special = "NAN";
> @@ -218,8 +217,7 @@ __printf_fphex (FILE *fp,
> }
> else
> {
> - int res = __isinf (fpnum.dbl.d);
> - if (res)
> + if (isinf (fpnum.dbl.d))
> {
> if (isupper (info->spec))
> {
> @@ -231,11 +229,9 @@ __printf_fphex (FILE *fp,
> special = "inf";
> wspecial = L"inf";
> }
> - negative = res < 0;
> }
> - else
> - negative = signbit (fpnum.dbl.d);
> }
> + negative = signbit (fpnum.dbl.d);
> }
>
> if (special)
> diff --git a/stdio-common/printf_size.c b/stdio-common/printf_size.c
> index 6ee753f..216f170 100644
> --- a/stdio-common/printf_size.c
> +++ b/stdio-common/printf_size.c
> @@ -108,7 +108,7 @@ __printf_size (FILE *fp, const struct printf_info *info,
> fpnum;
> const void *ptr = &fpnum;
>
> - int fpnum_sign = 0;
> + int is_neg = 0;
>
> /* "NaN" or "Inf" for the special cases. */
> const char *special = NULL;
> @@ -117,7 +117,6 @@ __printf_size (FILE *fp, const struct printf_info *info,
> struct printf_info fp_info;
> int done = 0;
> int wide = info->wide;
> - int res;
>
> /* Fetch the argument value. */
> #ifndef __NO_LONG_DOUBLE_MATH
> @@ -126,15 +125,15 @@ __printf_size (FILE *fp, const struct printf_info *info,
> fpnum.ldbl = *(const long double *) args[0];
>
> /* Check for special values: not a number or infinity. */
> - if (__isnanl (fpnum.ldbl))
> + if (isnan (fpnum.ldbl))
> {
> special = "nan";
> wspecial = L"nan";
> - // fpnum_sign = 0; Already zero
> + // is_neg = 0; Already zero
> }
> - else if ((res = __isinfl (fpnum.ldbl)))
> + else if (isinf (fpnum.ldbl))
> {
> - fpnum_sign = res;
> + is_neg = signbit (fpnum.ldbl);
> special = "inf";
> wspecial = L"inf";
> }
> @@ -151,15 +150,15 @@ __printf_size (FILE *fp, const struct printf_info *info,
> fpnum.dbl.d = *(const double *) args[0];
>
> /* Check for special values: not a number or infinity. */
> - if (__isnan (fpnum.dbl.d))
> + if (isnan (fpnum.dbl.d))
> {
> special = "nan";
> wspecial = L"nan";
> - // fpnum_sign = 0; Already zero
> + // is_neg = 0; Already zero
> }
> - else if ((res = __isinf (fpnum.dbl.d)))
> + else if (isinf (fpnum.dbl.d))
> {
> - fpnum_sign = res;
> + is_neg = signbit (fpnum.dbl.d);
> special = "inf";
> wspecial = L"inf";
> }
> @@ -175,14 +174,14 @@ __printf_size (FILE *fp, const struct printf_info *info,
> {
> int width = info->prec > info->width ? info->prec : info->width;
>
> - if (fpnum_sign < 0 || info->showsign || info->space)
> + if (is_neg || info->showsign || info->space)
> --width;
> width -= 3;
>
> if (!info->left && width > 0)
> PADN (' ', width);
>
> - if (fpnum_sign < 0)
> + if (is_neg)
> outchar ('-');
> else if (info->showsign)
> outchar ('+');
>