[COMMITTED] tile: define __NO_LONG_DOUBLE_MATH

Message ID 1448399819-2804-1-git-send-email-cmetcalf@ezchip.com
State Committed
Headers

Commit Message

Chris Metcalf Nov. 24, 2015, 9:16 p.m. UTC
  This avoids build failures in the tests, and matches what is in
bits/mathdef.h.

Update the libc and libm abilist files to include __finitel,
__isinfl, and __isnanl.
---
 ChangeLog                                                 | 14 ++++++++++++++
 sysdeps/tile/bits/mathdef.h                               |  6 ++++++
 sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist |  3 +++
 sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libm.abilist |  1 +
 sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist |  3 +++
 sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libm.abilist |  1 +
 sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist         |  3 +++
 sysdeps/unix/sysv/linux/tile/tilepro/libm.abilist         |  1 +
 8 files changed, 32 insertions(+)
  

Comments

Joseph Myers Nov. 24, 2015, 10:41 p.m. UTC | #1
On Tue, 24 Nov 2015, Chris Metcalf wrote:

> This avoids build failures in the tests, and matches what is in
> bits/mathdef.h.
> 
> Update the libc and libm abilist files to include __finitel,
> __isinfl, and __isnanl.

The abilist changes need more explanation.  Did you previously have ABI 
test failures for the baselines being incorrect?  How did such incorrect 
baselines arise?  Were the symbols in fact exported at version GLIBC_2.12 
in the binaries of that version that were the reason for starting symbols 
at that version rather than 2.15?  Unless those 2.12 binaries had these 
symbols, they should not be exported at version GLIBC_2.12.  You could add 
them to GLIBC_2.23 for tile, but really there should be no need for those 
symbols at all in the __NO_LONG_DOUBLE_MATH case so the only case for 
doing so would be consistency between architectures; it would be better 
for the preferred approach to be that these symbols aren't exported in 
that case, as with __issignalingl, with backwards compatibility only for 
existing such architectures that export them.  (The public interfaces 
finitel, isinfl, isnanl of course should be exported as they already were 
- although as legacy interfaces no new versions of them should be added 
for TS 18661-3 types, only versions such as __isnanf128 when actually 
needed.)
  
Chris Metcalf Nov. 25, 2015, 12:25 a.m. UTC | #2
On 11/24/2015 5:41 PM, Joseph Myers wrote:
> On Tue, 24 Nov 2015, Chris Metcalf wrote:
>
>> This avoids build failures in the tests, and matches what is in
>> bits/mathdef.h.
>>
>> Update the libc and libm abilist files to include __finitel,
>> __isinfl, and __isnanl.
> The abilist changes need more explanation.  Did you previously have ABI
> test failures for the baselines being incorrect?  How did such incorrect
> baselines arise?

No, there were no previous ABI test failures.  They were correct before, since without
__NO_LONG_DOUBLE_MATH, those functions are hidden (see include/math.h).
I added that define because I was getting new build failures on some signgam
tests without it, something like these:

https://sourceware.org/ml/libc-alpha/2015-11/msg00172.html

I noticed that every other platform defined that macro except hppa. And since
every platform except hppa seems to export __isinfl and friends, adding it to the
tile ABI list seemed like the right fix.  This fix is also due to my seeing references
to bug 19270 and a fix for hppa recently:

https://sourceware.org/bugzilla/show_bug.cgi?id=19270
https://sourceware.org/ml/libc-alpha/2015-11/msg00520.html

> Were the symbols in fact exported at version GLIBC_2.12
> in the binaries of that version that were the reason for starting symbols
> at that version rather than 2.15?  Unless those 2.12 binaries had these
> symbols, they should not be exported at version GLIBC_2.12.  You could add
> them to GLIBC_2.23 for tile, but really there should be no need for those
> symbols at all in the __NO_LONG_DOUBLE_MATH case so the only case for
> doing so would be consistency between architectures; it would be better
> for the preferred approach to be that these symbols aren't exported in
> that case, as with __issignalingl, with backwards compatibility only for
> existing such architectures that export them.  (The public interfaces
> finitel, isinfl, isnanl of course should be exported as they already were
> - although as legacy interfaces no new versions of them should be added
> for TS 18661-3 types, only versions such as __isnanf128 when actually
> needed.)

