[1/2] PR target/107299: Fix build issue when long double is IEEE 128-bit

  This patch is a repost of a patch:

| Date: Thu, 19 Jan 2023 11:37:27 -0500
| Subject: [PATCH] PR target/107299: Fix build issue when long double is IEEE 128-bit
| Message-ID: <Y8lxx+Jxfl1IkheJ@toto.the-meissners.org>

This patch updates the IEEE 128-bit types used in libgcc.

At the moment, we cannot build GCC when the target uses IEEE 128-bit long
doubles, such as building the compiler for a native Fedora 36 system.  The
build dies when it is trying to build the _mulkc3.c and _divkc3 modules.

This patch changes libgcc to use long double for the IEEE 128-bit base type if
long double is IEEE 128-bit, and it uses _Float128 otherwise.  The built-in
functions are adjusted to be the correct version based on the IEEE 128-bit base
type used.

While it is desirable to ultimately have __float128 and _Float128 use the same
internal type and mode within GCC, at present if you use the option
-mabi=ieeelongdouble, the __float128 type will use the long double type and not
the _Float128 type.  We get an internal compiler error if we combine the
signbitf128 built-in with a long double type.

I've gone through several iterations of trying to fix this within GCC, and
there are various problems that have come up.  I developed this alternative
patch that changes libgcc so that it does not tickle the issue.  I hope we can
fix the compiler at some point, but right now, this is preventing people on
Fedora 36 systems from building compilers where the default long double is IEEE
128-bit.

I have built a GCC compiler tool chain on the following platforms and there
were no regressions caused by these patches.

    *	Power10 little endian, IBM long double, --with-cpu=power10

    *	Power9 little endian, IBM long double, --with-cpu=power9

    *	Power8 big endian, IBM long double, --with-cpu=power8, both
	32-bit/64-bit tests.

In addition, I have built a GCC compiler tool chain on the following systems
with IEEE 128-bit long double as the default.  Comparing the test suite runs to
the runs for the toolchain with IBM long double as the default, I only get the
expected differences (C++ modules test fail on IEEE long double, 3 Fortran
tests pass on IEEE long double that fail on IBM long double, C test pr105334.c
fails, and C test fp128_conversions.c fails on power10):

    *	Power10 little endian, IEEE long double, --with-cpu=power10

    *	Power9 little endian, IEEE long double, --with-cpu=power9

Note, it is Friday February 3rd, and I will be on vacation from Tuesday
February 7th through Tuesday February 14th.

Can I check this change into the master branch?

2023-02-02   Michael Meissner  <meissner@linux.ibm.com>

	PR target/107299
	* config/rs6000/_divkc3.c (COPYSIGN): Use the correct built-in based on
	whether long double is IBM or IEEE.
	(INFINITY): Likewise.
	(FABS): Likewise.
	* config/rs6000/_mulkc3.c (COPYSIGN): Likewise.
	(INFINITY): Likewise.
	* config/rs6000/quad-float128.h (TF): Remove definition.
	(TFtype): Define to be long double or _Float128.
	(TCtype): Define to be _Complex long double or _Complex _Float128.
	* libgcc2.h (TFtype): Allow machine config files to override this.
	(TCtype): Likewise.
	* soft-fp/quad.h (TFtype): Likewise.
---
 libgcc/config/rs6000/_divkc3.c       |  8 ++++++++
 libgcc/config/rs6000/_mulkc3.c       |  7 +++++++
 libgcc/config/rs6000/quad-float128.h | 19 ++++++-------------
 libgcc/libgcc2.h                     |  4 ++++
 libgcc/soft-fp/quad.h                |  2 ++
 5 files changed, 27 insertions(+), 13 deletions(-)
  

Hi Mike,

