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
