From: Jakub Jelinek <jakub@redhat.com>
To: i.nixman@autistici.org
Cc: Gcc Patches <gcc-patches@gcc.gnu.org>
Subject: Re: libquadmath fix for 94756 and 87204
Date: Fri, 20 Jan 2023 15:20:53 +0100 [thread overview]
Message-ID: <Y8qjRTCBnm8AyK0y@tucnak> (raw)
In-Reply-To: <0e5ef1d5ce3e47e8431450ae8383a342@autistici.org>
On Fri, Jan 20, 2023 at 02:06:01PM +0000, i.nixman--- via Gcc-patches wrote:
> I have fixed:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94756
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87204
>
> tested on i686-mingw-w64, x86_64-mingw-w64, and for i686 and x86_64 linux.
>
> could anyone check and apply please?
>
>
>
> best!
> diff --git a/libquadmath/printf/gmp-impl.h b/libquadmath/printf/gmp-impl.h
> index 94d88efc57f..af0719321dc 100644
> --- a/libquadmath/printf/gmp-impl.h
> +++ b/libquadmath/printf/gmp-impl.h
> @@ -33,15 +33,30 @@ MA 02111-1307, USA. */
> #define MAX(h,i) ((h) > (i) ? (h) : (i))
> #endif
>
> -#define BITS_PER_MP_LIMB (__SIZEOF_LONG__ * __CHAR_BIT__)
> -#define BYTES_PER_MP_LIMB (BITS_PER_MP_LIMB / __CHAR_BIT__)
> -typedef unsigned long int mp_limb_t;
> -typedef long int mp_limb_signed_t;
> +#ifdef __MINGW32__
> + /* for MinGW targets the Microsoft ABI requires that `long`
> + types will always have 32 bit, because of that we will use
> + `int32_t` for 32-bit builds and `int64_t` for 64-bit builds */
> +# if __x86_64__
> + typedef long long int mp_limb_signed_t;
> + typedef unsigned long long int mp_limb_t;
> +# define BITS_PER_MP_LIMB (__SIZEOF_LONG_LONG__ * __CHAR_BIT__)
> +# else // !__x86_64__
> + typedef long int mp_limb_signed_t;
> + typedef unsigned long int mp_limb_t;
> +# define BITS_PER_MP_LIMB (__SIZEOF_LONG__ * __CHAR_BIT__)
> +# endif // __x86_64__
> +#else // !__MINGW32__
> + typedef long int mp_limb_signed_t;
> + typedef unsigned long int mp_limb_t;
> +# define BITS_PER_MP_LIMB (__SIZEOF_LONG__ * __CHAR_BIT__)
> +#endif // __MINGW32__
The above looks way too complicated for what it does.
If all you want to change mp_limb* to be long long for mingw 64-bit,
then just do:
#if defined(__MINGW32__) && defined(__x86_64__)
typedef unsigned long long int mp_limb_t;
typedef long long int mp_limb_signed_t;
#define BITS_PER_MP_LIMB (__SIZEOF_LONG_LONG__ * __CHAR_BIT__)
#else
typedef unsigned long int mp_limb_t;
typedef long int mp_limb_signed_t;
#define BITS_PER_MP_LIMB (__SIZEOF_LONG__ * __CHAR_BIT__)
#endif
and nothing else. Or port the changes from glibc stdlib/gmp.h
etc. where there are macros to control what type is used for mp_limb_t etc.
> -typedef mp_limb_t * mp_ptr;
> -typedef const mp_limb_t * mp_srcptr;
> -typedef long int mp_size_t;
> -typedef long int mp_exp_t;
> +#define BYTES_PER_MP_LIMB (BITS_PER_MP_LIMB / __CHAR_BIT__)
> +typedef long int mp_size_t;
> +typedef long int mp_exp_t;
> +typedef mp_limb_t *mp_ptr;
> +typedef const mp_limb_t *mp_srcptr;
Why?
As for the rest, it would help if you could list the exact glibc commits
which you've ported to libquadmath and indicate if it is solely those
and nothing else.
The patch needs a ChangeLog entry too.
> --- a/libquadmath/strtod/strtod_l.c
> +++ b/libquadmath/strtod/strtod_l.c
> @@ -200,7 +200,7 @@ round_and_return (mp_limb_t *retval, intmax_t exponent, int negative,
>
> round_limb = retval[RETURN_LIMB_SIZE - 1];
> round_bit = (MANT_DIG - 1) % BITS_PER_MP_LIMB;
> - for (i = 0; i < RETURN_LIMB_SIZE; ++i)
> + for (i = 0; i < RETURN_LIMB_SIZE - 1; ++i)
> more_bits |= retval[i] != 0;
> MPN_ZERO (retval, RETURN_LIMB_SIZE);
> }
> @@ -215,9 +215,14 @@ round_and_return (mp_limb_t *retval, intmax_t exponent, int negative,
> more_bits |= ((round_limb & ((((mp_limb_t) 1) << round_bit) - 1))
> != 0);
>
> - (void) mpn_rshift (retval, &retval[shift / BITS_PER_MP_LIMB],
> - RETURN_LIMB_SIZE - (shift / BITS_PER_MP_LIMB),
> - shift % BITS_PER_MP_LIMB);
> + /* mpn_rshift requires 0 < shift < BITS_PER_MP_LIMB. */
> + if ((shift % BITS_PER_MP_LIMB) != 0)
> + (void) mpn_rshift (retval, &retval[shift / BITS_PER_MP_LIMB],
> + RETURN_LIMB_SIZE - (shift / BITS_PER_MP_LIMB),
> + shift % BITS_PER_MP_LIMB);
> + else
> + for (i = 0; i < RETURN_LIMB_SIZE - (shift / BITS_PER_MP_LIMB); i++)
> + retval[i] = retval[i + (shift / BITS_PER_MP_LIMB)];
> MPN_ZERO (&retval[RETURN_LIMB_SIZE - (shift / BITS_PER_MP_LIMB)],
> shift / BITS_PER_MP_LIMB);
> }
> @@ -276,7 +281,7 @@ round_and_return (mp_limb_t *retval, intmax_t exponent, int negative,
> }
> }
>
> - if (exponent > MAX_EXP)
> + if (exponent >= MAX_EXP)
> goto overflow;
>
> #ifdef HAVE_FENV_H
> @@ -308,7 +313,7 @@ round_and_return (mp_limb_t *retval, intmax_t exponent, int negative,
> }
> #endif
>
> - if (exponent > MAX_EXP)
> + if (exponent >= MAX_EXP)
> overflow:
> return overflow_value (negative);
>
> @@ -688,7 +693,7 @@ ____STRTOF_INTERNAL (nptr, endptr, group)
> if (endptr != NULL)
> *endptr = (STRING_TYPE *) cp;
>
> - return retval;
> + return negative ? -retval : retval;
> }
>
> /* It is really a text we do not recognize. */
> @@ -1193,7 +1198,7 @@ ____STRTOF_INTERNAL (nptr, endptr, group)
> if (__builtin_expect (exponent > MAX_10_EXP + 1 - (intmax_t) int_no, 0))
> return overflow_value (negative);
>
> - if (__builtin_expect (exponent < MIN_10_EXP - (DIG + 1), 0))
> + if (__builtin_expect (exponent < MIN_10_EXP - (DIG + 2), 0))
> return underflow_value (negative);
>
> if (int_no > 0)
> @@ -1360,7 +1365,7 @@ ____STRTOF_INTERNAL (nptr, endptr, group)
>
> assert (dig_no > int_no
> && exponent <= 0
> - && exponent >= MIN_10_EXP - (DIG + 1));
> + && exponent >= MIN_10_EXP - (DIG + 2));
>
> /* We need to compute MANT_DIG - BITS fractional bits that lie
> within the mantissa of the result, the following bit for
> @@ -1651,8 +1656,8 @@ ____STRTOF_INTERNAL (nptr, endptr, group)
> d1 = den[densize - 2];
>
> /* The division does not work if the upper limb of the two-limb
> - numerator is greater than the denominator. */
> - if (mpn_cmp (num, &den[densize - numsize], numsize) > 0)
> + numerator is greater or equal to than the denominator. */
> + if (mpn_cmp (num, &den[densize - numsize], numsize) >= 0)
> num[numsize++] = 0;
>
> if (numsize < densize)
> @@ -1761,7 +1766,7 @@ ____STRTOF_INTERNAL (nptr, endptr, group)
> got_limb;
> }
>
> - for (i = densize; num[i] == 0 && i >= 0; --i)
> + for (i = densize; i >= 0 && num[i] == 0; --i)
> ;
> return round_and_return (retval, exponent - 1, negative,
> quot, BITS_PER_MP_LIMB - 1 - used,
Jakub
next prev parent reply other threads:[~2023-01-20 14:21 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-20 14:06 i.nixman
2023-01-20 14:20 ` Jakub Jelinek [this message]
2023-01-20 14:10 i.nixman
2023-01-21 16:31 i.nixman
2023-03-01 18:28 ` Jakub Jelinek
2023-01-26 17:49 i.nixman
2023-02-05 17:39 ` NightStrike
2023-02-08 18:13 ` NightStrike
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y8qjRTCBnm8AyK0y@tucnak \
--to=jakub@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=i.nixman@autistici.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).