nemanjai added inline comments.
================ Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:6964 + return DAG.getSetCC(DL, ResultVT, Op, DAG.getConstantFP(0.0, DL, OperandVT), + ISD::SETUO); + ---------------- sepavloff wrote: > nemanjai wrote: > > sepavloff wrote: > > > 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. > > > > > Out of curiosity, why was this added when you recognized that it results in > > worse code? This is certainly part of the reason for the regression for > > `ppc_fp128`. > > > > It would appear that before this patch, we would emit a comparison for all > > types that are not IEEE FP types (such as `ppc_fp128`). Those semantics do > > not seem to have carried over. > > Out of curiosity, why was this added when you recognized that it results in > > worse code? This is certainly part of the reason for the regression for > > ppc_fp128. > > It is my mistake. After experiments I forgot to remove this change. I am > sorry. > > For x86 and AArch64 I used modified `test-suite`, with changes from D106804. > Without proper tests it is hard to reveal why one intrinsic starts to fail. > > > It would appear that before this patch, we would emit a comparison for all > > types that are not IEEE FP types (such as ppc_fp128). Those semantics do > > not seem to have carried over. > > The previous behavior is not correct in non-default FP environment. Unordered > comparison raises Invalid exception if an operand is signaling NaN. On the > other hand, `isnan` must never raise exceptions. Well, if the **must never raise exceptions** is an IEEE-754 requirement (i.e. as noted in 5.7.2), I think it is reasonable that operations on types that do not conform to IEEE-754 are not bound by it. 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