[PATCH] riscv: Set __GLIBC_FLT_EVAL_METHOD to 0 if __FLT_EVAL_METHOD__ is -1

[PATCH] riscv: Set __GLIBC_FLT_EVAL_METHOD to 0 if __FLT_EVAL_METHOD__ is -1

[Wilco Dijkstra]

Hi Kito,

This looks like a recent bug in LLVM - even with -Ofast compilers continue to
use float and double for evaluation (obviously!), so float_t and double_t should
not change (which may break ABI if some code uses -Ofast and some doesn't,
leading to incompatible float_t/double_t definitions).

So this should be reported to and fixed in LLVM, and if we want/need to work
around it in GLIBC, it should be done in the generic flt-eval-method.h header
so that it works on all targets.

Cheers,
Wilco

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

[Kito Cheng]

For anyone who interested on this issue, but not tracking LLVM community:
LLVM has created a bug to track this:
https://github.com/llvm/llvm-project/issues/60781

Hi Wilco:

According to the comment in the flt-eval-method.h,
it says its mapping -1 to 2 is because it's the most conservatively
safe assumption[1],
so that's why I consider fixing that for RISC-V port only.

I am happy to send patches to update the generic flt-eval-method.h if
there are other maintainers that think it's reasonable to change the
default mapping in glibc.

[1] https://github.com/bminor/glibc/blob/master/bits/flt-eval-method.h#L23

On Wed, Feb 15, 2023 at 10:13 PM Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>
> Hi Kito,
>
> This looks like a recent bug in LLVM - even with -Ofast compilers continue to
> use float and double for evaluation (obviously!), so float_t and double_t should
> not change (which may break ABI if some code uses -Ofast and some doesn't,
> leading to incompatible float_t/double_t definitions).
>
> So this should be reported to and fixed in LLVM, and if we want/need to work
> around it in GLIBC, it should be done in the generic flt-eval-method.h header
> so that it works on all targets.
>
> Cheers,
> Wilco

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

[Wilco Dijkstra]

Hi Kito,

> According to the comment in the flt-eval-method.h,
> it says its mapping -1 to 2 is because it's the most conservatively
> safe assumption[1],
> so that's why I consider fixing that for RISC-V port only.

The most conservative option would be to map -1 to 0 since that is what
the undefined case already does, and this is correct on all targets (68k and x86
have overrides that force it to 2 - IIRC s390 used to use 1 but now uses 0).

> I am happy to send patches to update the generic flt-eval-method.h if
> there are other maintainers that think it's reasonable to change the
> default mapping in glibc.

This bug affects almost all targets, so we want a generic fix. Mapping -1 to 0
seems a reasonable default, but we could also decide to decouple the
float_t/double_t ABI from __FLT_EVAL_METHOD__ if it is no longer treated as
a fixed value for the whole application.

Cheers,
Wilco

[PATCH] riscv: Set __GLIBC_FLT_EVAL_METHOD to 0 if __FLT_EVAL_METHOD__ is -1

[Kito Cheng]

__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 which is 128-bits IEEE 754 floating point type.

However most RISC-V CPU isn't provide native 128-bits IEEE 754 floating point
type operations (Q extension), so this will made float_t and double_t
become very inefficient type, that's violate the spirit float_t and double_t.

RISC-V has native single and double precision floating point opeations
when F and D is present, even we don't have F or D extension, using
same precision for soft-float still most efficient.

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 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

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.
---
 sysdeps/riscv/bits/flt-eval-method.h | 31 ++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 sysdeps/riscv/bits/flt-eval-method.h

diff --git a/sysdeps/riscv/bits/flt-eval-method.h b/sysdeps/riscv/bits/flt-eval-method.h
new file mode 100644
index 0000000000..95c907516e
--- /dev/null
+++ b/sysdeps/riscv/bits/flt-eval-method.h
@@ -0,0 +1,31 @@
+/* Define __GLIBC_FLT_EVAL_METHOD.  RISC-V version.
+   Copyright (C) 2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _MATH_H
+# error "Never use <bits/flt-eval-method.h> directly; include <math.h> instead."
+#endif
+
+#ifdef __FLT_EVAL_METHOD__
+# if __FLT_EVAL_METHOD__ == -1
+#  define __GLIBC_FLT_EVAL_METHOD	0
+# else
+#  define __GLIBC_FLT_EVAL_METHOD	__FLT_EVAL_METHOD__
+# endif
+#else
+# define __GLIBC_FLT_EVAL_METHOD	0
+#endif
-- 
2.37.2