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 <c...@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
