rs6000: Fix typos in float128 ISA3.1 support

rs6000: Fix typos in float128 ISA3.1 support

[Kewen.Lin]

[-- Attachment #1: Type: text/plain, Size: 2417 bytes --]

Hi,

Recently if we build gcc on Power with the assembler which doesn't
have Power10 support, the build will fail when building libgcc with
one error message like:

Error: invalid switch -mpower10
Error: unrecognized option -mpower10
make[2]: *** [...gcc/gcc-base/libgcc/shared-object.mk:14: float128-p10.o] Error 1

By checking the culprit commit r12-1340, it's caused by some typos.

What the proposed patch does:
  - fix test target typo libgcc_cv_powerpc_3_1_float128_hw
    (written wrongly as libgcc_cv_powerpc_float128_hw, so it's going
     to build ISA3.1 stuffs just when detecting ISA3.0).
  - fix test case used for libgcc_cv_powerpc_3_1_float128_hw check.
  - fix test option used for libgcc_cv_powerpc_3_1_float128_hw check.
  - remove the ISA3.1 related contents from t-float128-hw.
  - add new macro FLOAT128_HW_INSNS_ISA3_1 to differentiate ISA3.1
    content from ISA3.0 part in ifunc support.

For the last two points, I think it's by design, when the assembler
only supports power9 insns but not power10 insns, we are unable to
build power10 hw supported function successfully, so we shouldn't
generate related ifunc stuffs which rely on p10 insns.

This patch makes the build happy even without p10 supported as.

Bootstrapped/regtested on:
  - powerpc64le-linux-gnu P10
  - powerpc64le-linux-gnu P9 (w/i and w/o p10 supported as)
  - powerpc64-linux-gnu P8 (w/i and w/o p10 supported as)

BTW, there are some noises during regression testings due to
newer versions binutils, but they were identified as unrelated
after some checkings.

Is it ok for trunk?

BR,
Kewen
-----
libgcc/ChangeLog:

	* configure: Regenerate.
	* configure.ac (test for libgcc_cv_powerpc_3_1_float128_hw): Fix
	typos among name, CFLAGS and test case.
	* config/rs6000/t-float128-hw(fp128_3_1_hw_funcs,
	fp128_3_1_hw_src, fp128_3_1_hw_static_obj, fp128_3_1_hw_shared_obj,
	fp128_3_1_hw_obj): Remove variables for ISA 3.1 support.
	* config/rs6000/t-float128-p10-hw (FLOAT128_HW_INSNS): Append
	macro FLOAT128_HW_INSNS_ISA3_1 for ISA 3.1 support.
	(FP128_3_1_CFLAGS_HW): Fix option typo.
	* config/rs6000/float128-ifunc.c (SW_OR_HW_ISA3_1): Guarded
	with FLOAT128_HW_INSNS_ISA3_1.
	(__floattikf_resolve): Likewise.
	(__floatuntikf_resolve): Likewise.
	(__fixkfti_resolve): Likewise.
	(__fixunskfti_resolve): Likewise.
	(__floattikf): Likewise.
	(__floatuntikf): Likewise.
	(__fixkfti): Likewise.
	(__fixunskfti): Likewise.

[-- Attachment #2: rs6000-libgcc-typo.diff --]
[-- Type: text/plain, Size: 8261 bytes --]

 libgcc/config/rs6000/float128-ifunc.c  |  9 ++++++++-
 libgcc/config/rs6000/t-float128-hw     | 16 ----------------
 libgcc/config/rs6000/t-float128-p10-hw |  4 ++--
 libgcc/configure                       | 18 +++++++++---------
 libgcc/configure.ac                    | 14 +++++++-------
 5 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/libgcc/config/rs6000/float128-ifunc.c b/libgcc/config/rs6000/float128-ifunc.c
index 57545dd7edb..ef7f731bf0b 100644
--- a/libgcc/config/rs6000/float128-ifunc.c
+++ b/libgcc/config/rs6000/float128-ifunc.c
@@ -46,7 +46,9 @@
 #endif
 
 #define SW_OR_HW(SW, HW) (__builtin_cpu_supports ("ieee128") ? HW : SW)
+#ifdef FLOAT128_HW_INSNS_ISA3_1
 #define SW_OR_HW_ISA3_1(SW, HW) (__builtin_cpu_supports ("arch_3_1") ? HW : SW)
+#endif
 
 /* Resolvers.  */
 static __typeof__ (__addkf3_sw) *
@@ -97,6 +99,7 @@ __floatdikf_resolve (void)
   return SW_OR_HW (__floatdikf_sw, __floatdikf_hw);
 }
 
+#ifdef FLOAT128_HW_INSNS_ISA3_1
 static __typeof__ (__floattikf_sw) *
 __floattikf_resolve (void)
 {
@@ -108,6 +111,7 @@ __floatuntikf_resolve (void)
 {
   return SW_OR_HW_ISA3_1 (__floatuntikf_sw, __floatuntikf_hw);
 }
+#endif
 
 static __typeof__ (__floatunsikf_sw) *
 __floatunsikf_resolve (void)
@@ -121,7 +125,7 @@ __floatundikf_resolve (void)
   return SW_OR_HW (__floatundikf_sw, __floatundikf_hw);
 }
 
-
+#ifdef FLOAT128_HW_INSNS_ISA3_1
 static __typeof__ (__fixkfti_sw) *
 __fixkfti_resolve (void)
 {
@@ -133,6 +137,7 @@ __fixunskfti_resolve (void)
 {
   return SW_OR_HW_ISA3_1 (__fixunskfti_sw, __fixunskfti_hw);
 }
+#endif
 
 static __typeof__ (__fixkfsi_sw) *
 __fixkfsi_resolve (void)
@@ -323,6 +328,7 @@ TFtype __floatsikf (SItype_ppc)
 TFtype __floatdikf (DItype_ppc)
   __attribute__ ((__ifunc__ ("__floatdikf_resolve")));
 
+#ifdef FLOAT128_HW_INSNS_ISA3_1
 TFtype __floattikf (TItype_ppc)
   __attribute__ ((__ifunc__ ("__floattikf_resolve")));
 
@@ -334,6 +340,7 @@ TItype_ppc __fixkfti (TFtype)
 
 UTItype_ppc __fixunskfti (TFtype)
   __attribute__ ((__ifunc__ ("__fixunskfti_resolve")));
+#endif
 
 TFtype __floatunsikf (USItype_ppc)
   __attribute__ ((__ifunc__ ("__floatunsikf_resolve")));
diff --git a/libgcc/config/rs6000/t-float128-hw b/libgcc/config/rs6000/t-float128-hw
index c0827366cc4..d64ca4dd694 100644
--- a/libgcc/config/rs6000/t-float128-hw
+++ b/libgcc/config/rs6000/t-float128-hw
@@ -13,13 +13,6 @@ fp128_hw_static_obj	= $(addsuffix $(objext),$(fp128_hw_funcs))
 fp128_hw_shared_obj	= $(addsuffix _s$(objext),$(fp128_hw_funcs))
 fp128_hw_obj		= $(fp128_hw_static_obj) $(fp128_hw_shared_obj)
 
-# New functions for ISA 3.1 hardware support
-fp128_3_1_hw_funcs	= float128-p10
-fp128_3_1_hw_src	= $(srcdir)/config/rs6000/float128-p10.c
-fp128_3_1_hw_static_obj	= $(addsuffix $(objext),$(fp128_3_1_hw_funcs))
-fp128_3_1_hw_shared_obj	= $(addsuffix _s$(objext),$(fp128_3_1_hw_funcs))
-fp128_3_1_hw_obj	= $(fp128_3_1_hw_static_obj) $(fp128_3_1_hw_shared_obj)
-
 fp128_ifunc_funcs	= float128-ifunc
 fp128_ifunc_src		= $(srcdir)/config/rs6000/float128-ifunc.c
 fp128_ifunc_static_obj	= float128-ifunc$(objext)
@@ -37,18 +30,9 @@ FP128_CFLAGS_HW		 = -Wno-type-limits -mvsx -mfloat128 \
 			   -I$(srcdir)/config/rs6000 \
 			   $(FLOAT128_HW_INSNS)
 
-FP128_3_1_CFLAGS_HW	 = -Wno-type-limits -mvsx -mcpu=power10 \
-			   -mfloat128-hardware -mno-gnu-attribute \
-			   -I$(srcdir)/soft-fp \
-			   -I$(srcdir)/config/rs6000 \
-			   $(FLOAT128_HW_INSNS)
-
 $(fp128_hw_obj)		 : INTERNAL_CFLAGS += $(FP128_CFLAGS_HW)
 $(fp128_hw_obj)		 : $(srcdir)/config/rs6000/t-float128-hw
 
-$(fp128_3_1_hw_obj)	 : INTERNAL_CFLAGS += $(FP128_3_1_CFLAGS_HW)
-$(fp128_3_1_hw_obj)	 : $(srcdir)/config/rs6000/t-float128-p10-hw
-
 $(fp128_ifunc_obj)	 : INTERNAL_CFLAGS += $(FP128_CFLAGS_SW)
 $(fp128_ifunc_obj)	 : $(srcdir)/config/rs6000/t-float128-hw
 
diff --git a/libgcc/config/rs6000/t-float128-p10-hw b/libgcc/config/rs6000/t-float128-p10-hw
index de36227c3d1..edaaee0e478 100644
--- a/libgcc/config/rs6000/t-float128-p10-hw
+++ b/libgcc/config/rs6000/t-float128-p10-hw
@@ -2,7 +2,7 @@
 # Tell the float128 functions that the ISA 3.1 hardware support can
 # be compiled it to be selected via IFUNC functions.
 
-FLOAT128_HW_INSNS	= -DFLOAT128_HW_INSNS
+FLOAT128_HW_INSNS	+= -DFLOAT128_HW_INSNS_ISA3_1
 
 # New functions for hardware support
 
@@ -14,7 +14,7 @@ fp128_3_1_hw_obj	= $(fp128_3_1_hw_static_obj) $(fp128_3_1_hw_shared_obj)
 
 # Build the hardware support functions with appropriate hardware support
 FP128_3_1_CFLAGS_HW	 = -Wno-type-limits -mvsx -mfloat128 \
-			   -mpower10 \
+			   -mcpu=power10 \
 			   -mfloat128-hardware -mno-gnu-attribute \
 			   -I$(srcdir)/soft-fp \
 			   -I$(srcdir)/config/rs6000 \
diff --git a/libgcc/configure b/libgcc/configure
index ce05e0dd48b..4919a56f518 100755
--- a/libgcc/configure
+++ b/libgcc/configure
@@ -5265,10 +5265,10 @@ $as_echo "$libgcc_cv_powerpc_float128_hw" >&6; }
   CFLAGS="$saved_CFLAGS"
 
   saved_CFLAGS="$CFLAGS"
-  CFLAGS="$CFLAGS -mpower10 -mfloat128-hardware"
+  CFLAGS="$CFLAGS -mcpu=power10 -mfloat128-hardware"
   { $as_echo "$as_me:${as_lineno-$LINENO}: checking for PowerPC ISA 3.1 to build hardware __float128 libraries" >&5
 $as_echo_n "checking for PowerPC ISA 3.1 to build hardware __float128 libraries... " >&6; }
-if ${libgcc_cv_powerpc_float128_hw+:} false; then :
+if ${libgcc_cv_powerpc_3_1_float128_hw+:} false; then :
   $as_echo_n "(cached) " >&6
 else
   cat confdefs.h - <<_ACEOF >conftest.$ac_ext
@@ -5280,15 +5280,15 @@ else
      #ifndef __BUILTIN_CPU_SUPPORTS__
      #error "__builtin_cpu_supports is not available"
      #endif
-     vector unsigned char add (vector unsigned char a, vector unsigned char b)
+     vector unsigned char conv (vector unsigned char qs)
      {
        vector unsigned char ret;
-       __asm__ ("xscvsqqp %0,%1,%2" : "=v" (ret) : "v" (a), "v" (b));
+       __asm__ ("xscvsqqp %0,%1" : "=v" (ret) : "v" (qs));
        return ret;
      }
-     void *add_resolver (void) { return (void *) add; }
-     __float128 add_ifunc (__float128, __float128)
-	__attribute__ ((__ifunc__ ("add_resolver")));
+     void *conv_resolver (void) { return (void *) conv; }
+     __float128 conv_ifunc (__float128)
+	__attribute__ ((__ifunc__ ("conv_resolver")));
 _ACEOF
 if ac_fn_c_try_compile "$LINENO"; then :
   libgcc_cv_powerpc_3_1_float128_hw=yes
@@ -5297,8 +5297,8 @@ else
 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_hw" >&5
-  $as_echo "$libgcc_cv_powerpc_float128_hw" >&6; }
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libgcc_cv_powerpc_3_1_float128_hw" >&5
+$as_echo "$libgcc_cv_powerpc_3_1_float128_hw" >&6; }
   CFLAGS="$saved_CFLAGS"
 esac
 
