sepavloff added a comment.
In D104854#2886328 <https://reviews.llvm.org/D104854#2886328>, @efriedma wrote:
>> 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.
According to the documentation, nnan/ninf may be applied to `fneg`, `fadd`,
`fsub`, `fmul`, `fdiv`, `frem`, `fcmp`, `phi`, `select` and `call`. We can
ignore this flag for calls of isnan and similar functions. Of course, if
conditions of using `-ffast-math` are broken, we have undefined behavior and
`isnan(x) != isnan(x)` becomes possible, like in this code:
%c = fadd %a, nan
%r = call llvm.isnan.f32(%c)
Similarly, it is legitimate to optimize `isnan` in the code:
%c = fadd %a, %b
%r = call llvm.isnan.f32(%c)
In this case the result of `fadd` cannot be NaN, otherwise contract of
`-ffast-math` is broken. So `isnan` in this case may be optimized to `false`.
================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:6964
+ return DAG.getSetCC(DL, ResultVT, Op, DAG.getConstantFP(0.0, DL,
OperandVT),
+ ISD::SETUO);
+
----------------
efriedma wrote:
> 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.
It makes sense, it could be beneficial for targets that have limited set of
floating point comparisons. However straightforward check like:
if (Flags.hasNoFPExcept() && isOperationLegalOrCustom(ISD::SETCC,
OperandVT))
results in worse code, mainly for vector types. It should be more complex check.
================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:6984
+ return DAG.getSetCC(DL, ResultVT, Sub, DAG.getConstant(0, DL, IntVT),
+ ISD::SETNE);
+}
----------------
efriedma wrote:
> Instead of emitting `ExpMaskV - AbsV != 0`, can we just emit `ExpMaskV !=
> AbsV`?
> Instead of emitting ExpMaskV - AbsV != 0, can we just emit ExpMaskV != AbsV?
Implemented.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104854/new/
https://reviews.llvm.org/D104854
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
