| Message ID | 1504896073.18797.26.camel@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [version,2,rs6000] Add builtins to convert from float/double to int/long using current rounding mode | expand |
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
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
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 +}
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
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!
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
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") +/* 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 +}
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
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.
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 +}
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