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