ARM-software / CMSIS_5

CMSIS Version 5 Development Repository
http://arm-software.github.io/CMSIS_5/index.html
Apache License 2.0
1.33k stars 1.08k forks source link

Float16 max #982

Closed sjoerdmeijer closed 3 years ago

sjoerdmeijer commented 4 years ago

Should this:

https://github.com/ARM-software/CMSIS_5/blob/729752f45b361b1b631fa34ce12000d1f2681ed3/CMSIS/DSP/Include/arm_math_types_f16.h#L137

not be:

define F16_MAX FLT16_MAX

?

christophe0606 commented 4 years ago

@sjoerdmeijer It looks cleaner with your way. I'll do the change.

christophe0606 commented 4 years ago

@sjoerdmeijer What about __STDC_WANT_IEC_60559_TYPES_EXT__ ? I am not very familiar with this and it looks like it is needed. Should it be defined by the user or the compiler (using a specific option ?)

sjoerdmeijer commented 4 years ago

I think a fully compliant compiler implementation of that C11 extension would define that, but we don't. But I don't think we need to worry about it though, because we define the FLT16 predefined macros when the target architecture supports float16. This can e.g. be checked by adding -dM -E t.c to compile command and do a "grep FLT16", for example:

..

define __FLT16_MAX_10_EXP__ 4

define FLT16_MAX_EXP 16

define __FLT16_MAX__ 6.5504e+4F16

define __FLT16_MIN_10_EXP__ (-4)

define FLT16_MIN_EXP (-13)

define __FLT16_MIN__ 6.103515625e-5F16

..

I did notice a little probem, it is actually defined in cases where it shouldn't, but that is not affecting anything of the story here.

christophe0606 commented 4 years ago

For some reason, it does not build with FLT16_MAX but it is working with __FLT16_MAX__ so I'll use this for now and remove my wrong define. (Tested with gcc and clang)