On Wed, 9 Aug 2023, Jakub Jelinek via Gcc-patches wrote:
> I know that soft-fp is owned by glibc and I think the op-common.h change
> should be propagated there, but the bitint stuff is really GCC specific
> and IMHO doesn't belong into the glibc copy.
The op-common.h change is OK for glibc.
Some additional tests I think should be added to the testsuite for
floating-point functionality in this patch, that I didn't spot in the
testsuite patches - if any of these aren't included initially, there
should at least be bugs filed in Bugzilla for the omissions:
1. Test overflowing conversions to integers (including from inf or NaN)
raise FE_INVALID. (Note: it's not specified in the standard whether
inexact conversions to integers raise FE_INEXACT or not, so testing that
seems less important.)
2. Test conversions from integers to floating point raise FE_INEXACT when
inexact, together with FE_OVERFLOW when overflowing (while exact
conversions don't raise exceptions).
3. Test conversions from integers to floating point respect the rounding
mode.
4. Test converting floating-point values in the range (-1.0, 0.0] to both
unsigned and signed _BitInt; I didn't see such tests for binary floating
types, only for decimal types, and the decimal tests didn't include tests
of negative zero itself as the value converted to _BitInt.
5. Test conversions of noncanonical BID zero to integers (these tests
would be specific to BID). See below for a bug in this area.
For points 2 and 3 above, it's probably appropriate to test only for
binary floating point, to avoid any issues with the separate DFP rounding
mode and with DFP arithmetic operations not necessarily working correctly
with exceptions - but then a bug should be filed in Bugzilla noting the
omission of such tests for DFP.
For points 1, 2 and 3 above, if the conversions for types such as
_BitInt(32) might end up using the same conversions as for types such as
int, then tests for such types should probably be omitted (again with a
bug filed) given the range of known bugs about exceptions from such
operations with types such as int.
> +__bid_fixtdbitint (UBILtype *r, SItype rprec, _Decimal128 a)
> +{
> + FP_DECL_EX;
> + USItype arprec = rprec < 0 ? -rprec : rprec;
> + USItype rn = (arprec + BIL_TYPE_SIZE - 1) / BIL_TYPE_SIZE;
> + union { _Decimal128 d; UDItype u[2]; } u;
> + UDItype mantissahi, mantissalo, t;
> + SItype sgn;
> + SItype exponent;
> + USItype exp_bits, mant_bits;
> + UBILtype *pow10v, *resv;
> + USItype pow10_limbs, res_limbs, min_limbs, mant_limbs, low_zeros;
> +
> + FP_INIT_EXCEPTIONS;
> + u.d = a;
> + mantissahi = u.u[__FLOAT_WORD_ORDER__ != __ORDER_BIG_ENDIAN__];
> + mantissalo = u.u[__FLOAT_WORD_ORDER__ == __ORDER_BIG_ENDIAN__];
> + t = mantissahi >> 47;
> + sgn = (DItype) mantissahi < 0;
> + if ((t & (3 << 14)) != (3 << 14))
> + {
> + mantissahi &= ((((UDItype) 1) << 49) - 1);
> + exponent = (t >> 2) & 0x3fff;
Overflow (thus producing a noncanonical zero) is possible in this case for
TDmode. An appropriate test of a noncanonical zero that goes through this
case should thus be added to the testsuite.
> + }
> + else if ((t & (3 << 12)) != (3 << 12))
> + {
> + mantissahi &= ((((UDItype) 1) << 47) - 1);
> + mantissahi |= ((UDItype) 1) << 49;
> + exponent = t & 0x3fff;
> + if (mantissahi > (UDItype) 0x1ed09bead87c0
> + || (mantissahi == (UDItype) 0x1ed09bead87c0
> + && mantissalo > 0x378d8e63ffffffff))
> + {
> + mantissahi = 0;
> + mantissalo = 0;
> + }
And in this case, overflow is guaranteed; the check for the overflow
threshold should thus move to the previous case.
This patch is OK with these fixes.
Note for powerpc architecture maintainers: adding _BitInt support on that
architecture will mean, as well as adding support for the conversions
to/from DPD (if S/390 doesn't get there first), also adding support for
conversions to/from IBM long double.
--
Joseph S. Myers
[email protected]
