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 jos...@codesourcery.com