[PATCH] D121122: Set FLT_EVAL_METHOD to -1 when fast-math is enabled.
Zahira Ammarguellat via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 23 07:05:16 PDT 2022
zahiraam added a comment.
In D121122#3402442 <https://reviews.llvm.org/D121122#3402442>, @aaron.ballman wrote:
> In D121122#3402334 <https://reviews.llvm.org/D121122#3402334>, @bjope wrote:
>
>> Hello. We've got some problem in our downstream tests after this patch and I'm trying to figure out how things are supposed to work. Maybe someone being part of this review knows.
>
> Sorry for the troubles!
>
>> Problem is that we have some libc verification suites that include test cases using `float_t`. But in math.h from for example newlib the type float_t isn't defined if FLT_EVAL_METHOD is set to -1.
>> The standard says that float_t is implementation defined when FLT_EVAL_METHOD isn't 0/1/2. But I'm not quite sure how that is supposed to be handled (such as where to define float_t if not in math.h).
>
> The way I read the requirements in C2x 7.12p3 are that the types `float_t` and `double_t` are always defined in `<math.h>`, but in the event `FLT_EVAL_METHOD` isn't 0, 1, or 2, the implementation has to pick whatever type makes the most sense for the underlying types.
>
>> One question is: If I have a piece of C99 code using float_t, is that code not allowed to be compiled using fast-math?
>
> The standard doesn't admit that fast math is a thing; it's up to the implementations to define what their fast math extension does.
>
>> I guess it should be seen as with fast-math the float_t precision is unknown. But the type still has to be defined somewhere or else the frontend will complain about "unknown type name". But I'm not sure where this is supposed to be handled.
>
> I think it should still be handled in `<math.h>`. *I am not a C floating point expert, so take this suggestion with a grain of salt*: I think it is defensible to fallback to defining `float_t` as `float` and `double_t` as `double` when the eval method is indeterminable. @andrew.w.kaylor may have more nuanced thoughts here.
Thanks for the answer @aaron.ballman.
Looking at this https://godbolt.org/z/drhGEjvns i see that float_t is defined in math.h (if I remove the include it will fail). Not sure what version of libs godbolt is using, but I tend to agree that it should be defined in math.h too. But Andy will have a more definite answer.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121122/new/
https://reviews.llvm.org/D121122
More information about the cfe-commits
mailing list