[PATCH-2,rs6000] Implement 32bit inline lrint [PR88558]
Checks
Commit Message
Hi,
This patch implements 32bit inline lrint by "fctiw". It depends on
the patch1 to do SImode move from FP register on P7.
Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
Thanks
Gui Haochen
ChangeLog
rs6000: support 32bit inline lrint
gcc/
PR target/88558
* config/rs6000/rs6000.md (lrint<mode>di2): Remove TARGET_FPRND
from insn condition.
(lrint<mode>si2): New insn pattern for 32bit lrint.
gcc/testsuite/
PR target/106769
* gcc.target/powerpc/pr88558.h: New.
* gcc.target/powerpc/pr88558-p7.c: New.
* gcc.target/powerpc/pr88558-p8v.c: New.
patch.diff
Comments
Hi Haochen,
on 2023/8/25 14:44, HAO CHEN GUI wrote:
> Hi,
> This patch implements 32bit inline lrint by "fctiw". It depends on
> the patch1 to do SImode move from FP register on P7.
>
> Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
>
> Thanks
> Gui Haochen
>
> ChangeLog
> rs6000: support 32bit inline lrint
>
> gcc/
> PR target/88558
> * config/rs6000/rs6000.md (lrint<mode>di2): Remove TARGET_FPRND
> from insn condition.
> (lrint<mode>si2): New insn pattern for 32bit lrint.
>
> gcc/testsuite/
> PR target/106769
> * gcc.target/powerpc/pr88558.h: New.
> * gcc.target/powerpc/pr88558-p7.c: New.
> * gcc.target/powerpc/pr88558-p8v.c: New.
>
> patch.diff
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index fd263e8dfe3..b36304de8c6 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -6655,10 +6655,18 @@ (define_insn "lrint<mode>di2"
> [(set (match_operand:DI 0 "gpc_reg_operand" "=d")
> (unspec:DI [(match_operand:SFDF 1 "gpc_reg_operand" "<rreg2>")]
> UNSPEC_FCTID))]
> - "TARGET_HARD_FLOAT && TARGET_FPRND"
> + "TARGET_HARD_FLOAT"
> "fctid %0,%1"
> [(set_attr "type" "fp")])
>
> +(define_insn "lrint<mode>si2"
> + [(set (match_operand:SI 0 "gpc_reg_operand" "=d")
> + (unspec:SI [(match_operand:SFDF 1 "gpc_reg_operand" "<rreg2>")]
> + UNSPEC_FCTIW))]
It surprises me that we have UNSPEC_FCTIW but it's unused before. :)
> + "TARGET_HARD_FLOAT && TARGET_POPCNTD"
> + "fctiw %0,%1"
> + [(set_attr "type" "fp")])
> +
> (define_insn "btrunc<mode>2"
> [(set (match_operand:SFDF 0 "gpc_reg_operand" "=d,wa")
> (unspec:SFDF [(match_operand:SFDF 1 "gpc_reg_operand" "d,wa")]
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr88558-p7.c b/gcc/testsuite/gcc.target/powerpc/pr88558-p7.c
> new file mode 100644
> index 00000000000..6437c55fa61
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr88558-p7.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-math-errno -mdejagnu-cpu=power7" } */
Nit: Maybe add one comment for why -fno-math-errno is needed,
such as: "-fno-math-errno is required to make {i,l,ll}rint inlined".
> +
> +#include "pr88558.h"
> +
> +/* { dg-final { scan-assembler-times {\mfctid\M} 2 { target lp64 } } } */
> +/* { dg-final { scan-assembler-times {\mfctid\M} 1 { target ilp32 } } } */
> +/* { dg-final { scan-assembler-times {\mfctiw\M} 1 { target lp64 } } } */
> +/* { dg-final { scan-assembler-times {\mfctiw\M} 2 { target ilp32 } } } */
> +/* { dg-final { scan-assembler-times {\mstfiwx\M} 1 } } */
Shouldn't we also expect different expected count for stfiwx for lp64 and ilp32?
1 for lp64 and 2 for ilp32? no?
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr88558-p8v.c b/gcc/testsuite/gcc.target/powerpc/pr88558-p8v.c
> new file mode 100644
> index 00000000000..fd22123ffb6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr88558-p8v.c
Nit: Maybe just name this with "-p8.c" instead of "-p8v.c".
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_p8vector_ok } */
> +/* { dg-options "-O2 -fno-math-errno -mdejagnu-cpu=power8" } */
> +
> +long int foo (double a)
> +{
> + return __builtin_lrint (a);
> +}
> +
> +long long bar (double a)
> +{
> + return __builtin_llrint (a);
> +}
> +
> +int baz (double a)
> +{
> + return __builtin_irint (a);
> +}
I think you want to use #include "pr88558.h" here, wrong revision?
> +
> +/* { dg-final { scan-assembler-times {\mfctid\M} 2 { target lp64 } } } */
> +/* { dg-final { scan-assembler-times {\mfctid\M} 1 { target ilp32 } } } */
> +/* { dg-final { scan-assembler-times {\mfctiw\M} 1 { target lp64 } } } */
> +/* { dg-final { scan-assembler-times {\mfctiw\M} 2 { target ilp32 } } } */
> +/* { dg-final { scan-assembler-times {\mmfvsrwz\M} 1 } } */
Similar question on mfvsrwz counts (to the above stfiwx).
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr88558.h b/gcc/testsuite/gcc.target/powerpc/pr88558.h
> new file mode 100644
> index 00000000000..0cc0c68dd4e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr88558.h
> @@ -0,0 +1,14 @@
> +long int foo (double a)
> +{
> + return __builtin_lrint (a);
> +}
> +
> +long long bar (double a)
> +{
> + return __builtin_llrint (a);
> +}
> +
> +int baz (double a)
> +{
> + return __builtin_irint (a);
> +}
>
The PR also mentioned lrintf, I think we can also add some cases for
the coverage?
BR,
Kewen
@@ -6655,10 +6655,18 @@ (define_insn "lrint<mode>di2"
[(set (match_operand:DI 0 "gpc_reg_operand" "=d")
(unspec:DI [(match_operand:SFDF 1 "gpc_reg_operand" "<rreg2>")]
UNSPEC_FCTID))]
- "TARGET_HARD_FLOAT && TARGET_FPRND"
+ "TARGET_HARD_FLOAT"
"fctid %0,%1"
[(set_attr "type" "fp")])
+(define_insn "lrint<mode>si2"
+ [(set (match_operand:SI 0 "gpc_reg_operand" "=d")
+ (unspec:SI [(match_operand:SFDF 1 "gpc_reg_operand" "<rreg2>")]
+ UNSPEC_FCTIW))]
+ "TARGET_HARD_FLOAT && TARGET_POPCNTD"
+ "fctiw %0,%1"
+ [(set_attr "type" "fp")])
+
(define_insn "btrunc<mode>2"
[(set (match_operand:SFDF 0 "gpc_reg_operand" "=d,wa")
(unspec:SFDF [(match_operand:SFDF 1 "gpc_reg_operand" "d,wa")]
new file mode 100644
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-math-errno -mdejagnu-cpu=power7" } */
+
+#include "pr88558.h"
+
+/* { dg-final { scan-assembler-times {\mfctid\M} 2 { target lp64 } } } */
+/* { dg-final { scan-assembler-times {\mfctid\M} 1 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\mfctiw\M} 1 { target lp64 } } } */
+/* { dg-final { scan-assembler-times {\mfctiw\M} 2 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\mstfiwx\M} 1 } } */
new file mode 100644
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-options "-O2 -fno-math-errno -mdejagnu-cpu=power8" } */
+
+long int foo (double a)
+{
+ return __builtin_lrint (a);
+}
+
+long long bar (double a)
+{
+ return __builtin_llrint (a);
+}
+
+int baz (double a)
+{
+ return __builtin_irint (a);
+}
+
+/* { dg-final { scan-assembler-times {\mfctid\M} 2 { target lp64 } } } */
+/* { dg-final { scan-assembler-times {\mfctid\M} 1 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\mfctiw\M} 1 { target lp64 } } } */
+/* { dg-final { scan-assembler-times {\mfctiw\M} 2 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times {\mmfvsrwz\M} 1 } } */
new file mode 100644
@@ -0,0 +1,14 @@
+long int foo (double a)
+{
+ return __builtin_lrint (a);
+}
+
+long long bar (double a)
+{
+ return __builtin_llrint (a);
+}
+
+int baz (double a)
+{
+ return __builtin_irint (a);
+}