[PATCH] D104854: Introduce intrinsic llvm.isnan

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

> The options '-ffast-math' and '-fno-honor-nans' imply that FP operation
> operands are never NaNs. This assumption however should not be applied
> to the functions that check FP number properties, including 'isnan'. If
> such function returns expected result instead of actually making
> checks, it becomes useless in many cases.

This doesn't work the way you want it to, at least given the way nnan/ninf are currently defined in LangRef.  It's possible to end up in a situation where `isnan(x) == isnan(x)` evaluates to false at runtime.  It doesn't matter how you compute isnan; the problem is that the input is poisoned.

I think the right solution to this sort of issue is to insert a "freeze" in the LLVM IR, or something like that.  Not sure how we'd expect users to write this in C.  Suggestions welcome.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:6964
+    return DAG.getSetCC(DL, ResultVT, Op, DAG.getConstantFP(0.0, DL, OperandVT),
+                        ISD::SETUO);
+
----------------
Maybe we want to consider falling back to the integer path if SETCC isn't legal for the given operand type?  We could do that as a followup, though.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:6984
+  return DAG.getSetCC(DL, ResultVT, Sub, DAG.getConstant(0, DL, IntVT),
+                      ISD::SETNE);
+}
----------------
Instead of emitting `ExpMaskV - AbsV != 0`, can we just emit `ExpMaskV != AbsV`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104854