public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Repost of patches for solving the build on Fedora 36 problem
@ 2023-02-03  5:43 Michael Meissner
  2023-02-03  5:49 ` [PATCH 1/2] PR target/107299: Fix build issue when long double is IEEE 128-bit Michael Meissner
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Michael Meissner @ 2023-02-03  5:43 UTC (permalink / raw)
  To: gcc-patches, Michael Meissner, Segher Boessenkool, Kewen.Lin,
	David Edelsohn, Peter Bergner, Will Schmidt

I'm reposting these two patches that allow GCC to build on Fedora 36 just to be
clear which patches I'm talking about.  The issue is that if GCC is configured
with long double using the IEEE 128-bit representation, it currently cannot
build _mulkc3 and _divkc3 in libgcc.

Note, these patches do not solve the underlying problem of mixing _Float128 and
long double types and using built-in functions (i.e. calling a _Float128
built-in function with long double arguments when long double is IEEE 128-bit,
or vice versa calling a long double built-in function with _Float128
arguments).  But they do allow the compiler to build.

Note, it is the morning of February 3rd, and I will be off on vacation from
February 7th through February 14th.

The first patch changes libgcc so that it uses either _Float128 or long double
as the base IEEE 128-bit type, depending on whether long double uses the IBM
double-double representation, or the IEEE 128-bit representation.  And for the
complex type it uses _Complex _Float128 or _Complex long double.  The _mulkc3
and _divkc3 functions are adjusted to use the f128 built-in functions or the
long double built-in functions, based on the long double type.

The second patch improves how the compiler generates the call to _mulkc3 and
_divkc3.  I've discovered as I have tried to fix underlying problem with the
IEEE 128-bit floating point types, it breaks the calls for IEEE 128-bit complex
multiply and divide.  This patch uses a cleaner approach to generate these
calls, and it will work with the current setup, and with the various fixes that
I've attempted to do to fix the underlying problem.

-- 
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meissner@linux.ibm.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/2] PR target/107299: Fix build issue when long double is IEEE 128-bit
  2023-02-03  5:43 [PATCH 0/2] Repost of patches for solving the build on Fedora 36 problem Michael Meissner
@ 2023-02-03  5:49 ` Michael Meissner
  2023-02-22 10:37   ` Kewen.Lin
                     ` (2 more replies)
  2023-02-03  5:53 ` [PATCH 2/2] Rework 128-bit complex multiply and divide Michael Meissner
  2023-02-03  7:42 ` [PATCH 0/2] Repost of patches for solving the build on Fedora 36 problem Richard Biener
  2 siblings, 3 replies; 19+ messages in thread
From: Michael Meissner @ 2023-02-03  5:49 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, Segher Boessenkool, Kewen.Lin,
	David Edelsohn, Peter Bergner, Will Schmidt

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(-)

diff --git a/libgcc/config/rs6000/_divkc3.c b/libgcc/config/rs6000/_divkc3.c
index 9f52428cfa0..e3bb97c9cb7 100644
--- a/libgcc/config/rs6000/_divkc3.c
+++ b/libgcc/config/rs6000/_divkc3.c
@@ -26,9 +26,17 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #include "soft-fp.h"
 #include "quad-float128.h"
 
+#ifndef __LONG_DOUBLE_IEEE128__
 #define COPYSIGN(x,y) __builtin_copysignf128 (x, y)
 #define INFINITY __builtin_inff128 ()
 #define FABS __builtin_fabsf128
+
+#else
+#define COPYSIGN(x,y) __builtin_copysignl (x, y)
+#define INFINITY __builtin_infl ()
+#define FABS __builtin_fabsl
+#endif
+
 #define isnan __builtin_isnan
 #define isinf __builtin_isinf
 #define isfinite __builtin_isfinite
diff --git a/libgcc/config/rs6000/_mulkc3.c b/libgcc/config/rs6000/_mulkc3.c
index 299d8d147b0..3d98436d1d4 100644
--- a/libgcc/config/rs6000/_mulkc3.c
+++ b/libgcc/config/rs6000/_mulkc3.c
@@ -26,8 +26,15 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #include "soft-fp.h"
 #include "quad-float128.h"
 
+#ifndef __LONG_DOUBLE_IEEE128__
 #define COPYSIGN(x,y) __builtin_copysignf128 (x, y)
 #define INFINITY __builtin_inff128 ()
+
+#else
+#define COPYSIGN(x,y) __builtin_copysignl (x, y)
+#define INFINITY __builtin_infl ()
+#endif
+
 #define isnan __builtin_isnan
 #define isinf __builtin_isinf
 
diff --git a/libgcc/config/rs6000/quad-float128.h b/libgcc/config/rs6000/quad-float128.h
index 68bd9b97f23..3354110004c 100644
--- a/libgcc/config/rs6000/quad-float128.h
+++ b/libgcc/config/rs6000/quad-float128.h
@@ -27,21 +27,14 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-/* quad.h defines the TFtype type by:
-   typedef float TFtype __attribute__ ((mode (TF)));
-
-   This define forces it to use KFmode (aka, ieee 128-bit floating point).
-   However, when the compiler's default is changed so that long double is IEEE
-   128-bit floating point, we need to go back to using TFmode and TCmode.  */
-#ifndef __LONG_DOUBLE_IEEE128__
-#define TF KF
-
-/* We also need TCtype to represent complex ieee 128-bit float for
-   __mulkc3 and __divkc3.  */
-typedef __complex float TCtype __attribute__ ((mode (KC)));
+/* Override the types for IEEE 128-bit scalar and complex.  */
+#ifdef __LONG_DOUBLE_IEEE128__
+#define TFtype long double
+#define TCtype _Complex long double
 
 #else
-typedef __complex float TCtype __attribute__ ((mode (TC)));
+#define TFtype _Float128
+#define TCtype _Complex _Float128
 #endif
 
 /* Force the use of the VSX instruction set.  */
diff --git a/libgcc/libgcc2.h b/libgcc/libgcc2.h
index 6f42db7f0be..3ec9bbd8164 100644
--- a/libgcc/libgcc2.h
+++ b/libgcc/libgcc2.h
@@ -156,9 +156,13 @@ typedef		float XFtype	__attribute__ ((mode (XF)));
 typedef _Complex float XCtype	__attribute__ ((mode (XC)));
 #endif
 #if LIBGCC2_HAS_TF_MODE
+#ifndef TFtype
 typedef		float TFtype	__attribute__ ((mode (TF)));
+#endif
+#ifndef TCtype
 typedef _Complex float TCtype	__attribute__ ((mode (TC)));
 #endif
+#endif
 
 typedef int cmp_return_type __attribute__((mode (__libgcc_cmp_return__)));
 typedef int shift_count_type __attribute__((mode (__libgcc_shift_count__)));
