Support __FLT_EVAL_METHOD__ == 16 in math.h

Message ID 20191003062238.25364-1-kito.cheng@sifive.com
State New
Headers show
Series
  • Support __FLT_EVAL_METHOD__ == 16 in math.h
Related show

Commit Message

Kito Cheng Oct. 3, 2019, 6:22 a.m.
- GCC will set __FLT_EVAL_METHOD__ to 16 if __fp16 supported, e.g.
    cortex-a55/aarch64.
    - $ aarch64-unknown-elf-gcc -v 2>&1 |grep version
      gcc version 9.2.0 (GCC)
    - $ aarch64-unknown-elf-gcc  -E -dM -mcpu=cortex-a55 - < /dev/null  |grep FLT_EVAL_METHOD
      #define __FLT_EVAL_METHOD__ 16
      #define __FLT_EVAL_METHOD_TS_18661_3__ 16
      #define __FLT_EVAL_METHOD_C99__ 16
  - The behavior of __FLT_EVAL_METHOD__ == 16 is same as
    __FLT_EVAL_METHOD__ == 0 except for float16_t, but newlib didn't
    support float16_t.
---
 newlib/libc/include/math.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.17.1

Comments

Howland, Craig D. - US via newlib Oct. 3, 2019, 6:18 p.m. | #1
> From: newlib-owner@sourceware.org <newlib-owner@sourceware.org> on behalf of Kito Cheng <kito.cheng@sifive.com>

> Sent: Thursday, October 3, 2019 2:22 AM

> To: newlib@sourceware.org; kito.cheng@gmail.com

> Cc: Kito Cheng

> Subject: [PATCH] Support __FLT_EVAL_METHOD__ == 16 in math.h

>

>   - GCC will set __FLT_EVAL_METHOD__ to 16 if __fp16 supported, e.g.

>     cortex-a55/aarch64.

>     - $ aarch64-unknown-elf-gcc -v 2>&1 |grep version

>       gcc version 9.2.0 (GCC)

>     - $ aarch64-unknown-elf-gcc  -E -dM -mcpu=cortex-a55 - < /dev/null  |grep FLT_EVAL_METHOD

>       #define __FLT_EVAL_METHOD__ 16

>       #define __FLT_EVAL_METHOD_TS_18661_3__ 16

>       #define __FLT_EVAL_METHOD_C99__ 16

>   - The behavior of __FLT_EVAL_METHOD__ == 16 is same as

>     __FLT_EVAL_METHOD__ == 0 except for float16_t, but newlib didn't

>     support float16_t.

> ---

>  newlib/libc/include/math.h | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/newlib/libc/include/math.h b/newlib/libc/include/math.h

> index 1efc5b92c..412ae318a 100644

> --- a/newlib/libc/include/math.h

> +++ b/newlib/libc/include/math.h

> @@ -146,7 +146,7 @@ extern int isnan (double);

>    #define __TMP_FLT_EVAL_METHOD

>  #endif /* FLT_EVAL_METHOD */

>  #if defined FLT_EVAL_METHOD

> -  #if FLT_EVAL_METHOD == 0

> +  #if (FLT_EVAL_METHOD == 0) || (FLT_EVAL_METHOD == 16)

>      typedef float  float_t;

>      typedef double double_t;

>     #elif FLT_EVAL_METHOD == 1

> --

> 2.17.1

>

     Where this belongs is an interesting question.
     The first thought is that this is a non-standard value according to
C11, C99, and POSIX, which only allow non-negative values of 0, 1, and 2.  As 
such, this value check is most properly put into the appropriate
machine/arch/machine/_types.h file.  (libc/machine/aarch64/machine/_types.h 
based on the given example of cortex-a55/aarch64, perhaps.)
     While I failed to find a definition of the value 16 in either the GCC 
7.2 GCC or CPP manuals, GCC does document a -fpermitted-flt-eval-methods 
option which references ISO/IEC TS 18661-3 changes/extensions for 
FLT_EVAL_METHOD values.  Being such an extension perhaps does say that math.h 
would be the right place, as it is not machine-specific, even though it is
non-standard.  But being an extension from this esoteric document, does it 
belong unadorned?  (That is, should it have some kind of gate that ensures 
this 18661-3 meaning is truly intended?)
     So, does this belong in math.h as a non-standard general extension, or
in machine as an implementation-defined value?  (By non-standard here, I
mean neither C nor POSIX, the base standards being followed.  Being an
ISO/IEC document it is standard from that perspective.)
     Being non C/POSIX should be noted in the source comments.  (Having found
a draft copy of 18661-3 to read, it also resolved an ambiguity in the given
explanation of what the value 16 means.  Does "except for float16_t" mean
that float16_t does not observe evaluation method 0, or does it mean that
float16_t type is also present?  It turns out to mean the latter.)  Perhaps
something like this?

+ /* FLT_EVAL_METHOD == 16 has meaning as defined in ISO/IEC TS 18661-3,
+  * which provides non-compliant extensions to C and POSIX (by adding
+  * additional positive values for FLT_EVAL_METHOD).  It effectively has
+  * same meaning as the C99 and C11 definitions for value 0, while also
+  * serving as a flag that the _Float16 (float16_t) type exists.  */

     Does anyone on the list have any connection with WG 14 N2405?  I found
that the 18661-3 document is apparently being worked on to be Annex X to the 
C standard.  (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2405.pdf)
I suggest that this altered use of the FLT_EVAL_METHOD macro is a bad one
and should not be adopted because it is mixing things up.  The way 16 is
being defined is as noted above:  'the present meaning of value 0 with a flag
that _Float16 is also available.'  Well, the latter has nothing to do with
the former.  In fact, they've redefined 0 in an incompatible way.  A new
define should be introduced to tell what floating point types are present.
Another flaw is that the present method does not allow for multiple new types 
to be indicated.  If I knew how to get it to them, I could write a more 
complete complaint if this brief note is insufficient.
                                Craig

Patch

diff --git a/newlib/libc/include/math.h b/newlib/libc/include/math.h
index 1efc5b92c..412ae318a 100644
--- a/newlib/libc/include/math.h
+++ b/newlib/libc/include/math.h
@@ -146,7 +146,7 @@  extern int isnan (double);
   #define __TMP_FLT_EVAL_METHOD
 #endif /* FLT_EVAL_METHOD */
 #if defined FLT_EVAL_METHOD
-  #if FLT_EVAL_METHOD == 0
+  #if (FLT_EVAL_METHOD == 0) || (FLT_EVAL_METHOD == 16)
     typedef float  float_t;
     typedef double double_t;
    #elif FLT_EVAL_METHOD == 1