[RFC PATCH 0/1] strold: Support for 128-bit long double

Hi all,

Newlib’s `strtold` currently handles only two formats of long double:
- `_LDBL_EQ_DBL` (long double == double), and
- 80-bit x87 extended (via `strtorx`).

On some platforms such as AArch64, `long double` uses the  IEEE-754 binary128 format
(with 113 bit long mantissa). And in that case, the current implementation of `strtold`
selects the 80-bit path (meant for x87 only), producing incorrect results.
This patch imports the missing piece `strtorQ` from FreeBSD’s gdtoa library
(David M. Gay implementation) to fix this case. Which is the same library
already (partially) used in newlib. strtorQ uses the existing underlying "engine"
strtodg, already used by strtorx.c.

For context, i've encountered this issue when running libstc++ testsuite. Since newlib 4.4.0
added the long double math functions in `c630a6a83`, the libstdc++ testsuite excutes `long_double.cc` and
`strtod.cc` tests that fail without this patch.

The strtorQ file is imported from:
- https://cgit.freebsd.org/src/tree/contrib/gdtoa/strtorQ.c

I had to make the following adaptations on that file to match newlib
history with this library:

* Make it reentrancy–aware
  - Rename `strtorQ_l` → `_strtorQ_l` and add `struct _reent *` (first arg).
  - Call `_strtodg_l(p, …)` instead of `strtodg_l(…)` and pass the reentrancy
  pointer.

* NaN handling:
  - Use `__builtin_nanl("")` instead of `ld_QNAN*` constants
  - Matches with what was done in newlib history:
    - Commit 6c212a8b used libm `nanl`, then commit 35555851 switched to
    `__builtin_nanl` to remove dependency on libm,
    then commit 4c8fa88e removed gd_qnan.h completely.

* Header adjustments to make homogeneous with strtorx and existing headers.
* Compile only if !_LDBL_EQ_DBL as done to strtorx in commit 10677229b.

Regarding the other changes in the patch:

* `strtold` integration:
  - Introduced `_strtold_impl` to select the right converter:
      - `_strtod_l` if `_LDBL_EQ_DBL`,
      - `_strtorx_l` if `LDBL_MANT_DIG == 64`,
      - `_strtorQ_l` if `LDBL_MANT_DIG == 113`.
      I usually avoid test with hardcoded values, but such tests on LDBL_MANT_DIG
      seem to be a common practice in newlib so i followed it for the sake
      of homogeneity, let me know if you prefer a different approach.
  - `_strtold_r`, `strtold_l`, and `strtold` now call this helper.

  Platforms that use different formats (none that i know of) will fallback to a
  _strtod_l so they will be less precise but not completely wrong as it was the
  case before.

* Build system:
  - Added `strtorQ.c` to Makefile.inc and Makefile.in under
    `HAVE_LONG_DOUBLE && !_LDBL_EQ_DBL`.
  - Declared `_strtorQ_l` in `mprec.h`.

Tested on riscv64-elf / x86_64-elf / arm-elf / aarch64-elf. 
on aarch64 and riscv64, it does effectively fixes the issue,
on arm and x86_64 no regression.

As you can see in my patch i didn't address the `FLT_ROUNDS` macro, and i still
use the default FLT_ROUNDS value of 0 if not x86. I feel that
this macro should be addressed separatly, in a proper file/header, since
it's a C99 mandated macro used beyong strtold.
It can be fixed properly by relying on `fegetround`, which is more portable
than redefining the macro for each arch. But `fegetround` in newlib
is in libm not libc, i wonder if having this dependency on libm is acceptable? 
I see that for `nanl` was deemed problematic in commit 35555851.

Let me know what you think.

Thanks

Best,


Zakaria Fadli (1):
  newlib: strtold: Import strtorQ for 128-bit long double support

 newlib/Makefile.in              |  19 +++++
 newlib/libc/stdlib/Makefile.inc |   1 +
 newlib/libc/stdlib/mprec.h      |   2 +
 newlib/libc/stdlib/strtold.c    |  56 +++++++-------
 newlib/libc/stdlib/strtorQ.c    | 129 ++++++++++++++++++++++++++++++++
 5 files changed, 178 insertions(+), 29 deletions(-)
 create mode 100644 newlib/libc/stdlib/strtorQ.c

-- 
2.43.0