diff --git a/libgcc/soft-fp/quad.h b/libgcc/soft-fp/quad.h
index 3889bb44f1f..71f87d36ba9 100644
--- a/libgcc/soft-fp/quad.h
+++ b/libgcc/soft-fp/quad.h
@@ -65,7 +65,9 @@
 #define _FP_HIGHBIT_DW_Q	\
   ((_FP_W_TYPE) 1 << (_FP_WFRACBITS_DW_Q - 1) % _FP_W_TYPE_SIZE)
 
+#ifndef TFtype
 typedef float TFtype __attribute__ ((mode (TF)));
+#endif
 
 #if _FP_W_TYPE_SIZE < 64
 
-- 
2.39.1


-- 
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meissner@linux.ibm.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 2/2] Rework 128-bit complex multiply and divide.
  2023-02-03  5:43 [PATCH 0/2] Repost of patches for solving the build on Fedora 36 problem Michael Meissner
  2023-02-03  5:49 ` [PATCH 1/2] PR target/107299: Fix build issue when long double is IEEE 128-bit Michael Meissner
@ 2023-02-03  5:53 ` Michael Meissner
  2023-02-22 10:13   ` Kewen.Lin
                     ` (2 more replies)
  2023-02-03  7:42 ` [PATCH 0/2] Repost of patches for solving the build on Fedora 36 problem Richard Biener
  2 siblings, 3 replies; 19+ messages in thread
From: Michael Meissner @ 2023-02-03  5:53 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, Segher Boessenkool, Kewen.Lin,
	David Edelsohn, Peter Bergner, Will Schmidt

This patch reworks how the complex multiply and divide built-in functions are
done.  Previously we created built-in declarations for doing long double complex
multiply and divide when long double is IEEE 128-bit.  The old code also did not
support __ibm128 complex multiply and divide if long double is IEEE 128-bit.

This patch was originally posted on December 13th, 2022:

| Date: Tue, 13 Dec 2022 01:21:06 -0500
| Subject: [PATCH V2] Rework 128-bit complex multiply and divide, PR target/107299
| Message-ID: <Y5gZ0o1nzCq9MmR9@toto.the-meissners.org>

In terms of history, I wrote the original code just as I was starting to test
GCC on systems where IEEE 128-bit long double was the default.  At the time, we
had not yet started mangling the built-in function names as a way to bridge
going from a system with 128-bit IBM long double to 128-bin IEEE long double.

The original code depends on there only being two 128-bit types invovled.  With
the next patch in this series, this assumption will no longer be true.  When
long double is IEEE 128-bit, there will be 2 IEEE 128-bit types (one for the
explicit __float128/_Float128 type and one for long double).

The problem is we cannot create two separate built-in functions that resolve to
the same name.  This is a requirement of add_builtin_function and the C front
end.  That means for the 3 possible modes (IFmode, KFmode, and TFmode), you can
only use 2 of them.

This code does not create the built-in declaration with the changed name.
Instead, it uses the TARGET_MANGLE_DECL_ASSEMBLER_NAME hook to change the name
before it is written out to the assembler file like it now does for all of the
other long double built-in functions.

When I wrote these patches, I discovered that __ibm128 complex multiply and
divide had originally not been supported if long double is IEEE 128-bit as it
would generate calls to __mulic3 and __divic3.  I added tests in the testsuite
to verify that the correct name (i.e. __multc3 and __divtc3) is used in this
case.

I had previously sent this patch out on November 1st.  Compared to that version,
this version no longer disables the special mapping when you are building
libgcc, as it turns out we don't need it.

I tested all 3 patchs for PR target/107299 on:

    1)	LE Power10 using --with-cpu=power10 --with-long-double-format=ieee
    2)	LE Power10 using --with-cpu=power10 --with-long-double-format=ibm
    3)	LE Power9  using --with-cpu=power9  --with-long-double-format=ibm
    4)	BE Power8  using --with-cpu=power8  --with-long-double-format=ibm

Once all 3 patches have been applied, we can once again build GCC when long
double is IEEE 128-bit.  There were no other regressions with these patches.
Can I check these patches into the trunk?

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

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

gcc/

	PR target/107299
	* config/rs6000/rs6000.cc (create_complex_muldiv): Delete.
	(init_float128_ieee): Delete code to switch complex multiply and divide
	for long double.
	(complex_multiply_builtin_code): New helper function.
	(complex_divide_builtin_code): Likewise.
	(rs6000_mangle_decl_assembler_name): Add support for mangling the name
	of complex 128-bit multiply and divide built-in functions.

gcc/testsuite/

	PR target/107299
	* gcc.target/powerpc/divic3-1.c: New test.
	* gcc.target/powerpc/divic3-2.c: Likewise.
	* gcc.target/powerpc/mulic3-1.c: Likewise.
	* gcc.target/powerpc/mulic3-2.c: Likewise.
---
 gcc/config/rs6000/rs6000.cc                 | 109 +++++++++++---------
 gcc/testsuite/gcc.target/powerpc/divic3-1.c |  18 ++++
 gcc/testsuite/gcc.target/powerpc/divic3-2.c |  17 +++
 gcc/testsuite/gcc.target/powerpc/mulic3-1.c |  18 ++++
 gcc/testsuite/gcc.target/powerpc/mulic3-2.c |  17 +++
 5 files changed, 132 insertions(+), 47 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/divic3-1.c
 create mode 100644 gcc/testsuite/gcc.target/powerpc/divic3-2.c
 create mode 100644 gcc/testsuite/gcc.target/powerpc/mulic3-1.c
 create mode 100644 gcc/testsuite/gcc.target/powerpc/mulic3-2.c

diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index 16ca3a31757..7e76c37fdab 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -11151,26 +11151,6 @@ init_float128_ibm (machine_mode mode)
     }
 }
 
-/* Create a decl for either complex long double multiply or complex long double
-   divide when long double is IEEE 128-bit floating point.  We can't use
-   __multc3 and __divtc3 because the original long double using IBM extended
-   double used those names.  The complex multiply/divide functions are encoded
-   as builtin functions with a complex result and 4 scalar inputs.  */
-
-static void
-create_complex_muldiv (const char *name, built_in_function fncode, tree fntype)
-{
-  tree fndecl = add_builtin_function (name, fntype, fncode, BUILT_IN_NORMAL,
-				      name, NULL_TREE);
-
-  set_builtin_decl (fncode, fndecl, true);
-
-  if (TARGET_DEBUG_BUILTIN)
-    fprintf (stderr, "create complex %s, fncode: %d\n", name, (int) fncode);
-
-  return;
-}
-
 /* Set up IEEE 128-bit floating point routines.  Use different names if the
    arguments can be passed in a vector register.  The historical PowerPC
    implementation of IEEE 128-bit floating point used _q_<op> for the names, so
@@ -11182,32 +11162,6 @@ init_float128_ieee (machine_mode mode)
 {
   if (FLOAT128_VECTOR_P (mode))
     {
-      static bool complex_muldiv_init_p = false;
-
-      /* Set up to call __mulkc3 and __divkc3 under -mabi=ieeelongdouble.  If
-	 we have clone or target attributes, this will be called a second
-	 time.  We want to create the built-in function only once.  */
-     if (mode == TFmode && TARGET_IEEEQUAD && !complex_muldiv_init_p)
-       {
-	 complex_muldiv_init_p = true;
-	 built_in_function fncode_mul =
-	   (built_in_function) (BUILT_IN_COMPLEX_MUL_MIN + TCmode
-				- MIN_MODE_COMPLEX_FLOAT);
-	 built_in_function fncode_div =
-	   (built_in_function) (BUILT_IN_COMPLEX_DIV_MIN + TCmode
-				- MIN_MODE_COMPLEX_FLOAT);
-
-	 tree fntype = build_function_type_list (complex_long_double_type_node,
-						 long_double_type_node,
-						 long_double_type_node,
-						 long_double_type_node,
-						 long_double_type_node,
-						 NULL_TREE);
-
-	 create_complex_muldiv ("__mulkc3", fncode_mul, fntype);
-	 create_complex_muldiv ("__divkc3", fncode_div, fntype);
-       }
-
       set_optab_libfunc (add_optab, mode, "__addkf3");
       set_optab_libfunc (sub_optab, mode, "__subkf3");
       set_optab_libfunc (neg_optab, mode, "__negkf2");
@@ -28225,6 +28179,25 @@ rs6000_starting_frame_offset (void)
   return RS6000_STARTING_FRAME_OFFSET;
 }
 \f
+/* Internal function to return the built-in function id for the complex
+   multiply operation for a given mode.  */
+
+static inline built_in_function
+complex_multiply_builtin_code (machine_mode mode)
+{
+  return (built_in_function) (BUILT_IN_COMPLEX_MUL_MIN + mode
+			      - MIN_MODE_COMPLEX_FLOAT);
+}
+
+/* Internal function to return the built-in function id for the complex divide
+   operation for a given mode.  */
+
+static inline built_in_function
+complex_divide_builtin_code (machine_mode mode)
+{
+  return (built_in_function) (BUILT_IN_COMPLEX_DIV_MIN + mode
+			      - MIN_MODE_COMPLEX_FLOAT);
+}
 
 /* On 64-bit Linux and Freebsd systems, possibly switch the long double library
    function names from <foo>l to <foo>f128 if the default long double type is
@@ -28243,11 +28216,53 @@ rs6000_starting_frame_offset (void)
    only do this transformation if the __float128 type is enabled.  This
    prevents us from doing the transformation on older 32-bit ports that might
    have enabled using IEEE 128-bit floating point as the default long double
-   type.  */
+   type.
+
+   We also use the TARGET_MANGLE_DECL_ASSEMBLER_NAME hook to change the
+   function names used for complex multiply and divide to the appropriate
+   names.  */
 
 static tree
 rs6000_mangle_decl_assembler_name (tree decl, tree id)
 {
+  /* Handle complex multiply/divide.  For IEEE 128-bit, use __mulkc3 or
+     __divkc3 and for IBM 128-bit use __multc3 and __divtc3.  */
+  if (TARGET_FLOAT128_TYPE
+      && TREE_CODE (decl) == FUNCTION_DECL
+      && DECL_IS_UNDECLARED_BUILTIN (decl)
+      && DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL)
+    {
+      built_in_function id = DECL_FUNCTION_CODE (decl);
+      const char *newname = NULL;
+
+      if (id == complex_multiply_builtin_code (KCmode))
+	newname = "__mulkc3";
+
+      else if (id == complex_multiply_builtin_code (ICmode))
+	newname = "__multc3";
+
+      else if (id == complex_multiply_builtin_code (TCmode))
+	newname = (TARGET_IEEEQUAD) ? "__mulkc3" : "__multc3";
+
+      else if (id == complex_divide_builtin_code (KCmode))
+	newname = "__divkc3";
+
+      else if (id == complex_divide_builtin_code (ICmode))
+	newname = "__divtc3";
+
+      else if (id == complex_divide_builtin_code (TCmode))
+	newname = (TARGET_IEEEQUAD) ? "__divkc3" : "__divtc3";
+
+      if (newname)
+	{
+	  if (TARGET_DEBUG_BUILTIN)
+	    fprintf (stderr, "Map complex mul/div => %s\n", newname);
+
+	  return get_identifier (newname);
+	}
+    }
+
+  /* Map long double built-in functions if long double is IEEE 128-bit.  */
   if (TARGET_FLOAT128_TYPE && TARGET_IEEEQUAD && TARGET_LONG_DOUBLE_128
       && TREE_CODE (decl) == FUNCTION_DECL
       && DECL_IS_UNDECLARED_BUILTIN (decl)
diff --git a/gcc/testsuite/gcc.target/powerpc/divic3-1.c b/gcc/testsuite/gcc.target/powerpc/divic3-1.c
new file mode 100644
index 00000000000..1cc6b1be904
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/divic3-1.c
@@ -0,0 +1,18 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-require-effective-target longdouble128 } */
+/* { dg-require-effective-target ppc_float128_sw } */
+/* { dg-options "-O2 -mpower8-vector -mabi=ieeelongdouble -Wno-psabi" } */
+
+/* Check that complex divide generates the right call for __ibm128 when long
+   double is IEEE 128-bit floating point.  */
+
+typedef _Complex long double c_ibm128_t __attribute__((mode(__IC__)));
+
+void
+divide (c_ibm128_t *p, c_ibm128_t *q, c_ibm128_t *r)
+{
+  *p = *q / *r;
+}
+
+/* { dg-final { scan-assembler "bl __divtc3" } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/divic3-2.c b/gcc/testsuite/gcc.target/powerpc/divic3-2.c
new file mode 100644
index 00000000000..8ff342e0116
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/divic3-2.c
@@ -0,0 +1,17 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-require-effective-target longdouble128 } */
+/* { dg-options "-O2 -mpower8-vector -mabi=ibmlongdouble -Wno-psabi" } */
+
+/* Check that complex divide generates the right call for __ibm128 when long
+   double is IBM 128-bit floating point.  */
+
+typedef _Complex long double c_ibm128_t __attribute__((mode(__TC__)));
+
+void
+divide (c_ibm128_t *p, c_ibm128_t *q, c_ibm128_t *r)
+{
+  *p = *q / *r;
+}
+
+/* { dg-final { scan-assembler "bl __divtc3" } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/mulic3-1.c b/gcc/testsuite/gcc.target/powerpc/mulic3-1.c
new file mode 100644
index 00000000000..4cd773c4b06
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/mulic3-1.c
@@ -0,0 +1,18 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-require-effective-target longdouble128 } */
+/* { dg-require-effective-target ppc_float128_sw } */
+/* { dg-options "-O2 -mpower8-vector -mabi=ieeelongdouble -Wno-psabi" } */
+
+/* Check that complex multiply generates the right call for __ibm128 when long
+   double is IEEE 128-bit floating point.  */
+
+typedef _Complex long double c_ibm128_t __attribute__((mode(__IC__)));
+
+void
+multiply (c_ibm128_t *p, c_ibm128_t *q, c_ibm128_t *r)
+{
+  *p = *q * *r;
+}
+
+/* { dg-final { scan-assembler "bl __multc3" } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/mulic3-2.c b/gcc/testsuite/gcc.target/powerpc/mulic3-2.c
new file mode 100644
index 00000000000..36fe8bc3061
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/mulic3-2.c
@@ -0,0 +1,17 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-require-effective-target longdouble128 } */
+/* { dg-options "-O2 -mpower8-vector -mabi=ibmlongdouble -Wno-psabi" } */
+
+/* Check that complex multiply generates the right call for __ibm128 when long
+   double is IBM 128-bit floating point.  */
+
+typedef _Complex long double c_ibm128_t __attribute__((mode(__TC__)));
+
+void
+multiply (c_ibm128_t *p, c_ibm128_t *q, c_ibm128_t *r)
+{
+  *p = *q * *r;
+}
+
+/* { dg-final { scan-assembler "bl __multc3" } } */
-- 
2.39.1


-- 
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meissner@linux.ibm.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Repost of patches for solving the build on Fedora 36 problem
  2023-02-03  5:43 [PATCH 0/2] Repost of patches for solving the build on Fedora 36 problem Michael Meissner
  2023-02-03  5:49 ` [PATCH 1/2] PR target/107299: Fix build issue when long double is IEEE 128-bit Michael Meissner
  2023-02-03  5:53 ` [PATCH 2/2] Rework 128-bit complex multiply and divide Michael Meissner
@ 2023-02-03  7:42 ` Richard Biener
  2023-02-06 18:28   ` Peter Bergner
  2 siblings, 1 reply; 19+ messages in thread
From: Richard Biener @ 2023-02-03  7:42 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, Segher Boessenkool, Kewen.Lin,
	David Edelsohn, Peter Bergner, Will Schmidt

On Fri, Feb 3, 2023 at 6:44 AM Michael Meissner via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> I'm reposting these two patches that allow GCC to build on Fedora 36 just to be
> clear which patches I'm talking about.  The issue is that if GCC is configured
> with long double using the IEEE 128-bit representation, it currently cannot
> build _mulkc3 and _divkc3 in libgcc.

It's interesting that we do not see this with openSUSE where I configure with

--with-cpu=power8 --with-tune=power9 --with-long-double-format=ieee
--with-long-double-128

note this is ppc64le, we leave ppc64 and ppc with their default.

> Note, these patches do not solve the underlying problem of mixing _Float128 and
> long double types and using built-in functions (i.e. calling a _Float128
> built-in function with long double arguments when long double is IEEE 128-bit,
> or vice versa calling a long double built-in function with _Float128
> arguments).  But they do allow the compiler to build.
>
> Note, it is the morning of February 3rd, and I will be off on vacation from
> February 7th through February 14th.
>
> The first patch changes libgcc so that it uses either _Float128 or long double
> as the base IEEE 128-bit type, depending on whether long double uses the IBM
> double-double representation, or the IEEE 128-bit representation.  And for the
> complex type it uses _Complex _Float128 or _Complex long double.  The _mulkc3
> and _divkc3 functions are adjusted to use the f128 built-in functions or the
> long double built-in functions, based on the long double type.
>
> The second patch improves how the compiler generates the call to _mulkc3 and
> _divkc3.  I've discovered as I have tried to fix underlying problem with the
> IEEE 128-bit floating point types, it breaks the calls for IEEE 128-bit complex
> multiply and divide.  This patch uses a cleaner approach to generate these
> calls, and it will work with the current setup, and with the various fixes that
> I've attempted to do to fix the underlying problem.
>
> --
> Michael Meissner, IBM
> PO Box 98, Ayer, Massachusetts, USA, 01432
> email: meissner@linux.ibm.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Repost of patches for solving the build on Fedora 36 problem
  2023-02-03  7:42 ` [PATCH 0/2] Repost of patches for solving the build on Fedora 36 problem Richard Biener