diff --git a/libgcc/configure.ac b/libgcc/configure.ac
index bc315dec7e4..13a80b2551b 100644
--- a/libgcc/configure.ac
+++ b/libgcc/configure.ac
@@ -460,9 +460,9 @@ powerpc*-*-linux*)
   CFLAGS="$saved_CFLAGS"
 
   saved_CFLAGS="$CFLAGS"
-  CFLAGS="$CFLAGS -mpower10 -mfloat128-hardware"
+  CFLAGS="$CFLAGS -mcpu=power10 -mfloat128-hardware"
   AC_CACHE_CHECK([for PowerPC ISA 3.1 to build hardware __float128 libraries],
-		 [libgcc_cv_powerpc_float128_hw],
+		 [libgcc_cv_powerpc_3_1_float128_hw],
 		 [AC_COMPILE_IFELSE(
     [AC_LANG_SOURCE([#include <sys/auxv.h>
      #ifndef AT_PLATFORM
@@ -471,15 +471,15 @@ powerpc*-*-linux*)
      #ifndef __BUILTIN_CPU_SUPPORTS__
      #error "__builtin_cpu_supports is not available"
      #endif
-     vector unsigned char add (vector unsigned char a, vector unsigned char b)
+     vector unsigned char conv (vector unsigned char qs)
      {
        vector unsigned char ret;
-       __asm__ ("xscvsqqp %0,%1,%2" : "=v" (ret) : "v" (a), "v" (b));
+       __asm__ ("xscvsqqp %0,%1" : "=v" (ret) : "v" (qs));
        return ret;
      }
-     void *add_resolver (void) { return (void *) add; }
-     __float128 add_ifunc (__float128, __float128)
-	__attribute__ ((__ifunc__ ("add_resolver")));])],
+     void *conv_resolver (void) { return (void *) conv; }
+     __float128 conv_ifunc (__float128)
+	__attribute__ ((__ifunc__ ("conv_resolver")));])],
     [libgcc_cv_powerpc_3_1_float128_hw=yes],
     [libgcc_cv_powerpc_3_1_float128_hw=no])])
   CFLAGS="$saved_CFLAGS"
