[PATCH] D137811: InstCombine: Port amdgcn.class intrinsic combines to is.fpclass

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:842
+    // Equivalent of isnan. Replace with standard fcmp.
+    Value *FCmp = Builder.CreateFCmpUNO(Src0, Src0);
+    FCmp->takeName(&II);
----------------
jcranmer-intel wrote:
> sepavloff wrote:
> > arsenm wrote:
> > > sepavloff wrote:
> > > > This replacement is not valid in general case, only if FP exceptions are ignored. If the argument is a signaling NaN, compare instruction raises `Invalid` exception.
> > > Isn't that implied by not being a constrained intrinsic?
> > > 
> > `is_fpclass` does not depend on FP environment and does not change it. So it does not have constrained variant.
> `Builder.CreateFCmp*` functions look like they create quiet comparison instructions in FP-constrained mode, and you need to use `CreateFCmpS` to generate one of the signalling comparisons. So replacing them even in strict mode is legal.
Quiet comparison instruction do not raise FP exception if an argument is quiet NaN. It still generates exceptions for signaling NaNs.


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

https://reviews.llvm.org/D137811