From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.gentoo.org (woodpecker.gentoo.org [140.211.166.183]) by sourceware.org (Postfix) with ESMTP id B59943858C60 for ; Fri, 21 Jan 2022 22:44:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B59943858C60 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gentoo.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gentoo.org Received: by smtp.gentoo.org (Postfix, from userid 559) id 45BA134342D; Fri, 21 Jan 2022 22:44:35 +0000 (UTC) Date: Fri, 21 Jan 2022 17:44:37 -0500 From: Mike Frysinger To: C Howland Cc: newlib@sourceware.org, Paul Eggert Subject: Re: [PATCH] newlib: switch to autoconf long double wider macro Message-ID: Mail-Followup-To: C Howland , newlib@sourceware.org, Paul Eggert References: <20220121050411.23094-1-vapier@gentoo.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="xwq7y7EfR4GEHPAp" Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, KAM_LOTSOFHASH, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: newlib@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Newlib mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Jan 2022 22:44:39 -0000 --xwq7y7EfR4GEHPAp Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 21 Jan 2022 12:03, C Howland wrote: > > On Jan 21 07:39, Mike Frysinger wrote: > > > On 21 Jan 2022 12:27, Corinna Vinschen wrote: > > > > On Jan 21 00:04, Mike Frysinger wrote: > > > > > Now that we require a recent version of autoconf, we can rely on = this > > > > > macro existing. It has inverted semantics from the existing test= (it > > > > > looks for "is wider" instead of "is equal"), so we have to invert= the > > > > > check when creating our _LDBL_EQ_DBL. > > > > > > > > Looks right to me, please push. > > > > > > for posterity, i'll note that autoconf uses a different (more > > comprehensive) > > > testing method that ultimately arrives at a diff answer than what new= lib > > is > > > atm. for aarch64, _LDBL_EQ_DBL is now defined when it wasn't before. > > > > > > newlib today is doing: > > > #if DBL_MANT_DIG =3D=3D LDBL_MANT_DIG && \ > > > LDBL_MIN_EXP =3D=3D DBL_MIN_EXP && \ > > > LDBL_MAX_EXP =3D=3D DBL_MAX_EXP > > > #define _LDBL_EQ_DBL > > > #else > > > #error "LDBL !=3D DBL" > > > #endif > > > > > > while autoconf is doing: > > > (0 < ((DBL_MAX_EXP < LDBL_MAX_EXP) > > > + (DBL_MANT_DIG < LDBL_MANT_DIG) > > > - (LDBL_MAX_EXP < DBL_MAX_EXP) > > > - (LDBL_MANT_DIG < DBL_MANT_DIG))) > > > && (int) LDBL_EPSILON =3D=3D 0 > > > > Erm... that's kind of weird. The newlib expression doesn't look wrong. > > > > So, out of curiosity of somebody not being familiar with aarch64, > > why is that? >=20 > Major problem, as that result is wrong, as they are definitely not equal: > $ aarch64-none-elf-cpp -dM null.c | grep DBL | sort > #define __DBL_DECIMAL_DIG__ 17 > #define __DBL_DENORM_MIN__ > ((double)4.94065645841246544176568792868221372e-324L) > #define __DBL_DIG__ 15 > #define __DBL_EPSILON__ ((double)2.22044604925031308084726333618164062e-1= 6L) > #define __DBL_HAS_DENORM__ 1 > #define __DBL_HAS_INFINITY__ 1 > #define __DBL_HAS_QUIET_NAN__ 1 > #define __DBL_MANT_DIG__ 53 > #define __DBL_MAX_10_EXP__ 308 > #define __DBL_MAX__ ((double)1.79769313486231570814527423731704357e+308L) > #define __DBL_MAX_EXP__ 1024 > #define __DBL_MIN_10_EXP__ (-307) > #define __DBL_MIN__ ((double)2.22507385850720138309023271733240406e-308L) > #define __DBL_MIN_EXP__ (-1021) > #define __DBL_NORM_MAX__ > ((double)1.79769313486231570814527423731704357e+308L) > #define __LDBL_DECIMAL_DIG__ 36 > #define __LDBL_DENORM_MIN__ 6.47517511943802511092443895822764655e-4966L > #define __LDBL_DIG__ 33 > #define __LDBL_EPSILON__ 1.92592994438723585305597794258492732e-34L > #define __LDBL_HAS_DENORM__ 1 > #define __LDBL_HAS_INFINITY__ 1 > #define __LDBL_HAS_QUIET_NAN__ 1 > #define __LDBL_MANT_DIG__ 113 > #define __LDBL_MAX_10_EXP__ 4932 > #define __LDBL_MAX__ 1.18973149535723176508575932662800702e+4932L > #define __LDBL_MAX_EXP__ 16384 > #define __LDBL_MIN_10_EXP__ (-4931) > #define __LDBL_MIN__ 3.36210314311209350626267781732175260e-4932L > #define __LDBL_MIN_EXP__ (-16381) > #define __LDBL_NORM_MAX__ 1.18973149535723176508575932662800702e+4932L >=20 > I am confused by the patch. The opening statement says "Now that we > require a recent version of autoconf, we can rely on this macro existing." > But then it proceeds to define a test for it. If it already exists, why = is > a test being defined? (Or is it just meaning to say that there is now, a= ll > this time later, a macro name that has become a de-facto standard to use?) i don't know what test you're talking about. the configure.ac just calls t= he autoconf macro directly. it isn't doing it conditionally. the autoconf ma= cro is supposed to be testing the same thing we're testing. > And a question about the test. There are two main segments in it: >=20 > + long double const a[] =3D > + { > + 0.0L, DBL_MIN, DBL_MAX, DBL_EPSILON, > + LDBL_MIN, LDBL_MAX, LDBL_EPSILON > + }; > + long double > + f (long double x) > + { > + return ((x + (unsigned long int) 10) * (-1 / x) + a[0] > + + (x ? f (x) : 'c')); > + } > + > +int > +main () > +{ > +static int test_array [1 - 2 * !((0 < ((DBL_MAX_EXP < LDBL_MAX_EXP) > + + (DBL_MANT_DIG < LDBL_MANT_DIG) > + - (LDBL_MAX_EXP < DBL_MAX_EXP) > + - (LDBL_MANT_DIG < DBL_MANT_DIG))) > + && (int) LDBL_EPSILON =3D=3D 0 > + )]; > +test_array [0] =3D 0; > +return test_array [0]; > + > + ; > + return 0; > +} >=20 > What's the first part doing? Only the second part (main) is performing t= he > size comparison. > Lastly, I tried this check with my GCC 10.2.0 aarch64 and it does > compile. (I hand-made a file to test. I did not make a configure file to > run.) I then changed the test_array definition to make it compare LDBL > against LDBL (to force them to be equal) and the compilation does fail, as > expected. So then what's going on that for Mike aarch64 _LDBL_EQ_DBL is > now defined when it wasn't before? The answer cannot be both different a= nd > also still correct. It is critical that _LDBL_EQ_DBL only be defined when > they are indeed the same size, as that gates using the double routines as > being long double. > Now a secondary item: the (int) LDBL_EPSILON =3D=3D 0 term in the c= heck > is degenerate and does nothing. (By rule (from the standard) the cast to > int discards the fraction so it will always be 0 and it becomes ... && 1.) > (In a minor quibble I have to disagree with the statement that the new > check is more comprehensive. In a rough way of looking at it they are bo= th > checking 3 values. But as noted the one term in the new check does > nothing, so it really is only checking 2 values. I actually made the > original check and had a positive reason for checking both min and max > exponent--although at the moment I'm not remembering what that reason was= =2E) to be clear, this code is all from autoconf. questions about it would be b= est answered by the authors/maintainers of it. if there is a problem, it affec= ts many more projects than newlib. autoconf used to have the same (or pretty close) test as newlib before it w= as changed to what it is now back in 2004. http://git.savannah.gnu.org/cgit/autoconf.git/commit/?h=3D9a76187b8c387598f= 63a9349a281d8b70963654e let's loop in Paul. -mike --xwq7y7EfR4GEHPAp Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEuQK1JxMl+JKsJRrUQWM7n+g39YEFAmHrN1UACgkQQWM7n+g3 9YFevxAAh83QiObvnLGoDpZgpBI4dY8kNVooW6UEYIoHJIC3bhRkq5OiZyEnPaBS 2uFbEyiY9B0xNRPcxyDzidKDhzUBJ562/UsDrr3ndq//2+fddKzMFh3SE/Vkrm52 lioOCWi4gzUFU8gF+T3si0ByATmy7+Lwvrf4wUrRawZKX6q02R0jzhYmt9B4vR2T ro24FD3iOWBdzz8q5ROwMpafW99FEhPvD8EaeYFyZvBl5cKxzwnqa+GckFFKQs8B w/f3NqZglEfU9qbjvtGIqKQwaRZepJmootvHXLczzijJ7yYsJ3/vgT+XG0hEP5am XHfIll1kETgRI63wJFvB7pxhweLj8Ot5Qb2ZrFCzn2g57/pvDetYyBGnCUwqywAe xDMVkt6hgRQ910KGoMh6i19mV/7OuEZoVzqpiKN0b1U545CjSolh8mTvfZrSp8ud L+JXzON96ucf6u5bScjMCEjzollPeW5M3aJ3C58TyHeRyh54s17RqJkCJYwb4gcW SXL/ql1bjz8SigUWaGZJw6ltUHmWumYIz5d94pq9j05EoMw1Eg/pD0PsGYKT7OEb YJicf+XrTJ08P/uMqRa9w6oNZplcE1YR8oVEkP30wW2ulPgw0MCZg9v4+zebwlX6 k4C/27CN21bH9VO19efOPVJc2EftjbM86WsXxpqznO19GVrRelY= =yA6d -----END PGP SIGNATURE----- --xwq7y7EfR4GEHPAp--