[PATCH] ieeefp.c: Auto-detect _LDBL_EQ

Discussion:

[PATCH] ieeefp.c: Auto-detect _LDBL_EQ_DBL

Keith Packard

2018-09-06 04:18:29 UTC

Make configuring the library a bit simpler

Signed-off-by: Keith Packard <***@keithp.com>
---
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

--
2.19.0.rc2

Corinna Vinschen

2018-09-06 12:22:55 UTC

Permalink

Post by Keith Packard
Make configuring the library a bit simpler
---
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
--
2.19.0.rc2

I leave that one to Jeff.

Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer
Red Hat

Craig Howland

2018-09-06 22:13:54 UTC

Permalink

    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 && ...

                Craig

Joel Sherrill

2018-09-06 23:08:46 UTC

Permalink

Post by Craig Howland

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
#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

Post by Craig Howland
Craig

Keith Packard

2018-09-07 04:36:48 UTC

Permalink

Post by Joel Sherrill
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.

Because multilib installs only a single set of header files, I can't see
how this can work without having those header files define this based on
the compilation environment.

--
-keith

Keith Packard

2018-09-07 04:34:57 UTC

Permalink

This post might be inappropriate. Click to display it.