On Wed, Oct 28, 2020 at 04:58:46PM -0500, Segher Boessenkool wrote:
> Hi Mike,
> 
> On Thu, Oct 22, 2020 at 06:10:37PM -0400, Michael Meissner wrote:
> > PowerPC: Use __builtin_pack_ieee128 if long double is IEEE 128-bit.
> 
> This title makes no sense, and thankfully is not what the patch does :-)

Thanks, every so often I accidently type __ieee128 instead of __ibm128.

> > This patch changes the __ibm128 emulator to use __builtin_pack_ieee128
> > instead of __builtin_pack_longdouble if long double is IEEE 128-bit, and
> > we need to use the __ibm128 type.  The code will run without this patch,
> > but this patch slightly optimizes it better.
> 
> It uses __builtin_pack_ibm128, instead?

Yes.

> > libgcc/
> > 2020-10-22  Michael Meissner  <meiss...@linux.ibm.com>
> > 
> >     * config/rs6000/ibm-ldouble.c (pack_ldouble): Use
> >     __builtin_pack_ieee128 if long double is IEEE 128-bit.
> 
> Here, too.
> 
> > ---
> >  libgcc/config/rs6000/ibm-ldouble.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/libgcc/config/rs6000/ibm-ldouble.c 
> > b/libgcc/config/rs6000/ibm-ldouble.c
> > index dd2a02373f2..767fdd72683 100644
> > --- a/libgcc/config/rs6000/ibm-ldouble.c
> > +++ b/libgcc/config/rs6000/ibm-ldouble.c
> > @@ -102,9 +102,17 @@ __asm__ (".symver __gcc_qadd,_xlqadd@GCC_3.4\n\t"
> >  static inline IBM128_TYPE
> >  pack_ldouble (double dh, double dl)
> >  {
> > +  /* If we are building on a non-VSX system, the __ibm128 type is not 
> > defined.
> 
> "Building on" does not matter in the least.  The compiler should
> generate the same code, no matter what it runs on.  Target matters, not
> host (and build not at all).

Yes.

> > +     This means we can't always use __builtin_pack_ibm128.  Instead, we use
> > +     __builtin_pack_longdouble if long double uses the IBM extended double
> > +     128-bit format, and use the explicit __builtin_pack_ibm128 if long 
> > double
> > +     is IEEE 128-bit.  */
> 
> And this comment is about the *next* case?
> 
> >  #if defined (__LONG_DOUBLE_128__) && defined (__LONG_DOUBLE_IBM128__)      
> > \
> >      && !(defined (_SOFT_FLOAT) || defined (__NO_FPRS__))
> >    return __builtin_pack_longdouble (dh, dl);
> > +#elif defined (__LONG_DOUBLE_128__) && defined (__LONG_DOUBLE_IEEE128__) \
> > +    && !(defined (_SOFT_FLOAT) || defined (__NO_FPRS__))
> > +  return __builtin_pack_ibm128 (dh, dl);
> 
> Given the above, _SOFT_FLOAT etc. are wrong.
> 
> Just use some more portable thing to repack?  Is __builtin_pack_ibm128
> not defined always here anyway?

That is the problem.  If you build a big endian PowerPC compiler where VSX is
not default, the __ibm128 stuff is not defined.  It is only defined when
__float128 is a possibility.  Hence __builtin_pack_ibm128 and
__builtin_unpack_ibm128 are not defined.

> /* 128-bit __ibm128 floating point builtins (use -mfloat128 to indicate that
>    __ibm128 is available).  */
> #define BU_IBM128_2(ENUM, NAME, ATTR, ICODE)                            \
>   RS6000_BUILTIN_2 (MISC_BUILTIN_ ## ENUM,              /* ENUM */      \
>                     "__builtin_" NAME,                  /* NAME */      \
>                     (RS6000_BTM_HARD_FLOAT              /* MASK */      \
>                      | RS6000_BTM_FLOAT128),                            \
>                     (RS6000_BTC_ ## ATTR                /* ATTR */      \
>                      | RS6000_BTC_BINARY),                              \
>                     CODE_FOR_ ## ICODE)                 /* ICODE */
> 
> (so just HARD_FLOAT and FLOAT128 are needed)
> 
> What am I missing?

As I said, the __ibm128 keyword is not enabled on non-VSX systems.

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