@ 2023-02-06 18:28   ` Peter Bergner
  2023-02-07 14:22     ` Richard Biener
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Bergner @ 2023-02-06 18:28 UTC (permalink / raw)
  To: Richard Biener, Michael Meissner, gcc-patches,
	Segher Boessenkool, Kewen.Lin, David Edelsohn, Will Schmidt,
	Bill Seurer

On 2/3/23 1:42 AM, Richard Biener wrote:
> On Fri, Feb 3, 2023 at 6:44 AM Michael Meissner via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> I'm reposting these two patches that allow GCC to build on Fedora 36 just to be
>> clear which patches I'm talking about.  The issue is that if GCC is configured
>> with long double using the IEEE 128-bit representation, it currently cannot
>> build _mulkc3 and _divkc3 in libgcc.
> 
> It's interesting that we do not see this with openSUSE where I configure with
> 
> --with-cpu=power8 --with-tune=power9 --with-long-double-format=ieee
> --with-long-double-128
> 
> note this is ppc64le, we leave ppc64 and ppc with their default.

That's strange, Bill just retested on our ppc64le openSUSE Tumbleweed system
using basically the same configure options and sees the ICE:

/home/seurer/gcc/git/gcc-trunk/libgcc/config/rs6000/_mulkc3.c: In function '__mulkc3_sw':
/home/seurer/gcc/git/gcc-trunk/libgcc/config/rs6000/_mulkc3.c:97:1: internal compiler error: in fold_stmt, at gimple-range-fold.cc:522

