* [PATCH 2/3] powerpc: Add redirection for finitef128, isinf128, and isnanf128
2017-09-07 10:44 [PATCH 0/3] Fixes for -mfloat128 being enabled by default on gcc for power Gabriel F. T. Gomes
@ 2017-09-07 10:44 ` Gabriel F. T. Gomes
2017-09-07 14:39 ` Joseph Myers
2017-09-07 10:44 ` [PATCH 3/3] powerpc64le: Disable compiler support for float128 when reusing ldbl-128 Gabriel F. T. Gomes
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Gabriel F. T. Gomes @ 2017-09-07 10:44 UTC (permalink / raw)
To: libc-alpha
On powerpc64le, compiler support for float128 is not enabled by default
on gcc. To enable it, the flag -mfloat128 must be passed as a command
line option to the compiler. This means that only the few files that
actively have -mfloat128 passed as an argument get compiler support for
float128, whereas all other files don't.
When -mfloat128 becomes enabled by default on powerpc [1], all the files
that do not currently have compiler support for float128 enabled during
their compilation, will start to have it. This will lead to build
errors in some of these files.
This patch defines __finitef128 as __redirect___finitef128 in
sysdeps/powerpc/powerpc64/fpu/multiarch/s_finite.c, similarly to what's
done for the float, double, and long double versions of these functions,
to get rid of the build error. Likewise for isinff128 and isnanf128.
[1] https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01028.html
Tested for powerpc64le with patched [1] and unpatched gcc.
* sysdeps/powerpc/powerpc64/fpu/multiarch/s_finite.c
(__finitef128): Define to __redirect___finitef128.
* sysdeps/powerpc/powerpc64/fpu/multiarch/s_isinf.c
(__isinff128): Define to __redirect___isinff128.
* sysdeps/powerpc/powerpc64/fpu/multiarch/s_isnan.c
(__isnanf128): Define to __redirect___isnanf128.
---
sysdeps/powerpc/powerpc64/fpu/multiarch/s_finite.c | 1 +
sysdeps/powerpc/powerpc64/fpu/multiarch/s_isinf.c | 1 +
sysdeps/powerpc/powerpc64/fpu/multiarch/s_isnan.c | 1 +
3 files changed, 3 insertions(+)
diff --git a/sysdeps/powerpc/powerpc64/fpu/multiarch/s_finite.c b/sysdeps/powerpc/powerpc64/fpu/multiarch/s_finite.c
index a5ec36b72f..19fc36e32d 100644
--- a/sysdeps/powerpc/powerpc64/fpu/multiarch/s_finite.c
+++ b/sysdeps/powerpc/powerpc64/fpu/multiarch/s_finite.c
@@ -19,6 +19,7 @@
#define __finite __redirect___finite
#define __finitef __redirect___finitef
#define __finitel __redirect___finitel
+#define __finitef128 __redirect___finitef128
#include <math.h>
#include <math_ldbl_opt.h>
#include <shlib-compat.h>
diff --git a/sysdeps/powerpc/powerpc64/fpu/multiarch/s_isinf.c b/sysdeps/powerpc/powerpc64/fpu/multiarch/s_isinf.c
index 9c6789c7bd..18c23c5884 100644
--- a/sysdeps/powerpc/powerpc64/fpu/multiarch/s_isinf.c
+++ b/sysdeps/powerpc/powerpc64/fpu/multiarch/s_isinf.c
@@ -19,6 +19,7 @@
#define __isinf __redirect___isinf
#define __isinff __redirect___isinff
#define __isinfl __redirect___isinfl
+#define __isinff128 __redirect___isinff128
#include <math.h>
#include <math_ldbl_opt.h>
#include <shlib-compat.h>
diff --git a/sysdeps/powerpc/powerpc64/fpu/multiarch/s_isnan.c b/sysdeps/powerpc/powerpc64/fpu/multiarch/s_isnan.c
index 3cfe1793da..e193670b0e 100644
--- a/sysdeps/powerpc/powerpc64/fpu/multiarch/s_isnan.c
+++ b/sysdeps/powerpc/powerpc64/fpu/multiarch/s_isnan.c
@@ -19,6 +19,7 @@
#define __isnan __redirect___isnan
#define __isnanf __redirect___isnanf
#define __isnanl __redirect___isnanl
+#define __isnanf128 __redirect___isnanf128
#include <math.h>
#include <math_ldbl_opt.h>
#include <shlib-compat.h>
--
2.13.5
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 2/3] powerpc: Add redirection for finitef128, isinf128, and isnanf128
2017-09-07 10:44 ` [PATCH 2/3] powerpc: Add redirection for finitef128, isinf128, and isnanf128 Gabriel F. T. Gomes
@ 2017-09-07 14:39 ` Joseph Myers
2017-09-08 15:25 ` Gabriel F. T. Gomes
0 siblings, 1 reply; 9+ messages in thread
From: Joseph Myers @ 2017-09-07 14:39 UTC (permalink / raw)
To: Gabriel F. T. Gomes; +Cc: libc-alpha
On Thu, 7 Sep 2017, Gabriel F. T. Gomes wrote:
> This patch defines __finitef128 as __redirect___finitef128 in
> sysdeps/powerpc/powerpc64/fpu/multiarch/s_finite.c, similarly to what's
> done for the float, double, and long double versions of these functions,
> to get rid of the build error. Likewise for isinff128 and isnanf128.
What is the build error? Why should s_finite.c need to redirect
__finitef128, or to redirect __finitef or __finitel for that matter?
s_finitef.c manages to redirect only __finitef, not any of the other
versions, after all.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] powerpc: Add redirection for finitef128, isinf128, and isnanf128
2017-09-07 14:39 ` Joseph Myers
@ 2017-09-08 15:25 ` Gabriel F. T. Gomes
2017-09-08 16:24 ` Joseph Myers
0 siblings, 1 reply; 9+ messages in thread
From: Gabriel F. T. Gomes @ 2017-09-08 15:25 UTC (permalink / raw)
To: Joseph Myers; +Cc: libc-alpha
On Thu, 7 Sep 2017 14:39:42 +0000
Joseph Myers <joseph@codesourcery.com> wrote:
>On Thu, 7 Sep 2017, Gabriel F. T. Gomes wrote:
>
>> This patch defines __finitef128 as __redirect___finitef128 in
>> sysdeps/powerpc/powerpc64/fpu/multiarch/s_finite.c, similarly to what's
>> done for the float, double, and long double versions of these functions,
>> to get rid of the build error. Likewise for isinff128 and isnanf128.
>
>What is the build error? Why should s_finite.c need to redirect
>__finitef128, or to redirect __finitef or __finitel for that matter?
>s_finitef.c manages to redirect only __finitef, not any of the other
>versions, after all.
I'm sorry for not making this clear in the patch itself. I'll try to
explain what was my rationale and if that sounds OK, I'll add that to the
commit message and comments.
This is the build error:
In file included from <command-line>:0:0:
../include/math.h:29:15: error: ‘__finitef128’ undeclared here (not in a function); did you mean ‘__rintf128’?
hidden_proto (__finitef128)
^~~~~~~~~~~~
./../include/libc-symbols.h:518:27: note: in definition of macro ‘__hidden_proto’
extern thread __typeof (name) name __hidden_proto_hiddenattr (attrs);
^~~~
../include/math.h:29:1: note: in expansion of macro ‘hidden_proto’
hidden_proto (__finitef128)
^~~~~~~~~~~~
The output of the preprocessor has the following lines (snippets):
# 21 "../math/bits/mathcalls-helper-functions.h"
[...]
extern int __redirect___finite (double __value) __attribute__ ((__nothrow__ )) __attribute__ ((__const__));
[...]
extern int __redirect___finitef (float __value) __attribute__ ((__nothrow__ )) __attribute__ ((__const__));
[...]
extern int __redirect___finitel (long double __value) __attribute__ ((__nothrow__ )) __attribute__ ((__const__));
[...]
extern int __redirect___finitef128 (_Float128 __value) __attribute__ ((__nothrow__ )) __attribute__ ((__const__));
(from the multiple inclusions of mathcalls-helper-functions.h (see comment
about the differences between s_finite.c and s_finitef.c further down this
message))
But include/math.h declares hidden prototypes for them (snippet):
# 9 "../include/math.h" 2
[...]
extern __typeof (__redirect___finite) __redirect___finite __attribute__ ((visibility ("hidden")));
[...]
extern __typeof (__redirect___finitef) __redirect___finitef __attribute__ ((visibility ("hidden")));
[...]
extern __typeof (__redirect___finitel) __redirect___finitel __attribute__ ((visibility ("hidden")));
[...]
extern __typeof (__finitef128) __finitef128 __attribute__ ((visibility ("hidden")));
~~~~~~~~~~~
Here's the source of the error.
As for the difference between s_finite.c and s_finitef.c, that happens
because include/math.h includes math/math.h before creating the hidden
prototypes.
On top of that, math/math.h includes bits/mathcalls-helper-functions.h
(once for each floating-point type). However, the declarations in
bits/mathcalls-helper-functions.h are macroized using __MATHDECL_1.
__MATHDECL_1 receives the suffix (f, l, f128, or nothing) as a parameter
and expands the base name (e.g.: __finite).
When s_finite.c defines __finite as __redirect_finite, all declarations in
bits/mathcalls-helper-functions.h are also redefined. That doesn't happen
with s_finitef.c because it only defines __finitef, which does not match
with anything in bits/mathcalls-helper-functions.h.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 2/3] powerpc: Add redirection for finitef128, isinf128, and isnanf128
2017-09-08 15:25 ` Gabriel F. T. Gomes
@ 2017-09-08 16:24 ` Joseph Myers
0 siblings, 0 replies; 9+ messages in thread
From: Joseph Myers @ 2017-09-08 16:24 UTC (permalink / raw)
To: Gabriel F. T. Gomes; +Cc: libc-alpha
On Fri, 8 Sep 2017, Gabriel F. T. Gomes wrote:
> On top of that, math/math.h includes bits/mathcalls-helper-functions.h
> (once for each floating-point type). However, the declarations in
> bits/mathcalls-helper-functions.h are macroized using __MATHDECL_1.
> __MATHDECL_1 receives the suffix (f, l, f128, or nothing) as a parameter
> and expands the base name (e.g.: __finite).
>
> When s_finite.c defines __finite as __redirect_finite, all declarations in
> bits/mathcalls-helper-functions.h are also redefined. That doesn't happen
> with s_finitef.c because it only defines __finitef, which does not match
> with anything in bits/mathcalls-helper-functions.h.
I think that this key point - that the function name gets macro expanded
before the concatenation - needs explaining in comments in all three
affected source files to justify why they are defining the macros for
other types.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] powerpc64le: Disable compiler support for float128 when reusing ldbl-128
2017-09-07 10:44 [PATCH 0/3] Fixes for -mfloat128 being enabled by default on gcc for power Gabriel F. T. Gomes
2017-09-07 10:44 ` [PATCH 2/3] powerpc: Add redirection for finitef128, isinf128, and isnanf128 Gabriel F. T. Gomes
@ 2017-09-07 10:44 ` Gabriel F. T. Gomes
2017-09-07 14:45 ` Joseph Myers
2017-09-07 10:44 ` [PATCH 1/3] powerpc: Add -mfloat128 to tst-strtod-nan-locale testcase Gabriel F. T. Gomes
2017-09-26 22:05 ` [PATCH 0/3] Fixes for -mfloat128 being enabled by default on gcc for power Joseph Myers
3 siblings, 1 reply; 9+ messages in thread
From: Gabriel F. T. Gomes @ 2017-09-07 10:44 UTC (permalink / raw)
To: libc-alpha
Some files under sysdeps/ieee754/ldbl-128ibm are able to reuse the
implementation in sysdeps/ieee754/ldbl-128 by defining _Float128 to long
double. This relies on compiler support for _Float128 being disabled.
On powerpc, such support is currently disabled by default, however, it
will get enabled by default [1], requiring that this Makefile actively
disable compiler support for float128 for the required files.
[1] https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01028.html
Tested for powerpc64le with patched [1] and unpatched gcc.
* sysdeps/powerpc/powerpc64le/Makefile (CFLAGS-e_expl.c)
(CFLAGS-e_j0l.c, CFLAGS-e_j1l.c, CFLAGS-e_lgammal_r.c)
(CFLAGS-s_cbrtl.c): New variables.
---
sysdeps/powerpc/powerpc64le/Makefile | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/sysdeps/powerpc/powerpc64le/Makefile b/sysdeps/powerpc/powerpc64le/Makefile
index 3fd9d9a715..a1d37135e5 100644
--- a/sysdeps/powerpc/powerpc64le/Makefile
+++ b/sysdeps/powerpc/powerpc64le/Makefile
@@ -20,6 +20,19 @@ CFLAGS-test-math-issignaling.cc += -mfloat128
CFLAGS-test-math-iszero.cc += -mfloat128
$(objpfx)test-float128% $(objpfx)test-ifloat128% $(objpfx)test-math-iszero: \
gnulib-tests += $(f128-loader-link)
+
+# Some files under sysdeps/ieee754/ldbl-128ibm are able to reuse the
+# implementation in sysdeps/ieee754/ldbl-128 by defining _Float128 to
+# long double. This relies on compiler support for _Float128 being
+# disabled. On powerpc, such support used to be disabled by default,
+# however, it will get enabled by default [1], requiring that this
+# Makefile actively disable it for the following files.
+# [1] https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01028.html
+CFLAGS-e_expl.c += -mno-float128
+CFLAGS-e_j0l.c += -mno-float128
+CFLAGS-e_j1l.c += -mno-float128
+CFLAGS-e_lgammal_r.c += -mno-float128
+CFLAGS-s_cbrtl.c += -mno-float128
endif
# Append flags to string <-> _Float128 routines.
--
2.13.5
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] powerpc64le: Disable compiler support for float128 when reusing ldbl-128
2017-09-07 10:44 ` [PATCH 3/3] powerpc64le: Disable compiler support for float128 when reusing ldbl-128 Gabriel F. T. Gomes
@ 2017-09-07 14:45 ` Joseph Myers
0 siblings, 0 replies; 9+ messages in thread
From: Joseph Myers @ 2017-09-07 14:45 UTC (permalink / raw)
To: Gabriel F. T. Gomes; +Cc: libc-alpha
On Thu, 7 Sep 2017, Gabriel F. T. Gomes wrote:
> Some files under sysdeps/ieee754/ldbl-128ibm are able to reuse the
> implementation in sysdeps/ieee754/ldbl-128 by defining _Float128 to long
> double. This relies on compiler support for _Float128 being disabled.
>
> On powerpc, such support is currently disabled by default, however, it
> will get enabled by default [1], requiring that this Makefile actively
> disable compiler support for float128 for the required files.
I think it would be better to stop #include of ldbl-128 files from
ldbl-128ibm. That is, copy the relevant source files (in the case of
e_expl.c that's actually t_expl.h) to ldbl-128ibm, with _Float128
converted to long double and L() also expanded back to adding L suffixes.
And, in the e_lgammal_r.c case, then eliminate the present LDBL_MANT_DIG
conditionals from both copies.
Long term I'd like the ldbl-128 files to e.g. use f128 constant suffixes
directly (once we can assume GCC >= 7 to build glibc), with such changes
cleaning up the code but being incompatible with its use also for
ldbl-128ibm.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] powerpc: Add -mfloat128 to tst-strtod-nan-locale testcase
2017-09-07 10:44 [PATCH 0/3] Fixes for -mfloat128 being enabled by default on gcc for power Gabriel F. T. Gomes
2017-09-07 10:44 ` [PATCH 2/3] powerpc: Add redirection for finitef128, isinf128, and isnanf128 Gabriel F. T. Gomes
2017-09-07 10:44 ` [PATCH 3/3] powerpc64le: Disable compiler support for float128 when reusing ldbl-128 Gabriel F. T. Gomes
@ 2017-09-07 10:44 ` Gabriel F. T. Gomes
2017-09-26 22:05 ` [PATCH 0/3] Fixes for -mfloat128 being enabled by default on gcc for power Joseph Myers
3 siblings, 0 replies; 9+ messages in thread
From: Gabriel F. T. Gomes @ 2017-09-07 10:44 UTC (permalink / raw)
To: libc-alpha
On powerpc64le, not all files can have the flag -mfloat128 passed as an
option on the compile command, since that could conflict with other
flags, such as -mno-vsx. Each file that needs the flag, gets it through
a CFLAGS-filename variable on sysdeps/powerpc/powerpc64le/Makefile.
The test cases tst-strtod-nan-locale and tst-wcstod-nan-locale are
missing this flag.
Tested for powerpc64le.
* sysdeps/powerpc/powerpc64le/Makefile
(CFLAGS-tst-strtod-nan-locale.c): New variable.
(CFLAGS-tst-wcstod-nan-locale.c): New variable.
---
sysdeps/powerpc/powerpc64le/Makefile | 3 +++
1 file changed, 3 insertions(+)
diff --git a/sysdeps/powerpc/powerpc64le/Makefile b/sysdeps/powerpc/powerpc64le/Makefile
index dea2290736..3fd9d9a715 100644
--- a/sysdeps/powerpc/powerpc64le/Makefile
+++ b/sysdeps/powerpc/powerpc64le/Makefile
@@ -33,12 +33,15 @@ CFLAGS-bug-strtod.c += -mfloat128
CFLAGS-bug-strtod2.c += -mfloat128
CFLAGS-tst-strtod-round.c += -mfloat128
CFLAGS-tst-wcstod-round.c += -mfloat128
+CFLAGS-tst-strtod-nan-locale.c += -mfloat128
+CFLAGS-tst-wcstod-nan-locale.c += -mfloat128
CFLAGS-tst-strtod6.c += -mfloat128
CFLAGS-tst-strfrom.c += -mfloat128
CFLAGS-tst-strfrom-locale.c += -mfloat128
CFLAGS-strfrom-skeleton.c += -mfloat128
$(foreach test,bug-strtod bug-strtod2 bug-strtod2 tst-strtod-round \
tst-wcstod-round tst-strtod6 tst-strrom tst-strfrom-locale \
+tst-strtod-nan-locale tst-wcstod-nan-locale \
strfrom-skeleton,$(objpfx)$(test)): gnulib-tests += $(f128-loader-link)
# When building glibc with support for _Float128, the powers of ten tables in
--
2.13.5
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] Fixes for -mfloat128 being enabled by default on gcc for power
2017-09-07 10:44 [PATCH 0/3] Fixes for -mfloat128 being enabled by default on gcc for power Gabriel F. T. Gomes
` (2 preceding siblings ...)
2017-09-07 10:44 ` [PATCH 1/3] powerpc: Add -mfloat128 to tst-strtod-nan-locale testcase Gabriel F. T. Gomes
@ 2017-09-26 22:05 ` Joseph Myers
3 siblings, 0 replies; 9+ messages in thread
From: Joseph Myers @ 2017-09-26 22:05 UTC (permalink / raw)
To: Gabriel F. T. Gomes; +Cc: libc-alpha
On Thu, 7 Sep 2017, Gabriel F. T. Gomes wrote:
> Besides these fixes, I was working on a problem that I got with the
> build of math/test-tgmath3.c, but since the change to gcc has been
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82333> now filed for
powerpc64le ICE on that test, since I couldn't locate any bug you may
have filed for it.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 9+ messages in thread