From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 187BC3858C20 for ; Tue, 16 Aug 2022 11:41:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 187BC3858C20 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 079D2348BA; Tue, 16 Aug 2022 11:41:07 +0000 (UTC) Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id E4F662C149; Tue, 16 Aug 2022 11:41:06 +0000 (UTC) Date: Tue, 16 Aug 2022 11:41:06 +0000 (UTC) From: Richard Biener To: Jakub Jelinek cc: gcc-patches@gcc.gnu.org, FX , "Joseph S. Myers" Subject: Re: [PATCH] Implement __builtin_issignaling In-Reply-To: Message-ID: References: User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Aug 2022 11:41:09 -0000 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.