He did not specify --with=cpu= or --with-tune=, which means he got
power8 defaults for both of those.  It's hard for me to believe that
--with-tune=power9 could hide the issue, but we'll try that configuration
too.  Do you have any other configure options that might affect things?

Peter



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Repost of patches for solving the build on Fedora 36 problem
  2023-02-06 18:28   ` Peter Bergner
@ 2023-02-07 14:22     ` Richard Biener
  2023-02-07 14:31       ` Jakub Jelinek
  0 siblings, 1 reply; 19+ messages in thread
From: Richard Biener @ 2023-02-07 14:22 UTC (permalink / raw)
  To: Peter Bergner
  Cc: Michael Meissner, gcc-patches, Segher Boessenkool, Kewen.Lin,
	David Edelsohn, Will Schmidt, Bill Seurer

On Mon, Feb 6, 2023 at 7:28 PM Peter Bergner <bergner@linux.ibm.com> wrote:
>
> On 2/3/23 1:42 AM, Richard Biener wrote:
> > On Fri, Feb 3, 2023 at 6:44 AM Michael Meissner via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >> I'm reposting these two patches that allow GCC to build on Fedora 36 just to be
> >> clear which patches I'm talking about.  The issue is that if GCC is configured
> >> with long double using the IEEE 128-bit representation, it currently cannot
> >> build _mulkc3 and _divkc3 in libgcc.
> >
> > It's interesting that we do not see this with openSUSE where I configure with
> >
> > --with-cpu=power8 --with-tune=power9 --with-long-double-format=ieee
> > --with-long-double-128
> >
> > note this is ppc64le, we leave ppc64 and ppc with their default.
>
> That's strange, Bill just retested on our ppc64le openSUSE Tumbleweed system
> using basically the same configure options and sees the ICE:
>
> /home/seurer/gcc/git/gcc-trunk/libgcc/config/rs6000/_mulkc3.c: In function '__mulkc3_sw':
> /home/seurer/gcc/git/gcc-trunk/libgcc/config/rs6000/_mulkc3.c:97:1: internal compiler error: in fold_stmt, at gimple-range-fold.cc:522
>
> He did not specify --with=cpu= or --with-tune=, which means he got
> power8 defaults for both of those.  It's hard for me to believe that
> --with-tune=power9 could hide the issue, but we'll try that configuration
> too.  Do you have any other configure options that might affect things?

The full configure is

../configure --prefix=/usr --infodir=/usr/share/info
--mandir=/usr/share/man --libdir=/usr/lib64 --libexecdir=/usr/lib64
--enable-languages=c,c++,objc,fortran,obj-c++,ada,go,jit,m2
--enable-host-shared --enable-checking=release --disable-werror
--with-gxx-include-dir=/usr/include/c++/13
--with-libstdcxx-zoneinfo=/usr/share/zoneinfo --enable-ssp
--disable-libssp --disable-libvtv --enable-cet=auto --disable-libcc1
--enable-plugin --with-bugurl=https://bugs.opensuse.org/
'--with-pkgversion=SUSE Linux' --with-slibdir=/lib64
--with-system-zlib --enable-libstdcxx-allocator=new
--disable-libstdcxx-pch --enable-version-specific-runtime-libs
--with-gcc-major-version-only --enable-linker-build-id
--enable-linux-futex --enable-gnu-indirect-function
--program-suffix=-13 --without-system-libunwind --with-cpu=power8
--with-tune=power9 --with-long-double-format=ieee --enable-secureplt
--with-long-double-128 --enable-targets=powerpcle-linux
--disable-multilib --with-build-config=bootstrap-lto-lean
--enable-link-mutex --build=powerpc64le-suse-linux
--host=powerpc64le-suse-linux

and _mulkc3.c is built like

/home/abuild/rpmbuild/BUILD/gcc-13.0.1+git5679/obj-powerpc64le-suse-linux/./gcc/xgcc
-B/home/abuild/rpmbuild/BUILD/gcc-13.0.1+git5679/obj-powerpc64le-suse-linux/./gcc/
-B/usr/powerpc64le-suse-linux/bin/ -B/usr/powerpc64le-suse-linux/lib/
-isystem /usr/powerpc64le-suse-linux/include -isystem
/usr/powerpc64le-suse-linux/sys-include   -fno-checking -O2
-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -funwind-tables
-fasynchronous-unwind-tables -fstack-clash-protection
-Werror=return-type -g -U_FORTIFY_SOURCE -O2  -O2 -U_FORTIFY_SOURCE
-D_FORTIFY_SOURCE=3 -funwind-tables -fasynchronous-unwind-tables
-fstack-clash-protection -Werror=return-type -g -U_FORTIFY_SOURCE
-DIN_GCC -fPIC   -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual
-Wno-format -Wstrict-prototypes -Wmissing-prototypes
-Wold-style-definition  -isystem ./include  -fPIC -mlong-double-128
-mno-minimal-toc -g -DIN_LIBGCC2 -fbuilding-libgcc
-fno-stack-protector  -fPIC -mlong-double-128 -mno-minimal-toc -I. -I.
-I../.././gcc -I../../../libgcc -I../../../libgcc/.
-I../../../libgcc/../gcc -I../../../libgcc/../include
-I../../../libgcc/../libdecnumber/dpd
-I../../../libgcc/../libdecnumber -DHAVE_CC_TLS  -Wno-type-limits
-mvsx -mfloat128 -mno-float128-hardware -mno-gnu-attribute
-I../../../libgcc/soft-fp -I../../../libgcc/config/rs6000
-DFLOAT128_HW_INSNS -DFLOAT128_HW_INSNS_ISA3_1  -o _mulkc3.o -MT
_mulkc3.o -MD -MP -MF _mulkc3.dep  -c
../../../libgcc/config/rs6000/_mulkc3.c -fvisibility=hidden
-DHIDE_EXPORTS

>
> Peter
>
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Repost of patches for solving the build on Fedora 36 problem
  2023-02-07 14:22     ` Richard Biener
