[PATCH 0/3 V2] Honor --disable-decimal-float in PowerPC libgcc _Float128

[PATCH 0/3 V2] Honor --disable-decimal-float in PowerPC libgcc _Float128

[Michael Meissner]

I have broken the patches I submitted on Friday February 26th into 3 patches.
These patches allow us to build the libgcc library on PowerPC for VSX systems
and optionally enable/disable the Decimal support.  If Decimal support is
disabled, then the Float128 <-> Decimal conversions are not built.

I have done bootstraps on a little endian power9 system with each of the long
double variants (128-bit IBM, 128-bit IEEE, 64-bit) enabled, and there were no
regressions in each of the builds with the previous version.

In addition, I have built 2 cross compilers from my x86_64 system to little
endian PowerPC Linux.  One build enabled decimal support and one disabled the
decimal support.  On the build that disabled decimal support, I verified that
the _Float128 <-> Decimal conversions were not built.

Can I check these patches into the master branch for GCC 11?

The 3 patches are:

   #1: Fix __sprintfkf prototype to match between .h and .c files;
   #2: Eliminate including stdio.h in _sprintfkf.c; (and)
   #3: Disable building _Float128 <-> Decimal conversions if
       --disable-decimal-float.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

[PATCH 1/3 V2] Fix __sprintfkf prototype in libgcc.

[Michael Meissner]

[PATCH 1/3 V2] Fix __sprintfkf prototype in libgcc.

The prototype of __sprintfkf in _sprintfkf.h did not match the function in
_sprintfkf.c.  This patch fixes the prototype.  I also included the
_sprintfkf.h file in _sprintfkf.c to make sure the prototype is correct and to
eliminate a warning about declaring the function without a previous
declaration.

I have done bootstraps on a little endian power9 system with each of the long
double variants (128-bit IBM, 128-bit IEEE, 64-bit) enabled, and there were no
regressions in each of the builds with the previous version.

In addition, I have built 2 cross compilers from my x86_64 system to little
endian PowerPC Linux.  One build enabled decimal support and one disabled the
decimal support.  On the build that disabled decimal support, I verified that
the _Float128 <-> Decimal conversions were not built.

Can I check this patch into the master branch for GCC 11?

libgcc/
2021-03-01  Michael Meissner  <meissner@linux.ibm.com>

	* config/rs6000/_sprintfkf.h (__sprintfkf): Fix prototype to match
	the function.
	* config/rs6000/_sprintfkf.c: Include _sprintfkf.h.
---
 libgcc/config/rs6000/_sprintfkf.c | 1 +
 libgcc/config/rs6000/_sprintfkf.h | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libgcc/config/rs6000/_sprintfkf.c b/libgcc/config/rs6000/_sprintfkf.c
index a7fdfb483c9..2d624f14e25 100644
--- a/libgcc/config/rs6000/_sprintfkf.c
+++ b/libgcc/config/rs6000/_sprintfkf.c
@@ -28,6 +28,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #include <soft-fp.h>
 #include <quad-float128.h>
 #include <stdio.h>
+#include <_sprintfkf.h>
 
 /* This function must be built with IBM 128-bit as long double, so that we can
    access the strfroml function if do not have an IEEE 128-bit version, and if
diff --git a/libgcc/config/rs6000/_sprintfkf.h b/libgcc/config/rs6000/_sprintfkf.h
index 637d104c882..de9d7137f69 100644
--- a/libgcc/config/rs6000/_sprintfkf.h
+++ b/libgcc/config/rs6000/_sprintfkf.h
@@ -24,5 +24,4 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 /* Declaration of the conversion function to IEEE 128-bit floating point from
    string using snprintf.  */
 
-extern int __sprintfkf (char *restrict, const char *restrict, ...);
-
+extern int __sprintfkf (char *restrict, const char *restrict, _Float128);
-- 
2.22.0


-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

[PATCH 2/3 V2] Do not include stdio.h in libgcc's Decimal/Float128 conversions.

[Michael Meissner]

[PATCH 2/3 V2] Do not include stdio.h in libgcc's Decimal/Float128 conversions.

The _sprintfkf.c file was including stdio.h to get the definition of sprintf.
This patch modifies this so that stdio.h is not included in order to support
freestanding cross compilers that might not provide stdio.h.

I have done bootstraps on a little endian power9 system with each of the long
double variants (128-bit IBM, 128-bit IEEE, 64-bit) enabled, and there were no
regressions in each of the builds with the previous version.

