Re: [PATCH] Remap __GLIBC_FLT_EVAL_METHOD to 0 if __FLT_EVAL_METHOD__ is -1

[Kito Cheng <kito.cheng@sifive.com>]

ping


On Sun, Feb 19, 2023 at 2:29 AM Palmer Dabbelt <palmer@rivosinc.com> wrote:
>
> On Thu, 16 Feb 2023 18:26:46 PST (-0800), kito.cheng@sifive.com wrote:
> > ---
> > We were intending to update RISC-V's setting only, but after discussion
> > with Wilco Dijkstra, we decide to change the generic one instead of RISC-V only
> > since it also fix inefficient issue for float_t and double_t.
> > ---
> >
> > __GLIBC_FLT_EVAL_METHOD will effect the definition of float_t and
> > double_t, currently we'll set __GLIBC_FLT_EVAL_METHOD to 2 when
> > __FLT_EVAL_METHOD__ is -1, that means we'll define float_t and double_t
> > to long double.
> >
> > However some target isn't natively (HW) support long double like AArch64 and
> > RISC-V, they defined long double as 128-bits IEEE 754 floating point type.
> >
> > That means setting __GLIBC_FLT_EVAL_METHOD to 2 will cause very inefficient
> > code gen for those target who didn't provide native support for long
> > double, and that's violate the spirit float_t and double_t - most efficient
> > types at least as wide as float and double.
> >
> > So this patch propose to remap __GLIBC_FLT_EVAL_METHOD to 0 rather than
> > 2 when __FLT_EVAL_METHOD__ is -1, which means we'll use float/double
> > rather than long double for float_t and double_t.
> >
> > Note: __FLT_EVAL_METHOD__ == -1 means the precision is indeterminable,
> >       which means compiler might using indeterminable precision during
> >       optimization/code gen, clang will set this value to -1 when fast
> >       math is enabled.
> >
> > Note: Default definition float_t and double_t in current glibc:
> >       |  __GLIBC_FLT_EVAL_METHOD | float_t     | double_t
> >       |               0 or 16    | float       | double
> >       |               1          | double      | doulbe
> >       |               2          | long double | long double
> >       More complete list see math/math.h
> >
> > Note: RISC-V has defined ISA extension to support 128-bits IEEE 754
> >       floating point operations, but only rare RISC-V core will implement that.
> >
> > Related link:
> >
> > [1] LLVM issue (__FLT_EVAL_METHOD__ is set to -1 with Ofast. #60781):
> >     https://github.com/llvm/llvm-project/issues/60781
> > [2] Last version of this patch: https://sourceware.org/pipermail/libc-alpha/2023-February/145622.html
> >
> > Ref:
> >
> > [1] Definition of FLT_EVAL_METHOD from C99 spec:
> > C99 Spec draft: (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf)
> >
> > Except for assignment and cast (which remove all extra range and precision), the values
> > of operations with floating operands and values subject to the usual arithmetic
> > conversions and of floating constants are evaluated to a format whose range and precision
> > may be greater than required by the type. The use of evaluation formats is characterized
> > by the implementation-defined value of FLT_EVAL_METHOD:
> > 19)
> >
> >   -1 indeterminable;
> >   0 evaluate all operations and constants just to the range and precision of the
> >     type;
> >   1 evaluate operations and constants of type float and double to the
> >     range and precision of the double type, evaluate long double
> >     operations and constants to the range and precision of the long double
> >     type;
> >   2 evaluate all operations and constants to the range and precision of the
> >     long double type.
> >
> > All other negative values for FLT_EVAL_METHOD characterize implementation-defined
> > behavior.
> >
> > [2] Definition of float_t and double_t in C99 spec:
> >
> > C99 Spec draft: (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf)
> >
> > 7.12
> >
> > ...
> >
> > The types
> > float_t
> > double_t
> > are floating types at least as wide as float and double, respectively, and such that
> > double_t is at least as wide as float_t. If FLT_EVAL_METHOD equals 0,
> > float_t and double_t are float and double, respectively; if
> > FLT_EVAL_METHOD equals 1, they are both double; if FLT_EVAL_METHOD equals
> > 2, they are both long double; and for other values of FLT_EVAL_METHOD, they are
> > otherwise implementation-defined.199)
> >
> > 199) The types float_t and double_t are intended to be the implementation’s most efficient types at
> > least as wide as float and double, respectively. For FLT_EVAL_METHOD equal 0, 1, or 2, the
> > type float_t is the narrowest type used by the implementation to
> > evaluate floating expressions.
> > ---
> >  bits/flt-eval-method.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/bits/flt-eval-method.h b/bits/flt-eval-method.h
> > index 75f57b9a0e..b6be0a1e43 100644
> > --- a/bits/flt-eval-method.h
> > +++ b/bits/flt-eval-method.h
> > @@ -26,14 +26,14 @@
> >     -1.  */
> >
> >  /* In the default version of this header, follow __FLT_EVAL_METHOD__.
> > -   -1 is mapped to 2 (considering evaluation as long double to be a
> > +   -1 is mapped to 0 (considering evaluation as same precision to be a
> >     conservatively safe assumption), and if __FLT_EVAL_METHOD__ is not
> >     defined then assume there is no excess precision and use the value
> >     0.  */
> >
> >  #ifdef __FLT_EVAL_METHOD__
> >  # if __FLT_EVAL_METHOD__ == -1
> > -#  define __GLIBC_FLT_EVAL_METHOD    2
> > +#  define __GLIBC_FLT_EVAL_METHOD    0
> >  # else
> >  #  define __GLIBC_FLT_EVAL_METHOD    __FLT_EVAL_METHOD__
> >  # endif
>
> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> # RISC-V
>
> though we should have a global reviewed look at this now that it's
> touching everything.