@ 2023-02-07 14:31       ` Jakub Jelinek
  0 siblings, 0 replies; 19+ messages in thread
From: Jakub Jelinek @ 2023-02-07 14:31 UTC (permalink / raw)
  To: Richard Biener
  Cc: Peter Bergner, Michael Meissner, gcc-patches, Segher Boessenkool,
	Kewen.Lin, David Edelsohn, Will Schmidt, Bill Seurer

On Tue, Feb 07, 2023 at 03:22:41PM +0100, Richard Biener via Gcc-patches wrote:
> > He did not specify --with=cpu= or --with-tune=, which means he got
> > power8 defaults for both of those.  It's hard for me to believe that
> > --with-tune=power9 could hide the issue, but we'll try that configuration
> > too.  Do you have any other configure options that might affect things?
> 
> The full configure is
> 
> ../configure --prefix=/usr --infodir=/usr/share/info
> --mandir=/usr/share/man --libdir=/usr/lib64 --libexecdir=/usr/lib64
> --enable-languages=c,c++,objc,fortran,obj-c++,ada,go,jit,m2
> --enable-host-shared --enable-checking=release --disable-werror
> --with-gxx-include-dir=/usr/include/c++/13
> --with-libstdcxx-zoneinfo=/usr/share/zoneinfo --enable-ssp
> --disable-libssp --disable-libvtv --enable-cet=auto --disable-libcc1
> --enable-plugin --with-bugurl=https://bugs.opensuse.org/
> '--with-pkgversion=SUSE Linux' --with-slibdir=/lib64
> --with-system-zlib --enable-libstdcxx-allocator=new
> --disable-libstdcxx-pch --enable-version-specific-runtime-libs
> --with-gcc-major-version-only --enable-linker-build-id
> --enable-linux-futex --enable-gnu-indirect-function
> --program-suffix=-13 --without-system-libunwind --with-cpu=power8
> --with-tune=power9 --with-long-double-format=ieee --enable-secureplt
> --with-long-double-128 --enable-targets=powerpcle-linux
> --disable-multilib --with-build-config=bootstrap-lto-lean
> --enable-link-mutex --build=powerpc64le-suse-linux
> --host=powerpc64le-suse-linux
> 
> and _mulkc3.c is built like

We do not get the ICE in Fedora builds either.  The important thing is
--enable-checking=release
With release checking there is no ICE, with checking there is one.

	Jakub


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] Rework 128-bit complex multiply and divide.
  2023-02-03  5:53 ` [PATCH 2/2] Rework 128-bit complex multiply and divide Michael Meissner
@ 2023-02-22 10:13   ` Kewen.Lin
  2023-02-22 18:01     ` Michael Meissner
  2023-03-02 22:46   ` Ping: " Michael Meissner
  2023-03-03 21:35   ` Segher Boessenkool
  2 siblings, 1 reply; 19+ messages in thread
