[PATCH] D112025: Intrinsic for checking floating point class

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff marked 2 inline comments as done.
sepavloff added inline comments.


================
Comment at: llvm/docs/LangRef.rst:22485
+if the element value satisfies the specified test. The function never raises
+floating point exceptions.
+
----------------
efriedma wrote:
> Maybe give an example here, e.g. 0x104 tests for a normal number.
> 
> Is there some reason the bits are arranged in this particular order?  Not that I really have a better suggestion.
I put a small example explaining the use of 'test' argument.

As for bit representation of particular tests, it is practically arbitrary. The sequence of bits agrees with the sequence of classes specified by IEEE-754 in 5.7.2. If there were a better sequence, it would be no problems to change the bits.


================
Comment at: llvm/include/llvm/IR/Intrinsics.td:734
+                            [llvm_anyfloat_ty, llvm_i32_ty],
+                            [IntrNoMem, IntrWillReturn]>;
+
----------------
efriedma wrote:
> Did you mean to add `ImmArg<ArgIndex<1>>`?
Exactly! Thank you!


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7269
+      return DAG.getSetCC(DL, ResultVT, Op, Op,
+                          IsInverted ? ISD::SETO : ISD::SETUO);
+  }
----------------
efriedma wrote:
> You could try to handle more cases here, e.g. fcPosNormal is two comparisons.  Not sure how worthwhile that is.
These cases can be implemented with a single comparison operation. Other cases require more than one comparison. For example, the test `fcPosNormal` can be implemented as `(x >= SmallestNormalized) && (x <= Largest)`. Also floating-point constant materialization may have some cost. The resulting gain is not obvious. For x86 floating point operations produce better code if SSE is available, but much worse code if x87 is used. So the advantage of floating point comparisons is target-dependent.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112025