Yes, I agree that exporting them seems pointless, but every other architecture does
as well, so it seemed uncontroversial.  And I only chose 2.12 on the theory that they
should have been exported then.  I don't really care what version I export them as,
since there's really no way for userspace to generate a reference to them, since the
macrology in math/math.h can't generate a reference to them no matter whether
__NO_LONG_DOUBLE_MATH is defined or not.  Let me know if you have an idea for
what would be cleaner here and I am happy to make the change, but it does seem
like it should be a change for every platform without long double, not just tile.
Thanks!
  
Joseph Myers Nov. 25, 2015, 1:09 a.m. UTC | #3
On Tue, 24 Nov 2015, Chris Metcalf wrote:

> as well, so it seemed uncontroversial.  And I only chose 2.12 on the theory
> that they
> should have been exported then.  I don't really care what version I export

That's not how things work.  The version is always the version where the 
symbols were actually first exported, not where they "should" have been 
exported; you never add symbols to a past release version.  See exp2l in 
math/Versions or fallocate64 in sysdeps/unix/sysv/linux/<various 32-bit 
architectures>/Versions for examples.

> them as, since there's really no way for userspace to generate a 
> reference to them,

Which is why it would be optimal not to export them at all.

> what would be cleaner here and I am happy to make the change, but it does seem
> like it should be a change for every platform without long double, not just
> tile.

Well, you don't remove symbols from old versions either.  Optimally the 
default should be that they aren't exported, with existing no-long-double 
platforms that had the exports defining some macro in their sysdeps files 
(e.g. math_private.h) to cause them to be exported (and ideally causing 
them to be exported as compat symbols only, given that it's not possible 
for the headers to generate references to them).
  
Chris Metcalf Nov. 26, 2015, 4:15 a.m. UTC | #4
On 11/24/2015 5:41 PM, Joseph Myers wrote:
> You could add them to GLIBC_2.23 for tile

OK, I'm having trouble making this happen.  Adding an appropriate
clause to sysdeps/tile/Versions doesn't seem to work.  Doing small tests
with --version-script suggests that the linker honors the first entry
it finds in the version script, and math/Versions is put into the
concatenated Versions.v.i before sysdeps/tile/Versions.

So, am I missing something?  It seems like other platforms try to
do this and presumably it works, e.g. the sparc64 exp2l version.
I don't see how that works any more than what I was trying to do...
  
Andreas Schwab Nov. 26, 2015, 8:44 a.m. UTC | #5
Chris Metcalf <cmetcalf@ezchip.com> writes:

> OK, I'm having trouble making this happen.  Adding an appropriate
> clause to sysdeps/tile/Versions doesn't seem to work.  Doing small tests
> with --version-script suggests that the linker honors the first entry
> it finds in the version script, and math/Versions is put into the
> concatenated Versions.v.i before sysdeps/tile/Versions.

You would need to move the symbols out of the generic Versions file into
a more specific one that isn't seen by tile.

Andreas.
  
Joseph Myers Nov. 26, 2015, 4:13 p.m. UTC | #6
On Thu, 26 Nov 2015, Andreas Schwab wrote:

> Chris Metcalf <cmetcalf@ezchip.com> writes:
> 
> > OK, I'm having trouble making this happen.  Adding an appropriate
> > clause to sysdeps/tile/Versions doesn't seem to work.  Doing small tests
> > with --version-script suggests that the linker honors the first entry
> > it finds in the version script, and math/Versions is put into the
> > concatenated Versions.v.i before sysdeps/tile/Versions.
> 
> You would need to move the symbols out of the generic Versions file into
> a more specific one that isn't seen by tile.

