On Thu, Jun 14, 2018 at 05:48:42PM -0400, Michael Meissner wrote:
> On Thu, Jun 14, 2018 at 04:27:26PM -0500, Segher Boessenkool wrote:
> > On Mon, Jun 11, 2018 at 07:31:44PM -0400, Michael Meissner wrote:
> > > Since we do not define arithmetic insns for IFmode, other than
> > > negate/absolute value (that we define for the other types), we will not
> > > have
> > > undesirable widening.
> >
> > I don't understand this part. We _do_ need to have all the basic operations
> > for IFmode. How else can __ibm128 variables work (with
> > -mabi=ieeelongdouble)?
>
> It calls the external function if the operation is not available.
> I.e. __gcc_qadd. We don't define addif3, and addtf3 is only defined if long
> double is IEEE and we are on power9.
Ah, so we *do* have the operations. But they are in optabs, not in
define_insn or similar. Gotcha.
And the expand code will not widen in this case.
Please clarify this in whatever code comments or commit message applies.
> > > --- gcc/config/rs6000/rs6000.md (revision 261349)
> > > +++ gcc/config/rs6000/rs6000.md (working copy)
> > > @@ -8159,8 +8159,8 @@ (define_expand "extendtfkf2"
> > > })
> > >
> > > (define_expand "trunciftf2"
> > > - [(set (match_operand:IF 0 "gpc_reg_operand")
> > > - (float_truncate:IF (match_operand:TF 1 "gpc_reg_operand")))]
> > > + [(set (match_operand:TF 0 "gpc_reg_operand")
> > > + (float_truncate:TF (match_operand:IF 1 "gpc_reg_operand")))]
> > > "TARGET_FLOAT128_TYPE"
> > > {
> > > rs6000_expand_float128_convert (operands[0], operands[1], false);
> > > @@ -8168,8 +8168,8 @@ (define_expand "trunciftf2"
> > > })
> > >
> > > (define_expand "truncifkf2"
> > > - [(set (match_operand:IF 0 "gpc_reg_operand")
> > > - (float_truncate:IF (match_operand:KF 1 "gpc_reg_operand")))]
> > > + [(set (match_operand:KF 0 "gpc_reg_operand")
> > > + (float_truncate:KF (match_operand:IF 1 "gpc_reg_operand")))]
> > > "TARGET_FLOAT128_TYPE"
> > > {
> > > rs6000_expand_float128_convert (operands[0], operands[1], false);
> >
> > These bugfixes really should have been a separate patch.
>
> Why? Before hand they were never called, so it didn't matter that the
> arguments were wrong. Now that the order of the modes is different, it calls
> trunc instead of expand.
It would help anyone unfortunate enough to have to bisect this in the future.
And makes reviewing easier, too. Knowing what parts play together with what
other parts helps a *lot*.
(Note I said "should have", no need to change it anymore).
OK for trunk if I didn't say so before (and for 8.2 together with the rest).
Segher
