| Message ID | 20191015190529.11559-27-gabriel@inconstante.net.br |
|---|---|
| State | New |
| Headers | show |
| Series | Add IEEE long double <-> string functions for powerpc64le | expand |
* Gabriel F. T. Gomes: > From: Tulio Magno Quites Machado Filho <tuliom@linux.ibm.com> > > On platforms where long double has IEEE binary128 format as a third > option (initially, only powerpc64le), many exported functions are > redirected to their __*ieee128 equivalents. This redirection is > provided by installed headers such as stdio-ldbl.h, and is supposed to > work correctly with user code. > > However, during the build of glibc, similar redirections are employed, > in internal headers such as include/stdio.h, in order to avoid extra PLT > entries. These redirections conflict with the redirections to > __*ieee128, and must be avoided during the build. This patch protects > the second redirections with a test for __LONG_DOUBLE_USES_FLOAT128. Why don't we see this problem with the existing dual ABIs for long double? > diff --git a/include/stdio.h b/include/stdio.h > index bea2066cd1..8ec801c989 100644 > --- a/include/stdio.h > +++ b/include/stdio.h > @@ -13,7 +13,10 @@ extern int __fcloseall (void) attribute_hidden; > extern int __snprintf (char *__restrict __s, size_t __maxlen, > const char *__restrict __format, ...) > __attribute__ ((__format__ (__printf__, 3, 4))); > +# if !defined __LONG_DOUBLE_USES_FLOAT128 \ > + || (defined __LONG_DOUBLE_USES_FLOAT128 && __LONG_DOUBLE_USES_FLOAT128 == 0) > libc_hidden_proto (__snprintf) > +# endif > extern int __vfscanf (FILE *__restrict __s, > const char *__restrict __format, > __gnuc_va_list __arg) This leads to a build failure on most architectures because __LONG_DOUBLE_USES_FLOAT128 is not defined at this point. (I tried the gabriel/powerpc-ieee128-printscan branch at commit 1c4f7fffc4f1cc186906dd47812e725e51bb036a.) Thanks, Florian
Hi, Florian, On Wed, 16 Oct 2019, Florian Weimer wrote: >Why don't we see this problem with the existing dual ABIs for long >double? That would be because, on the previous transition (from long double being the same as double to long double with some other, whatever format), the old format was the same as double, which meant that no files had to be compiled in the old mode (-mlong-double-64). The [at-the-time-]new compat symbols got created during the compilation of the double-typed functions, which could be built with -mlong-double-128. For the new ABI, we rely on a lot of builds with -mlong-double-128 and -mfloat128, which work fine without this patch. However, we need a couple of builds in -mabi=ieeelongdouble mode, and the use of this flag causes build errors such as these: In file included from ../sysdeps/ieee754/ldbl-128ibm-compat/ieee128-qefgcvt.c:20: ../include/stdio.h:174:1: error: asm declaration ignored due to conflict with previous rename [-Werror=pragmas] 174 | libc_hidden_proto (dprintf) | ^~~~~~~~~~~~~~~~~ ../include/stdio.h:178:1: error: asm declaration ignored due to conflict with previous rename [-Werror=pragmas] 178 | libc_hidden_proto (fprintf) | ^~~~~~~~~~~~~~~~~ ../include/stdio.h:179:1: error: asm declaration ignored due to conflict with previous rename [-Werror=pragmas] 179 | libc_hidden_proto (vfprintf) | ^~~~~~~~~~~~~~~~~ ../include/stdio.h:180:1: error: asm declaration ignored due to conflict with previous rename [-Werror=pragmas] 180 | libc_hidden_proto (sprintf) | ^~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors make[2]: *** [/home/gabriel/build/powerpc64le/glibc/sysd-rules:1663: /home/gabriel/build/powerpc64le/glibc/misc/ieee128-qefgcvt.os] Error 1 make[2]: Leaving directory '/home/gabriel/src/glibc/misc' When we started working on this effort, we went on a path that led to a lot more files being built with -mabi=ieeelongdouble, now it's just cvt* functions, really. So I'm updating this patch to touch less files. >> +# if !defined __LONG_DOUBLE_USES_FLOAT128 \ >> + || (defined __LONG_DOUBLE_USES_FLOAT128 && __LONG_DOUBLE_USES_FLOAT128 == 0) >> libc_hidden_proto (__snprintf) >> +# endif >> extern int __vfscanf (FILE *__restrict __s, >> const char *__restrict __format, >> __gnuc_va_list __arg) > >This leads to a build failure on most architectures because >__LONG_DOUBLE_USES_FLOAT128 is not defined at this point. > >(I tried the gabriel/powerpc-ieee128-printscan branch at commit >1c4f7fffc4f1cc186906dd47812e725e51bb036a.) I'm sorry. It is perfectly reasonable that I should have tested it on other platforms before sending this patchset. I'm fixing this and testing on more platforms using build-many-glibcs.py (using b-m-g sent me on a search for *other* problems with the patchset, so thanks for testing the branch, much appreciated :). The actual snippet that causes the problem is the following: >@@ -72,7 +75,8 @@ libc_hidden_proto (__isoc99_vfscanf) > Unfortunately, symbol redirection is not transitive, so the > __REDIRECT in the public header does not link up with the above > libc_hidden_proto. Bridge the gap with a macro. */ >-# if !__GLIBC_USE (DEPRECATED_SCANF) >+# if !__GLIBC_USE (DEPRECATED_SCANF) \ >+ && __LONG_DOUBLE_USES_FLOAT128 == 0 > # undef sscanf > # define sscanf __isoc99_sscanf > # endif It's missing the check for __LONG_DOUBLE_USES_FLOAT128 being defined. I have locally changed this to a check for the macro being defined, then for a value (which solves the problem according to b-m-g), but I'm wondering if I should switch to defining it to 0 (zero) on all other long-double.h files (for all platforms), thus avoiding the problems with undefined macros [1]. Do you have an opinion on that? I'll send a v2, soon. [1] https://sourceware.org/glibc/wiki/Wundef
* Gabriel F. T. Gomes: > It's missing the check for __LONG_DOUBLE_USES_FLOAT128 being defined. I > have locally changed this to a check for the macro being defined, then for > a value (which solves the problem according to b-m-g), but I'm wondering if > I should switch to defining it to 0 (zero) on all other long-double.h > files (for all platforms), thus avoiding the problems with undefined > macros [1]. Do you have an opinion on that? Defining it everywhere is currently what we prefer. We have many exceptions (SHARED, the __ASSUME_* macros), but they are supposed to bhe historic. Thanks, Florian
diff --git a/include/err.h b/include/err.h index 7c05cd1dbb..4bfd0f1769 100644 --- a/include/err.h +++ b/include/err.h @@ -12,12 +12,15 @@ __vwarn_internal (const char *format, __gnuc_va_list ap, # ifndef _ISOMAC +#if !defined __LONG_DOUBLE_USES_FLOAT128 \ + || (defined __LONG_DOUBLE_USES_FLOAT128 && __LONG_DOUBLE_USES_FLOAT128 == 0) libc_hidden_proto (warn) libc_hidden_proto (warnx) libc_hidden_proto (vwarn) libc_hidden_proto (vwarnx) libc_hidden_proto (verr) libc_hidden_proto (verrx) +# endif # endif /* !_ISOMAC */ #endif /* err.h */ diff --git a/include/stdio.h b/include/stdio.h index bea2066cd1..8ec801c989 100644 --- a/include/stdio.h +++ b/include/stdio.h @@ -13,7 +13,10 @@ extern int __fcloseall (void) attribute_hidden; extern int __snprintf (char *__restrict __s, size_t __maxlen, const char *__restrict __format, ...) __attribute__ ((__format__ (__printf__, 3, 4))); +# if !defined __LONG_DOUBLE_USES_FLOAT128 \ + || (defined __LONG_DOUBLE_USES_FLOAT128 && __LONG_DOUBLE_USES_FLOAT128 == 0) libc_hidden_proto (__snprintf) +# endif extern int __vfscanf (FILE *__restrict __s, const char *__restrict __format, __gnuc_va_list __arg) @@ -72,7 +75,8 @@ libc_hidden_proto (__isoc99_vfscanf) Unfortunately, symbol redirection is not transitive, so the __REDIRECT in the public header does not link up with the above libc_hidden_proto. Bridge the gap with a macro. */ -# if !__GLIBC_USE (DEPRECATED_SCANF) +# if !__GLIBC_USE (DEPRECATED_SCANF) \ + && __LONG_DOUBLE_USES_FLOAT128 == 0 # undef sscanf # define sscanf __isoc99_sscanf # endif @@ -150,7 +154,10 @@ libc_hidden_proto (__libc_readline_unlocked); extern const char *const _sys_errlist_internal[] attribute_hidden; extern int _sys_nerr_internal attribute_hidden; +#if !defined __LONG_DOUBLE_USES_FLOAT128 \ + || (defined __LONG_DOUBLE_USES_FLOAT128 && __LONG_DOUBLE_USES_FLOAT128 == 0) libc_hidden_proto (__asprintf) +#endif # if IS_IN (libc) extern FILE *_IO_new_fopen (const char*, const char*); # define fopen(fname, mode) _IO_new_fopen (fname, mode) @@ -171,13 +178,16 @@ extern int _IO_new_fgetpos (FILE *, __fpos_t *); # define fgetpos(fp, posp) _IO_new_fgetpos (fp, posp) # endif -libc_hidden_proto (dprintf) extern __typeof (dprintf) __dprintf __attribute__ ((__format__ (__printf__, 2, 3))); libc_hidden_proto (__dprintf) +#if !defined __LONG_DOUBLE_USES_FLOAT128 \ + || (defined __LONG_DOUBLE_USES_FLOAT128 && __LONG_DOUBLE_USES_FLOAT128 == 0) +libc_hidden_proto (dprintf) libc_hidden_proto (fprintf) libc_hidden_proto (vfprintf) libc_hidden_proto (sprintf) +#endif libc_hidden_proto (fwrite) libc_hidden_proto (perror) libc_hidden_proto (remove) diff --git a/include/sys/syslog.h b/include/sys/syslog.h index 89d3479ebc..e10c58f6ef 100644 --- a/include/sys/syslog.h +++ b/include/sys/syslog.h @@ -3,7 +3,12 @@ #include <misc/sys/syslog.h> #ifndef _ISOMAC +#include <bits/floatn.h> + +#if !defined __LONG_DOUBLE_USES_FLOAT128 \ + || (defined __LONG_DOUBLE_USES_FLOAT128 && __LONG_DOUBLE_USES_FLOAT128 == 0) libc_hidden_proto (syslog) +#endif /* __vsyslog_internal uses the same mode_flags bits as __v*printf_internal; see libio/libioP.h. */ diff --git a/include/wchar.h b/include/wchar.h index 2cb44954fc..4875c9e84c 100644 --- a/include/wchar.h +++ b/include/wchar.h @@ -114,7 +114,10 @@ libc_hidden_proto (fputws_unlocked) libc_hidden_proto (putwc_unlocked) libc_hidden_proto (putwc) +#if !defined __LONG_DOUBLE_USES_FLOAT128 \ + || (defined __LONG_DOUBLE_USES_FLOAT128 && __LONG_DOUBLE_USES_FLOAT128 == 0) libc_hidden_proto (vswscanf) +#endif libc_hidden_proto (mbrtowc) libc_hidden_proto (wcrtomb)
From: Tulio Magno Quites Machado Filho <tuliom@linux.ibm.com> On platforms where long double has IEEE binary128 format as a third option (initially, only powerpc64le), many exported functions are redirected to their __*ieee128 equivalents. This redirection is provided by installed headers such as stdio-ldbl.h, and is supposed to work correctly with user code. However, during the build of glibc, similar redirections are employed, in internal headers such as include/stdio.h, in order to avoid extra PLT entries. These redirections conflict with the redirections to __*ieee128, and must be avoided during the build. This patch protects the second redirections with a test for __LONG_DOUBLE_USES_FLOAT128. --- include/err.h | 3 +++ include/stdio.h | 14 ++++++++++++-- include/sys/syslog.h | 5 +++++ include/wchar.h | 3 +++ 4 files changed, 23 insertions(+), 2 deletions(-)