In addition, I have built 2 cross compilers from my x86_64 system to little
endian PowerPC Linux.  One build enabled decimal support and one disabled the
decimal support.  On the build that disabled decimal support, I verified that
the _Float128 <-> Decimal conversions were not built.

Can I check this patch into the master branch for GCC 11?

libgcc/
2021-03-01  Michael Meissner  <meissner@linux.ibm.com>

	* config/rs6000/_sprintfkf.c: Do not include stdio.h.
	(__sprintfkf): Call __builtin_sprintf not sprintf.
---
 libgcc/config/rs6000/_sprintfkf.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/libgcc/config/rs6000/_sprintfkf.c b/libgcc/config/rs6000/_sprintfkf.c
index 2d624f14e25..9bbc26145ab 100644
--- a/libgcc/config/rs6000/_sprintfkf.c
+++ b/libgcc/config/rs6000/_sprintfkf.c
@@ -27,7 +27,6 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #include <stdlib.h>
 #include <soft-fp.h>
 #include <quad-float128.h>
-#include <stdio.h>
 #include <_sprintfkf.h>
 
 /* This function must be built with IBM 128-bit as long double, so that we can
@@ -42,7 +41,11 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
    If we are linked against an earlier library, we will have fake it by
    converting the value to long double, and using sprintf to do the conversion.
    This isn't ideal, as IEEE 128-bit has more exponent range than IBM
-   128-bit.  */
+   128-bit.
+
+   We use __builtin_sprintf so that we don't have to include stdio.h to define
+   sprintf.  Stdio.h might not be present for freestanding cross compilers that
+   do not need to include a library.  */
 
 extern int __sprintfieee128 (char *restrict, const char *restrict, ...)
   __attribute__ ((__weak__));
@@ -54,5 +57,5 @@ int __sprintfkf (char *restrict string,
   if (__sprintfieee128)
     return __sprintfieee128 (string, format, number);
 
-  return sprintf (string, format, (long double) number);
+  return __builtin_sprintf (string, format, (long double) number);
 }
-- 
2.22.0


-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

[PATCH 3/3 V2] Do not build Decimal/Float128 conversions if decimal is disabled.

[Michael Meissner]

[PATCH 3/3 V2] Do not build Decimal/Float128 conversions if decimal is disabled.

This patch suppresses building the Decimal <-> Float128 conversions if the user
used --disable-decimal-float when configuring GCC.

I have done bootstraps on a little endian power9 system with each of the long
double variants (128-bit IBM, 128-bit IEEE, 64-bit) enabled, and there were no
regressions in each of the builds with the previous version.

In addition, I have built 2 cross compilers from my x86_64 system to little
endian PowerPC Linux.  One build enabled decimal support and one disabled the
decimal support.  On the build that disabled decimal support, I verified that
the _Float128 <-> Decimal conversions were not built.

Can I check this patch into the master branch for GCC 11?

libgcc/
2021-03-01  Michael Meissner  <meissner@linux.ibm.com>

	* config.host (powerpc*-*-linux*): Add t-float128-dec if Decimal
	arithmetic is supported.
	* config/rs6000/t-float128: Add conditions to suppress building the
	Decimal <-> Float128 conversions if --disable-decimal-float.
	* config/rs6000/t-float128-dec: New file.
	* configure.ac (powerpc*-*-linux*): Record whether decimal arithmetic
	is supported.
	* configure: Regenerate.
---
 libgcc/config.host                  |  6 ++++++
 libgcc/config/rs6000/t-float128     |  4 ++++
 libgcc/config/rs6000/t-float128-dec |  4 ++++
 libgcc/configure                    | 21 ++++++++++++++++++++-
 libgcc/configure.ac                 |  7 +++++++
 5 files changed, 41 insertions(+), 1 deletion(-)
 create mode 100644 libgcc/config/rs6000/t-float128-dec

diff --git a/libgcc/config.host b/libgcc/config.host
index f808b61be70..4ab1952899f 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -1217,6 +1217,12 @@ powerpc*-*-linux*)
 	esac
 
 	if test $libgcc_cv_powerpc_float128 = yes; then
+		# Enable building the decimal/Float128 conversions if decimal
+		# arithmetic is supported in the compiler.
+		if test $libgcc_cv_powerpc_float128_dec = yes; then
+			tmake_file="${tmake_file} rs6000/t-float128-dec"
+		fi
+
 		tmake_file="${tmake_file} rs6000/t-float128"
 	fi
 
