diff mbox

[3/8] float128: Add wrappers for IEEE functions.

Message ID 20161207173544.36123848@keller.br.ibm.com
State Superseded
Headers show

Commit Message

Gabriel F. T. Gomes Dec. 7, 2016, 7:35 p.m. UTC 
On Mon, 5 Dec 2016 18:50:39 +0000
Joseph Myers <joseph@codesourcery.com> wrote:

> On Mon, 5 Dec 2016, Gabriel F. T. Gomes wrote:

> 

> > On Wed, 9 Nov 2016 21:38:07 +0000

> > Joseph Myers <joseph@codesourcery.com> wrote:

> >   

> > > On Wed, 9 Nov 2016, Gabriel F. T. Gomes wrote:

> > >   

> > > I don't think float128-specific wrappers like this are appropriate.  All 

> > > these wrappers should be type-generic.  If you put type-generic wrappers 

> > > as math/w_*_template.c, will the existing wrappers with matherr support 

> > > still take precedence for existing types?  

> > 

> > They won't.

> > 

> > The files generated for the functions listed in gen-libm-calls

> > (in <builddir>/math/) are always generated, regardless of the existence

> > of the same file in the source dir.  And the generated-files take

> > precedence over the files in the source dir.

> > 

> > Do you have any suggestions on how to proceed?  

> 

> Source files in sysdeps certainly take precedence; that's by design, so 

> that architectures can easily have their own implementations that override 

> the templates.  Are you saying that this only works for source files in 

> sysdeps and not for those in math/?


When building from sysdeps (e.g. sysdeps/ieee754/ldbl-opt/w_acosl.c),
the command that is used to build it, puts <build_dir>/math first in
the include search path:

gcc ../sysdeps/ieee754/ldbl-opt/w_acosl.c [...] -I../include
-I/home/gftg/build/glibc/math  -I/home/gftg/build/glibc
-I../sysdeps/unix/sysv/linux/powerpc/powerpc64le/fpu [...]

And that will make sysdeps files try to use the new, auto-generated
wrappers:

In file included from /home/gftg/build/glibc/math/w_acosl.c:2:0,
                 from ../sysdeps/ieee754/ldbl-opt/w_acosl.c:4:
./w_acos_template.c:1:2: error: #error 

Is that what you expected?
Is it OK to have <build_dir>/math be searched first for include files?

> If so, then the existing log1p and scalbln wrappers should just be made 

> into type-generic templates like I did with ilogb; there is nothing in 

> those wrappers using the _LIB_VERSION / matherr / __kernel_standard 

> functionality that we want to obsolete and so don't want to form part of 

> the interface to new functions.  For the rest of the existing wrappers, 

> this indicates more of the steps towards deprecation are needed as part of 

> adding the new wrappers.


