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

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff added a comment.

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

>> On most targets turning off FP exceptions means reading content of FP control register, changing value of mask bits and putting the modified register value back. It is expensive operation and cannot be made invisible to LLVM. Some targets (like RISCV) do not have possibility to mask FP exceptions at all, so at IR level there is no way to turn exception off. Default FP environment supposes that the exceptions are ignored, not disabled. In general case they are raised always.
>
> Managing the FP mode is a user responsibility. If the operation in unconstrained we can assume the default floating point environment without preserved exceptions.

This is true for rounding direction, denormal behavior or any other FP control mode. They are represented by bits in some registers, may be set/read by appropriate API functions.  Exception behavior is a very different thing. There is no register that keeps "current exception behavior". It is only a compiler hint that facilitates code generation. So user has limited means to control exception behavior.

> Floating point exception doesn’t mean trap, but that’s a possible mode. The IR semantics are not dependent on the possible set of lowerings. Ultimately between the user and codegen, this code must never have trapped and may never be transformed to a form that could trap.

Absolutely.

> The replacement fcmp must not trap, but it’s not the middle end’s responsibility to ensure that. This is correct regardless of what any particular processor may do or whatever the source did.

It is the middle end's responsibility to make only transformations that keep FP semantics. Replacement `is_fpclass` -> `fcmp` changes the semantics and can produce incorrect programs.

In D137811#4083368 <https://reviews.llvm.org/D137811#4083368>, @kpn wrote:

> Ok, true, it takes executing code to change the floating point environment. Yes, there would be inline assembly or a function call to change the FP environment. LLVM would see that code because there would be IR for it. All true.
>
> But LLVM wouldn't know what the inline assembly was doing. It wouldn't recognize the function call and thus wouldn't know what it was doing. LLVM would not know the floating point environment had changed. Rephrased, the change in the floating point environment would, to LLVM, be invisible. That's the "invisible" I was referring to earlier.

Ok, I see what is "invisible". Do you think inline assembly should be decorated as def/use of FP environment similar to function calls as is done in D111433 <https://reviews.llvm.org/D111433> and D139549 <https://reviews.llvm.org/D139549>?

> It's true that a blind replacement of is_fpclass with fcmp would be incorrect _if_ the floating point environment is not the default environment. But if we are in the default FP environment we can assume that any CPU trap will be hidden from us by the OS and therefore is not a part of our discussion of IR correctness.

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.

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

> Non-`strictfp` functions are assumed to be in the default FP environment, which implies there's some form of undefined behavior if you call a non-`strictfp` function with a non-default FP environment. The precise, formal semantics that effect this rule is of course underdefined,
>
> This is how we define default FP environment in the LangRef today:
>
>> The default LLVM floating-point environment assumes that floating-point instructions do not have side effects. Results assume the round-to-nearest rounding mode. No floating-point exception state is maintained in this environment. Therefore, there is no attempt to create or preserve invalid operation (SNaN) or division-by-zero exceptions.
>
> From this definition, it seems clear to me that we are legally allowed to insert instructions that would cause FP exceptions in non-`strictfp` functions.

LLVM must be able to represent semantics of the supported languages. The code snippet above represents a program that is valid from viewpoint of C standard but would be broken if the middle end makes replacement `isnan(x)`->`x!=x`. So such replacement must not be made.


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

https://reviews.llvm.org/D137811