sepavloff added a comment. In D104854#2905430 <https://reviews.llvm.org/D104854#2905430>, @efriedma wrote:
> In D104854#2904826 <https://reviews.llvm.org/D104854#2904826>, @sepavloff > wrote: > >> 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: > > Right... so how can you produce a NaN in these circumstances? You could load > one from memory, I guess? Yes, they come from structures in memory. I think they can also come from function arguments, if some source files are compiled with option `-ffast-math` and some without. > It would probably be a good idea to have an instcombine that combines away > isnan on a value produced by an operation marked nnan, so we don't confuse > people reading assembly into assuming isnan is actually reliable in that > context. Added such transformation (file llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp, test in llvm/test/Transforms/InstSimplify/ConstProp/fpclassify.ll). 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