For this first part.. this is independent of the other patches, right?
Is the attached patch in the lines of what you had in mind?
(I just wrote it for log1p, because I still don't know what to do about
scalbln (I think I can't use declare_mgen_alias because w_scalbln
doesn't create a strong alias).  Besides, I still don't know how to
deal with the IS_IN (libm) test in
sysdeps/ieee754/ldbl-64-128/w_scalblnl.c)

Gabriel

Comments

Joseph Myers Dec. 7, 2016, 9:47 p.m. UTC | #1 
On Wed, 7 Dec 2016, Gabriel F. T. Gomes wrote:

> > Source files in sysdeps certainly take precedence; that's by design, so 

> > that architectures can easily have their own implementations that override 

> > the templates.  Are you saying that this only works for source files in 

> > sysdeps and not for those in math/?

> 

> When building from sysdeps (e.g. sysdeps/ieee754/ldbl-opt/w_acosl.c),

> the command that is used to build it, puts <build_dir>/math first in

> the include search path:

> 

> gcc ../sysdeps/ieee754/ldbl-opt/w_acosl.c [...] -I../include

> -I/home/gftg/build/glibc/math  -I/home/gftg/build/glibc

> -I../sysdeps/unix/sysv/linux/powerpc/powerpc64le/fpu [...]

> 

> And that will make sysdeps files try to use the new, auto-generated

> wrappers:

> 

> In file included from /home/gftg/build/glibc/math/w_acosl.c:2:0,

>                  from ../sysdeps/ieee754/ldbl-opt/w_acosl.c:4:

> ./w_acos_template.c:1:2: error: #error 

> 

> Is that what you expected?


Well, the detailed design is Paul's; I don't know if he considered the 
case of wanting a non-type-generic file in math/ to override a 
type-generic one.

> Is it OK to have <build_dir>/math be searched first for include files?


It looks like there are two possible issues involved.

(a) Which .c file gets built if there aren't any in sysdeps?  Is it the 
one in math/ in the source directory, or the one generated in the build 
directory?

(b) When sysdeps files do a #include of w_* files, where do those 
#included files come from?

Your command line shows a problem with (b): because of the -I of the 
*top-level* build directory (not that of the math/ subdirectory), 
"#include <math/w_acosl.c>" wrongly includes the generated file.

In my previous comments I was thinking of (a).  But it doesn't really 
matter, as renaming the existing files like I suggested (and updating the 
#includes of them) would solve both issues.  (Moving them into 
sysdeps/generic without renaming would probably also do the trick, but 
that's uglier and I don't think it's really any easier since you still 
need to update the #includes wherever you move them.)

> For this first part.. this is independent of the other patches, right?

> Is the attached patch in the lines of what you had in mind?

> (I just wrote it for log1p, because I still don't know what to do about

> scalbln (I think I can't use declare_mgen_alias because w_scalbln

> doesn't create a strong alias).  Besides, I still don't know how to

> deal with the IS_IN (libm) test in

> sysdeps/ieee754/ldbl-64-128/w_scalblnl.c)


For scalbln, look at math/s_ldexp_template.c and the ldbl-opt s_ldexp* 
files for an example - the issues are pretty similar.

For log1p, I'd expect the template file to use -1 (or M_LIT (-1.0) in the 
islessequal call) rather than the double value -1.0.  And I'd hope that 
you can remove the ldbl-128ibm/w_log1pl.c and ldbl-64-128/w_log1pl.c files 
completely and the generated file will do the right thing automatically.

-- 
Joseph S. Myers
joseph@codesourcery.com
diff mbox

Patch

From d3a7d7240eb4cf15a878e5fd2bcd96d164b14c86 Mon Sep 17 00:00:00 2001
From: "Gabriel F. T. Gomes" <gftg@linux.vnet.ibm.com>
Date: Wed, 7 Dec 2016 16:19:11 -0200
Subject: [PATCH] Make w_log1p type-generic

This patch converts the wrapper log1p (which set errno directly rather
than doing anything with __kernel_standard) to use the type-generic
template machinery, in the same way that has been done for ilogb.

Tested for powerpc64le, s390, and x86_64.

2016-12-06  Gabriel F. T. Gomes  <gftg@linux.vnet.ibm.com>

	* math/Makefile (gen-libm-calls): Add w_log1pF.
	(libm-calls): Remove w_log1pF.
	* math/w_log1p.c: Remove file.
	* math/w_log1pf.c: Likewise.
	* math/w_log1pl.c: Likewise.
	* math/w_log1p_template.c: New file with type-generic
	implementation based on math/w_log1p.c.
	* sysdeps/ieee754/ldbl-128ibm/w_log1pl.c: Remove use of macro
	long_double_symbol.
	* sysdeps/ieee754/ldbl-64-128/w_log1pl.c: Likewise.
---
 math/Makefile                          |  5 +++--
 math/w_log1p.c                         | 41 ----------------------------------
 math/w_log1p_template.c                | 36 +++++++++++++++++++++++++++++
 math/w_log1pf.c                        | 36 -----------------------------
 math/w_log1pl.c                        | 36 -----------------------------
 sysdeps/ieee754/ldbl-128ibm/w_log1pl.c |  1 -
 sysdeps/ieee754/ldbl-64-128/w_log1pl.c |  1 -
 7 files changed, 39 insertions(+), 117 deletions(-)
 delete mode 100644 math/w_log1p.c
 create mode 100644 math/w_log1p_template.c
 delete mode 100644 math/w_log1pf.c
 delete mode 100644 math/w_log1pl.c

diff --git a/math/Makefile b/math/Makefile
index 848b093..50ce418 100644
--- a/math/Makefile
+++ b/math/Makefile
@@ -52,7 +52,8 @@  gen-libm-calls = cargF conjF cimagF crealF cabsF s_cacosF		  \
 		 k_casinhF s_csinhF k_casinhF s_csinhF s_catanhF s_catanF \
 		 s_ctanF s_ctanhF s_cexpF s_clogF s_cprojF s_csqrtF	  \
 		 s_cpowF s_clog10F s_fdimF s_nextdownF s_fmaxF s_fminF	  \
-		 s_nanF s_iseqsigF s_canonicalizeF w_ilogbF w_llogbF
+		 s_nanF s_iseqsigF s_canonicalizeF w_ilogbF w_llogbF	  \
+		 w_log1pF
 
 libm-calls =								  \
 	e_acosF e_acoshF e_asinF e_atan2F e_atanhF e_coshF e_expF e_fmodF \
@@ -61,7 +62,7 @@  libm-calls =								  \
 	e_ilogbF							  \
 	k_cosF k_sinF k_tanF s_asinhF s_atanF s_cbrtF			  \
 	s_ceilF s_cosF s_erfF s_expm1F s_fabsF				  \
-	s_floorF s_log1pF w_log1pF s_logbF				  \
+	s_floorF s_log1pF s_logbF				  \
 	s_nextafterF s_nexttowardF s_rintF s_scalblnF w_scalblnF	  \
 	s_significandF s_sinF s_tanF s_tanhF w_acosF w_acoshF w_asinF	  \
 	w_atan2F w_atanhF w_coshF w_expF w_exp2F w_exp10F w_fmodF	  \
diff --git a/math/w_log1p.c b/math/w_log1p.c
deleted file mode 100644
index 282c85c..0000000
--- a/math/w_log1p.c
+++ /dev/null
@@ -1,41 +0,0 @@ 
-/* Wrapper for __log1p that handles setting errno.
-   Copyright (C) 2015-2016 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#include <errno.h>
-#include <math.h>
-#include <math_private.h>
-
-double
-__w_log1p (double x)
-{
-  if (__glibc_unlikely (islessequal (x, -1.0)))
-    {
-      if (x == -1.0)
-	__set_errno (ERANGE);
-      else
-	__set_errno (EDOM);
-    }
-
-  return __log1p (x);
-}
-weak_alias (__w_log1p, log1p)
-
-#ifdef NO_LONG_DOUBLE
-strong_alias (__w_log1p, __log1pl)
-weak_alias (__w_log1p, log1pl)
-#endif
diff --git a/math/w_log1p_template.c b/math/w_log1p_template.c
new file mode 100644
index 0000000..bb91975
--- /dev/null
+++ b/math/w_log1p_template.c
@@ -0,0 +1,36 @@ 
+/* Wrapper for __log1p that handles setting errno.
+   Copyright (C) 2015-2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <math.h>
+#include <math_private.h>
+
+FLOAT
+M_DECL_FUNC (__w_log1p) (FLOAT x)
+{
+  if (__glibc_unlikely (islessequal (x, -1.0)))
+    {
+      if (x == -1.0)
+	__set_errno (ERANGE);
+      else
+	__set_errno (EDOM);
+    }
+
+  return M_SUF (__log1p) (x);
+}
+declare_mgen_alias (__w_log1p, log1p)
diff --git a/math/w_log1pf.c b/math/w_log1pf.c
deleted file mode 100644
index ed9992a..0000000
--- a/math/w_log1pf.c
+++ /dev/null
@@ -1,36 +0,0 @@ 
-/* Wrapper for __log1pf that handles setting errno.
-   Copyright (C) 2015-2016 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#include <errno.h>
-#include <math.h>
-#include <math_private.h>
-
-float
-__w_log1pf (float x)
-{
-  if (__glibc_unlikely (islessequal (x, -1.0f)))
-    {
-      if (x == -1.0f)
-	__set_errno (ERANGE);
-      else
-	__set_errno (EDOM);
-    }
-
-  return __log1pf (x);
-}
-weak_alias (__w_log1pf, log1pf)
diff --git a/math/w_log1pl.c b/math/w_log1pl.c
deleted file mode 100644
index 3478c1c..0000000
--- a/math/w_log1pl.c
+++ /dev/null
@@ -1,36 +0,0 @@ 
-/* Wrapper for __log1pl that handles setting errno.
-   Copyright (C) 2015-2016 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#include <errno.h>
-#include <math.h>
-#include <math_private.h>
-
-long double
-__w_log1pl (long double x)
-{
-  if (__glibc_unlikely (islessequal (x, -1.0L)))
-    {
-      if (x == -1.0L)
-	__set_errno (ERANGE);
-      else
-	__set_errno (EDOM);
-    }
-
-  return __log1pl (x);
-}
-weak_alias (__w_log1pl, log1pl)
diff --git a/sysdeps/ieee754/ldbl-128ibm/w_log1pl.c b/sysdeps/ieee754/ldbl-128ibm/w_log1pl.c
index 969fadc..375e768 100644
--- a/sysdeps/ieee754/ldbl-128ibm/w_log1pl.c
+++ b/sysdeps/ieee754/ldbl-128ibm/w_log1pl.c
@@ -20,4 +20,3 @@ 
 #undef weak_alias
 #define weak_alias(n,a)
 #include <math/w_log1pl.c>
-long_double_symbol (libm, __w_log1pl, log1pl);
diff --git a/sysdeps/ieee754/ldbl-64-128/w_log1pl.c b/sysdeps/ieee754/ldbl-64-128/w_log1pl.c
index 969fadc..375e768 100644
--- a/sysdeps/ieee754/ldbl-64-128/w_log1pl.c
+++ b/sysdeps/ieee754/ldbl-64-128/w_log1pl.c
@@ -20,4 +20,3 @@ 
 #undef weak_alias
 #define weak_alias(n,a)
 #include <math/w_log1pl.c>
-long_double_symbol (libm, __w_log1pl, log1pl);
-- 
2.4.11