On Thu, 2021-01-28 at 21:42 -0500, Michael Meissner via Gcc-patches wrote:
> [PATCH] Add conversions between _Float128 and Decimal.
> 

Hi, 
Just a couple cosmetic nits in the description.  The changelog seems to
match that patch contents OK.  


> This patch implements conversions between _Float128 and the 3 Decimal
> floating types.  It does by extendending the dfp-bit conversions to add a

nit: 'does so' or 'does this' 

> new binary floating point type (KF), and doing the conversions in the same
> mannor as the other binary/decimal conversions.

manner.

> 
> In particular for conversions from _Float128 to Decimal, it uses a sprintf
> variant to convert _Float128 to strings, and a type specific function that
> converts the string output to the appropriate Decimal type
> 
> For conversions from one of the Decimal types to _Float128, it uses a decimal
> function to convert to string (i.e. __decimal<n>ToString), and then uses a
> variant of strtold to convert to _Float128.


Are the sprintf and strtold functions called actually variants? 


> 
> If the user is linked against GLIBC 2.32 or newer, then the sprintf and 
> strtold
> variant functions can use the features directly in GLIBC 2.32 to do this
> conversion.
> 
> If you have an older GLIBC and want to convert _Float128 to one of the Decimal
> types, it will convert the _Float128 to __ibm128 and then convert that to
> Decimal.
> 
> Similarly if you have one of the Decimal types, and want to convert to
> _Float128, it will first convert the Decimal type to __ibm128, and then 
> convert
> __ibm128 to _Float128.
> 
> These functions will primarily be used if/when the default PowerPC long double
> type is changed to IEEE 128-bit, but they could also be used if the user
> explicitly converts _Float128 to/from a Decimal type.
> 
> One test case relating to Decimal fails if I build a compiler where the 
> default
> is IEEE 128-bit:
> 
>     * c-c++-common/dfp/convert-bfp-11.c
> 
> I have patches for this test, and they have been submitted separately.

ok

> 
> I have tested this patch by doing builds, bootstraps, and make check with 3
> builds on a power9 little endian server:
> 
>     *   Build one used the default long double being IBM 128-bit;
>     *   Build two set the long double default to IEEE 128-bit; (and)
>     *   Build three set the long double default to 64-bit.
> 
> The compilers built fine providing I recompiled gmp, mpc, and mpfr with the
> appropriate long double options.  There were a few differences in the test
> suite runs that will be addressed in later patches, but over all it works
> well.  This patch is required to be able to build a toolchain where the 
> default
> long double is IEEE 128-bit.

>                                 Can I check this patch into the master branch 
> for
> GCC 11?

Separate the check-in question from the description paragraph.