on 2023/2/3 13:49, Michael Meissner wrote:
> This patch is a repost of a patch:
> 
> | Date: Thu, 19 Jan 2023 11:37:27 -0500
> | Subject: [PATCH] PR target/107299: Fix build issue when long double is IEEE 128-bit
> | Message-ID: <Y8lxx+Jxfl1IkheJ@toto.the-meissners.org>
> 
> This patch updates the IEEE 128-bit types used in libgcc.
> 
> At the moment, we cannot build GCC when the target uses IEEE 128-bit long
> doubles, such as building the compiler for a native Fedora 36 system.  The
> build dies when it is trying to build the _mulkc3.c and _divkc3 modules.
> 
> This patch changes libgcc to use long double for the IEEE 128-bit base type if
> long double is IEEE 128-bit, and it uses _Float128 otherwise.  The built-in
> functions are adjusted to be the correct version based on the IEEE 128-bit base
> type used.
> 
> While it is desirable to ultimately have __float128 and _Float128 use the same
> internal type and mode within GCC, at present if you use the option
> -mabi=ieeelongdouble, the __float128 type will use the long double type and not
> the _Float128 type.  We get an internal compiler error if we combine the
> signbitf128 built-in with a long double type.
> 
> I've gone through several iterations of trying to fix this within GCC, and
> there are various problems that have come up.  I developed this alternative
> patch that changes libgcc so that it does not tickle the issue.  I hope we can
> fix the compiler at some point, but right now, this is preventing people on
> Fedora 36 systems from building compilers where the default long double is IEEE
> 128-bit.

Thanks for working on this!  If updating libgcc source to workaround this issue
is the best option we can have at this moment, it's fine.  Comparing to one
previous proposal which removes the workaround in build_common_tree_nodes for
rs6000 KFmode, a bit concern on this one is that users can still meet the ICE
with a simple case like:

typedef float TFtype __attribute__((mode (TF)));

TFtype
test (TFtype t)
{
  return __builtin_copysignf128 (1.0q, t);
}

but I guess they would write this kind of code very rarely?

BR,
Kewen
  

On Wed, Feb 22, 2023 at 06:37:39PM +0800, Kewen.Lin wrote:
> Thanks for working on this!  If updating libgcc source to workaround this issue
> is the best option we can have at this moment, it's fine.

Thanks.  Yes, I agree that it does not fix the root issue.

> Comparing to one
> previous proposal which removes the workaround in build_common_tree_nodes for
> rs6000 KFmode, a bit concern on this one is that users can still meet the ICE
> with a simple case like:
> 
> typedef float TFtype __attribute__((mode (TF)));
> 
> TFtype
> test (TFtype t)
> {
>   return __builtin_copysignf128 (1.0q, t);
> }
> 
> but I guess they would write this kind of code very rarely?

I tend to think that it is better to consistantly use __float128/_Float128
types with the 'f128' functions, and use long double with the 'l'.  It would be
nice to fix the root cause (of __float128 and _Float128 not being the same type
within the compiler).

It is complicated by the fact that until C++2x, you can't use the _Float128
type.  You can use the __float128 and __ibm128 extensions, but you can't use
those extensions with _Complex.  This means for C++, you have to use the
__attrbibute__((mode)) to get to the complex type.  And due to the way I
initially implemented it, whether you use T{C,F}, K{C,F}, and I{C,F} depends on
the switches.

But without fixing that (which is fairly complex), I really want the master
branch fixed so you can build GCC with long double defaulting to IEEE 128-bit.
  

This is the most important patch.  It is needed to allow the boostrap to work
again when long double is IEEE 128-bit.

| Date: Fri, 3 Feb 2023 00:49:12 -0500
| From: Michael Meissner <meissner@linux.ibm.com>
| Subject: [PATCH 1/2] PR target/107299: Fix build issue when long double is IEEE 128-bit
| Message-ID: <Y9ygWHc7w3DeIr9O@toto.the-meissners.org>
  

Hi!

On Fri, Feb 03, 2023 at 12:49:12AM -0500, Michael Meissner wrote:
> This patch updates the IEEE 128-bit types used in libgcc.
> 
> At the moment, we cannot build GCC when the target uses IEEE 128-bit long
> doubles, such as building the compiler for a native Fedora 36 system.  The
> build dies when it is trying to build the _mulkc3.c and _divkc3 modules.
> 
> This patch changes libgcc to use long double for the IEEE 128-bit base type if
> long double is IEEE 128-bit, and it uses _Float128 otherwise.  The built-in
> functions are adjusted to be the correct version based on the IEEE 128-bit base
> type used.

Please make it much clearer (in the code as well as in the commit
message) that this is a workaround for problems elsewhere.  It
complicates already complicated things that should not be all that
complex in the first place :-(

It is not clear to me that this is good to have at all -- it causes new
non-trivial problems after all -- but you say it allows people to at
least bootstrap, in more cases than before.

So with comments like I said above: okay for trunk.  And not okay for
any backports.

Thanks,


Segher