From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa1.mentor.iphmx.com (esa1.mentor.iphmx.com [68.232.129.153]) by sourceware.org (Postfix) with ESMTPS id 355203858D1E for ; Fri, 1 Sep 2023 21:48:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 355203858D1E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="6.02,221,1688457600"; d="scan'208";a="18002237" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa1.mentor.iphmx.com with ESMTP; 01 Sep 2023 13:48:26 -0800 IronPort-SDR: cHkvqEulgaYMT/13V2piCXRGV7fHQE7PXXSKv6ERXsQoq36MgyeVpMbNnZNDyLU7C2sFO5nH9r 3/kopiIG6+nmoKALttRoz/VuTZ/ee/Du9NWIPrhV1QjE/84zZrqRMzO167+yDPNT9Q6bFodxlj dTCGe9RfOZzULbSj0f8mVvXQo86VVxFXhAmNg3VxZSs3ESJvChfdZdPeRes82D2wTS0xOy3iFa /pGoFPZ6Q4pJpQG6M1CJJQnbM8ZZamq5i+YXsxg4mQjAsEGpkXpHXRhUUqQPpVidw9fBOCUPR3 qbo= Date: Fri, 1 Sep 2023 21:48:22 +0000 From: Joseph Myers To: Jakub Jelinek CC: Ian Lance Taylor , Subject: Re: [PATCH 9/12] libgcc _BitInt support [PR102989] In-Reply-To: Message-ID: <18ed8557-5c75-4f3-e9a9-3cba6ab0a5b1@codesourcery.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-15.mgc.mentorg.com (139.181.222.15) To svr-ies-mbx-10.mgc.mentorg.com (139.181.222.10) X-Spam-Status: No, score=-3104.6 required=5.0 tests=BAYES_00,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 joseph@codesourcery.com