[PATCH] D88913: [FPEnv] Use strictfp metadata in casting nodes

Tue Oct 27 09:55:36 PDT 2020

sepavloff added a comment.

In D88913#2356152 <https://reviews.llvm.org/D88913#2356152>, @kpn wrote:

> In D88913#2353379 <https://reviews.llvm.org/D88913#2353379>, @sepavloff wrote:
>
>> Generally the patch looks good. But the need to expect incorrect values in tests is a concern. Maybe this is a consequence of storing exception behavior in a separate field of CGFPOptionsRAII. This misbehavior should be fixed.
>
> In this patch? Because that's going to be a huge patch. Fixing all the issues with strictfp AST->IRBuilder is going to be large. Fixing them incrementally seems better to me. Eliminating the incorrect values in the tests marked FIXME would sweep the problem under the rug.
>
> It's true that incorrect values in tests is _not_ the desired end state. But it seems to me that as a _temporary_ incremental step it beats the alternatives.

OK. Users that called compiler with `-ffp-exception-behavior=strict` shouldn't notice any changes, right? Other non-default FP mode combinations anyway doesn't work now. So this patch shouldn't break existing code.

The only missing thing is tests that check non-default rounding mode. `#pragma STDC FENV_ROUND` can be used to prepare them, as it is done in D90026 <https://reviews.llvm.org/D90026>. Could you please add such test, to check if rounding mode is correctly processed in cast nodes?

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88913/new/

https://reviews.llvm.org/D88913