public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH-2, rs6000] Implement 32bit inline lrint [PR88558]
@ 2023-08-25  6:44 HAO CHEN GUI
  2023-08-28  9:20 ` Kewen.Lin
  0 siblings, 1 reply; 2+ messages in thread
From: HAO CHEN GUI @ 2023-08-25  6:44 UTC (permalink / raw)
  To: gcc-patches; +Cc: Segher Boessenkool, David, Kewen.Lin, Peter Bergner

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))]
+  "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" } */
+
+#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 } } */
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
@@ -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 } } */
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);
+}




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

* Re: [PATCH-2, rs6000] Implement 32bit inline lrint [PR88558]
  2023-08-25  6:44 [PATCH-2, rs6000] Implement 32bit inline lrint [PR88558] HAO CHEN GUI
@ 2023-08-28  9:20 ` Kewen.Lin
  0 siblings, 0 replies; 2+ messages in thread
From: Kewen.Lin @ 2023-08-28  9:20 UTC (permalink / raw)
  To: HAO CHEN GUI; +Cc: Segher Boessenkool, David, Peter Bergner, gcc-patches

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

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

end of thread, other threads:[~2023-08-28  9:20 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-25  6:44 [PATCH-2, rs6000] Implement 32bit inline lrint [PR88558] HAO CHEN GUI
2023-08-28  9:20 ` Kewen.Lin

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