Re: [PATCH] rs6000: Don't use TFmode for 128 bits fp constant in toc [PR110011]

["Kewen.Lin" <linkw@linux.ibm.com>]

on 2023/6/11 10:04, David Edelsohn wrote:
> On Tue, Jun 6, 2023 at 5: 20 AM Kewen. Lin <linkw@ linux. ibm. com> wrote: Hi, As PR110011 shows, when encoding 128 bits fp constant into toc, we adopts REAL_VALUE_TO_TARGET_LONG_DOUBLE which is to find the first float mode with LONG_DOUBLE_TYPE_SIZE
> ZjQcmQRYFpfptBannerStart
> This Message Is From an External Sender
> This message came from outside your organization.
>  
> ZjQcmQRYFpfptBannerEnd
> On Tue, Jun 6, 2023 at 5:20 AM Kewen.Lin <linkw@linux.ibm.com <mailto:linkw@linux.ibm.com>> wrote:
> 
>     Hi,
> 
>     As PR110011 shows, when encoding 128 bits fp constant into
>     toc, we adopts REAL_VALUE_TO_TARGET_LONG_DOUBLE which is
>     to find the first float mode with LONG_DOUBLE_TYPE_SIZE
>     bits of precision, it would be TFmode here.  But the 128
>     bits fp constant can be with mode IFmode or KFmode, which
>     doesn't necessarily have the same underlying float format
>     as the one of TFmode, like this PR exposes, with option
>     -mabi=ibmlongdouble TFmode has ibm_extended_format while
>     KFmode has ieee_quad_format, mixing up the formats (the
>     encoding/decoding ways) would cause unexpected results.
> 
>     This patch is to make it use constant's own mode instead
>     of TFmode for real_to_target call.
> 
>     Bootstrapped and regtested on powerpc64-linux-gnu P7/P8/P9 and
>     powerpc64le-linux-gnu P9 and P10.
> 
>     I'll push this next week if no objections.
> 
>     BR,
>     Kewen
>     -----
>             PR target/110011
> 
>     gcc/ChangeLog:
> 
>             * config/rs6000/rs6000.cc (output_toc): Use its own mode of the
>             128-bit float constant for real_to_target call.
> 
> 
> The comment wording can be worded better.  Maybe 
> 
> Use the mode of the 128-bit floating constant itself for real_to_target call.
> 
> This is okay.

Thanks David, I've updated changelog as you suggested and pushed as r14-1703.
Will backport after burn-in time.

BR,
Kewen

> 
> Thanks, David
> 
> 
>     gcc/testsuite/ChangeLog:
> 
>             * gcc.target/powerpc/pr110011.c: New test.
>     ---
>      gcc/config/rs6000/rs6000.cc                 |  2 +-
>      gcc/testsuite/gcc.target/powerpc/pr110011.c | 42 +++++++++++++++++++++
>      2 files changed, 43 insertions(+), 1 deletion(-)
>      create mode 100644 gcc/testsuite/gcc.target/powerpc/pr110011.c
> 
>     diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
>     index 3f129ea37d2..330c6a6fa5f 100644
>     --- a/gcc/config/rs6000/rs6000.cc
>     +++ b/gcc/config/rs6000/rs6000.cc
>     @@ -17314,7 +17314,7 @@ output_toc (FILE *file, rtx x, int labelno, machine_mode mode)
>            if (DECIMAL_FLOAT_MODE_P (GET_MODE (x)))
>             REAL_VALUE_TO_TARGET_DECIMAL128 (*CONST_DOUBLE_REAL_VALUE (x), k);
>            else
>     -       REAL_VALUE_TO_TARGET_LONG_DOUBLE (*CONST_DOUBLE_REAL_VALUE (x), k);
>     +       real_to_target (k, CONST_DOUBLE_REAL_VALUE (x), GET_MODE (x));
> 
>            if (TARGET_64BIT)
>             {
>     diff --git a/gcc/testsuite/gcc.target/powerpc/pr110011.c b/gcc/testsuite/gcc.target/powerpc/pr110011.c
>     new file mode 100644
>     index 00000000000..5b04d3e298a
>     --- /dev/null
>     +++ b/gcc/testsuite/gcc.target/powerpc/pr110011.c
>     @@ -0,0 +1,42 @@
>     +/* { dg-do run } */
>     +/* { dg-require-effective-target float128_runtime } */
>     +/* Force long double to be with IBM format here, to verify
>     +   _Float128 constant still uses its own format (IEEE) for
>     +   encoding rather than IBM format.  */
>     +/* { dg-options "-mfp-in-toc -mabi=ibmlongdouble" } */
>     +/* { dg-add-options float128 } */
>     +
>     +#define MPFR_FLOAT128_MAX 0x1.ffffffffffffffffffffffffffffp+16383f128
>     +
>     +__attribute__ ((noipa))
>     +_Float128 f128_max ()
>     +{
>     +  return MPFR_FLOAT128_MAX;
>     +}
>     +
>     +typedef union
>     +{
>     +  int w[4];
>     +  _Float128 f128;
>     +} U;
>     +
>     +int main ()
>     +{
>     +
>     +  U umax;
>     +  umax.f128 = f128_max ();
>     +  /* ieee float128 max:
>     +     7ffeffff ffffffff ffffffff ffffffff.  */
>     +  if (umax.w[1] != 0xffffffff || umax.w[2] != 0xffffffff)
>     +    __builtin_abort ();
>     +#ifdef __LITTLE_ENDIAN__
>     +  if (umax.w[0] != 0xffffffff || umax.w[3] != 0x7ffeffff)
>     +    __builtin_abort ();
>     +#else
>     +  if (umax.w[3] != 0xffffffff || umax.w[0] != 0x7ffeffff)
>     +    __builtin_abort ();
>     +#endif
>     +
>     +  return 0;
>     +}
>     +
>     --
>     2.31.1
>