diff mbox series

rs6000: Relax some FLOAT128 expander condition for FLOAT128_IEEE_P [PR105359]

Message ID 1d9e1052-e1c6-91f7-50e8-47cd0963212b@linux.ibm.com
State New
Headers show
Series rs6000: Relax some FLOAT128 expander condition for FLOAT128_IEEE_P [PR105359] | expand

Commit Message

Kewen.Lin July 17, 2024, 9:09 a.m. UTC 
Hi,

As PR105359 shows, we disable some FLOAT128 expanders for
64-bit long double, but in fact IEEE float128 types like
__ieee128 are only guarded with TARGET_FLOAT128_TYPE and
TARGET_LONG_DOUBLE_128 is only checked when determining if
we can reuse long_double_type_node.  So this patch is to
relax all affected FLOAT128 expander conditions for
FLOAT128_IEEE_P.  By the way, currently IBM double double
type __ibm128 is guarded by TARGET_LONG_DOUBLE_128, so we
have to use TARGET_LONG_DOUBLE_128 for it.  IMHO, it's not
necessary and can be enhanced later.

Btw, for all test cases mentioned in PR105359, I removed
the xfails and tested them with explicit -mlong-double-64,
both pr79004.c and float128-hw.c are tested well and
float128-hw4.c isn't tested (unsupported due to 64 bit
long double conflicts with -mabi=ieeelongdouble).

Bootstrapped and regtested on powerpc64-linux-gnu P8/P9
and powerpc64le-linux-gnu P9/P10.

I'm going to push this next week if no objections.

BR,
Kewen
-----
	PR target/105359

gcc/ChangeLog:

	* config/rs6000/rs6000.md (@extenddf<FLOAT128:mode>2): Remove condition
	TARGET_LONG_DOUBLE_128 for FLOAT128_IEEE_P modes.
	(extendsf<FLOAT128:mode>2): Likewise.
	(trunc<FLOAT128:mode>df2): Likewise.
	(trunc<FLOAT128:mode>sf2): Likewise.
	(floatsi<FLOAT128:mode>2): Likewise.
	(fix_trunc<FLOAT128:mode>si2): Likewise.

gcc/testsuite/ChangeLog:

	* gcc.target/powerpc/pr79004.c: Remove xfails.
---
 gcc/config/rs6000/rs6000.md                | 18 ++++++++++++------
 gcc/testsuite/gcc.target/powerpc/pr79004.c | 14 ++++++--------
 2 files changed, 18 insertions(+), 14 deletions(-)

--
2.39.1

Comments

Peter Bergner July 17, 2024, 1 p.m. UTC | #1 
On 7/17/24 4:09 AM, Kewen.Lin wrote:
> 	* config/rs6000/rs6000.md (@extenddf<FLOAT128:mode>2): Remove condition
> 	TARGET_LONG_DOUBLE_128 for FLOAT128_IEEE_P modes.

This all LGTM, except this ChangeLog fragment doesn't match the code changes
below.  Rather than removing TARGET_LONG_DOUBLE_128, you've added
FLOAT128_IEEE_P (<MODE>mode)).


> -  "TARGET_HARD_FLOAT && TARGET_LONG_DOUBLE_128"
> +  "TARGET_HARD_FLOAT
> +   && (TARGET_LONG_DOUBLE_128 || FLOAT128_IEEE_P (<MODE>mode))"


Peter
Kewen.Lin July 18, 2024, 5:19 a.m. UTC | #2 
Hi Peter,

on 2024/7/17 21:00, Peter Bergner wrote:
> On 7/17/24 4:09 AM, Kewen.Lin wrote:
>> 	* config/rs6000/rs6000.md (@extenddf<FLOAT128:mode>2): Remove condition
>> 	TARGET_LONG_DOUBLE_128 for FLOAT128_IEEE_P modes.
> 
> This all LGTM, except this ChangeLog fragment doesn't match the code changes
> below.  Rather than removing TARGET_LONG_DOUBLE_128, you've added
> FLOAT128_IEEE_P (<MODE>mode)).