From: Kewen.Lin @ 2023-02-22 10:13 UTC (permalink / raw)
  To: Michael Meissner
  Cc: gcc-patches, Segher Boessenkool, David Edelsohn, Peter Bergner,
	Will Schmidt

Hi Mike,

on 2023/2/3 13:53, Michael Meissner wrote:
> This patch reworks how the complex multiply and divide built-in functions are
> done.  Previously we created built-in declarations for doing long double complex
> multiply and divide when long double is IEEE 128-bit.  The old code also did not
> support __ibm128 complex multiply and divide if long double is IEEE 128-bit.
> 
> This patch was originally posted on December 13th, 2022:
> 
> | Date: Tue, 13 Dec 2022 01:21:06 -0500
> | Subject: [PATCH V2] Rework 128-bit complex multiply and divide, PR target/107299
> | Message-ID: <Y5gZ0o1nzCq9MmR9@toto.the-meissners.org>
> 
> In terms of history, I wrote the original code just as I was starting to test
> GCC on systems where IEEE 128-bit long double was the default.  At the time, we
> had not yet started mangling the built-in function names as a way to bridge
> going from a system with 128-bit IBM long double to 128-bin IEEE long double.
> 
> The original code depends on there only being two 128-bit types invovled.  With
> the next patch in this series, this assumption will no longer be true.  When
> long double is IEEE 128-bit, there will be 2 IEEE 128-bit types (one for the
> explicit __float128/_Float128 type and one for long double).
> 
> The problem is we cannot create two separate built-in functions that resolve to
> the same name.  This is a requirement of add_builtin_function and the C front
> end.  That means for the 3 possible modes (IFmode, KFmode, and TFmode), you can
> only use 2 of them.
> 
> This code does not create the built-in declaration with the changed name.
> Instead, it uses the TARGET_MANGLE_DECL_ASSEMBLER_NAME hook to change the name
> before it is written out to the assembler file like it now does for all of the
> other long double built-in functions.
> 
> When I wrote these patches, I discovered that __ibm128 complex multiply and
> divide had originally not been supported if long double is IEEE 128-bit as it
> would generate calls to __mulic3 and __divic3.  I added tests in the testsuite
> to verify that the correct name (i.e. __multc3 and __divtc3) is used in this
> case.
> 
> I had previously sent this patch out on November 1st.  Compared to that version,
> this version no longer disables the special mapping when you are building
> libgcc, as it turns out we don't need it.
> 
> I tested all 3 patchs for PR target/107299 on:
> 
>     1)	LE Power10 using --with-cpu=power10 --with-long-double-format=ieee
>     2)	LE Power10 using --with-cpu=power10 --with-long-double-format=ibm
>     3)	LE Power9  using --with-cpu=power9  --with-long-double-format=ibm
>     4)	BE Power8  using --with-cpu=power8  --with-long-double-format=ibm
> 
> Once all 3 patches have been applied, we can once again build GCC when long
> double is IEEE 128-bit.  There were no other regressions with these patches.
> Can I check these patches into the trunk?

These two above paragraphs look a bit out of date (two patches now). :)

IIUC this patch actually fixes a latent issue, so it is independent of the one
fixing the bootstrapping issue, right?  This updated version of patch looks
good to me, but I'd leave the approval to Segher/David.  Thanks!

BR,
Kewen

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] PR target/107299: Fix build issue when long double is IEEE 128-bit
  2023-02-03  5:49 ` [PATCH 1/2] PR target/107299: Fix build issue when long double is IEEE 128-bit Michael Meissner
@ 2023-02-22 10:37   ` Kewen.Lin
  2023-02-22 18:11     ` Michael Meissner
  2023-03-02 22:45   ` Ping: " Michael Meissner
  2023-03-03 20:56   ` Segher Boessenkool
  2 siblings, 1 reply; 19+ messages in thread
From: Kewen.Lin @ 2023-02-22 10:37 UTC (permalink / raw)
  To: Michael Meissner
  Cc: gcc-patches, Segher Boessenkool, David Edelsohn, Peter Bergner,
	Will Schmidt

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] Rework 128-bit complex multiply and divide.
  2023-02-22 10:13   ` Kewen.Lin
@ 2023-02-22 18:01     ` Michael Meissner
  0 siblings, 0 replies; 19+ messages in thread
From: Michael Meissner @ 2023-02-22 18:01 UTC (permalink / raw)
  To: Kewen.Lin
  Cc: Michael Meissner, gcc-patches, Segher Boessenkool,
	David Edelsohn, Peter Bergner, Will Schmidt

On Wed, Feb 22, 2023 at 06:13:07PM +0800, Kewen.Lin wrote:
> These two above paragraphs look a bit out of date (two patches now). :)

Thanks.

> IIUC this patch actually fixes a latent issue, so it is independent of the one
> fixing the bootstrapping issue, right?  This updated version of patch looks
> good to me, but I'd leave the approval to Segher/David.  Thanks!

Yes, I've been waiting for Segher or David's approval for this for awhile.

The history is it is indeed a latent issue (not supporting __ibm128 complex
multiply and divide when long double is IEEE 128-bit).  However, at the time I
wrote it, the other changes had broken the complex multiply and divide, and I
wrote this patch as part of the series.  I separated the patch from the other 2
to make it simpler to go in.  But it seems to be in limbo.

-- 
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meissner@linux.ibm.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] PR target/107299: Fix build issue when long double is IEEE 128-bit
  2023-02-22 10:37   ` Kewen.Lin
@ 2023-02-22 18:11     ` Michael Meissner
  0 siblings, 0 replies; 19+ messages in thread
From: Michael Meissner @ 2023-02-22 18:11 UTC (permalink / raw)
  To: Kewen.Lin
  Cc: Michael Meissner, gcc-patches, Segher Boessenkool,
	David Edelsohn, Peter Bergner, Will Schmidt

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.

-- 
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meissner@linux.ibm.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Ping: [PATCH 1/2] PR target/107299: Fix build issue when long double is IEEE 128-bit
  2023-02-03  5:49 ` [PATCH 1/2] PR target/107299: Fix build issue when long double is IEEE 128-bit Michael Meissner
  2023-02-22 10:37   ` Kewen.Lin
@ 2023-03-02 22:45   ` Michael Meissner
  2023-03-03 20:56   ` Segher Boessenkool
  2 siblings, 0 replies; 19+ messages in thread
From: Michael Meissner @ 2023-03-02 22:45 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, Segher Boessenkool, Kewen.Lin,
	David Edelsohn, Peter Bergner, Will Schmidt

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>

-- 
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meissner@linux.ibm.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Ping: [PATCH 2/2] Rework 128-bit complex multiply and divide.
  2023-02-03  5:53 ` [PATCH 2/2] Rework 128-bit complex multiply and divide Michael Meissner
  2023-02-22 10:13   ` Kewen.Lin
@ 2023-03-02 22:46   ` Michael Meissner
  2023-03-03 21:35   ` Segher Boessenkool
  2 siblings, 0 replies; 19+ messages in thread
From: Michael Meissner @ 2023-03-02 22:46 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, Segher Boessenkool, Kewen.Lin,
	David Edelsohn, Peter Bergner, Will Schmidt

This patch is second in importance after the first patch in the series.  It is
needed to allow complex IBM 128-bit multiply/divide when long double is IEEE
128-bit.

| Date: Fri, 3 Feb 2023 00:53:05 -0500
| From: Michael Meissner <meissner@linux.ibm.com>
| Subject: [PATCH 2/2] Rework 128-bit complex multiply and divide.
| Message-ID: <Y9yhQQdUaM+z6IYD@toto.the-meissners.org>

-- 
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meissner@linux.ibm.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] PR target/107299: Fix build issue when long double is IEEE 128-bit
  2023-02-03  5:49 ` [PATCH 1/2] PR target/107299: Fix build issue when long double is IEEE 128-bit Michael Meissner
  2023-02-22 10:37   ` Kewen.Lin
  2023-03-02 22:45   ` Ping: " Michael Meissner