-- 
2.25.1

Re: rs6000: Fix typos in float128 ISA3.1 support

[Segher Boessenkool]

Hi!

On Mon, Jun 21, 2021 at 05:27:06PM +0800, Kewen.Lin wrote:
> Recently if we build gcc on Power with the assembler which doesn't
> have Power10 support, the build will fail when building libgcc with
> one error message like:
> 
> Error: invalid switch -mpower10
> Error: unrecognized option -mpower10
> make[2]: *** [...gcc/gcc-base/libgcc/shared-object.mk:14: float128-p10.o] Error 1

In general, it is recommended to use a binutils of approximately the
same age as the GCC you are trying to build.  This is similar to us not
supporting most other non-sensical configurations.  An important reason
for that is it cannot ever be tested, there are just too many strange
combinations possible.

That said :-)

> By checking the culprit commit r12-1340, it's caused by some typos.

(That is 9090f4807161.)

>   - fix test case used for libgcc_cv_powerpc_3_1_float128_hw check.

I was confused here for a bit, "test case" usually means something in
testsuite/, I'd call this just "test" :-)

> BTW, there are some noises during regression testings due to
> newer versions binutils, but they were identified as unrelated
> after some checkings.

Hrm, what kind of noise?

> 	* config/rs6000/t-float128-hw(fp128_3_1_hw_funcs,
> 	fp128_3_1_hw_src, fp128_3_1_hw_static_obj, fp128_3_1_hw_shared_obj,
> 	fp128_3_1_hw_obj): Remove variables for ISA 3.1 support.

