[PATCH version 2, rs6000] Add builtins to convert from float/double to int/long using current rounding mode

[PATCH version 2, rs6000] Add builtins to convert from float/double to int/long using current rounding mode

[Carl Love]

GCC Maintainers:

The following patch adds support for a couple of requested builtins that
convert from float/double to int / long using the current rounding
mode.  I initially posted an early version of this patch which generated
redundant instructions.  

The patch has been tested on powerpc64le-unknown-linux-gnu (Power 8 LE).

Please let me know if the following patch is acceptable.  Thanks.

                        Carl Love
------------------------------------------------------------------

gcc/ChangeLog:

2017-09-08  Carl Love  <cel@us.ibm.com>

	* config/rs6000/rs6000-builtin.def (FCTID, FCTIW): Add BU_P7_MISC_1
	macro expansion for builtins.
	* config/rs6000/rs6000.md (fctid, fctiw): Add define_insn for the
	fctid and fctiw instructions.

gcc/testsuite/ChangeLog:

2017-09-08 Carl Love  <cel@us.ibm.com>
	* gcc.target/powerpc/builtin-fctid-fctiw-runnable.c: New test file
	for the __builtin_fctid and __builtin_fctiw builtins.
---
 gcc/config/rs6000/rs6000-builtin.def               |   2 +
 gcc/config/rs6000/rs6000.md                        |  16 +++
 .../powerpc/builtin-fctid-fctiw-runnable.c         | 140 +++++++++++++++++++++
 3 files changed, 158 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c

diff --git a/gcc/config/rs6000/rs6000-builtin.def b/gcc/config/rs6000/rs6000-builtin.def
index 850164a..c7983e9 100644
--- a/gcc/config/rs6000/rs6000-builtin.def
+++ b/gcc/config/rs6000/rs6000-builtin.def
@@ -2231,6 +2231,8 @@ BU_DFP_MISC_2 (DSCRIQ,		"dscriq",	CONST,	dfp_dscri_td)
 /* 1 argument BCD functions added in ISA 2.06.  */
 BU_P7_MISC_1 (CDTBCD,		"cdtbcd",	CONST,	cdtbcd)
 BU_P7_MISC_1 (CBCDTD,		"cbcdtd",	CONST,	cbcdtd)
+BU_P7_MISC_1 (FCTID,		"fctid",	CONST,  fctid)
+BU_P7_MISC_1 (FCTIW,		"fctiw",	CONST,	fctiw)
 
 /* 2 argument BCD functions added in ISA 2.06.  */
 BU_P7_MISC_2 (ADDG6S,		"addg6s",	CONST,	addg6s)
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 20873ac..3113fe7 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -115,6 +115,8 @@
    UNSPEC_CMPB
    UNSPEC_FCTIW
    UNSPEC_FCTID
+   UNSPEC_FCTID_INST
+   UNSPEC_FCTIW_INST
    UNSPEC_LFIWAX
    UNSPEC_LFIWZX
    UNSPEC_FCTIWUZ
@@ -14054,6 +14056,20 @@
   [(set_attr "type" "integer")
    (set_attr "length" "4")])
 