@ 2023-03-03 20:56   ` Segher Boessenkool
  2 siblings, 0 replies; 19+ messages in thread
From: Segher Boessenkool @ 2023-03-03 20:56 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, Kewen.Lin, David Edelsohn,
	Peter Bergner, Will Schmidt

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] Rework 128-bit complex multiply and divide.
  2023-02-03  5:53 ` [PATCH 2/2] Rework 128-bit complex multiply and divide Michael Meissner
  2023-02-22 10:13   ` Kewen.Lin
  2023-03-02 22:46   ` Ping: " Michael Meissner
@ 2023-03-03 21:35   ` Segher Boessenkool
  2023-03-09 16:11     ` Michael Meissner
  2023-03-09 16:42     ` Michael Meissner
  2 siblings, 2 replies; 19+ messages in thread
From: Segher Boessenkool @ 2023-03-03 21:35 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, Kewen.Lin, David Edelsohn,
	Peter Bergner, Will Schmidt

Hi!

On Fri, Feb 03, 2023 at 12:53:05AM -0500, Michael Meissner wrote:
> This patch reworks how the complex multiply and divide built-in functions are
> done.

> I tested all 3 patchs for PR target/107299 on:

Is this part of the proposed commit message?  As Ke Wen pointed out, it
is wrong.  Most of your mail does not belong in a commit message at all,
but some probably does?  Please do this clearer with future patches.

> 	* config/rs6000/rs6000.cc (create_complex_muldiv): Delete.
> 	(init_float128_ieee): Delete code to switch complex multiply and divide
> 	for long double.

I like this kind of patch :-)

> +/* Internal function to return the built-in function id for the complex
> +   multiply operation for a given mode.  */
> +
> +static inline built_in_function
> +complex_multiply_builtin_code (machine_mode mode)
> +{
> +  return (built_in_function) (BUILT_IN_COMPLEX_MUL_MIN + mode
> +			      - MIN_MODE_COMPLEX_FLOAT);
> +}

There should be an assert that the mode is as expected
  gcc_assert (IN_RANGE (mode, MIN_MODE_COMPLEX_FLOAT, MAX_MODE_COMPLEX_FLOAT));
or such.

Using more temporaries should make this simpler as well, obviate the
need for explicit casts, and make everything fit on short lines.

> +static inline built_in_function
> +complex_divide_builtin_code (machine_mode mode)
> +{
> +  return (built_in_function) (BUILT_IN_COMPLEX_DIV_MIN + mode
> +			      - MIN_MODE_COMPLEX_FLOAT);
> +}

Ditto ofc.

> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/divic3-1.c
> @@ -0,0 +1,18 @@
> +/* { dg-do compile { target { powerpc*-*-* } } } */

Leave the target clause out.

> +/* { dg-require-effective-target powerpc_p8vector_ok } */
> +/* { dg-require-effective-target longdouble128 } */
> +/* { dg-require-effective-target ppc_float128_sw } */
> +/* { dg-options "-O2 -mpower8-vector -mabi=ieeelongdouble -Wno-psabi" } */

It would be nice if you did not try to add -mpower8-vector in more
testcases :-(

Is -Wno-psabi needed here?  What is the error you get without it / on
which configurations?  Cargo-culting hiding the warnings makes you see
fewer warnings, but that is the opposite of a good idea.

> +/* { dg-final { scan-assembler "bl __divtc3" } } */

This name depends on what object format and ABI is in use (some have an
extra leading underscore, or a dot, or whatever).


Segher

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] Rework 128-bit complex multiply and divide.
  2023-03-03 21:35   ` Segher Boessenkool
@ 2023-03-09 16:11     ` Michael Meissner
  2023-03-09 22:16       ` Segher Boessenkool
  2023-03-09 16:42     ` Michael Meissner
  1 sibling, 1 reply; 19+ messages in thread
From: Michael Meissner @ 2023-03-09 16:11 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Michael Meissner, gcc-patches, Kewen.Lin, David Edelsohn,
	Peter Bergner, Will Schmidt

On Fri, Mar 03, 2023 at 03:35:44PM -0600, Segher Boessenkool wrote:
> Hi!
> 
> On Fri, Feb 03, 2023 at 12:53:05AM -0500, Michael Meissner wrote:
> > This patch reworks how the complex multiply and divide built-in functions are
> > done.
> 
> > I tested all 3 patchs for PR target/107299 on:
> 
> Is this part of the proposed commit message?  As Ke Wen pointed out, it
> is wrong.  Most of your mail does not belong in a commit message at all,
> but some probably does?  Please do this clearer with future patches.
> 
> > 	* config/rs6000/rs6000.cc (create_complex_muldiv): Delete.
> > 	(init_float128_ieee): Delete code to switch complex multiply and divide
> > 	for long double.
> 
> I like this kind of patch :-)
> 
> > +/* Internal function to return the built-in function id for the complex
> > +   multiply operation for a given mode.  */
> > +
> > +static inline built_in_function
> > +complex_multiply_builtin_code (machine_mode mode)
> > +{
> > +  return (built_in_function) (BUILT_IN_COMPLEX_MUL_MIN + mode
> > +			      - MIN_MODE_COMPLEX_FLOAT);
> > +}
> 
> There should be an assert that the mode is as expected
>   gcc_assert (IN_RANGE (mode, MIN_MODE_COMPLEX_FLOAT, MAX_MODE_COMPLEX_FLOAT));
> or such.

Ok.

> Using more temporaries should make this simpler as well, obviate the
> need for explicit casts, and make everything fit on short lines.
> 
> > +static inline built_in_function
> > +complex_divide_builtin_code (machine_mode mode)
> > +{
> > +  return (built_in_function) (BUILT_IN_COMPLEX_DIV_MIN + mode
> > +			      - MIN_MODE_COMPLEX_FLOAT);
> > +}
> 
> Ditto ofc.
> 
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/divic3-1.c
> > @@ -0,0 +1,18 @@
> > +/* { dg-do compile { target { powerpc*-*-* } } } */
> 
> Leave the target clause out.

Ok.

> > +/* { dg-require-effective-target powerpc_p8vector_ok } */
> > +/* { dg-require-effective-target longdouble128 } */
> > +/* { dg-require-effective-target ppc_float128_sw } */
> > +/* { dg-options "-O2 -mpower8-vector -mabi=ieeelongdouble -Wno-psabi" } */
> 
> It would be nice if you did not try to add -mpower8-vector in more
> testcases :-(

Yep.

> Is -Wno-psabi needed here?  What is the error you get without it / on
> which configurations?  Cargo-culting hiding the warnings makes you see
> fewer warnings, but that is the opposite of a good idea.
> 
> > +/* { dg-final { scan-assembler "bl __divtc3" } } */
> 
> This name depends on what object format and ABI is in use (some have an
> extra leading underscore, or a dot, or whatever).

Yes it is needed if GCC is configured against an older GLIBC before the full
IEEE 128-bit support was added.  For example, on my big endian test system, you
get warnings if you switch the floating point format.  I would imagine it would
also fail on little endian system with older libraries.

-- 
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meissner@linux.ibm.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] Rework 128-bit complex multiply and divide.
  2023-03-03 21:35   ` Segher Boessenkool
  2023-03-09 16:11     ` Michael Meissner
@ 2023-03-09 16:42     ` Michael Meissner
  1 sibling, 0 replies; 19+ messages in thread