Needs a space before the opening paren.  Doesn't need a line break so
early on that line btw.

Just "Remove." or "Delete." is less confusing btw: what you wrote can be
read as "Remove the variables from these declarations" or similar.  And
of course terseness is usually best in a changelog.

> 	* config/rs6000/t-float128-p10-hw (FLOAT128_HW_INSNS): Append
> 	macro FLOAT128_HW_INSNS_ISA3_1 for ISA 3.1 support.

Don't say what it is for, just say what changed :-)

> 	(FP128_3_1_CFLAGS_HW): Fix option typo.
> 	* config/rs6000/float128-ifunc.c (SW_OR_HW_ISA3_1): Guarded
> 	with FLOAT128_HW_INSNS_ISA3_1.

"Guard", not "Guarded", all entries are written in the imperative, like,
"Do this" or "Guard that".

> +#ifdef FLOAT128_HW_INSNS_ISA3_1
>  TFtype __floattikf (TItype_ppc)
>    __attribute__ ((__ifunc__ ("__floattikf_resolve")));

I wonder if we now need TItype_ppc at all anymore, btw?

Okay for trunk with the changelog slightly massaged.  Thanks!


Segher

Re: [EXTERNAL] Re: rs6000: Fix typos in float128 ISA3.1 support

[Kewen.Lin]

