From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from server.nextmovesoftware.com (server.nextmovesoftware.com [69.48.154.134]) by sourceware.org (Postfix) with ESMTPS id DC9343858D20 for ; Thu, 2 May 2024 13:48:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DC9343858D20 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=nextmovesoftware.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=nextmovesoftware.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org DC9343858D20 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=69.48.154.134 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714657736; cv=none; b=HnHiaNqUo3pwRyFT30VDXWeiaSnq4DZ/AzBjUoO6QdtSkYa6L1j21BEx8yoMbZqlmYfSl1xqKwGr79m/oW5Rg++mrTOCigGLLi7M276sRQGaF+wbSn6Gpvr5fOfGkrYpBWcwvFzc5pmn4IFu+zQMqT/cDr/hqLCbxn67/zzflO0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714657736; c=relaxed/simple; bh=rgEDgQHk70O9Er2yUKeGHR63BlDotvhDvN9VeplsKHE=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=b1jPZ1fy2Nhkg2PwbNLNZebM2yj4xVuUm8YVQiNfXqOxECutbRyWza0UyELhscrL9ZAxPAB49NXyQgnz/SFhCKcKtbXjn3G2z7XktfbYta/sl2O67LeCL7BBXxsBAmkL1hp+VhwhEGTFCymUxBE79PrV8dZIdPD5QYOPPvmix1Y= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nextmovesoftware.com; s=default; h=Content-Transfer-Encoding:Content-Type: MIME-Version:Message-ID:Date:Subject:In-Reply-To:References:Cc:To:From:Sender :Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help: List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=MurcI8P8GNIQrsIX54gGYrUkPloybU0lrjjZHH/fwm0=; b=U7efq5WZ4sdmZLM8Ld4t5QiipN WLa6ifY3QZ5fdLfA5LmhySBsJxsDnERUlh8I33qv74Z3303efo2HDHQU6dzy82h+TXU8177qZGm02 taCRgw28Ju8ocuOKOw8KsrxXp6HlNylf+pTGYWlOkl1GT0De97AzScCXzMIJciSTgSh3zyUm+xZnH vn4xDDRZNv86hzs9dPBEl4HQhjfKGINzCPi+Q1ocCiEJ/TKyHdptGsZMXJjjSGDplvS9u1XUPkb3X StuycMqnRc5tzUcsGSCJsIKlXcHOuRdB9HyIAJNeHID/+IGzUgreAlMxYsI+bs1agPOuHKBSWV4Di fE+cDGhA==; Received: from [168.86.198.148] (port=55547 helo=Dell) by server.nextmovesoftware.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96.2) (envelope-from ) id 1s2Wnu-00ENkz-0y; Thu, 02 May 2024 09:48:54 -0400 From: "Roger Sayle" To: "'Richard Biener'" Cc: "'Joseph Myers'" , References: <009201da97b2$325fc140$971f43c0$@nextmovesoftware.com> <001401da9c73$ff2bbec0$fd833c40$@nextmovesoftware.com> In-Reply-To: Subject: RE: [PATCH] PR middle-end/111701: signbit(x*x) vs -fsignaling-nans Date: Thu, 2 May 2024 14:48:52 +0100 Message-ID: <00a201da9c97$78ab54e0$6a01fea0$@nextmovesoftware.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQFpJDeNQ4XW3FzPfj+oivxBOnliZQI7ZQOPAUCMPpIBQlFyobJA8lYQ Content-Language: en-gb X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - server.nextmovesoftware.com X-AntiAbuse: Original Domain - gcc.gnu.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - nextmovesoftware.com X-Get-Message-Sender-Via: server.nextmovesoftware.com: authenticated_id: roger@nextmovesoftware.com X-Authenticated-Sender: server.nextmovesoftware.com: roger@nextmovesoftware.com X-Source: X-Source-Args: X-Source-Dir: X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: > From: Richard Biener > On Thu, May 2, 2024 at 11:34=E2=80=AFAM Roger Sayle = > wrote: > > > > > > > From: Richard Biener On Fri, Apr 26, > > > 2024 at 10:19=E2=80=AFAM Roger Sayle > > > wrote: > > > > > > > > This patch addresses PR middle-end/111701 where optimization of > > > > signbit(x*x) using tree_nonnegative_p incorrectly eliminates a > > > > floating point multiplication when the operands may potentially = be > > > > signaling > > > NaNs. > > > > > > > > The above bug fix also provides a solution or work-around to the > > > > tricky issue in PR middle-end/111701, that the results of IEEE > > > > operations on NaNs are specified to return a NaN result, but = fail > > > > to > > > > (precisely) specify the exact NaN representation of this result. > > > > Hence for the operation "-NaN*-NaN" different hardware > > > > implementations > > > > (targets) return different results. Ultimately knowing what the > > > > resulting NaN "payload" of an operation is can only be known by > > > > executing that operation at run-time, and I'd suggest that GCC's > > > > -fsignaling-nans provides a mechanism for handling code that = uses > > > > NaN representations for communication/signaling (which is a > > > > different but related > > > concept to IEEE's sNaN). > > > > > > > > One nice thing about this patch, which may or may not be a P2 > > > > regression fix, is that it only affects (improves) code compiled > > > > with -fsignaling-nans so should be extremely safe even for this = point in stage > 3. > > > > > > > > This patch has been tested on x86_64-pc-linux-gnu with make > > > > bootstrap and make -k check, both with and without > > > > --target_board=3Dunix{-m32} with no new failures. Ok for = mainline? > > > > > > Hmm, but the bugreports are not about sNaN but about the fact that > > > the sign of the NaN produced by 0/0 or by -NaN*-NaN is not = well-defined. > > > So I don't think this is the correct approach to fix this. We'd > > > instead have to use tree_expr_maybe_nan_p () - and if NaN*NaN = cannot > > > be -NaN (is that at least > > > specified?) then the RECURSE path should still work as well. > > > > If we ignore the bugzilla PR for now, can we agree that if x is a > > signaling NaN, that we shouldn't be eliminating x*x? i.e. that this > > patch fixes a real bug, but perhaps not (precisely) the one = described in PR > middle-end/111701. >=20 > This might or might not be covered by -fdelete-dead-exceptions - at = least in the > past we were OK with removing traps like for -ftrapv (-ftrapv makes = signed > overflow no longer invoke undefined behavior) or when deleting loads = that might > trap (but those would invoke undefined behavior). >=20 > I bet the C standard doesn't say anything about sNaNs or how an = operation with > it has to behave in the abstract machine. We do document though that = it > "disables optimizations that may change the number of exceptions = visible with > signaling NaNs" which suggests that with -fsignaling-nans we have to = preserve all > such traps but I am very sure DCE will simply elide unused ops here = (also for other > FP operations with -ftrapping-math - but there we do not document that = we > preserve all traps). >=20 > With the patch the multiplication is only preserved because = __builtin_signbit still > uses it. A plain >=20 > void foo(double x) > { > x*x; > } >=20 > has the multiplication elided during gimplification already (even at = -O0). void foo(double x) { double t =3D x*x; } when compiled with -fsignaling-nans -fexceptions -fnon-call-exceptions doesn't exhibit the above bug. Perhaps this short-coming of = gimplification deserves its own Bugzilla PR? =20 > So I don't think the patch is a meaningful improvement as to preserve > multiplications of sNaNs. >=20 > Richard. >=20 > > Once the signaling NaN case is correctly handled, the use of > > -fsignaling-nans can be used as a workaround for PR 111701, allowing > > it to perhaps be reduced from a P2 to a P3 regression (or even not a = bug if the > qNaN case is undefined behavior). > > When I wrote this patch I was trying to help with GCC 14's stage 3. > > > > > > 2024-04-26 Roger Sayle > > > > > > > > gcc/ChangeLog > > > > PR middle-end/111701 > > > > * fold-const.cc (tree_binary_nonnegative_warnv_p) MULT_EXPR>: > > > > Split handling of floating point and integer types. For = equal > > > > floating point operands, avoid optimization if the = operand may be > > > > a signaling NaN. > > > > > > > > gcc/testsuite/ChangeLog > > > > PR middle-end/111701 > > > > * gcc.dg/pr111701-1.c: New test case. > > > > * gcc.dg/pr111701-2.c: Likewise. > > > > > > > >