[PATCH] D137811: InstCombine: Perform basic isnan combines on llvm.is.fpclass

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff added a comment.

In D137811#4085644 <https://reviews.llvm.org/D137811#4085644>, @arsenm wrote:

> In D137811#4084663 <https://reviews.llvm.org/D137811#4084663>, @sepavloff wrote:
>
>> What about such code?
>>
>>   int get_code(float x) {
>>     return isnan(x) ? 1 : 2;
>>   }
>>   void func1(float x) {
>>     int code1 = get_code(x);
>>     ...
>>   }
>>   #pragma STDC FENV_ACCESS ON
>>   void func2(float x) {
>>     int code1 = get_code(x);
>>     ...
>>   }
>>
>> Acoording to C standard it must work as intended. If compiler replaces `isnan` with comparison, the code would be broken.
>
> This code didn't actually manipulate the floating point mode, just made it legal to do so for the scope of the pragma. If you were to insert code to modify the mode to enable FP exceptions inside func2, it would be illegal because you did not restore the mode before calling into code outside of the pragma scope. FENV_ACCESS doesn't require the compiler to fixup the floating point mode for you before calling into other code.

Pragma STDC FENV_ACCESS ON may be used not only for code that manipulates control modes, but also when exception status is examined (7.6.1p2):

  The FENV_ACCESS pragma provides a means to inform the implementation when a program might
   access the floating-point environment to test floating-point status flags or run under non-default
   floating-point control modes.

A call to `fetestexcep` could be put somewhere inside `func2` to have an access to FP environment, but it does not change the behavior of `isnan` inside `get_code`.

In D137811#4085761 <https://reviews.llvm.org/D137811#4085761>, @jcranmer-intel wrote:

> From the C standard (7.6.1p2 from draft N3054):
>
>> (When execution passes from a part of the program translated with `FENV_ACCESS` "off" to a part translated with `FENV_ACCESS` "on", the state of the floating-point status flags is unspecified and the floating-point control modes have their default settings.)
>
>
>
>> 263. The purpose of the `FENV_ACCESS` pragma is to allow certain optimizations that could subvert flag tests and mode changes (e.g., global common subexpression elimination, code motion, and constant folding). In general, if the state of `FENV_ACCESS` is "off", the translator can assume that the flags are not tested, and that default modes are in effect, except where specified otherwise by an `FENV_ROUND` pragma.
>
> On a strict reading of the standard, on transition from `get_code` (which has `FENV_ACCESS OFF`) back to `func2`, the FP flags are unspecified, which means it is legal to transform the non-flag-setting `isnan` into flag-setting `fcmp`. There is a slight wording mismatch, as `FENV_ACCESS` is about //testing//, not //setting// flags, but the footnote seems to indicate that the correct reading is that `FENV_ACCESS` is necessary if you want guarantees about flags being set or not set.

This statement is not pertinent to this case because the only operation in `get_code` is a call to `isnan`, but it must not raise any exception according to the same standard (F3p6):

  The C classification macros fpclassify, iscanonical, isfinite, isinf, isnan, isnormal,
  issignaling, issubnormal, iszero, and signbit provide the IEC 60559 operations indicated
  in the table above provided their arguments are in the format of their semantic type. Then these
  macros raise no floating-point exceptions, even if an argument is a signaling NaN.

and this property must be kept no matter if exceptions are ignored or not.

In a broader context, it looks natural that a non-strictfp function may lose some exceptions that strictfp function would raise. But it would be counterintuitive if it raised new exceptions, which cannot be observed in strictfp function.


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

https://reviews.llvm.org/D137811