Re: [PATCH] ieeefp.c: Auto-detect _LDBL_EQ_DBL

On Thu, Sep 6, 2018 at 5:14 PM Craig Howland <[email protected]>
wrote:

> On 09/06/2018 12:18 AM, Keith Packard wrote:
> > Make configuring the library a bit simpler
> >
> > Signed-off-by: Keith Packard <[email protected]>
> > ---
> > This seems simpler than computing this value during configure, and
> > will make using meson easier in the future.
> >
> >   newlib/libc/include/ieeefp.h | 5 +++++
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/newlib/libc/include/ieeefp.h b/newlib/libc/include/ieeefp.h
> > index 2d6421a4c..71749ccc0 100644
> > --- a/newlib/libc/include/ieeefp.h
> > +++ b/newlib/libc/include/ieeefp.h
> > @@ -143,6 +143,11 @@ typedef union
> >
> >   #endif /* __IEEE_LITTLE_ENDIAN */
> >
> > +#if LDBL_MANT_DIG == DBL_MANT_DIG && LDBL_MIN_EXP == DBL_MIN_EXP && \
> > +    LDBL_MAX_EXP == DBL_MAX_EXP
> > +#define _LDBL_EQ_DBL
> > +#endif
> > +
> >   #ifndef _LDBL_EQ_DBL
> >
> >   #ifndef LDBL_MANT_DIG
>      Back when _LDBL_EQ_DBL was added, the reason for a configure-time
> test was
> that there were compiler options that choose long double size.  (See
> https://sourceware.org/ml/newlib/2009/msg00497.html)  GCC does have
> -mlong-double options which allow the size of long double to be changed,
> so the
> original reason for the decision still applies.  So unless some kind of
> multilib
> magic can be added to take care of this, the proposal is a non-starter.
>       However, even assuming that the configure-time problem can be
> solved, this
> patch by itself seems to be a bad idea because it is leaving the
> config-generated define in newlib.h.  I don't think the math routines all
> include include/ieeefp.h (they probably all get include/machine/ieeefp.h),
> so
> that the definitions could end up coming from different places.
> (Realistically
> they should never give a different result, but it is not good practice
> have a
> decision like this in 2 places.)  So to do something like this, it 1)
> needs a
> place where all files using it can get it (or all files using it would
> need to
> be sure they included ieeefp.h), and 2)  get rid of the configured part of
> it
> and the definition in newlib.h.  (That is, this patch is the start of a
> configuration simplification, but is incomplete.)
>       In addition, this could/would produce warnings on platforms without
> long
> double, so an added term would be needed if this bit of it is kept,
> something like:
>
> #if defined(LDBL_MANT_DIG) && LDBL_MANT_DIG == DBL_MANT_DIG && ...
>

There is some old thread on newlib and maybe gcc where I took a
swing at the same issue. I got stuck on the multilib variant issue and
decided I couldn't spend more time. I recall problems on architectures
like the sh where some variant has only single precision floating point.
I think a Coldfire variant also tripped it up.

It needs to vary on a per-multilib basis. I am not sure where the logic
goes for a probe like that would end up in a .h file so it could vary.

--joel

>
>
>
>                  Craig
>