Or do the proper fix I suggested in 
<https://sourceware.org/ml/libc-alpha/2015-11/msg00571.html> that avoids 
exporting them for tile at all.  Make the code creating __*l aliases do 
e.g.

# if defined LDBL_CLASSIFY_COMPAT && SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_23)
compat_symbol (libc, __isnan, __isnanl, GLIBC_2_0)
# endif

(in the case of __finitel you'll need to do that for libm as well as 
libc), then, for each architecture that already defines 
__NO_LONG_DOUBLE_MATH (before the tile patch) and so should have those 
aliases (arm coldfire microblaze mips32 nios2 sh, I think), define 
LDBL_CLASSIFY_COMPAT in its math_private.h.  That should also ensure that 
the hppa fix doesn't cause ABI problems by adding new exports of these 
functions for hppa.
  

Patch

diff --git a/ChangeLog b/ChangeLog
index 0c8e4b2ad469..ed638349d01e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@ 
+2015-11-24  Chris Metcalf  <cmetcalf@ezchip.com>
+
+	* sysdeps/tile/bits/mathdef.h (__NO_LONG_DOUBLE_MATH): Define.
+	* sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist: Add
+	__finitel, __isinfl, and __isnanl.
+	* sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist:
+	Likewise.
+	* sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist: Likewise.
+	* sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libm.abilist: Add
+	__finitel.
+	* sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libm.abilist:
+	Likewise.
+	* sysdeps/unix/sysv/linux/tile/tilepro/libm.abilist: Likewise.
+
 2015-11-24  Gleb Fotengauer-Malinovskiy  <glebfm@altlinux.org>
 
 	* malloc/memusage.c (me): Remove redundant getenv call.
diff --git a/sysdeps/tile/bits/mathdef.h b/sysdeps/tile/bits/mathdef.h
index afbf77c8c524..8f570aae5566 100644
--- a/sysdeps/tile/bits/mathdef.h
+++ b/sysdeps/tile/bits/mathdef.h
@@ -46,3 +46,9 @@  typedef double double_t;
 # endif
 
 #endif /* ISO C99 */
+
+#ifndef __NO_LONG_DOUBLE_MATH
+/* Signal that we do not really have a `long double'.  This disables the
+   declaration of all the `long double' function variants.  */
+# define __NO_LONG_DOUBLE_MATH	1
+#endif
diff --git a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist
index ffcc4a0a2b4a..152adb0b4deb 100644
--- a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist
@@ -182,6 +182,7 @@  GLIBC_2.12 __fgetws_chk F
 GLIBC_2.12 __fgetws_unlocked_chk F
 GLIBC_2.12 __finite F
 GLIBC_2.12 __finitef F
+GLIBC_2.12 __finitel F
 GLIBC_2.12 __flbf F
 GLIBC_2.12 __fork F
 GLIBC_2.12 __fpending F
@@ -227,9 +228,11 @@  GLIBC_2.12 __isdigit_l F
 GLIBC_2.12 __isgraph_l F
 GLIBC_2.12 __isinf F
 GLIBC_2.12 __isinff F
+GLIBC_2.12 __isinfl F
 GLIBC_2.12 __islower_l F
 GLIBC_2.12 __isnan F
 GLIBC_2.12 __isnanf F
+GLIBC_2.12 __isnanl F
 GLIBC_2.12 __isoc99_fscanf F
 GLIBC_2.12 __isoc99_fwscanf F
 GLIBC_2.12 __isoc99_scanf F
diff --git a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libm.abilist b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libm.abilist
index 18b8d00becc6..9ba58e25c372 100644
--- a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libm.abilist
+++ b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libm.abilist
@@ -5,6 +5,7 @@  GLIBC_2.12 __clog10f F
 GLIBC_2.12 __clog10l F
 GLIBC_2.12 __finite F
 GLIBC_2.12 __finitef F