diff --git a/libgcc/config/rs6000/t-float128 b/libgcc/config/rs6000/t-float128
index 6fb1a3d871b..1d9a0a5e7d7 100644
--- a/libgcc/config/rs6000/t-float128
+++ b/libgcc/config/rs6000/t-float128
@@ -23,6 +23,9 @@ fp128_softfp_shared_obj	= $(addsuffix -sw_s$(objext),$(fp128_softfp_funcs))
 fp128_softfp_obj	= $(fp128_softfp_static_obj) $(fp128_softfp_shared_obj)
 
 # Decimal <-> _Float128 conversions
+# FP128_DECIMAL_CONVERT is set in t-float128-dec if decimal arithmetic is
+# supported.
+ifeq ($(FP128_DECIMAL_CONVERT),yes)
 fp128_dec_funcs		= _kf_to_sd _kf_to_dd _kf_to_td \
 			  _sd_to_kf _dd_to_kf _td_to_kf
 
@@ -33,6 +36,7 @@ fp128_decstr_funcs	= _strtokf _sprintfkf
 # Decimal <-> __ibm128 conversions
 ibm128_dec_funcs	= _tf_to_sd _tf_to_dd _tf_to_td \
 			  _sd_to_tf _dd_to_tf _td_to_tf
+endif
 
 # New functions for software emulation
 fp128_ppc_funcs		= floattikf floatuntikf fixkfti fixunskfti \
diff --git a/libgcc/config/rs6000/t-float128-dec b/libgcc/config/rs6000/t-float128-dec
new file mode 100644
index 00000000000..2873006bb36
--- /dev/null
+++ b/libgcc/config/rs6000/t-float128-dec
@@ -0,0 +1,4 @@
+# Enable building the Float128/Decimal conversion routines.  If decimal support
+# is not enabled, this makefile fragment is not included.
+
+FP128_DECIMAL_CONVERT	= yes
diff --git a/libgcc/configure b/libgcc/configure
index 78fc22a5784..23de3a3adfc 100755
--- a/libgcc/configure
+++ b/libgcc/configure
@@ -4913,7 +4913,7 @@ case "$host" in
     case "$enable_cet" in
       auto)
 	# Check if target supports multi-byte NOPs
-	# and if assembler supports CET insn.
+	# and if compiler and assembler support CET insn.
 	cet_save_CFLAGS="$CFLAGS"
 	CFLAGS="$CFLAGS -fcf-protection"
 	cat confdefs.h - <<_ACEOF >conftest.$ac_ext
@@ -5228,6 +5228,25 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libgcc_cv_powerpc_float128" >&5
 $as_echo "$libgcc_cv_powerpc_float128" >&6; }
 
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for PowerPC __float128 libraries with decimal support" >&5
+$as_echo_n "checking for PowerPC __float128 libraries with decimal support... " >&6; }
+if ${libgcc_cv_powerpc_float128_dec+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+_Float128 convert (_Decimal128 a) { return a; }
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+  libgcc_cv_powerpc_float128_dec=yes
+else
+  libgcc_cv_powerpc_float128_dec=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libgcc_cv_powerpc_float128_dec" >&5
+$as_echo "$libgcc_cv_powerpc_float128_dec" >&6; }
+
   CFLAGS="$CFLAGS -mpower9-vector -mfloat128-hardware"
   { $as_echo "$as_me:${as_lineno-$LINENO}: checking for PowerPC ISA 3.0 to build hardware __float128 libraries" >&5
 $as_echo_n "checking for PowerPC ISA 3.0 to build hardware __float128 libraries... " >&6; }
diff --git a/libgcc/configure.ac b/libgcc/configure.ac
index ed50c0e9b49..389dcad7701 100644
--- a/libgcc/configure.ac
+++ b/libgcc/configure.ac
@@ -435,6 +435,13 @@ powerpc*-*-linux*)
     [libgcc_cv_powerpc_float128=yes],
     [libgcc_cv_powerpc_float128=no])])
 
