[PATCH] D71897: [X86] Adding fp128 support for strict fcmp

[Pengfei Wang via Phabricator via llvm-commits]

pengfei marked an inline comment as done.
pengfei added a comment.

In D71897#1796679 <https://reviews.llvm.org/D71897#1796679>, @craig.topper wrote:

> There are libcalls for OGT/OLT/OLE/OGE that are always signalling. There is a libcall for OEQ/UNE that not signalling. And there's a libcall for Ordered/Unordered that is not signalling.


Is the signaling/no signaling action by design (the API guarantees the behavior) or depended on targets. I don't find the description about signaling on GCC document. I guess it depends on targets.

> Note, that despite having multiple names there are really only 3 total libcalls. The names are aliases and the return value is used to check direction. The OGT/OLT/OLE/OGE libcall returns -1, 0, or 1. The other oeq/une and the ordered/unordered just return true or false.

In fact, there're only two. one is used for OGT/OLT/OLE/OGE/OEQ/ONE, one for ORD/UOR.
Why didn't we expand form these two functions directly? I think it can reduce the code complicity. And it can improve performance by reducing potential calls.

> We may need to expand the unsupported calls into multiple compares and control flow in IR. For example for non-signalling ogt/olt/ole/oge we'll need to check unordered first and branch accordingly for qnans without calling the signalling ogt/olt/ole/oge libcall which will signal. Technically we could use a select to change the value we pass to the ogt/olt/ole/oge when its signalling, but select has to be expanded into control flow on many targets anyway. @uweigand, what are you thoughts on this?

I think it can be simplified to directly use `__unordXf2` and `__cmpXf2`, if we know it's a NaN from the former, we can call a new function `TLI.raiseNaNException` to handle exception respectively.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:295
+                                         SDValue &Chain,
+                                         bool IsSignaling) const {
   assert((VT == MVT::f32 || VT == MVT::f64 || VT == MVT::f128 || VT == MVT::ppcf128)
----------------
craig.topper wrote:
> How are you handling the fact that the available libcalls don’t cover all the signaling behaviors? You don’t seem to be using the IsSignalling parameter?
I realized I can't handle signaling, but forgot to delete it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71897