+GLIBC_2.12 __finitel F
 GLIBC_2.12 __fpclassify F
 GLIBC_2.12 __fpclassifyf F
 GLIBC_2.12 __signbit F
diff --git a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist
index a66e8ec877bb..f8377a01c1e9 100644
--- a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist
@@ -182,6 +182,7 @@  GLIBC_2.12 __fgetws_chk F
 GLIBC_2.12 __fgetws_unlocked_chk F
 GLIBC_2.12 __finite F
 GLIBC_2.12 __finitef F
+GLIBC_2.12 __finitel F
 GLIBC_2.12 __flbf F
 GLIBC_2.12 __fork F
 GLIBC_2.12 __fpending F
@@ -227,9 +228,11 @@  GLIBC_2.12 __isdigit_l F
 GLIBC_2.12 __isgraph_l F
 GLIBC_2.12 __isinf F
 GLIBC_2.12 __isinff F
+GLIBC_2.12 __isinfl F
 GLIBC_2.12 __islower_l F
 GLIBC_2.12 __isnan F
 GLIBC_2.12 __isnanf F
+GLIBC_2.12 __isnanl F
 GLIBC_2.12 __isoc99_fscanf F
 GLIBC_2.12 __isoc99_fwscanf F
 GLIBC_2.12 __isoc99_scanf F
diff --git a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libm.abilist b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libm.abilist
index 18b8d00becc6..9ba58e25c372 100644
--- a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libm.abilist
+++ b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libm.abilist
@@ -5,6 +5,7 @@  GLIBC_2.12 __clog10f F
 GLIBC_2.12 __clog10l F
 GLIBC_2.12 __finite F
 GLIBC_2.12 __finitef F
+GLIBC_2.12 __finitel F
 GLIBC_2.12 __fpclassify F
 GLIBC_2.12 __fpclassifyf F
 GLIBC_2.12 __signbit F
diff --git a/sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist b/sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist
index ffcc4a0a2b4a..152adb0b4deb 100644
--- a/sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist
+++ b/sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist
@@ -182,6 +182,7 @@  GLIBC_2.12 __fgetws_chk F
 GLIBC_2.12 __fgetws_unlocked_chk F
 GLIBC_2.12 __finite F
 GLIBC_2.12 __finitef F
+GLIBC_2.12 __finitel F
 GLIBC_2.12 __flbf F
 GLIBC_2.12 __fork F
 GLIBC_2.12 __fpending F
@@ -227,9 +228,11 @@  GLIBC_2.12 __isdigit_l F
 GLIBC_2.12 __isgraph_l F
 GLIBC_2.12 __isinf F
 GLIBC_2.12 __isinff F
+GLIBC_2.12 __isinfl F
 GLIBC_2.12 __islower_l F
 GLIBC_2.12 __isnan F
 GLIBC_2.12 __isnanf F
+GLIBC_2.12 __isnanl F
 GLIBC_2.12 __isoc99_fscanf F
 GLIBC_2.12 __isoc99_fwscanf F
 GLIBC_2.12 __isoc99_scanf F
diff --git a/sysdeps/unix/sysv/linux/tile/tilepro/libm.abilist b/sysdeps/unix/sysv/linux/tile/tilepro/libm.abilist
index 18b8d00becc6..9ba58e25c372 100644
--- a/sysdeps/unix/sysv/linux/tile/tilepro/libm.abilist
+++ b/sysdeps/unix/sysv/linux/tile/tilepro/libm.abilist
@@ -5,6 +5,7 @@  GLIBC_2.12 __clog10f F
 GLIBC_2.12 __clog10l F
 GLIBC_2.12 __finite F
 GLIBC_2.12 __finitef F
+GLIBC_2.12 __finitel F
 GLIBC_2.12 __fpclassify F
 GLIBC_2.12 __fpclassifyf F
 GLIBC_2.12 __signbit F