Thanks for the comments!

For FLOAT128_IEEE_P modes, the previous condition is (TARGET_HARD_FLOAT &&
TARGET_LONG_DOUBLE_128), with this change the condition becomes to TARGET_HARD_FLOAT,
from this perspective it still matches.  I guess you meant "Remove" is expected to
remove some code explicitly and can be misleading here, if so how about "Don't check
TARGET_LONG_DOUBLE_128 for FLOAT128_IEEE_P modes"?

> 
> 
>> -  "TARGET_HARD_FLOAT && TARGET_LONG_DOUBLE_128"
>> +  "TARGET_HARD_FLOAT
>> +   && (TARGET_LONG_DOUBLE_128 || FLOAT128_IEEE_P (<MODE>mode))"
> 
> 
> Peter
> 

BR,
Kewen
Peter Bergner July 18, 2024, 1 p.m. UTC | #3 
On 7/18/24 12:19 AM, Kewen.Lin wrote:
> I guess you meant "Remove" is expected to remove some code explicitly and
> can be misleading here, if so how about "Don't check TARGET_LONG_DOUBLE_128
> for FLOAT128_IEEE_P modes"?

Yeah, I think that is more clear.  Thanks!

Peter
diff mbox series

Patch

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 276a5c9cf2d..c79858ba064 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -8845,7 +8845,8 @@  (define_insn_and_split "*mov<mode>_softfloat"
 (define_expand "@extenddf<mode>2"
   [(set (match_operand:FLOAT128 0 "gpc_reg_operand")
 	(float_extend:FLOAT128 (match_operand:DF 1 "gpc_reg_operand")))]
-  "TARGET_HARD_FLOAT && TARGET_LONG_DOUBLE_128"
+  "TARGET_HARD_FLOAT
+   && (TARGET_LONG_DOUBLE_128 || FLOAT128_IEEE_P (<MODE>mode))"
 {
   if (FLOAT128_IEEE_P (<MODE>mode))
     rs6000_expand_float128_convert (operands[0], operands[1], false);
@@ -8903,7 +8904,8 @@  (define_insn_and_split "@extenddf<mode>2_vsx"
 (define_expand "extendsf<mode>2"
   [(set (match_operand:FLOAT128 0 "gpc_reg_operand")
 	(float_extend:FLOAT128 (match_operand:SF 1 "gpc_reg_operand")))]
-  "TARGET_HARD_FLOAT && TARGET_LONG_DOUBLE_128"
+  "TARGET_HARD_FLOAT
+   && (TARGET_LONG_DOUBLE_128 || FLOAT128_IEEE_P (<MODE>mode))"
 {
   if (FLOAT128_IEEE_P (<MODE>mode))
     rs6000_expand_float128_convert (operands[0], operands[1], false);
@@ -8919,7 +8921,8 @@  (define_expand "extendsf<mode>2"
 (define_expand "trunc<mode>df2"
   [(set (match_operand:DF 0 "gpc_reg_operand")
 	(float_truncate:DF (match_operand:FLOAT128 1 "gpc_reg_operand")))]
-  "TARGET_HARD_FLOAT && TARGET_LONG_DOUBLE_128"
+  "TARGET_HARD_FLOAT
+   && (TARGET_LONG_DOUBLE_128 || FLOAT128_IEEE_P (<MODE>mode))"
 {
   if (FLOAT128_IEEE_P (<MODE>mode))
     {
@@ -8956,7 +8959,8 @@  (define_insn "trunc<mode>df2_internal2"
 (define_expand "trunc<mode>sf2"
   [(set (match_operand:SF 0 "gpc_reg_operand")
 	(float_truncate:SF (match_operand:FLOAT128 1 "gpc_reg_operand")))]
-  "TARGET_HARD_FLOAT && TARGET_LONG_DOUBLE_128"
+  "TARGET_HARD_FLOAT
+   && (TARGET_LONG_DOUBLE_128 || FLOAT128_IEEE_P (<MODE>mode))"
 {
   if (FLOAT128_IEEE_P (<MODE>mode))
     rs6000_expand_float128_convert (operands[0], operands[1], false);
@@ -8973,7 +8977,8 @@  (define_expand "floatsi<mode>2"
   [(parallel [(set (match_operand:FLOAT128 0 "gpc_reg_operand")
 		   (float:FLOAT128 (match_operand:SI 1 "gpc_reg_operand")))
 	      (clobber (match_scratch:DI 2))])]
-  "TARGET_HARD_FLOAT && TARGET_LONG_DOUBLE_128"
+  "TARGET_HARD_FLOAT
+   && (TARGET_LONG_DOUBLE_128 || FLOAT128_IEEE_P (<MODE>mode))"
 {
   rtx op0 = operands[0];
   rtx op1 = operands[1];
@@ -9009,7 +9014,8 @@  (define_insn "fix_trunc_helper<mode>"
 (define_expand "fix_trunc<mode>si2"
   [(set (match_operand:SI 0 "gpc_reg_operand")
 	(fix:SI (match_operand:FLOAT128 1 "gpc_reg_operand")))]
-  "TARGET_HARD_FLOAT && TARGET_LONG_DOUBLE_128"
+  "TARGET_HARD_FLOAT
+   && (TARGET_LONG_DOUBLE_128 || FLOAT128_IEEE_P (<MODE>mode))"
 {
   rtx op0 = operands[0];
   rtx op1 = operands[1];
diff --git a/gcc/testsuite/gcc.target/powerpc/pr79004.c b/gcc/testsuite/gcc.target/powerpc/pr79004.c
index 60c576cd36b..ac89a4c9f32 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr79004.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr79004.c
@@ -100,12 +100,10 @@  void to_uns_short_store_n (TYPE a, unsigned short *p, long n) { p[n] = (unsigned
 void to_uns_int_store_n (TYPE a, unsigned int *p, long n) { p[n] = (unsigned int)a; }
 void to_uns_long_store_n (TYPE a, unsigned long *p, long n) { p[n] = (unsigned long)a; }

-/* On targets with 64-bit long double, some opcodes to deal with __float128 are
-   disabled, see PR target/105359.  */
-/* { dg-final { scan-assembler-not {\mbl __}       { xfail longdouble64 } } } */
-/* { dg-final { scan-assembler     {\mxscvdpqp\M}  { xfail longdouble64 } } } */
-/* { dg-final { scan-assembler     {\mxscvqpdp\M}  { xfail longdouble64 } } } */
-/* { dg-final { scan-assembler     {\mxscvqpdpo\M} { xfail longdouble64 } } } */
+/* { dg-final { scan-assembler-not {\mbl __}       } } */
+/* { dg-final { scan-assembler     {\mxscvdpqp\M}  } } */
+/* { dg-final { scan-assembler     {\mxscvqpdp\M}  } } */
+/* { dg-final { scan-assembler     {\mxscvqpdpo\M} } } */
 /* { dg-final { scan-assembler     {\mxscvqpsdz\M} } } */
 /* { dg-final { scan-assembler     {\mxscvqpswz\M} } } */
 /* { dg-final { scan-assembler     {\mxscvsdqp\M}  } } */
@@ -113,7 +111,7 @@  void to_uns_long_store_n (TYPE a, unsigned long *p, long n) { p[n] = (unsigned l
 /* { dg-final { scan-assembler     {\mlxsd\M}      } } */
 /* { dg-final { scan-assembler     {\mlxsiwax\M}   } } */
 /* { dg-final { scan-assembler     {\mlxsiwzx\M}   } } */
-/* { dg-final { scan-assembler     {\mlxssp\M}     { xfail longdouble64 } } } */
+/* { dg-final { scan-assembler     {\mlxssp\M}     } } */
 /* { dg-final { scan-assembler     {\mstxsd\M}     } } */
 /* { dg-final { scan-assembler     {\mstxsiwx\M}   } } */
-/* { dg-final { scan-assembler     {\mstxssp\M}    { xfail longdouble64 } } } */
+/* { dg-final { scan-assembler     {\mstxssp\M}    } } */