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 newlib
> > is
> > > atm.  for aarch64, _LDBL_EQ_DBL is now defined when it wasn't before.
> > >
> > > newlib today is doing:
> > > #if DBL_MANT_DIG == LDBL_MANT_DIG && \
> > >     LDBL_MIN_EXP == DBL_MIN_EXP   && \
> > >     LDBL_MAX_EXP == DBL_MAX_EXP
> > >   #define _LDBL_EQ_DBL
> > >  #else
> > >   #error "LDBL != 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 == 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?
> 
> 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-16L)
> #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
> 
>      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, all
> 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 the
autoconf macro directly.  it isn't doing it conditionally.  the autoconf macro
is supposed to be testing the same thing we're testing.

>      And a question about the test.  There are two main segments in it:
> 
> +           long double const a[] =
> +             {
> +                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 == 0
> +         )];
> +test_array [0] = 0;
> +return test_array [0];
> +
> +  ;
> +  return 0;
> +}
> 
> What's the first part doing?  Only the second part (main) is performing the
> 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 and
> 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 == 0 term in the check
> 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 both
> 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.)

to be clear, this code is all from autoconf.  questions about it would be best
answered by the authors/maintainers of it.  if there is a problem, it affects
many more projects than newlib.

autoconf used to have the same (or pretty close) test as newlib before it was
changed to what it is now back in 2004.
http://git.savannah.gnu.org/cgit/autoconf.git/commit/?h=9a76187b8c387598f63a9349a281d8b70963654e

let's loop in Paul.
-mike