From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x131.google.com (mail-lf1-x131.google.com [IPv6:2a00:1450:4864:20::131]) by sourceware.org (Postfix) with ESMTPS id C172D3858D34 for ; Thu, 2 May 2024 10:10:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C172D3858D34 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C172D3858D34 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::131 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714644614; cv=none; b=p7muLUJCuH3jBpzGcNrTkXCMb8EFNxaZzI7/vs/AwnvgnSw5yptZNClrQJ0QmINrt+Oq8Mp/eHALdXuV20DnA2gH9PgUZZEqYfkVtsoowrNtDVy4xW3Ne+cgTr6E7xiKTL90ZuMgpbnpA44sT2ECkPCX6+iIa4AoEgSXOI6yQAs= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714644614; c=relaxed/simple; bh=GMhs9vf8lo0VT5cXfK7rBBbqqNQDPew2pxIiiXIUwHw=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=vjirZZIGtqBhsj3d09aTU8BYRzcj5jLYSzr8nMFN4l6Lc6eUIhcct7vPxKb4Zi0T0GiBh+UhdLQXpK7rVwK36qH/m4lpxfYznEMj70QrMsR6ScUm9Er0mPr23rzQMCLdMcEkC9zD07aKvekvRkipaHiZ4q3k1V/4EIpQZRNlbFE= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lf1-x131.google.com with SMTP id 2adb3069b0e04-51ab4ee9df8so9679923e87.1 for ; Thu, 02 May 2024 03:10:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1714644608; x=1715249408; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=Aw7fPEtjatAQij5zJOHIZlGnDMmsmbJ8tY41C88Teqc=; b=GqVqKVjeA8mC1ELuKe7UrR8gAncufvwTRWSq7f5+L2brVAkQn9jckghtLR+6+bR9xM Gyk5CuMfe25AM6KPixKKXjRC2g/xsEUeNaYmCFrEu338rOH9uYs+Tr57fw5iuPoRnFhU kr11Ec+PXhWW4J0073vbjpalfZxOhMqxY18bgDomnd9U3B5hkleMVXaVw1Mkovc6sBHc 0SOlyLhQOFyEzfU/JwkbAaUfH9Ym8s1VkEflamuNB8tRF3NbFq7lBp9pZfkd5wqkVlz/ RC7QKOd4XmKzs6gXAMIAS89SZ0NSs3y/3WjFLwRgWH/sOizjYbpjvsLDrCwUDzOlO9XY 0LlA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714644608; x=1715249408; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Aw7fPEtjatAQij5zJOHIZlGnDMmsmbJ8tY41C88Teqc=; b=dH7B611V3igIWNRSUgqjOrhoI/vwyhcv5+/BFSYNLPzsJ9GTqViN6b72FTtdZOE4xH YfyfBG48ekyTGabeKZAt8c7mP0sYJOxLgFxnCNGCC0m4x6ziYTGwhGXOg8p+eUucvqXS kye4DN6zpo0NnGKedPqbcRghbgWRMzxNFzysRXbzI6jdqD6Tp6sxKhwsFuCxjsn4s5C2 0OOEyltWQZV9c36OKOFZb53JTC98QQNmf4zGgOpecp0dMvE1FJzmM/8BZMBSg5hG8srB YCSt3n40vHSoBfaemw97QSnqTxZZ7LODroaaYG1NLCsob2c3wXC1TpReTFtnVe9Mnieh eQDA== X-Forwarded-Encrypted: i=1; AJvYcCXBa4C3VJiaGKjdHkMQ3bgVaKqU+mAzb1/jBLw4hzDLNtv8ubU9OEFjvQx+/tFU0GOuZd5YPW7O20L/ZrN5UVCKqjSDwC0c+Q== X-Gm-Message-State: AOJu0YzF7kkryyKIQOKt16w64q0pzGk2perQzib6nYe7e/IHQaL6WW88 Wcl1yWyko18NYUMnhEeuxywKSS+J/SSvW09dNwe457CwCdifVbb6rtiCPQN2QSJuPID7DPU+F3s QLVaLrxJ2qlSUbTdAddB+2yP+HPwonT0A X-Google-Smtp-Source: AGHT+IFH54BQfsHfo4k3Q7E/4zzGhbkgGJhPCZPDWDRHCpS9Hpv5+MstZMuo80pFr9P7rejD8AQRbVAOx5I3N9z/JPE= X-Received: by 2002:a05:6512:3055:b0:51f:c8f:630e with SMTP id b21-20020a056512305500b0051f0c8f630emr1390534lfb.35.1714644608115; Thu, 02 May 2024 03:10:08 -0700 (PDT) MIME-Version: 1.0 References: <009201da97b2$325fc140$971f43c0$@nextmovesoftware.com> <001401da9c73$ff2bbec0$fd833c40$@nextmovesoftware.com> In-Reply-To: <001401da9c73$ff2bbec0$fd833c40$@nextmovesoftware.com> From: Richard Biener Date: Thu, 2 May 2024 12:09:57 +0200 Message-ID: Subject: Re: [PATCH] PR middle-end/111701: signbit(x*x) vs -fsignaling-nans To: Roger Sayle Cc: Joseph Myers , gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-1.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,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: 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 si= gnaling > > 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 implementation= s > > > (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 s= tage 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 instea= d have to use > > tree_expr_maybe_nan_p () - and if NaN*NaN cannot be -NaN (is that at le= ast > > 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 signali= ng 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. This might or might not be covered by -fdelete-dead-exceptions - at least i= n 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). 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 preser= ve 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). With the patch the multiplication is only preserved because __builtin_signb= it still uses it. A plain void foo(double x) { x*x; } has the multiplication elided during gimplification already (even at -O0). So I don't think the patch is a meaningful improvement as to preserve multiplications of sNaNs. Richard. > 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 redu= ced > from a P2 to a P3 regression (or even not a bug if the qNaN case is undef= ined 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) : > > > Split handling of floating point and integer types. For equa= l > > > floating point operands, avoid optimization if the operand ma= y 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. > > > > >