+  AC_CACHE_CHECK([for PowerPC __float128 libraries with decimal support],
+		 [libgcc_cv_powerpc_float128_dec],
+		 [AC_COMPILE_IFELSE(
+    [AC_LANG_SOURCE([_Float128 convert (_Decimal128 a) { return a; }])],
+    [libgcc_cv_powerpc_float128_dec=yes],
+    [libgcc_cv_powerpc_float128_dec=no])])
+
   CFLAGS="$CFLAGS -mpower9-vector -mfloat128-hardware"
   AC_CACHE_CHECK([for PowerPC ISA 3.0 to build hardware __float128 libraries],
 		 [libgcc_cv_powerpc_float128_hw],
-- 
2.22.0


-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

Re: [PATCH 1/3 V2] Fix __sprintfkf prototype in libgcc.

[Segher Boessenkool]

On Mon, Mar 01, 2021 at 12:17:34PM -0500, Michael Meissner wrote:
> The prototype of __sprintfkf in _sprintfkf.h did not match the function in
> _sprintfkf.c.  This patch fixes the prototype.  I also included the
> _sprintfkf.h file in _sprintfkf.c to make sure the prototype is correct and to
> eliminate a warning about declaring the function without a previous
> declaration.

> libgcc/
> 2021-03-01  Michael Meissner  <meissner@linux.ibm.com>
> 
> 	* config/rs6000/_sprintfkf.h (__sprintfkf): Fix prototype to match
> 	the function.
> 	* config/rs6000/_sprintfkf.c: Include _sprintfkf.h.

This is okay for trunk.  Thank you!


Segher

Re: [PATCH 2/3 V2] Do not include stdio.h in libgcc's Decimal/Float128 conversions.

[Segher Boessenkool]

On Mon, Mar 01, 2021 at 12:18:52PM -0500, Michael Meissner wrote:
> The _sprintfkf.c file was including stdio.h to get the definition of sprintf.

(declaration of)

> This patch modifies this so that stdio.h is not included in order to support
> freestanding cross compilers that might not provide stdio.h.

So the code cannot work at all there?  Will not link?

> +   We use __builtin_sprintf so that we don't have to include stdio.h to define
> +   sprintf.  Stdio.h might not be present for freestanding cross compilers that
> +   do not need to include a library.  */

"declare sprintf", and the function is called stdio.g (all lowercase).
It is often written <stdio.h>, which makes it easier to handle in
sentences.

> @@ -54,5 +57,5 @@ int __sprintfkf (char *restrict string,
>    if (__sprintfieee128)
>      return __sprintfieee128 (string, format, number);
>  
> -  return sprintf (string, format, (long double) number);
> +  return __builtin_sprintf (string, format, (long double) number);

sprintf as well as __builtin_sprintf do not exist exactly when <stdio.h>
does not (or are not guaranteed to exist, anyway).


Segher

Re: [PATCH 2/3 V2] Do not include stdio.h in libgcc's Decimal/Float128 conversions.

[Michael Meissner]

On Mon, Mar 01, 2021 at 05:15:44PM -0600, Segher Boessenkool wrote:
> On Mon, Mar 01, 2021 at 12:18:52PM -0500, Michael Meissner wrote:
> > The _sprintfkf.c file was including stdio.h to get the definition of sprintf.
> 
> (declaration of)
> 
> > This patch modifies this so that stdio.h is not included in order to support
> > freestanding cross compilers that might not provide stdio.h.
> 
> So the code cannot work at all there?  Will not link?
> 
> > +   We use __builtin_sprintf so that we don't have to include stdio.h to define
> > +   sprintf.  Stdio.h might not be present for freestanding cross compilers that
> > +   do not need to include a library.  */
> 
> "declare sprintf", and the function is called stdio.g (all lowercase).
> It is often written <stdio.h>, which makes it easier to handle in
> sentences.
> 
> > @@ -54,5 +57,5 @@ int __sprintfkf (char *restrict string,
> >    if (__sprintfieee128)
> >      return __sprintfieee128 (string, format, number);
> >  
> > -  return sprintf (string, format, (long double) number);
> > +  return __builtin_sprintf (string, format, (long double) number);
> 
> sprintf as well as __builtin_sprintf do not exist exactly when <stdio.h>
> does not (or are not guaranteed to exist, anyway).

There are 2 issues:

The first issue is whether you get an error when you are building a cross
compiler and the target you are using does not provide a stdio.h.  This is the
error that this patch fixes.  I tend to regard not being able to build the
toolchain are higher in priority than whether it works at runtime.

The second is whether you get an error at link time because there is not
sprintf or strtold functions.  For normal archive libraries, this would only be
an issue if you actually do the conversion.  I have my doubts whether there is
any extant code that wants to convert _Float128 to one of the Decimal types or
vice versa.

Note the second issue would affect x86_64 cross compilers as well, since they
use those two functions to do the _Float128/Decimal versions.

I am open to suggestions of how to move forward.  I think we have to have a way
to build cross compilers without decimal support (i.e. my third patch).
Secondarily, I think we should allow the compiler to be built if there is no
stdio.h, but the user did not disable decimal floating point.

I don't think rewriting the Decimal conversions not to use GLIBC is really
practical.  I certainly do not have the FP math skills to do this without
losing precision, etc.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

Re: [PATCH 2/3 V2] Do not include stdio.h in libgcc's Decimal/Float128 conversions.

[Segher Boessenkool]

On Tue, Mar 02, 2021 at 04:25:33PM -0500, Michael Meissner wrote:
> On Mon, Mar 01, 2021 at 05:15:44PM -0600, Segher Boessenkool wrote:
> > On Mon, Mar 01, 2021 at 12:18:52PM -0500, Michael Meissner wrote:
> > > The _sprintfkf.c file was including stdio.h to get the definition of sprintf.
> > 
> > (declaration of)
> > 
> > > This patch modifies this so that stdio.h is not included in order to support
> > > freestanding cross compilers that might not provide stdio.h.
> > 
> > So the code cannot work at all there?  Will not link?
> > 
> > > +   We use __builtin_sprintf so that we don't have to include stdio.h to define
> > > +   sprintf.  Stdio.h might not be present for freestanding cross compilers that
> > > +   do not need to include a library.  */
> > 
> > "declare sprintf", and the function is called stdio.g (all lowercase).
> > It is often written <stdio.h>, which makes it easier to handle in
> > sentences.
> > 
> > > @@ -54,5 +57,5 @@ int __sprintfkf (char *restrict string,
> > >    if (__sprintfieee128)
> > >      return __sprintfieee128 (string, format, number);
> > >  
> > > -  return sprintf (string, format, (long double) number);
> > > +  return __builtin_sprintf (string, format, (long double) number);
> > 
> > sprintf as well as __builtin_sprintf do not exist exactly when <stdio.h>
> > does not (or are not guaranteed to exist, anyway).
> 
> There are 2 issues:
> 
> The first issue is whether you get an error when you are building a cross
> compiler and the target you are using does not provide a stdio.h.  This is the
> error that this patch fixes.  I tend to regard not being able to build the
> toolchain are higher in priority than whether it works at runtime.

And I see both as basic requirements.  A patch that adds X should make X
work.

If you want to make decimal and/or QP float work only on 64-bit LE Linux
you should say so.  And in that case, that is certainly not acceptable
if it doesn't "sorry" at configure time already.

> The second is whether you get an error at link time because there is not
> sprintf or strtold functions.  For normal archive libraries, this would only be
> an issue if you actually do the conversion.  I have my doubts whether there is
> any extant code that wants to convert _Float128 to one of the Decimal types or
> vice versa.

So what are these patches for at all, again?

Anyway, if some conversion doesn't work (or cannot work correctly at
all, which is the same thing), you should simply disable the conversion
routines themselves (and then cascade that through the possible callers:just make them say "sorry, unimplemented").

> Note the second issue would affect x86_64 cross compilers as well, since they
> use those two functions to do the _Float128/Decimal versions.

Yes, it cannot work correctly at all there, either.

> I am open to suggestions of how to move forward.  I think we have to have a way
> to build cross compilers without decimal support (i.e. my third patch).
> Secondarily, I think we should allow the compiler to be built if there is no
> stdio.h, but the user did not disable decimal floating point.

Yes.  So just do not use it then.  Disable the feature that would use it.

> I don't think rewriting the Decimal conversions not to use GLIBC is really
> practical.

It is necessary if you want to support it on any other systems than the
one you use.


Segher

Re: [PATCH 2/3 V2] Do not include stdio.h in libgcc's Decimal/Float128 conversions.

[Michael Meissner]

On Tue, Mar 02, 2021 at 03:53:06PM -0600, Segher Boessenkool wrote:
> If you want to make decimal and/or QP float work only on 64-bit LE Linux
> you should say so.  And in that case, that is certainly not acceptable
> if it doesn't "sorry" at configure time already.

Well in general the only supported configuration for _Float128 is 64-bit LE
Linux, but this is more due to the infrastructure not supporting it.

If you want to support _Float128 on big endian, you need a GLIBC that provides
the necessary support.  As far as I know, GLIBC does not build the _Float128
support on big endian.

You also need to set the minimum CPU to power7, because _Float128 is passed in
Altivec registers.  I think some of the configure tests use the default cpu
used in building GCC (i.e. power4 if you don't use --with-cpu).

As we have discussed many times, on 32-bit BE, you cannot use hardware
_Float128 support on power9/power10 because there is no TImode in 32-bit.
Various machine independent parts of GCC require an integer type to be the same
size as basic types.  If somebody made TImode work, we can remove the
restriction and allow _Float128 to work on 32-bit.

With the current compiler on BE, if you use -mcpu=power7 or newer, it will
enable the _Float128/__float128 keywords, and generate the code.  But until
there is an expectation of library support, it won't work for the general
user.

> > The second is whether you get an error at link time because there is not
> > sprintf or strtold functions.  For normal archive libraries, this would only be
> > an issue if you actually do the conversion.  I have my doubts whether there is
> > any extant code that wants to convert _Float128 to one of the Decimal types or
> > vice versa.
> 
> So what are these patches for at all, again?

The whole reason for the patches is to provide the support when we flip the
default long double type from the IBM 128-bit type to the IEEE 128-bit type
that conversions between long double and the Decimal types will succeed.  I
suspect in real life it won't be an issue, since Decimal is not used that
much.  But it is more likely people will want to convert between binary long
double and Decimal128.

I missed the fact that it had hidden dependencies for cross compilers.  So I am
trying to fix this.

> Anyway, if some conversion doesn't work (or cannot work correctly at
> all, which is the same thing), you should simply disable the conversion
> routines themselves (and then cascade that through the possible callers:just make them say "sorry, unimplemented").
> 
> > Note the second issue would affect x86_64 cross compilers as well, since they
> > use those two functions to do the _Float128/Decimal versions.
> 
> Yes, it cannot work correctly at all there, either.

If you remember, the original versions of the patch would only work if you
configured the compiler to use GLIBC 2.32 or newer (such as using the
--with-advance-toolchain at14.0 option).  You did not like this because as you
pointed out, you might use a different GLIBC when linking.

So I wrote the current patch that uses weak references to see if we did link
with GLIBC 2.32.  If the library is present, we use the functions in that
library.  If not, we have to give an approximate answer.  I used the existing
IBM 128-bit support to do the conversion.

Given GLIBC 2.32 is the minimum version of GLIBC that supports IEEE 128-bit
long doubles, if you link against an earlier library, you will not get the
conversions from long double.  You will only get the conversion if you
explicitly use _Float128.

> > I am open to suggestions of how to move forward.  I think we have to have a way
> > to build cross compilers without decimal support (i.e. my third patch).
> > Secondarily, I think we should allow the compiler to be built if there is no
> > stdio.h, but the user did not disable decimal floating point.
> 
> Yes.  So just do not use it then.  Disable the feature that would use it.
>
> > I don't think rewriting the Decimal conversions not to use GLIBC is really
> > practical.
> 
> It is necessary if you want to support it on any other systems than the
> one you use.

Sure, but it is a massive amount of work.  And I don't have the necessary
skills to insure that the conversion process will not suffer from errors.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

Re: [PATCH 2/3 V2] Do not include stdio.h in libgcc's Decimal/Float128 conversions.

[Joseph Myers]

On Wed, 3 Mar 2021, Michael Meissner via Gcc-patches wrote:

> As we have discussed many times, on 32-bit BE, you cannot use hardware
> _Float128 support on power9/power10 because there is no TImode in 32-bit.
> Various machine independent parts of GCC require an integer type to be the same
> size as basic types.  If somebody made TImode work, we can remove the
> restriction and allow _Float128 to work on 32-bit.

I'm not sure exactly what the machine-independent requirement is, but 
_Float128 is supported for 32-bit x86, riscv, sparc and s390 at least.

There's no support for _Float128 in the 32-bit powerpc ABI, but I don't 
think there is anything architecture-independent preventing such support 
on 32-bit platforms where TImode is unsupported (i.e. not supported by the 
scalar_mode_supported_p hook).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 2/3 V2] Do not include stdio.h in libgcc's Decimal/Float128 conversions.

[Michael Meissner]

On Wed, Mar 03, 2021 at 11:33:52PM +0000, Joseph Myers wrote:
> On Wed, 3 Mar 2021, Michael Meissner via Gcc-patches wrote:
> 
> > As we have discussed many times, on 32-bit BE, you cannot use hardware
> > _Float128 support on power9/power10 because there is no TImode in 32-bit.
> > Various machine independent parts of GCC require an integer type to be the same
> > size as basic types.  If somebody made TImode work, we can remove the
> > restriction and allow _Float128 to work on 32-bit.
> 
> I'm not sure exactly what the machine-independent requirement is, but 
> _Float128 is supported for 32-bit x86, riscv, sparc and s390 at least.
> 
> There's no support for _Float128 in the 32-bit powerpc ABI, but I don't 
> think there is anything architecture-independent preventing such support 
> on 32-bit platforms where TImode is unsupported (i.e. not supported by the 
> scalar_mode_supported_p hook).

The problem is several parts of GCC insists that there be an integer MODE that
is the same size as the scalar MODE.  I recall it happens in moves (such as
during calls), but it could be other places as well.

In theory, it would be nice to get this fixed, but the practicality is you can
go down a lot of places to find the next bug and fix that.

Maybe there is somebody who has enough time and patience to fix all of the
places that demand that there be an integer type large enough to hold scalar
types like _Float128 and generate alternate code.

Or perhaps implement enough of TImode so that it doesn't cause issues.

But in order to do the work and make sure it does not cause issues for 32-bit
and hardware _Float128, you need to build on a big endian power9 system.  In
the compile farm there is gcc135, but it is run little endian.

BTW, during the initial development of MMA, I ran into the same issue for the
vector pair and vector quad types.  Each of the approaches that we took had
some serious issues.  What I did in the initial stages of development is add
dummy moves that would never succeed for 256 and 512-bit integers, and then use
partial integers to represent that these weren't quite real integers.  However,
even then we discovered various parts of the compiler would strip off the
partial integer type and use the full integer type, even though there was no
real support for that type.  Peter and Aaron eventually solved this by adding
support for opaque modes.

However, Float128 needs to be a scalar floating point mode, not an opaque mode.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

Re: [PATCH 2/3 V2] Do not include stdio.h in libgcc's Decimal/Float128 conversions.

[Segher Boessenkool]

Hi!

On Wed, Mar 03, 2021 at 02:12:56PM -0500, Michael Meissner wrote:
> On Tue, Mar 02, 2021 at 03:53:06PM -0600, Segher Boessenkool wrote:
> > If you want to make decimal and/or QP float work only on 64-bit LE Linux
> > you should say so.  And in that case, that is certainly not acceptable
> > if it doesn't "sorry" at configure time already.
> 
> Well in general the only supported configuration for _Float128 is 64-bit LE
> Linux, but this is more due to the infrastructure not supporting it.
> 
> If you want to support _Float128 on big endian, you need a GLIBC that provides
> the necessary support.  As far as I know, GLIBC does not build the _Float128
> support on big endian.

This isn't the point.  You make it harder to support in the future, for
no reason (except a little convenience now).

> You also need to set the minimum CPU to power7, because _Float128 is passed in
> Altivec registers.

Yes, another unnecessary big restriction.

> As we have discussed many times, on 32-bit BE, you cannot use hardware
> _Float128 support on power9/power10 because there is no TImode in 32-bit.

That is a) another thing that needs fixing, and b) you *can* have
floating point modes of a size that does not exist as integer mode.

> Various machine independent parts of GCC require an integer type to be the same
> size as basic types.

And now explain float80 and float96?

> With the current compiler on BE, if you use -mcpu=power7 or newer, it will
> enable the _Float128/__float128 keywords, and generate the code.  But until
> there is an expectation of library support, it won't work for the general
> user.

So?  That does not mean you can break it further!

> > > Note the second issue would affect x86_64 cross compilers as well, since they
> > > use those two functions to do the _Float128/Decimal versions.
> > 
> > Yes, it cannot work correctly at all there, either.
> 
> If you remember, the original versions of the patch would only work if you
> configured the compiler to use GLIBC 2.32 or newer (such as using the
> --with-advance-toolchain at14.0 option).  You did not like this because as you
> pointed out, you might use a different GLIBC when linking.

It has nothing to do with me liking or not liking a patch.  You are
piling on more and more dependencies, when we cannot have any.  *That*
is the problem.  And it is a *problem*.

If we have a maze of limited situations that do and don't work, testing
becomes impossible already, let alone all further support.


Segher