| Message ID | ca60bf46-6ec1-47f1-a5d9-b4fe94db47fe@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCH-4,rs6000] Implement optab_isnormal for SFmode, DFmode and TFmode [PR97786] | expand |
Hi! On Fri, Apr 12, 2024 at 04:24:23PM +0800, HAO CHEN GUI wrote: > This patch implemented optab_isnormal for SF/DF/TFmode by rs6000 test > data class instructions. > > This patch relies on former patch which adds optab_isnormal. > https://gcc.gnu.org/pipermail/gcc-patches/2024-April/649366.html > gcc/ > PR target/97786 > * config/rs6000/vsx.md (isnormal<mode>2): New expand for SFmode and > DFmode. * config/rs6000/vsx.md (isnormal<mode>2 for SFDF): New expand. (isnormal<mode>2 for IEEE128): New expand. > --- a/gcc/config/rs6000/vsx.md > +++ b/gcc/config/rs6000/vsx.md > @@ -5357,6 +5357,30 @@ (define_expand "isfinite<mode>2" > DONE; > }) > > +(define_expand "isnormal<mode>2" > + [(use (match_operand:SI 0 "gpc_reg_operand")) > + (use (match_operand:SFDF 1 "gpc_reg_operand"))] > + "TARGET_HARD_FLOAT > + && TARGET_P9_VECTOR" Please put the condition on just one line if it is as simple and short as this. Why is TARGET_P9_VECTOR the correct condition? > +{ > + rtx tmp = can_create_pseudo_p () ? gen_reg_rtx (SImode) : operands[0]; This is an expander. can_create_pseudo_p always return true. Please simplify the code, keeping that in mind :-) > +(define_expand "isnormal<mode>2" > + [(use (match_operand:SI 0 "gpc_reg_operand")) > + (use (match_operand:IEEE128 1 "gpc_reg_operand"))] > + "TARGET_HARD_FLOAT > + && TARGET_P9_VECTOR" > +{ > + rtx tmp = can_create_pseudo_p () ? gen_reg_rtx (SImode) : operands[0]; > + emit_insn (gen_xststdcqp_<mode> (tmp, operands[1], GEN_INT (0x7f))); > + emit_insn (gen_xorsi3 (operands[0], tmp, const1_rtx)); > + DONE; > +}) Same issues here, of course. > + Why add radom white lines? Pleaase don't. > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr97786-7.c > @@ -0,0 +1,16 @@ > +/* { dg-do compile } */ > +/* { dg-require-effective-target powerpc_vsx_ok } */ > +/* { dg-options "-O2 -mdejagnu-cpu=power9 -mvsx" } */ If you use a -mcpu=, don't use vsx_ok. If you use a -mcpu=, don't use -mvsx. > +int test1 (double x) > +{ > + return __builtin_isnormal (x); > +} > + > +int test2 (float x) > +{ > + return __builtin_isnormal (x); > +} > + > +/* { dg-final { scan-assembler-not {\mfcmpu\M} } } */ Just \mfcmp please (so that it also catches fcmpo, if we ever generate that). > +/* { dg-final { scan-assembler-times {\mxststdc[sd]p\M} 2 } } */ Maybe you should test for one each of the s and d version? So just /* { dg-final { scan-assembler-times {\mxststdcsp\M} 1 } } */ /* { dg-final { scan-assembler-times {\mxststdcdp\M} 1 } } */ > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr97786-8.c > @@ -0,0 +1,13 @@ > +/* { dg-do compile { target lp64 } } */ Why run this on 64-bit systems only? If there is a reason, document that here (but is there a reason?) > +/* { dg-require-effective-target ppc_float128_sw } */ > +/* { dg-require-effective-target powerpc_vsx_ok } */ > +/* { dg-options "-O2 -mdejagnu-cpu=power9 -mvsx -mabi=ieeelongdouble -Wno-psabi" } */ Same comments here: If you have a -mcpu you do not want vsx_ok or -mvsx. Please fix these things and resend. Thanks! Segher
Hi Segher, Thanks for your review comments. I will modify it and resend. Just one question on the insn condition. 在 2024/5/17 1:25, Segher Boessenkool 写道: >> +(define_expand "isnormal<mode>2" >> + [(use (match_operand:SI 0 "gpc_reg_operand")) >> + (use (match_operand:SFDF 1 "gpc_reg_operand"))] >> + "TARGET_HARD_FLOAT >> + && TARGET_P9_VECTOR" > Please put the condition on just one line if it is as simple and short > as this. > > Why is TARGET_P9_VECTOR the correct condition? This expand calls gen_xststdc<sd>p which is a P9 vector instruction and relies on "TARGET_P9_VECTOR". So I set the condition.
On Fri, May 17, 2024 at 10:38:54AM +0800, HAO CHEN GUI wrote: > This expand calls gen_xststdc<sd>p which is a P9 vector instruction and > relies on "TARGET_P9_VECTOR". So I set the condition. Why? It needs P9, sure, and MSR[VSX] set, but the operands being VSX registers takes care of that, heh. But it's fine, the insn patterns it uses use the same conditions already. Segher
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md index a6c72ae33b0..d1c9ef5447c 100644 --- a/gcc/config/rs6000/vsx.md +++ b/gcc/config/rs6000/vsx.md @@ -5357,6 +5357,30 @@ (define_expand "isfinite<mode>2" DONE; }) +(define_expand "isnormal<mode>2" + [(use (match_operand:SI 0 "gpc_reg_operand")) + (use (match_operand:SFDF 1 "gpc_reg_operand"))] + "TARGET_HARD_FLOAT + && TARGET_P9_VECTOR" +{ + rtx tmp = can_create_pseudo_p () ? gen_reg_rtx (SImode) : operands[0]; + emit_insn (gen_xststdc<sd>p (tmp, operands[1], GEN_INT (0x7f))); + emit_insn (gen_xorsi3 (operands[0], tmp, const1_rtx)); + DONE; +}) + +(define_expand "isnormal<mode>2" + [(use (match_operand:SI 0 "gpc_reg_operand")) + (use (match_operand:IEEE128 1 "gpc_reg_operand"))] + "TARGET_HARD_FLOAT + && TARGET_P9_VECTOR" +{ + rtx tmp = can_create_pseudo_p () ? gen_reg_rtx (SImode) : operands[0]; + emit_insn (gen_xststdcqp_<mode> (tmp, operands[1], GEN_INT (0x7f))); + emit_insn (gen_xorsi3 (operands[0], tmp, const1_rtx)); + DONE; +}) + ;; The VSX Scalar Test Negative Quad-Precision (define_expand "xststdcnegqp_<mode>" diff --git a/gcc/testsuite/gcc.target/powerpc/pr97786-7.c b/gcc/testsuite/gcc.target/powerpc/pr97786-7.c new file mode 100644 index 00000000000..a0d848497b9 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/pr97786-7.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target powerpc_vsx_ok } */ +/* { dg-options "-O2 -mdejagnu-cpu=power9 -mvsx" } */ + +int test1 (double x) +{ + return __builtin_isnormal (x); +} + +int test2 (float x) +{ + return __builtin_isnormal (x); +} + +/* { dg-final { scan-assembler-not {\mfcmpu\M} } } */ +/* { dg-final { scan-assembler-times {\mxststdc[sd]p\M} 2 } } */ diff --git a/gcc/testsuite/gcc.target/powerpc/pr97786-8.c b/gcc/testsuite/gcc.target/powerpc/pr97786-8.c new file mode 100644 index 00000000000..d591073d281 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/pr97786-8.c @@ -0,0 +1,13 @@ +/* { dg-do compile { target lp64 } } */ +/* { dg-require-effective-target ppc_float128_sw } */ +/* { dg-require-effective-target powerpc_vsx_ok } */ +/* { dg-options "-O2 -mdejagnu-cpu=power9 -mvsx -mabi=ieeelongdouble -Wno-psabi" } */ + +int test1 (long double x) +{ + return __builtin_isnormal (x); +} + + +/* { dg-final { scan-assembler-not {\mxscmpuqp\M} } } */ +/* { dg-final { scan-assembler {\mxststdcqp\M} } } */