On Thu, Jun 14, 2018 at 04:27:26PM -0500, Segher Boessenkool wrote:
> Hi!
> 
> Many thanks for all your work on this.
> 
> On Mon, Jun 11, 2018 at 07:31:44PM -0400, Michael Meissner wrote:
> > This patch is a complete rework of the previous patch.  Previously I used 
> > new
> > target hooks to provide IFmode (__ibm128) from being widened by default to
> > TFmode (long double) on power9 systems when long double is IEEE 128-bit.
> > 
> > This patch reorganizes the 3 128-bit floating point types, so that IFmode is
> > numerically higher than TFmode/KFmode.  This means IFmode is considered the
> > widest type.
> 
> Sneaky.  It probably helps.  I like it :-)
> 
> > 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.

Basically when it wants to do something (such as an add) and there is no insn
to support the operation, it goes up the chain of widder types to see if
perhaps there is support in the wider mode (such as we do for 8/16/32-bit
integer arithmetic).

In this case, if the operation is not ABS or NEG, there is no IFmode operation,
and the widening does not happen.  Instead it calls the appropriate external
function.

Before the change, when IFmode had less precision than TFmode, it would widen
IFmode to TFmode to do the operation, and then convert it back again.

> > @@ -865,9 +872,8 @@ extern unsigned char rs6000_recip_bits[]
> >     words.  */
> >  #define DOUBLE_TYPE_SIZE 64
> >  
> > -/* A C expression for the size in bits of the type `long double' on
> > -   the target machine.  If you don't define this, the default is two
> > -   words.  */
> > +/* A C expression for the size in bits of the type `long double' on the 
> > target
> > +   machine.  If you don't define this, the default is two words.  */
> 
> Please don't change things that don't change anything.

Ok, I think I had a longer comment, and then rewrote it.

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

> Please explain that IFmode arith part?

See above.

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

Reply via email to