Patchwork nldbl-compat.c: Include math.h before nldbl-compat.h.

login
register
mail settings
Submitter Zack Weinberg
Date March 11, 2018, 6:25 p.m.
Message ID <20180311182523.25975-1-zackw@panix.com>
Download mbox | patch
Permalink /patch/26279/
State Committed, archived
Headers show

Comments

Zack Weinberg - March 11, 2018, 6:25 p.m.
Jeff Law noticed that native PowerPC builds were broken by my having
made math_ldbl_opt.h not include math.h.  nldbl-compat.c formerly got
math.h via libioP.h and math_ldbl_opt.h, *without* __NO_LONG_DOUBLE_MATH;
after my change it got it via nldbl-compat.h *with* __NO_LONG_DOUBLE_MATH,
but __NO_LONG_DOUBLE_MATH mode is forbidden on hosts that define
__HAVE_DISTINCT_FLOAT128, so the build breaks.  I don't know why this
didn't come up in a build-many-glibcs cycle.

I pushed this as a quick fix, but there's a deeper problem: presumably
nldbl-compat.h defines __NO_LONG_DOUBLE_MATH for a reason, but if we
can't use __NO_LONG_DOUBLE_MATH on any architecture that supports
_Float128, then nldbl-compat.h needs to find some other way to achieve
its goal.  Or maybe it's vestigial and we could just stop doing it?
Do we still need __NO_LONG_DOUBLE_MATH at all?  I leave these questions
to people more versed in the intricacies of long double.

	* sysdeps/ieee754/ldbl-opt/nldbl-compat.c: Include math.h
	before nldbl-compat.h.
---
 sysdeps/ieee754/ldbl-opt/nldbl-compat.c | 1 +
 1 file changed, 1 insertion(+)
Joseph Myers - March 12, 2018, 12:47 p.m.
On Sun, 11 Mar 2018, Zack Weinberg wrote:

> Jeff Law noticed that native PowerPC builds were broken by my having
> made math_ldbl_opt.h not include math.h.  nldbl-compat.c formerly got
> math.h via libioP.h and math_ldbl_opt.h, *without* __NO_LONG_DOUBLE_MATH;
> after my change it got it via nldbl-compat.h *with* __NO_LONG_DOUBLE_MATH,
> but __NO_LONG_DOUBLE_MATH mode is forbidden on hosts that define
> __HAVE_DISTINCT_FLOAT128, so the build breaks.  I don't know why this
> didn't come up in a build-many-glibcs cycle.

It did, with GCC mainline (older GCC doesn't default to supporting 
_Float128 on powerpc64le but only supports it when certain options are 
passed).

https://sourceware.org/ml/libc-testresults/2018-q1/msg00447.html

sysdeps/powerpc/bits/floatn.h makes sure not to enable __HAVE_FLOAT128 and 
__HAVE_DISTINCT_FLOAT128 in the __NO_LONG_DOUBLE_MATH case, so you should 
only have a problem when nldbl-compat.h is included late (after 
bits/floatn.h is included) but other headers disliking that combination 
are included even later.

> I pushed this as a quick fix, but there's a deeper problem: presumably
> nldbl-compat.h defines __NO_LONG_DOUBLE_MATH for a reason, but if we

To disable long double function declarations, so that e.g. nldbl-sin.c can 
define sinl with the "wrong" type (but the type correct for the ABI for 
which it's being built).

The files such as nldbl-sin.c are for libnldbl_nonshared.a, which 
supposedly supports -mlong-double-64 with compilers not supporting asm 
redirection (if asm redirection is supported, it's used so that calls to 
sinl etc. call sin etc. in that case).  I suspect in fact this case hasn't 
worked for a long time - see 
<https://sourceware.org/ml/libc-alpha/2014-08/msg00497.html> with an 
incorrect patch attempting to fix a build failure with such a compiler but 
without actually resulting in correct declarations.  So there may be a 
case for eliminating libnldbl_nonshared.a, in which case this special 
__NO_LONG_DOUBLE_MATH override might not be needed (and it would become a 
more normal macro, only ever defined by bits/long-double.h).
Zack Weinberg - March 12, 2018, 1:29 p.m.
On Mon, Mar 12, 2018 at 8:47 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Sun, 11 Mar 2018, Zack Weinberg wrote:
>
>> Jeff Law noticed that native PowerPC builds were broken by my having
>> made math_ldbl_opt.h not include math.h.  nldbl-compat.c formerly got
>> math.h via libioP.h and math_ldbl_opt.h, *without* __NO_LONG_DOUBLE_MATH;
>> after my change it got it via nldbl-compat.h *with* __NO_LONG_DOUBLE_MATH,
>> but __NO_LONG_DOUBLE_MATH mode is forbidden on hosts that define
>> __HAVE_DISTINCT_FLOAT128, so the build breaks.  I don't know why this
>> didn't come up in a build-many-glibcs cycle.
>
> It did, with GCC mainline (older GCC doesn't default to supporting
> _Float128 on powerpc64le but only supports it when certain options are
> passed).

Oh, OK.  I used the defaults, which are still 7.3.

When you get around to it, it sure would be nice if your buildbot
would post links to logs for failing builds.  I didn't see that
message, but given only its contents I would have assumed it wasn't my
mistake.

>> I pushed this as a quick fix, but there's a deeper problem: presumably
>> nldbl-compat.h defines __NO_LONG_DOUBLE_MATH for a reason, but if we
>
> To disable long double function declarations, so that e.g. nldbl-sin.c can
> define sinl with the "wrong" type (but the type correct for the ABI for
> which it's being built).

Hmm.  I _think_ nldbl-compat.c doesn't need to define any names from
math.h.  So we should be OK with the status quo for now.

> The files such as nldbl-sin.c are for libnldbl_nonshared.a, which
> supposedly supports -mlong-double-64 with compilers not supporting asm
> redirection (if asm redirection is supported, it's used so that calls to
> sinl etc. call sin etc. in that case).  I suspect in fact this case hasn't
> worked for a long time

I would back an official policy change that starting with version N we
will only support compilers for which __REDIRECT can be implemented
_somehow_ (not necessarily just compilers for which the GNU definition
of __REDIRECT works, but that's the only definition we have right now,
patches welcome).  That would cut a great deal of questionable cruft
out of public headers.  It might mean we can't support that Compaq
compiler in the message you referenced, but I doubt anyone would mind
at this point.

zw
Joseph Myers - March 12, 2018, 4:16 p.m.
On Mon, 12 Mar 2018, Zack Weinberg wrote:

> When you get around to it, it sure would be nice if your buildbot
> would post links to logs for failing builds.  I didn't see that
> message, but given only its contents I would have assumed it wasn't my
> mistake.

The basic difficulty with build-many-glibcs.py bot-cycle providing logs, 
either attached or linked from mails, is that they are very big (over 4GB 
for a single complete run; the bot only keeps around the logs for the 
current and one previous run), so there's a problem of where to put them.  
Logs of just failing build steps should be smaller, but still not small.

Patch

diff --git a/sysdeps/ieee754/ldbl-opt/nldbl-compat.c b/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
index bf54090d4f..ffb5fabebe 100644
--- a/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
+++ b/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
@@ -20,6 +20,7 @@ 
 #include <stdarg.h>
 #include <stdio.h>
 #include <libioP.h>
+#include <math.h>
 #include <wchar.h>
 #include <printf.h>
 #include <monetary.h>