On Tue, 16 Aug 2022, Jakub Jelinek wrote: > On Mon, Aug 15, 2022 at 03:06:16PM +0200, Jakub Jelinek via Gcc-patches wrote: > > On Mon, Aug 15, 2022 at 12:07:38PM +0000, Richard Biener wrote: > > > Ah, I misread > > > > > > +static rtx > > > +expand_builtin_issignaling (tree exp, rtx target) > > > +{ > > > + if (!validate_arglist (exp, REAL_TYPE, VOID_TYPE)) > > > + return NULL_RTX; > > > + > > > + tree arg = CALL_EXPR_ARG (exp, 0); > > > + scalar_float_mode fmode = SCALAR_FLOAT_TYPE_MODE (TREE_TYPE (arg)); > > > + const struct real_format *fmt = REAL_MODE_FORMAT (fmode); > > > + > > > + /* Expand the argument yielding a RTX expression. */ > > > + rtx temp = expand_normal (arg); > > > + > > > + /* If mode doesn't support NaN, always return 0. */ > > > + if (!HONOR_NANS (fmode)) > > > + { > > > + emit_move_insn (target, const0_rtx); > > > + return target; > > > > I think I can expand on the comment why HONOR_NANS instead of HONOR_SNANS > > and also add comment to the folding case. > > So what about like in this incremental patch: > > --- gcc/builtins.cc.jj 2022-08-16 13:23:04.220103861 +0200 > +++ gcc/builtins.cc 2022-08-16 13:32:03.411257574 +0200 > @@ -2765,7 +2765,13 @@ expand_builtin_issignaling (tree exp, rt > /* Expand the argument yielding a RTX expression. */ > rtx temp = expand_normal (arg); > > - /* If mode doesn't support NaN, always return 0. */ > + /* If mode doesn't support NaN, always return 0. > + Don't use !HONOR_SNANS (fmode) here, so there is some possibility of > + __builtin_issignaling working without -fsignaling-nans. Especially > + when -fno-signaling-nans is the default. > + On the other side, MODE_HAS_NANS (fmode) is unnecessary, with > + -ffinite-math-only even __builtin_isnan or __builtin_fpclassify > + fold to 0 or non-NaN/Inf classification. */ > if (!HONOR_NANS (fmode)) > { > emit_move_insn (target, const0_rtx); > @@ -9259,6 +9265,12 @@ fold_builtin_classify (location_t loc, t > return fold_build2_loc (loc, UNORDERED_EXPR, type, arg, arg); > > case BUILT_IN_ISSIGNALING: > + /* Folding to true for REAL_CST is done in fold_const_call_ss. > + Don't use tree_expr_signaling_nan_p (arg) -> integer_one_node > + and !tree_expr_maybe_signaling_nan_p (arg) -> integer_zero_node > + here, so there is some possibility of __builtin_issignaling working > + without -fsignaling-nans. Especially when -fno-signaling-nans is > + the default. */ > if (!tree_expr_maybe_nan_p (arg)) > return omit_one_operand_loc (loc, type, integer_zero_node, arg); > return NULL_TREE;
Can you also amend the extend.texi documentation? I think the behavior will be special enough to worth mentioning it (I don't see any of -ffinite-math-only effect on isnan/isinf mentioned though). I'm OK with the rest of the patch if Joseph doesn't have comments on the actual issignaling lowerings (which I didn't review for correctness due to lack of knowledge). > > > > That seems like a glibc bug/weird feature in the __MATH_TG macro > > > > or _Generic. > > > > When compiled with C++ it is rejected. > > > > > > So what about __builtin_issignaling then? Do we want to silently > > > ignore errors there? > > > > I think we should just restrict it to the scalar floating point types. > > After all, other typegeneric builtins that are or can be used similarly > > do the same thing. > > Note, that is what the patch does currently (rejecting _Complex > {float,double,long double} etc. arguments). I see. Richard.