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

["Keith Packard" <keithp at keithp dot com>]

Craig Howland <howland@LGSInnovations.com> writes:

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

Thanks for the history; I figured there was a good reason this was done
at configure time and not build time, but I couldn't think of what it
could be.

multilib is precisely the reason I want to have this detected at compile
time and not configure time. multilib installations do not have
per-variant include files, so if there is any difference in this value
across variants, then the application will not be able to know how its
particular library was compiled, only how the version which produced the
installed newlib.h file was compiled.

In a correctly configured multilib environment, the compiler options in
force for the application will direct the linking process to locate the
matching libraries, so having the application use the compile-time
definition of this value should cause it to match the selected library.

I think the current mechanism can only work in a non-multilib
environment where the application can rely on the newlib.h file
precisely matching the library it will use.

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

Yes, the autotools stuff would also need changes; right now I'm looking
for feedback on whether this change in the code makes sense. My
meson-based configuration bits do not define this value in newlib.h, so
it ends up only defined in ieeefp.h.

If there's a version of this patch which could make sense to upstream, I
could work on hacking the autotools files, but frankly those are really
hard to work with given the requirement for a specific version of
automake and autoconf which are no longer packaged for my distribution.

>   (Realistically they should never give a different result, but it is
> not good practice have a decision like this in 2 places.)

Yup, one version of the truth is always easier to keep synced than two :-)

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

To ensure that applications continue to be able to rely on the symbol
being set correctly, I think we would need to define this in a file
equivalent to newlib.h; I think sys/config.h could work?

> , 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.)

Yup, I don't have the ability to hack the autotools bits though.

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

Fortunately, cpp substitutes 0 for undefined names in these expressions
so this shouldn't be necessary.

-- 
-keith

Attachment: signature.asc
Description: PGP signature