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 :-) > 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? > 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). > + 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? /* 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? Segher