Hi Segher,

Thanks for the prompt review!

on 2021/6/23 上午2:56, Segher Boessenkool wrote:
> Hi!
> 
> On Mon, Jun 21, 2021 at 05:27:06PM +0800, Kewen.Lin wrote:
>> Recently if we build gcc on Power with the assembler which doesn't
>> have Power10 support, the build will fail when building libgcc with
>> one error message like:
>>
>> Error: invalid switch -mpower10
>> Error: unrecognized option -mpower10
>> make[2]: *** [...gcc/gcc-base/libgcc/shared-object.mk:14: float128-p10.o] Error 1
> 
> In general, it is recommended to use a binutils of approximately the
> same age as the GCC you are trying to build.  This is similar to us not
> supporting most other non-sensical configurations.  An important reason
> for that is it cannot ever be tested, there are just too many strange
> combinations possible.
> 
> That said :-)
> 

Ah, ok!  It explained why no one reported this.  I only used the customized
binutils for Power10 build/testing before, will work with one new binutils
from now on.  It did have more testing coverage.

>> By checking the culprit commit r12-1340, it's caused by some typos.
> 
> (That is 9090f4807161.)
> 
>>   - fix test case used for libgcc_cv_powerpc_3_1_float128_hw check.
> 
> I was confused here for a bit, "test case" usually means something in
> testsuite/, I'd call this just "test" :-)
> 
>> BTW, there are some noises during regression testings due to
>> newer versions binutils, but they were identified as unrelated
>> after some checkings.
> 
> Hrm, what kind of noise?
> 