From: Michael Meissner @ 2023-03-09 16:42 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Michael Meissner, gcc-patches, Kewen.Lin, David Edelsohn,
	Peter Bergner, Will Schmidt

On Fri, Mar 03, 2023 at 03:35:44PM -0600, Segher Boessenkool wrote:
> > +complex_multiply_builtin_code (machine_mode mode)
> > +{
> > +  return (built_in_function) (BUILT_IN_COMPLEX_MUL_MIN + mode
> > +			      - MIN_MODE_COMPLEX_FLOAT);
> > +}
> 
> There should be an assert that the mode is as expected
>   gcc_assert (IN_RANGE (mode, MIN_MODE_COMPLEX_FLOAT, MAX_MODE_COMPLEX_FLOAT));
> or such.
> 
> Using more temporaries should make this simpler as well, obviate the
> need for explicit casts, and make everything fit on short lines.

While I can use a temporary to shorten the line, I can't eliminate the case, or
I'll get a warning about implicit conversion from int to the enum
built_in_function.  Here is what I will use:

static inline built_in_function
complex_multiply_builtin_code (machine_mode mode)
{
  gcc_assert (IN_RANGE (mode, MIN_MODE_COMPLEX_FLOAT, MAX_MODE_COMPLEX_FLOAT));
  int func = BUILT_IN_COMPLEX_MUL_MIN + mode - MIN_MODE_COMPLEX_FLOAT;
  return (built_in_function) func;
}

-- 
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meissner@linux.ibm.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] Rework 128-bit complex multiply and divide.
  2023-03-09 16:11     ` Michael Meissner
@ 2023-03-09 22:16       ` Segher Boessenkool
  2023-03-09 23:23         ` Michael Meissner
  0 siblings, 1 reply; 19+ messages in thread
From: Segher Boessenkool @ 2023-03-09 22:16 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, Kewen.Lin, David Edelsohn,
	Peter Bergner, Will Schmidt

On Thu, Mar 09, 2023 at 11:11:34AM -0500, Michael Meissner wrote:
> On Fri, Mar 03, 2023 at 03:35:44PM -0600, Segher Boessenkool wrote:
> > > +/* { dg-final { scan-assembler "bl __divtc3" } } */
> > 
> > This name depends on what object format and ABI is in use (some have an
> > extra leading underscore, or a dot, or whatever).
> 
> Yes it is needed if GCC is configured against an older GLIBC before the full
> IEEE 128-bit support was added.  For example, on my big endian test system, you
> get warnings if you switch the floating point format.  I would imagine it would
> also fail on little endian system with older libraries.

The regexp is not good enough, that is all.  Maybe
  {bl .?__divtc3}
or similar?  We have many examples in the tests already.


Segher

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] Rework 128-bit complex multiply and divide.
  2023-03-09 22:16       ` Segher Boessenkool
@ 2023-03-09 23:23         ` Michael Meissner
  0 siblings, 0 replies; 19+ messages in thread
From: Michael Meissner @ 2023-03-09 23:23 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Michael Meissner, gcc-patches, Kewen.Lin, David Edelsohn,
	Peter Bergner, Will Schmidt

On Thu, Mar 09, 2023 at 04:16:21PM -0600, Segher Boessenkool wrote:
> On Thu, Mar 09, 2023 at 11:11:34AM -0500, Michael Meissner wrote:
> > On Fri, Mar 03, 2023 at 03:35:44PM -0600, Segher Boessenkool wrote:
> > > > +/* { dg-final { scan-assembler "bl __divtc3" } } */
> > > 
> > > This name depends on what object format and ABI is in use (some have an
> > > extra leading underscore, or a dot, or whatever).
> > 
> > Yes it is needed if GCC is configured against an older GLIBC before the full
> > IEEE 128-bit support was added.  For example, on my big endian test system, you
> > get warnings if you switch the floating point format.  I would imagine it would
> > also fail on little endian system with older libraries.
> 
> The regexp is not good enough, that is all.  Maybe
>   {bl .?__divtc3}
> or similar?  We have many examples in the tests already.

I forgot the mention the regexp.  I think just doing:

/* { dg-final { scan-assembler "__multc3" } } */

is sufficient.

-- 
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meissner@linux.ibm.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-03-09 23:23 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-03  5:43 [PATCH 0/2] Repost of patches for solving the build on Fedora 36 problem Michael Meissner
2023-02-03  5:49 ` [PATCH 1/2] PR target/107299: Fix build issue when long double is IEEE 128-bit Michael Meissner
2023-02-22 10:37   ` Kewen.Lin
2023-02-22 18:11     ` Michael Meissner
2023-03-02 22:45   ` Ping: " Michael Meissner
2023-03-03 20:56   ` Segher Boessenkool
2023-02-03  5:53 ` [PATCH 2/2] Rework 128-bit complex multiply and divide Michael Meissner
2023-02-22 10:13   ` Kewen.Lin
2023-02-22 18:01     ` Michael Meissner
2023-03-02 22:46   ` Ping: " Michael Meissner
2023-03-03 21:35   ` Segher Boessenkool
2023-03-09 16:11     ` Michael Meissner
2023-03-09 22:16       ` Segher Boessenkool
2023-03-09 23:23         ` Michael Meissner
2023-03-09 16:42     ` Michael Meissner
2023-02-03  7:42 ` [PATCH 0/2] Repost of patches for solving the build on Fedora 36 problem Richard Biener
2023-02-06 18:28   ` Peter Bergner
2023-02-07 14:22     ` Richard Biener
2023-02-07 14:31       ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).