+(define_insn "fctid"
+  [(set (match_operand:DI 0 "gpc_reg_operand" "=d")
+        (unspec:DI [(match_operand:DF 1 "gpc_reg_operand" "d")]
+                   UNSPEC_FCTID_INST))]
+  ""
+  "fctid %0,%1")
+
+(define_insn "fctiw"
+  [(set (match_operand:SI 0 "gpc_reg_operand" "=d")
+	(unspec:SI [(match_operand:DF 1 "gpc_reg_operand" "d")]
+		   UNSPEC_FCTIW_INST))]
+  ""
+  "fctiw %0,%1")
+
 (define_int_iterator UNSPEC_DIV_EXTEND [UNSPEC_DIVE
 					UNSPEC_DIVEO
 					UNSPEC_DIVEU
diff --git a/gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c b/gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c
new file mode 100644
index 0000000..e6cf46f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c
@@ -0,0 +1,140 @@
+/* { dg-do run { target { powerpc64*-*-* && { lp64 && p8vector_hw } } } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-options "-mcpu=power8" } */
+
+#define DEBUG 1
+
+#ifdef DEBUG
+#include <stdio.h>
+#endif
+
+void abort (void);
+
+long
+test_bi_lrint_1 (float __A)
+{
+	return (__builtin_fctid (__A));
+}
+long
+test_bi_lrint_2 (double __A)
+{
+	return (__builtin_fctid (__A));
+}
+
+int
+test_bi_rint_1 (float __A)
+{
+	return (__builtin_fctiw (__A));
+}
+
+int
+test_bi_rint_2 (double __A)
+{
+	return (__builtin_fctiw (__A));
+}
+
+
+int main( void)
+{
+  signed long lx, expected_l;
+  double dy;
+
+  signed int x, expected_i;
+  float y;
+  
+  dy = 1.45;
+  expected_l = 1;
+  lx = __builtin_fctid (dy);
+
+  if( lx != expected_l)
+#ifdef DEBUG
+    printf("ERROR: __builtin_fctid(dy= %f) = %ld, expected %ld\n",
+	   dy, lx, expected_l);
+#else
+    abort();
+#endif
+
+  dy = 3.51;
+  expected_l = 4;
+  lx = __builtin_fctid (dy);
+  
+  if( lx != expected_l)
+#ifdef DEBUG
+    printf("ERROR: __builtin_fctid(dy= %f) = %ld, expected %ld\n",
+	   dy, lx, expected_l);
+#else
+    abort();
+#endif
+
+  dy = 5.57;
+  expected_i = 6;
+  x = __builtin_fctiw (dy);
+
+  if( x != expected_i)
+#ifdef DEBUG
+    printf("ERROR: __builtin_fctiw(dy= %f) = %d, expected %d\n",
+	   dy, x, expected_i);
+#else
+    abort();
+#endif
+
+  y = 11.47;
+  expected_i = 11;
+  x = __builtin_fctiw (y);
+
+  if( x != expected_i)
+#ifdef DEBUG
+    printf("ERROR: __builtin_fctiw(y = %f) = %d, expected %d\n",
+	   y, x, expected_i);
+#else
+    abort();
+#endif
+
+  y = 17.77;
+  expected_l = 18;
+  lx = test_bi_lrint_1 (y);
+
+  if( lx != expected_l)
+#ifdef DEBUG
+    printf("ERROR: function call test_bi_lrint_1 (y = %f) = %ld, expected %ld\n",
+	   y, lx, expected_l);
+#else
+    abort();
+#endif
+
+  dy = 7.1;
+  expected_l = 7;
+  lx = test_bi_lrint_2 (dy);
+
+  if( lx != expected_l)
+#ifdef DEBUG
+    printf("ERROR: function call test_bi_lrint_2 (dy = %f) = %ld, expected %ld\n",
+	   dy, lx, expected_l);
+#else
+    abort();
+#endif
+
+  y = 0.001;
+  expected_i = 0;
+  x = test_bi_rint_1 (y);
+
+  if( x != expected_i)
+#ifdef DEBUG
+    printf("ERROR: function call test_bi_rint_1 (y = %f) = %d, expected %d\n",
+	   y, x, expected_i);
+#else
+    abort();
+#endif
+  
+  dy = 0.9999;
+  expected_i = 1;
+  x = test_bi_rint_2 (dy);
+
+  if( x != expected_i)
+#ifdef DEBUG
+    printf("ERROR: function call test_bi_rint_2 (dy = %f) = %d, expected %d\n",
+	   dy, x, expected_i);
+#else
+    abort();
+#endif
+}
-- 
1.9.1

Re: [PATCH version 2, rs6000] Add builtins to convert from float/double to int/long using current rounding mode

[Pat Haugen]

On 09/08/2017 01:41 PM, Carl Love wrote:
> +(define_insn "fctid"
> +  [(set (match_operand:DI 0 "gpc_reg_operand" "=d")
> +        (unspec:DI [(match_operand:DF 1 "gpc_reg_operand" "d")]
> +                   UNSPEC_FCTID_INST))]
> +  ""
> +  "fctid %0,%1")
> +
> +(define_insn "fctiw"
> +  [(set (match_operand:SI 0 "gpc_reg_operand" "=d")
> +	(unspec:SI [(match_operand:DF 1 "gpc_reg_operand" "d")]
> +		   UNSPEC_FCTIW_INST))]
> +  ""
> +  "fctiw %0,%1")

These should have (set_attr "type" "fp").

-Pat

Re: [PATCH version 2, rs6000] Add builtins to convert from float/double to int/long using current rounding mode

[Segher Boessenkool]

Hi Carl,

On Fri, Sep 08, 2017 at 11:41:13AM -0700, Carl Love wrote:
> The following patch adds support for a couple of requested builtins that
> convert from float/double to int / long using the current rounding
> mode.  I initially posted an early version of this patch which generated
> redundant instructions.  

> 2017-09-08  Carl Love  <cel@us.ibm.com>
> 
> 	* config/rs6000/rs6000-builtin.def (FCTID, FCTIW): Add BU_P7_MISC_1
> 	macro expansion for builtins.
> 	* config/rs6000/rs6000.md (fctid, fctiw): Add define_insn for the
> 	fctid and fctiw instructions.

There already is fctiwz_<mode> for SF, DF; how do these relate?  Could they
be joined?  Similar for lrint<mode>di2.

Or do we really want fcti* here instead of potentially the VSX equivalent?

> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c
> @@ -0,0 +1,140 @@
> +/* { dg-do run { target { powerpc64*-*-* && { lp64 && p8vector_hw } } } } */

powerpc*-*-*


Segher

Re: [PATCH version 2, rs6000] Add builtins to convert from float/double to int/long using current rounding mode

[Carl Love]

On Tue, 2017-09-12 at 11:49 -0500, Segher Boessenkool wrote:
> Hi Carl,
> 
> On Fri, Sep 08, 2017 at 11:41:13AM -0700, Carl Love wrote:
> > The following patch adds support for a couple of requested builtins that
> > convert from float/double to int / long using the current rounding
> > mode.  I initially posted an early version of this patch which generated
> > redundant instructions.  
> 
> > 2017-09-08  Carl Love  <cel@us.ibm.com>
> > 
> > 	* config/rs6000/rs6000-builtin.def (FCTID, FCTIW): Add BU_P7_MISC_1
> > 	macro expansion for builtins.
> > 	* config/rs6000/rs6000.md (fctid, fctiw): Add define_insn for the
> > 	fctid and fctiw instructions.
> 
> There already is fctiwz_<mode> for SF, DF; how do these relate?  Could they
> be joined?  Similar for lrint<mode>di2.
Segher:

The requirement from Steve Munroe for the builtins was to have the
conversions round using the current rounding mode.  Hence we need fctiw
not fctiwz which only rounds to zero.

I looked at the lrint<mode>di2 and for mode=df it is identical to my
fctid code.  So, I removed my define_insn "fctid" and just mapped the
builtin to lrintdfdi2.

Pat:
>> +  ""
>> +  "fctiw %0,%1")

>These should have (set_attr "type" "fp").

Fixed that thanks.

I retested the updated patch powerpc64le-unknown-linux-gnu (Power 8 LE)
only.

Here is the updated patch.  Let me know if it still needs any work.
Thanks.

                       Carl Love
----------------------------------------------------------------------

gcc/ChangeLog:

2017-09-12  Carl Love  <cel@us.ibm.com>

	* config/rs6000/rs6000-builtin.def (FCTID, FCTIW): Add BU_P7_MISC_1
	macro expansion for builtins.
	* config/rs6000/rs6000.md (fctid, fctiw): Add define_insn for the
	fctiw instruction.

gcc/testsuite/ChangeLog:

2017-09-12 Carl Love  <cel@us.ibm.com>
	* gcc.target/powerpc/builtin-fctid-fctiw-runnable.c: New test file
	for the __builtin_fctid and __builtin_fctiw builtins.
---
 gcc/config/rs6000/rs6000-builtin.def               |   2 +
 gcc/config/rs6000/rs6000.md                        |   8 ++
 .../powerpc/builtin-fctid-fctiw-runnable.c         | 138 +++++++++++++++++++++
 3 files changed, 148 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c

diff --git a/gcc/config/rs6000/rs6000-builtin.def b/gcc/config/rs6000/rs6000-builtin.def
index 850164a..e85e1b3 100644
--- a/gcc/config/rs6000/rs6000-builtin.def
+++ b/gcc/config/rs6000/rs6000-builtin.def
@@ -2231,6 +2231,8 @@ BU_DFP_MISC_2 (DSCRIQ,		"dscriq",	CONST,	dfp_dscri_td)
 /* 1 argument BCD functions added in ISA 2.06.  */
 BU_P7_MISC_1 (CDTBCD,		"cdtbcd",	CONST,	cdtbcd)
 BU_P7_MISC_1 (CBCDTD,		"cbcdtd",	CONST,	cbcdtd)
+BU_P7_MISC_1 (FCTID,		"fctid",	CONST,  lrintdfdi2)
+BU_P7_MISC_1 (FCTIW,		"fctiw",	CONST,	fctiw)
 
 /* 2 argument BCD functions added in ISA 2.06.  */
 BU_P7_MISC_2 (ADDG6S,		"addg6s",	CONST,	addg6s)
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 20873ac..237be24 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -14054,6 +14054,14 @@
   [(set_attr "type" "integer")
    (set_attr "length" "4")])
 