Some check selectors will fail without new binutils, those cases becomes
unsupported, like Power10 support checks.  The location of our pre-built
binutils in our system has not only binutils but also some other tools
like newer version gdb, which caused some differences such as for guality
cases.
>> 	* config/rs6000/t-float128-hw(fp128_3_1_hw_funcs,
>> 	fp128_3_1_hw_src, fp128_3_1_hw_static_obj, fp128_3_1_hw_shared_obj,
>> 	fp128_3_1_hw_obj): Remove variables for ISA 3.1 support.
> 
> Needs a space before the opening paren.  Doesn't need a line break so
> early on that line btw.
> 

Good catch, fixed.

> Just "Remove." or "Delete." is less confusing btw: what you wrote can be
> read as "Remove the variables from these declarations" or similar.  And
> of course terseness is usually best in a changelog.
> 

Fixed.

>> 	* config/rs6000/t-float128-p10-hw (FLOAT128_HW_INSNS): Append
>> 	macro FLOAT128_HW_INSNS_ISA3_1 for ISA 3.1 support.
> 
> Don't say what it is for, just say what changed :-)
> 

Fixed.

>> 	(FP128_3_1_CFLAGS_HW): Fix option typo.
>> 	* config/rs6000/float128-ifunc.c (SW_OR_HW_ISA3_1): Guarded
>> 	with FLOAT128_HW_INSNS_ISA3_1.
> 
> "Guard", not "Guarded", all entries are written in the imperative, like,
> "Do this" or "Guard that".
> 

Got it, fixed.

>> +#ifdef FLOAT128_HW_INSNS_ISA3_1
>>  TFtype __floattikf (TItype_ppc)
>>    __attribute__ ((__ifunc__ ("__floattikf_resolve")));
> 
> I wonder if we now need TItype_ppc at all anymore, btw?
> 

Sorry that I don't quite follow this question.

I think it stands for type signed int128, the function is to
convert signed int128 to float128, it would be needed here.
But I think that's not what you asked.  Or you are referring
to replace this type with signed int128 without shielding it
with mode?  If yes, it sounds like a question for Mike.

> Okay for trunk with the changelog slightly massaged.  Thanks!
> 

Thanks for catching changelog problems and the fix suggestion!

Fixed all of them accordingly and committed in r12-1738.


BR,
Kewen

Re: [EXTERNAL] Re: rs6000: Fix typos in float128 ISA3.1 support

[Segher Boessenkool]

On Wed, Jun 23, 2021 at 12:17:07PM +0800, Kewen.Lin wrote:
> >> +#ifdef FLOAT128_HW_INSNS_ISA3_1
> >>  TFtype __floattikf (TItype_ppc)
> >>    __attribute__ ((__ifunc__ ("__floattikf_resolve")));
> > 
> > I wonder if we now need TItype_ppc at all anymore, btw?
> 
> Sorry that I don't quite follow this question.

I thought it may do the same as just TItype now, but the ifunc stuff
probably makes it different still :-)


Segher

Re: [EXTERNAL] Re: rs6000: Fix typos in float128 ISA3.1 support

[Kewen.Lin]

on 2021/6/24 上午12:58, Segher Boessenkool wrote:
> On Wed, Jun 23, 2021 at 12:17:07PM +0800, Kewen.Lin wrote:
>>>> +#ifdef FLOAT128_HW_INSNS_ISA3_1
>>>>  TFtype __floattikf (TItype_ppc)
>>>>    __attribute__ ((__ifunc__ ("__floattikf_resolve")));
>>>
>>> I wonder if we now need TItype_ppc at all anymore, btw?
>>
>> Sorry that I don't quite follow this question.
> 
> I thought it may do the same as just TItype now, but the ifunc stuff
> probably makes it different still :-)
> 

Ah, thanks for the clarification!  If I read it right, TItype is defined
with __attribute__ ((mode (TI))) while TItype_ppc is defined with 
__attribute__ ((__mode__ (__TI__))), the later writing looks special.

BR,
Kewen

Re: [EXTERNAL] Re: rs6000: Fix typos in float128 ISA3.1 support

[Segher Boessenkool]