> 
> I have also built compilers with this patch on a big endian power8 system that
> has both 32-bit and 64-bit support.  There were no regressions in running 
> these
> tests on the system.
> 
> Can I check this patch into the master branch?
> 
> libgcc/
> 2021-01-28  Michael Meissner  <meiss...@linux.ibm.com>
> 
>       * config/rs6000/_dd_to_kf.c: New file.
>       * config/rs6000/_kf_to_dd.c: New file.
>       * config/rs6000/_kf_to_sd.c: New file.
>       * config/rs6000/_kf_to_td.c: New file.
>       * config/rs6000/_sd_to_kf.c: New file.
>       * config/rs6000/_sprintfkf.c: New file.
>       * config/rs6000/_sprintfkf.h: New file.
>       * config/rs6000/_strtokf.h: New file.
>       * config/rs6000/_strtokf.c: New file.
>       * config/rs6000/_td_to_kf.c: New file.
>       * config/rs6000/quad-float128.h: Add new declarations.
>       * config/rs6000/t-float128 (fp128_dec_funcs): New macro.
>       (fp128_decstr_funcs): New macro.
>       (ibm128_dec_funcs): New macro.
>       (fp128_ppc_funcs): Add the new conversions.
>       (fp128_dec_objs): Force Decimal <-> __float128 conversions to be
>       compiled with -mabi=ieeelongdouble.
>       (fp128_decstr_objs): Force __float128 <-> string conversions to be
>       compiled with -mabi=ibmlongdouble.
>       (ibm128_dec_objs): Force Decimal <-> __float128 conversions to be
>       compiled with -mabi=ieeelongdouble.
>       (FP128_CFLAGS_DECIMAL): New macro.
>       (IBM128_CFLAGS_DECIMAL): New macro.
>       * dfp-bit.c (DFP_TO_BFP): Add PowerPC _Float128 support.
>       (BFP_TO_DFP): Add PowerPC _Float128 support.
>       * dfp-bit.h (BFP_KIND): Add new binary floating point kind for
>       IEEE 128-bit floating point.
>       (DFP_TO_BFP): Add PowerPC _Float128 support.
>       (BFP_TO_DFP): Add PowerPC _Float128 support.
>       (BFP_SPRINTF): New macro.
> ---
>  libgcc/config/rs6000/_dd_to_kf.c     | 37 ++++++++++++++++++
>  libgcc/config/rs6000/_kf_to_dd.c     | 37 ++++++++++++++++++
>  libgcc/config/rs6000/_kf_to_sd.c     | 37 ++++++++++++++++++
>  libgcc/config/rs6000/_kf_to_td.c     | 37 ++++++++++++++++++
>  libgcc/config/rs6000/_sd_to_kf.c     | 37 ++++++++++++++++++
>  libgcc/config/rs6000/_sprintfkf.c    | 57 ++++++++++++++++++++++++++++
>  libgcc/config/rs6000/_sprintfkf.h    | 28 ++++++++++++++
>  libgcc/config/rs6000/_strtokf.c      | 56 +++++++++++++++++++++++++++
>  libgcc/config/rs6000/_strtokf.h      | 27 +++++++++++++
>  libgcc/config/rs6000/_td_to_kf.c     | 37 ++++++++++++++++++
>  libgcc/config/rs6000/quad-float128.h |  8 ++++
>  libgcc/config/rs6000/t-float128      | 37 +++++++++++++++++-
>  libgcc/dfp-bit.c                     | 12 +++++-
>  libgcc/dfp-bit.h                     | 26 +++++++++++++
>  14 files changed, 470 insertions(+), 3 deletions(-)
>  create mode 100644 libgcc/config/rs6000/_dd_to_kf.c
>  create mode 100644 libgcc/config/rs6000/_kf_to_dd.c
>  create mode 100644 libgcc/config/rs6000/_kf_to_sd.c
>  create mode 100644 libgcc/config/rs6000/_kf_to_td.c
>  create mode 100644 libgcc/config/rs6000/_sd_to_kf.c
>  create mode 100644 libgcc/config/rs6000/_sprintfkf.c
>  create mode 100644 libgcc/config/rs6000/_sprintfkf.h
>  create mode 100644 libgcc/config/rs6000/_strtokf.c
>  create mode 100644 libgcc/config/rs6000/_strtokf.h
>  create mode 100644 libgcc/config/rs6000/_td_to_kf.c
> 
> diff --git a/libgcc/config/rs6000/_dd_to_kf.c 
> b/libgcc/config/rs6000/_dd_to_kf.c
> new file mode 100644
> index 00000000000..3b4d8500a57
> --- /dev/null
> +++ b/libgcc/config/rs6000/_dd_to_kf.c
> @@ -0,0 +1,37 @@
> +/* Copyright (C) 1989-2021 Free Software Foundation, Inc.

Other source files in libgcc/config appear to have the copyright range
starting at a later date, presumably when the file was created.
So.. I think this should be Copyright (C) 2021 FSF....

> +
> +This file is part of GCC.
> +

I have read through the rest of the patch, no further issues or
comments noted by me. 
Thanks
-WIll



Reply via email to