+(define_insn "fctiw"
+  [(set (match_operand:SI 0 "gpc_reg_operand" "=d")
+	(unspec:SI [(match_operand:DF 1 "gpc_reg_operand" "d")]
+		   UNSPEC_FCTIW))]
+  ""
+  "fctiw %0,%1"
+  [(set_attr "type" "fp")])
+
 (define_int_iterator UNSPEC_DIV_EXTEND [UNSPEC_DIVE
 					UNSPEC_DIVEO
 					UNSPEC_DIVEU
diff --git a/gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c b/gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c
new file mode 100644
index 0000000..5a0e20e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c
@@ -0,0 +1,138 @@
+/* { dg-do run { target { powerpc*-*-* && { lp64 && p8vector_hw } } } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-options "-mcpu=power8" } */
+
+#ifdef DEBUG
+#include <stdio.h>
+#endif
+
+void abort (void);
+
+long
+test_bi_lrint_1 (float __A)
+{
+	return (__builtin_fctid (__A));
+}
+long
+test_bi_lrint_2 (double __A)
+{
+	return (__builtin_fctid (__A));
+}
+
+int
+test_bi_rint_1 (float __A)
+{
+	return (__builtin_fctiw (__A));
+}
+
+int
+test_bi_rint_2 (double __A)
+{
+	return (__builtin_fctiw (__A));
+}
+
+
+int main( void)
+{
+  signed long lx, expected_l;
+  double dy;
+
+  signed int x, expected_i;
+  float y;
+  
+  dy = 1.45;
+  expected_l = 1;
+  lx = __builtin_fctid (dy);
+
+  if( lx != expected_l)
+#ifdef DEBUG
+    printf("ERROR: __builtin_fctid(dy= %f) = %ld, expected %ld\n",
+	   dy, lx, expected_l);
+#else
+    abort();
+#endif
+
+  dy = 3.51;
+  expected_l = 4;
+  lx = __builtin_fctid (dy);
+  
+  if( lx != expected_l)
+#ifdef DEBUG
+    printf("ERROR: __builtin_fctid(dy= %f) = %ld, expected %ld\n",
+	   dy, lx, expected_l);
+#else
+    abort();
+#endif
+
+  dy = 5.57;
+  expected_i = 6;
+  x = __builtin_fctiw (dy);
+
+  if( x != expected_i)
+#ifdef DEBUG
+    printf("ERROR: __builtin_fctiw(dy= %f) = %d, expected %d\n",
+	   dy, x, expected_i);
+#else
+    abort();
+#endif
+
+  y = 11.47;
+  expected_i = 11;
+  x = __builtin_fctiw (y);
+
+  if( x != expected_i)
+#ifdef DEBUG
+    printf("ERROR: __builtin_fctiw(y = %f) = %d, expected %d\n",
+	   y, x, expected_i);
+#else
+    abort();
+#endif
+
+  y = 17.77;
+  expected_l = 18;
+  lx = test_bi_lrint_1 (y);
+
+  if( lx != expected_l)
+#ifdef DEBUG
+    printf("ERROR: function call test_bi_lrint_1 (y = %f) = %ld, expected %ld\n",
+	   y, lx, expected_l);
+#else
+    abort();
+#endif
+
+  dy = 7.1;
+  expected_l = 7;
+  lx = test_bi_lrint_2 (dy);
+
+  if( lx != expected_l)
+#ifdef DEBUG
+    printf("ERROR: function call test_bi_lrint_2 (dy = %f) = %ld, expected %ld\n",
+	   dy, lx, expected_l);
+#else
+    abort();
+#endif
+
+  y = 0.001;
+  expected_i = 0;
+  x = test_bi_rint_1 (y);
+
+  if( x != expected_i)
+#ifdef DEBUG
+    printf("ERROR: function call test_bi_rint_1 (y = %f) = %d, expected %d\n",
+	   y, x, expected_i);
+#else
+    abort();
+#endif
+  
+  dy = 0.9999;
+  expected_i = 1;
+  x = test_bi_rint_2 (dy);
+
+  if( x != expected_i)
+#ifdef DEBUG
+    printf("ERROR: function call test_bi_rint_2 (dy = %f) = %d, expected %d\n",
+	   dy, x, expected_i);
+#else
+    abort();
+#endif
+}
-- 
1.9.1

Re: [PATCH version 2, rs6000] Add builtins to convert from float/double to int/long using current rounding mode

[Segher Boessenkool]

On Tue, Sep 12, 2017 at 02:07:43PM -0700, Carl Love wrote:
> On Tue, 2017-09-12 at 11:49 -0500, Segher Boessenkool wrote:
> > On Fri, Sep 08, 2017 at 11:41:13AM -0700, Carl Love wrote:
> > > The following patch adds support for a couple of requested builtins that
> > > convert from float/double to int / long using the current rounding
> > > mode.  I initially posted an early version of this patch which generated
> > > redundant instructions.  
> > 
> > > 2017-09-08  Carl Love  <cel@us.ibm.com>
> > > 
> > > 	* config/rs6000/rs6000-builtin.def (FCTID, FCTIW): Add BU_P7_MISC_1
> > > 	macro expansion for builtins.
> > > 	* config/rs6000/rs6000.md (fctid, fctiw): Add define_insn for the
> > > 	fctid and fctiw instructions.
> > 
> > There already is fctiwz_<mode> for SF, DF; how do these relate?  Could they
> > be joined?  Similar for lrint<mode>di2.
> Segher:
> 
> The requirement from Steve Munroe for the builtins was to have the
> conversions round using the current rounding mode.  Hence we need fctiw
> not fctiwz which only rounds to zero.

Ah, okay, I completely missed that "z" distinction.  I'm not actually
awake it seems.  Sorry.

> I looked at the lrint<mode>di2 and for mode=df it is identical to my
> fctid code.  So, I removed my define_insn "fctid" and just mapped the
> builtin to lrintdfdi2.

Great!  Maybe name the other one similarly, if that makes sense?  And
then move them together too.

Some questions below.  Mostly nitpicking :-)

> 	* config/rs6000/rs6000.md (fctid, fctiw): Add define_insn for the
> 	fctiw instruction.

This needs updating now.

> diff --git a/gcc/config/rs6000/rs6000-builtin.def b/gcc/config/rs6000/rs6000-builtin.def
> index 850164a..e85e1b3 100644
> --- a/gcc/config/rs6000/rs6000-builtin.def
> +++ b/gcc/config/rs6000/rs6000-builtin.def
> @@ -2231,6 +2231,8 @@ BU_DFP_MISC_2 (DSCRIQ,		"dscriq",	CONST,	dfp_dscri_td)
>  /* 1 argument BCD functions added in ISA 2.06.  */
>  BU_P7_MISC_1 (CDTBCD,		"cdtbcd",	CONST,	cdtbcd)
>  BU_P7_MISC_1 (CBCDTD,		"cbcdtd",	CONST,	cbcdtd)
> +BU_P7_MISC_1 (FCTID,		"fctid",	CONST,  lrintdfdi2)

You have spaces instead of a tab before lrintdfdi2 here.

> +BU_P7_MISC_1 (FCTIW,		"fctiw",	CONST,	fctiw)

Also, those two shouldn't be below the "1 arg BCD" heading; they also aren't
ISA 2.06 (the instructions are in ISA 1 already; the builtins are new).

> +(define_insn "fctiw"
> +  [(set (match_operand:SI 0 "gpc_reg_operand" "=d")
> +	(unspec:SI [(match_operand:DF 1 "gpc_reg_operand" "d")]
> +		   UNSPEC_FCTIW))]
> +  ""

This needs "TARGET_HARD_FLOAT && TARGET_DOUBLE_FLOAT" I think?  Which
is the same as "TARGET_DF_FPR".  "lrint<mode>di2" also has "TARGET_FPRND"
but that is a mistake I think.  Cc:ing Mike for that.

The rest looks fine.  Okay for trunk with those changes, or resend if
you prefer I take another look.  Thanks!


Segher

Re: [PATCH version 2, rs6000] Add builtins to convert from float/double to int/long using current rounding mode

[Michael Meissner]

On Tue, Sep 12, 2017 at 05:41:34PM -0500, Segher Boessenkool wrote:
> This needs "TARGET_HARD_FLOAT && TARGET_DOUBLE_FLOAT" I think?  Which
> is the same as "TARGET_DF_FPR".  "lrint<mode>di2" also has "TARGET_FPRND"
> but that is a mistake I think.  Cc:ing Mike for that.

TARGET_FPRND is for ISA 2.02 (power5+) which added the various round to integer
instructions.  So, unless we are going to stop supporting older machines, it is
needed.

> The rest looks fine.  Okay for trunk with those changes, or resend if
> you prefer I take another look.  Thanks!

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

Re: [PATCH version 2, rs6000] Add builtins to convert from float/double to int/long using current rounding mode

[Segher Boessenkool]

On Tue, Sep 12, 2017 at 07:17:07PM -0400, Michael Meissner wrote:
> On Tue, Sep 12, 2017 at 05:41:34PM -0500, Segher Boessenkool wrote:
> > This needs "TARGET_HARD_FLOAT && TARGET_DOUBLE_FLOAT" I think?  Which
> > is the same as "TARGET_DF_FPR".  "lrint<mode>di2" also has "TARGET_FPRND"
> > but that is a mistake I think.  Cc:ing Mike for that.
> 
> TARGET_FPRND is for ISA 2.02 (power5+) which added the various round to integer
> instructions.  So, unless we are going to stop supporting older machines, it is
> needed.

I think you have this a bit wrong?

ISA 2.02 added fri*, but that's round to integer, not convert.

ISA 2.06 added some fc[ft]i* instructions, but most are in base PowerPC
already.

And FPRND is enabled at ISA 2.04 and later?


Segher

Re: [PATCH version 3, rs6000] Add builtins to convert from float/double to int/long using current rounding mode

[Carl Love]

GCC maintainers:

The following patch has been updated to address Segher's comments.

-- rename define insn "fctiw" to  define_insn "lrintsfsi2" to match the
fctid implementation

-- add "TARGET_SF_FPR && TARGET_FPRND" to the define_insn "lrintsfsi2"
as mentioned it was missing on the original define_insn for fctiw.

>> +BU_P7_MISC_1 (FCTIW,         "fctiw",        CONST,  fctiw)

> Also, those two shouldn't be below the "1 arg BCD" heading; they also
> aren't ISA 2.06 (the instructions are in ISA 1 already; the builtins
> are new).

I looked and really couldn't find a macro that really fit for these
builtins.  I added a new macro for miscellaneous pre ISA 2.04 builtins.
I called it BU_P1_MISC_1 for lack of a better name.  Open to suggestions
on a better name.  I really don't know what the ISA numbers are prior to
2.04 so called it P1 to catch all builtins prior to isa 2.04.

Let me know if you have suggestions/comments.  Thanks.

                     Carl Love


-------------------------------------------------------------------------------------

gcc/ChangeLog:

2017-09-14  Carl Love  <cel@us.ibm.com>

	* config/rs6000/rs6000-builtin.def (BU_P1MISC_1): Add define macro.
	(FCTID, FCTIW): Add BU_P1_MISC_1
	macro expansion for builtins.
	* config/rs6000/rs6000.md (lrintsfsi2): Add define_insn for the
	fctiw instruction.

gcc/testsuite/ChangeLog:

2017-09-14 Carl Love  <cel@us.ibm.com>
	* gcc.target/powerpc/builtin-fctid-fctiw-runnable.c: New test file
	for the __builtin_fctid and __builtin_fctiw builtins.
---
 gcc/config/rs6000/rs6000-builtin.def               |  14 +++
 gcc/config/rs6000/rs6000.md                        |   8 ++
 .../powerpc/builtin-fctid-fctiw-runnable.c         | 138 +++++++++++++++++++++
 3 files changed, 160 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c

diff --git a/gcc/config/rs6000/rs6000-builtin.def b/gcc/config/rs6000/rs6000-builtin.def
index 850164a..6308009 100644
--- a/gcc/config/rs6000/rs6000-builtin.def
+++ b/gcc/config/rs6000/rs6000-builtin.def
@@ -608,6 +608,16 @@
 		    CODE_FOR_ ## ICODE)			/* ICODE */
 
 
+/* Miscellaneous builtins for instructions added prior to ISA 2.04.  These
+  operate on general purpose registers.  */
+#define BU_P1_MISC_1(ENUM, NAME, ATTR, ICODE)				\
+  RS6000_BUILTIN_1 (MISC_BUILTIN_ ## ENUM,		/* ENUM */	\
+		    "__builtin_" NAME,			/* NAME */	\
+		    RS6000_BTM_ALWAYS,			/* MASK */	\
+		    (RS6000_BTC_ ## ATTR		/* ATTR */	\
+		     | RS6000_BTC_UNARY),				\
+		    CODE_FOR_ ## ICODE)			/* ICODE */
+
 /* Miscellaneous builtins for instructions added in ISA 2.06.  These
    instructions don't require either the DFP or VSX options, just the basic ISA
    2.06 (popcntd) enablement since they operate on general purpose
@@ -1846,6 +1856,10 @@ BU_VSX_OVERLOAD_X (XL,	     "xl")
 BU_VSX_OVERLOAD_X (XL_BE,    "xl_be")
 BU_VSX_OVERLOAD_X (XST,	     "xst")
 \f
+/* 1 argument builtins pre ISA 2.04.  */
+BU_P1_MISC_1 (FCTID,		"fctid",	CONST,  lrintdfdi2)
+BU_P1_MISC_1 (FCTIW,		"fctiw",	CONST,	lrintsfsi2)
+
 /* 2 argument CMPB instructions added in ISA 2.05. */
 BU_P6_2 (CMPB_32,        "cmpb_32",	CONST,	cmpbsi3)
 BU_P6_64BIT_2 (CMPB,     "cmpb",	CONST,	cmpbdi3)
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 20873ac..ad39dc9 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -5899,6 +5899,14 @@
   [(set_attr "type" "fpload")
    (set_attr "length" "16")])
 
+(define_insn "lrintsfsi2"
+  [(set (match_operand:SI 0 "gpc_reg_operand" "=d")
+	(unspec:SI [(match_operand:DF 1 "gpc_reg_operand" "d")]
+		   UNSPEC_FCTIW))]
+  "TARGET_SF_FPR && TARGET_FPRND"
+  "fctiw %0,%1"
+  [(set_attr "type" "fp")])
+
 ;; No VSX equivalent to fctid
 (define_insn "lrint<mode>di2"
   [(set (match_operand:DI 0 "gpc_reg_operand" "=d")
diff --git a/gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c b/gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c
new file mode 100644
index 0000000..5a0e20e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c
@@ -0,0 +1,138 @@
+/* { dg-do run { target { powerpc*-*-* && { lp64 && p8vector_hw } } } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-options "-mcpu=power8" } */
+
+#ifdef DEBUG
+#include <stdio.h>
+#endif
+
+void abort (void);
+
+long
+test_bi_lrint_1 (float __A)
+{
+	return (__builtin_fctid (__A));
+}
+long
+test_bi_lrint_2 (double __A)
+{
+	return (__builtin_fctid (__A));
+}
+
+int
+test_bi_rint_1 (float __A)
+{
+	return (__builtin_fctiw (__A));
+}
+
+int
+test_bi_rint_2 (double __A)
+{
+	return (__builtin_fctiw (__A));
+}
+
+
+int main( void)
+{
+  signed long lx, expected_l;
+  double dy;
+
+  signed int x, expected_i;
+  float y;
+  
+  dy = 1.45;
+  expected_l = 1;
+  lx = __builtin_fctid (dy);
+
+  if( lx != expected_l)
+#ifdef DEBUG
+    printf("ERROR: __builtin_fctid(dy= %f) = %ld, expected %ld\n",
+	   dy, lx, expected_l);
+#else
+    abort();
+#endif
+
+  dy = 3.51;
+  expected_l = 4;
+  lx = __builtin_fctid (dy);
+  
+  if( lx != expected_l)
+#ifdef DEBUG
+    printf("ERROR: __builtin_fctid(dy= %f) = %ld, expected %ld\n",
+	   dy, lx, expected_l);
+#else
+    abort();
+#endif
+
+  dy = 5.57;
+  expected_i = 6;
+  x = __builtin_fctiw (dy);
+
+  if( x != expected_i)
+#ifdef DEBUG
+    printf("ERROR: __builtin_fctiw(dy= %f) = %d, expected %d\n",
+	   dy, x, expected_i);
+#else
+    abort();
+#endif
+
+  y = 11.47;
+  expected_i = 11;
+  x = __builtin_fctiw (y);
+
+  if( x != expected_i)
+#ifdef DEBUG
+    printf("ERROR: __builtin_fctiw(y = %f) = %d, expected %d\n",
+	   y, x, expected_i);
+#else
+    abort();
+#endif
+
+  y = 17.77;
+  expected_l = 18;
+  lx = test_bi_lrint_1 (y);
+
+  if( lx != expected_l)
+#ifdef DEBUG
+    printf("ERROR: function call test_bi_lrint_1 (y = %f) = %ld, expected %ld\n",
+	   y, lx, expected_l);
+#else
+    abort();
+#endif
+
+  dy = 7.1;
+  expected_l = 7;
+  lx = test_bi_lrint_2 (dy);
+
+  if( lx != expected_l)
+#ifdef DEBUG
+    printf("ERROR: function call test_bi_lrint_2 (dy = %f) = %ld, expected %ld\n",
+	   dy, lx, expected_l);
+#else
+    abort();
+#endif
+
+  y = 0.001;
+  expected_i = 0;
+  x = test_bi_rint_1 (y);
+
+  if( x != expected_i)
+#ifdef DEBUG
+    printf("ERROR: function call test_bi_rint_1 (y = %f) = %d, expected %d\n",
+	   y, x, expected_i);
+#else
+    abort();
+#endif
+  
+  dy = 0.9999;
+  expected_i = 1;
+  x = test_bi_rint_2 (dy);
+
+  if( x != expected_i)
+#ifdef DEBUG
+    printf("ERROR: function call test_bi_rint_2 (dy = %f) = %d, expected %d\n",
+	   dy, x, expected_i);
+#else
+    abort();
+#endif
+}
-- 
1.9.1

Re: [PATCH version 3, rs6000] Add builtins to convert from float/double to int/long using current rounding mode

[Segher Boessenkool]

Hi Carl,

On Wed, Sep 13, 2017 at 04:29:01PM -0700, Carl Love wrote:
> -- add "TARGET_SF_FPR && TARGET_FPRND" to the define_insn "lrintsfsi2"
> as mentioned it was missing on the original define_insn for fctiw.

I don't think TARGET_FPRND is correct: this instruction is in the original
PowerPC specification already.  As noted, this is wrong in more patterns,
we can deal with all of them at once later.

> I looked and really couldn't find a macro that really fit for these
> builtins.  I added a new macro for miscellaneous pre ISA 2.04 builtins.
> I called it BU_P1_MISC_1 for lack of a better name.  Open to suggestions
> on a better name.  I really don't know what the ISA numbers are prior to
> 2.04 so called it P1 to catch all builtins prior to isa 2.04.

P7 stands for "POWER7" here, not some ISA version (and POWER1 would be
incorrect, fctiw was introduced on POWER2, and fctid on PowerPC).

BU_FP_1 maybe?

> +#define BU_P1_MISC_1(ENUM, NAME, ATTR, ICODE)				\
> +  RS6000_BUILTIN_1 (MISC_BUILTIN_ ## ENUM,		/* ENUM */	\
> +		    "__builtin_" NAME,			/* NAME */	\
> +		    RS6000_BTM_ALWAYS,			/* MASK */	\
> +		    (RS6000_BTC_ ## ATTR		/* ATTR */	\
> +		     | RS6000_BTC_UNARY),				\
> +		    CODE_FOR_ ## ICODE)			/* ICODE */

I wonder if this needs some test for hard float (try to build a testcase
with -msoft-float, what happens?  Pretty much anything that isn't an ICE
is fine).

> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c
> @@ -0,0 +1,138 @@
> +/* { dg-do run { target { powerpc*-*-* && { lp64 && p8vector_hw } } } } */
> +/* { dg-require-effective-target powerpc_p8vector_ok } */
> +/* { dg-options "-mcpu=power8" } */

Do you still need the p8vector_ok test if you already have p8vector_hw?

Would this test work on 32-bit if you used "long long" instead of "long"?

The patch looks good with those last few details taking care of :-)


Segher

Re: [PATCH version 2, rs6000] Add builtins to convert from float/double to int/long using current rounding mode

[Michael Meissner]

On Wed, Sep 13, 2017 at 06:08:45PM -0500, Segher Boessenkool wrote:
> On Tue, Sep 12, 2017 at 07:17:07PM -0400, Michael Meissner wrote:
> > On Tue, Sep 12, 2017 at 05:41:34PM -0500, Segher Boessenkool wrote:
> > > This needs "TARGET_HARD_FLOAT && TARGET_DOUBLE_FLOAT" I think?  Which
> > > is the same as "TARGET_DF_FPR".  "lrint<mode>di2" also has "TARGET_FPRND"
> > > but that is a mistake I think.  Cc:ing Mike for that.
> > 
> > TARGET_FPRND is for ISA 2.02 (power5+) which added the various round to integer
> > instructions.  So, unless we are going to stop supporting older machines, it is
> > needed.
> 
> I think you have this a bit wrong?
> 
> ISA 2.02 added fri*, but that's round to integer, not convert.
> 
> ISA 2.06 added some fc[ft]i* instructions, but most are in base PowerPC
> already.
> 
> And FPRND is enabled at ISA 2.04 and later?

Perhaps, I wasn't paying close attention of the exact instructions.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

[PATCH version 4, rs6000] Add builtins to convert from float/double to int/long using current rounding mode

[Carl Love]

GCC maintainers:

The following patch has been updated to address Segher's comments.  The
patch.  The new BU_FP_MISC_1 macro has been changed so gcc will
gracefully exit when the command line option -msoft-float is used.

I have tested the patch on powerpc64le-unknown-linux-gnu (Power 8 LE).
I also manually ran make -k check with --target_board=unix`{-m64,-m32}`
against the test case to make sure it worked fine for both 32-bit and
64-bit.

Please let me know if the following patch is acceptable.  Thanks.

                        Carl Love

-------------------------------------------------------------------------

gcc/ChangeLog:

2017-09-21  Carl Love  <cel@us.ibm.com>

	* config/rs6000/rs6000-builtin.def (BU_FP_1MISC_1): Add define macro.
	(FCTID, FCTIW): Add BU_FP_MISC_1 macro expansion for builtins.
	* config/rs6000/rs6000.md (lrintsfsi2): Add define_insn for the
	fctiw instruction.

gcc/testsuite/ChangeLog:

2017-09-21 Carl Love  <cel@us.ibm.com>
	* gcc.target/powerpc/builtin-fctid-fctiw-runnable.c: New test file
	for the __builtin_fctid and __builtin_fctiw.
---
 gcc/config/rs6000/rs6000-builtin.def               |  14 +++
 gcc/config/rs6000/rs6000.md                        |   8 ++
 .../powerpc/builtin-fctid-fctiw-runnable.c         | 137 +++++++++++++++++++++
 3 files changed, 159 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c

diff --git a/gcc/config/rs6000/rs6000-builtin.def b/gcc/config/rs6000/rs6000-builtin.def
index 850164a..90c10af 100644
--- a/gcc/config/rs6000/rs6000-builtin.def
+++ b/gcc/config/rs6000/rs6000-builtin.def
@@ -608,6 +608,16 @@
 		    CODE_FOR_ ## ICODE)			/* ICODE */
 
 
+/* Miscellaneous builtins for instructions added prior to ISA 2.04.  These
+  operate on floating point registers.  */
+#define BU_FP_MISC_1(ENUM, NAME, ATTR, ICODE)				\
+  RS6000_BUILTIN_1 (MISC_BUILTIN_ ## ENUM,		/* ENUM */	\
+		    "__builtin_" NAME,			/* NAME */	\
+		   RS6000_BTM_HARD_FLOAT,		/* MASK */	\
+		    (RS6000_BTC_ ## ATTR		/* ATTR */	\
+		     | RS6000_BTC_UNARY),				\
+		    CODE_FOR_ ## ICODE)			/* ICODE */
+
 /* Miscellaneous builtins for instructions added in ISA 2.06.  These
    instructions don't require either the DFP or VSX options, just the basic ISA
    2.06 (popcntd) enablement since they operate on general purpose
@@ -1846,6 +1856,10 @@ BU_VSX_OVERLOAD_X (XL,	     "xl")
 BU_VSX_OVERLOAD_X (XL_BE,    "xl_be")
 BU_VSX_OVERLOAD_X (XST,	     "xst")
 \f
+/* 1 argument builtins pre ISA 2.04.  */
+BU_FP_MISC_1 (FCTID,		"fctid",	CONST,  lrintdfdi2)
+BU_FP_MISC_1 (FCTIW,		"fctiw",	CONST,	lrintsfsi2)
+
 /* 2 argument CMPB instructions added in ISA 2.05. */
 BU_P6_2 (CMPB_32,        "cmpb_32",	CONST,	cmpbsi3)
 BU_P6_64BIT_2 (CMPB,     "cmpb",	CONST,	cmpbdi3)
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 20873ac..ad39dc9 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -5899,6 +5899,14 @@
   [(set_attr "type" "fpload")
    (set_attr "length" "16")])
 
+(define_insn "lrintsfsi2"
+  [(set (match_operand:SI 0 "gpc_reg_operand" "=d")
+	(unspec:SI [(match_operand:DF 1 "gpc_reg_operand" "d")]
+		   UNSPEC_FCTIW))]
+  "TARGET_SF_FPR && TARGET_FPRND"
+  "fctiw %0,%1"
+  [(set_attr "type" "fp")])
+
 ;; No VSX equivalent to fctid
 (define_insn "lrint<mode>di2"
   [(set (match_operand:DI 0 "gpc_reg_operand" "=d")
diff --git a/gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c b/gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c
new file mode 100644
index 0000000..b99fae3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/builtin-fctid-fctiw-runnable.c
@@ -0,0 +1,137 @@
+/* { dg-do run { target { powerpc*-*-* && { lp64 && p8vector_hw } } } } */
+/* { dg-options "-mcpu=power8" } */
+
+#ifdef DEBUG
+#include <stdio.h>
+#endif
+
+void abort (void);
+
+long
+test_bi_lrint_1 (float __A)
+{
+	return (__builtin_fctid (__A));
+}
+long
+test_bi_lrint_2 (double __A)
+{
+	return (__builtin_fctid (__A));
+}
+
+int
+test_bi_rint_1 (float __A)
+{
+	return (__builtin_fctiw (__A));
+}
+
+int
+test_bi_rint_2 (double __A)
+{
+	return (__builtin_fctiw (__A));
+}
+
+
+int main( void)
+{
+  signed long lx, expected_l;
+  double dy;
+
+  signed int x, expected_i;
+  float y;
+  
+  dy = 1.45;
+  expected_l = 1;
+  lx = __builtin_fctid (dy);
+
+  if( lx != expected_l)
+#ifdef DEBUG
+    printf("ERROR: __builtin_fctid(dy= %f) = %ld, expected %ld\n",
+	   dy, lx, expected_l);
+#else
+    abort();
+#endif
+
+  dy = 3.51;
+  expected_l = 4;
+  lx = __builtin_fctid (dy);
+  
+  if( lx != expected_l)
+#ifdef DEBUG
+    printf("ERROR: __builtin_fctid(dy= %f) = %ld, expected %ld\n",
+	   dy, lx, expected_l);
+#else
+    abort();
+#endif
+
+  dy = 5.57;
+  expected_i = 6;
+  x = __builtin_fctiw (dy);
+
+  if( x != expected_i)
+#ifdef DEBUG
+    printf("ERROR: __builtin_fctiw(dy= %f) = %d, expected %d\n",
+	   dy, x, expected_i);
+#else
+    abort();
+#endif
+
+  y = 11.47;
+  expected_i = 11;
+  x = __builtin_fctiw (y);
+
+  if( x != expected_i)
+#ifdef DEBUG
+    printf("ERROR: __builtin_fctiw(y = %f) = %d, expected %d\n",
+	   y, x, expected_i);
+#else
+    abort();
+#endif
+
+  y = 17.77;
+  expected_l = 18;
+  lx = test_bi_lrint_1 (y);
+
+  if( lx != expected_l)
+#ifdef DEBUG
+    printf("ERROR: function call test_bi_lrint_1 (y = %f) = %ld, expected %ld\n",
+	   y, lx, expected_l);
+#else
+    abort();
+#endif
+
+  dy = 7.1;
+  expected_l = 7;
+  lx = test_bi_lrint_2 (dy);
+
+  if( lx != expected_l)
+#ifdef DEBUG
+    printf("ERROR: function call test_bi_lrint_2 (dy = %f) = %ld, expected %ld\n",
+	   dy, lx, expected_l);
+#else
+    abort();
+#endif
+
+  y = 0.001;
+  expected_i = 0;
+  x = test_bi_rint_1 (y);
+
+  if( x != expected_i)
+#ifdef DEBUG
+    printf("ERROR: function call test_bi_rint_1 (y = %f) = %d, expected %d\n",
+	   y, x, expected_i);
+#else
+    abort();
+#endif
+  
+  dy = 0.9999;
+  expected_i = 1;
+  x = test_bi_rint_2 (dy);
+
+  if( x != expected_i)
+#ifdef DEBUG
+    printf("ERROR: function call test_bi_rint_2 (dy = %f) = %d, expected %d\n",
+	   dy, x, expected_i);
+#else
+    abort();
+#endif
+}
-- 
1.9.1

Re: [PATCH version 4, rs6000] Add builtins to convert from float/double to int/long using current rounding mode

[Segher Boessenkool]

Hi Carl,

Some comments, mostly trivial:

On Thu, Sep 21, 2017 at 04:09:11PM -0700, Carl Love wrote:
> --- a/gcc/config/rs6000/rs6000-builtin.def
> +++ b/gcc/config/rs6000/rs6000-builtin.def
> @@ -608,6 +608,16 @@
>  		    CODE_FOR_ ## ICODE)			/* ICODE */
>  
>  
> +/* Miscellaneous builtins for instructions added prior to ISA 2.04.  These
> +  operate on floating point registers.  */

"operate" should align with "Miscellaneous".

> +#define BU_FP_MISC_1(ENUM, NAME, ATTR, ICODE)				\
> +  RS6000_BUILTIN_1 (MISC_BUILTIN_ ## ENUM,		/* ENUM */	\
> +		    "__builtin_" NAME,			/* NAME */	\
> +		   RS6000_BTM_HARD_FLOAT,		/* MASK */	\
> +		    (RS6000_BTC_ ## ATTR		/* ATTR */	\
> +		     | RS6000_BTC_UNARY),				\
> +		    CODE_FOR_ ## ICODE)			/* ICODE */

RS6000_BTM_HARD_FLOAT needs to be indented one extra space, too.

> +(define_insn "lrintsfsi2"
> +  [(set (match_operand:SI 0 "gpc_reg_operand" "=d")
> +	(unspec:SI [(match_operand:DF 1 "gpc_reg_operand" "d")]
> +		   UNSPEC_FCTIW))]
> +  "TARGET_SF_FPR && TARGET_FPRND"
> +  "fctiw %0,%1"
> +  [(set_attr "type" "fp")])

Should this be match_operand:SF?

Okay for trunk if that works.  Thanks,


Segher