On Thu, Jun 24, 2021 at 05:32:20PM +0800, Kewen.Lin wrote:
> on 2021/6/24 上午12:58, Segher Boessenkool wrote:
> > On Wed, Jun 23, 2021 at 12:17:07PM +0800, Kewen.Lin wrote:
> >>>> +#ifdef FLOAT128_HW_INSNS_ISA3_1
> >>>>  TFtype __floattikf (TItype_ppc)
> >>>>    __attribute__ ((__ifunc__ ("__floattikf_resolve")));
> >>>
> >>> I wonder if we now need TItype_ppc at all anymore, btw?
> >>
> >> Sorry that I don't quite follow this question.
> > 
> > I thought it may do the same as just TItype now, but the ifunc stuff
> > probably makes it different still :-)
> 
> Ah, thanks for the clarification!  If I read it right, TItype is defined
> with __attribute__ ((mode (TI))) while TItype_ppc is defined with 
> __attribute__ ((__mode__ (__TI__))), the later writing looks special.

I managed to read things wrong, I thought there was some ifunc stuff in
the definition of TItype_ppc.  Of course there is not, it is just
setting the mode.

mode(__TI__) is just the more portable way of writing mode(TI), the
latter will not work if something #define's TI (you cannot do that with
__TI__, you are not allowed to by the C standard, in application code).

So it looks like we could just use {U,}TItype here, no _ppc.  Carl, is
there some reason I'm not seeing you used it?


Segher

Re: [EXTERNAL] Re: rs6000: Fix typos in float128 ISA3.1 support

[Kewen.Lin]

on 2021/6/25 上午3:36, Segher Boessenkool wrote:
> On Thu, Jun 24, 2021 at 05:32:20PM +0800, Kewen.Lin wrote:
>> on 2021/6/24 上午12:58, Segher Boessenkool wrote:
>>> On Wed, Jun 23, 2021 at 12:17:07PM +0800, Kewen.Lin wrote:
>>>>>> +#ifdef FLOAT128_HW_INSNS_ISA3_1
>>>>>>  TFtype __floattikf (TItype_ppc)
>>>>>>    __attribute__ ((__ifunc__ ("__floattikf_resolve")));
>>>>>
>>>>> I wonder if we now need TItype_ppc at all anymore, btw?
>>>>
>>>> Sorry that I don't quite follow this question.
>>>
>>> I thought it may do the same as just TItype now, but the ifunc stuff
>>> probably makes it different still :-)
>>
>> Ah, thanks for the clarification!  If I read it right, TItype is defined
>> with __attribute__ ((mode (TI))) while TItype_ppc is defined with 
>> __attribute__ ((__mode__ (__TI__))), the later writing looks special.
> 
> I managed to read things wrong, I thought there was some ifunc stuff in
> the definition of TItype_ppc.  Of course there is not, it is just
> setting the mode.
> 
> mode(__TI__) is just the more portable way of writing mode(TI), the
> latter will not work if something #define's TI (you cannot do that with
> __TI__, you are not allowed to by the C standard, in application code).
> 

Yeah, thanks for the note.  It looks better to update the generic
macro with this ppc style "__" writting and remove ppc one. :-)

One related bug PR101235 was just opened, I noticed the culprit commit
was backported to GCC11, is it OK to backport this fix to GCC 11 if
everything goes well in one more week?


BR,
Kewen

Re: [EXTERNAL] Re: rs6000: Fix typos in float128 ISA3.1 support

[Segher Boessenkool]

On Mon, Jun 28, 2021 at 04:15:15PM +0800, Kewen.Lin wrote:
> on 2021/6/25 上午3:36, Segher Boessenkool wrote:
> > mode(__TI__) is just the more portable way of writing mode(TI), the
> > latter will not work if something #define's TI (you cannot do that with
> > __TI__, you are not allowed to by the C standard, in application code).
> 
> Yeah, thanks for the note.  It looks better to update the generic
> macro with this ppc style "__" writting and remove ppc one. :-)
> 
> One related bug PR101235 was just opened, I noticed the culprit commit
> was backported to GCC11, is it OK to backport this fix to GCC 11 if
> everything goes well in one more week?

Please backport this immediately.  Thanks!


Segher