* [PATCH] PR middle-end/111701: signbit(x*x) vs -fsignaling-nans
@ 2024-04-26 8:17 Roger Sayle
2024-05-02 9:19 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Roger Sayle @ 2024-04-26 8:17 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1819 bytes --]
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=unix{-m32}
with no new failures. Ok for mainline?
2024-04-26 Roger Sayle <roger@nextmovesoftware.com>
gcc/ChangeLog
PR middle-end/111701
* fold-const.cc (tree_binary_nonnegative_warnv_p) <case 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.
Thanks in advance,
Roger
--
[-- Attachment #2: patchnn2.txt --]
[-- Type: text/plain, Size: 2223 bytes --]
diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
index 7b26896..f7f174d 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -15076,16 +15076,27 @@ tree_binary_nonnegative_warnv_p (enum tree_code code, tree type, tree op0,
break;
case MULT_EXPR:
- if (FLOAT_TYPE_P (type) || TYPE_OVERFLOW_UNDEFINED (type))
+ if (FLOAT_TYPE_P (type))
{
- /* x * x is always non-negative for floating point x
- or without overflow. */
+ /* x * x is non-negative for floating point x except
+ that -NaN*-NaN may return -NaN. PR middle-end/111701. */
+ if (operand_equal_p (op0, op1, 0))
+ {
+ if (!tree_expr_maybe_signaling_nan_p (op0) || RECURSE (op0))
+ return true;
+ }
+ else if (RECURSE (op0) && RECURSE (op1))
+ return true;
+ }
+
+ if (ANY_INTEGRAL_TYPE_P (type)
+ && TYPE_OVERFLOW_UNDEFINED (type))
+ {
+ /* x * x is always non-negative without overflow. */
if (operand_equal_p (op0, op1, 0)
|| (RECURSE (op0) && RECURSE (op1)))
{
- if (ANY_INTEGRAL_TYPE_P (type)
- && TYPE_OVERFLOW_UNDEFINED (type))
- *strict_overflow_p = true;
+ *strict_overflow_p = true;
return true;
}
}
diff --git a/gcc/testsuite/gcc.dg/pr111701-1.c b/gcc/testsuite/gcc.dg/pr111701-1.c
new file mode 100644
index 0000000..5cbfac2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr111701-1.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fsignaling-nans -fdump-tree-optimized" } */
+
+int foo(double x)
+{
+ return __builtin_signbit(x*x);
+}
+
+int bar(float x)
+{
+ return __builtin_signbit(x*x);
+}
+
+/* { dg-final { scan-tree-dump-times " \\* " 2 "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/pr111701-2.c b/gcc/testsuite/gcc.dg/pr111701-2.c
new file mode 100644
index 0000000..f79c7ba
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr111701-2.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+int foo(double x)
+{
+ return __builtin_signbit(x*x);
+}
+
+int bar(float x)
+{
+ return __builtin_signbit(x*x);
+}
+
+/* { dg-final { scan-tree-dump-not " \\* " "optimized" } } */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PR middle-end/111701: signbit(x*x) vs -fsignaling-nans
2024-04-26 8:17 [PATCH] PR middle-end/111701: signbit(x*x) vs -fsignaling-nans Roger Sayle
@ 2024-05-02 9:19 ` Richard Biener
2024-05-02 9:34 ` Roger Sayle
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2024-05-02 9:19 UTC (permalink / raw)
To: Roger Sayle, Joseph Myers; +Cc: gcc-patches
On Fri, Apr 26, 2024 at 10:19 AM Roger Sayle <roger@nextmovesoftware.com> 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=unix{-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.
Richard.
>
> 2024-04-26 Roger Sayle <roger@nextmovesoftware.com>
>
> gcc/ChangeLog
> PR middle-end/111701
> * fold-const.cc (tree_binary_nonnegative_warnv_p) <case 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.
>
>
> Thanks in advance,
> Roger
> --
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] PR middle-end/111701: signbit(x*x) vs -fsignaling-nans
2024-05-02 9:19 ` Richard Biener
@ 2024-05-02 9:34 ` Roger Sayle
2024-05-02 10:09 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Roger Sayle @ 2024-05-02 9:34 UTC (permalink / raw)
To: 'Richard Biener', 'Joseph Myers'; +Cc: gcc-patches
> From: Richard Biener <richard.guenther@gmail.com>
> On Fri, Apr 26, 2024 at 10:19 AM Roger Sayle <roger@nextmovesoftware.com>
> 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=unix{-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.
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 <roger@nextmovesoftware.com>
> >
> > gcc/ChangeLog
> > PR middle-end/111701
> > * fold-const.cc (tree_binary_nonnegative_warnv_p) <case 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.
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PR middle-end/111701: signbit(x*x) vs -fsignaling-nans
2024-05-02 9:34 ` Roger Sayle
@ 2024-05-02 10:09 ` Richard Biener
2024-05-02 13:48 ` Roger Sayle
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2024-05-02 10:09 UTC (permalink / raw)
To: Roger Sayle; +Cc: Joseph Myers, gcc-patches
On Thu, May 2, 2024 at 11:34 AM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> > From: Richard Biener <richard.guenther@gmail.com>
> > On Fri, Apr 26, 2024 at 10:19 AM Roger Sayle <roger@nextmovesoftware.com>
> > 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=unix{-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.
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).
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).
With the patch the multiplication is only preserved because __builtin_signbit
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 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 <roger@nextmovesoftware.com>
> > >
> > > gcc/ChangeLog
> > > PR middle-end/111701
> > > * fold-const.cc (tree_binary_nonnegative_warnv_p) <case 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.
> > >
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] PR middle-end/111701: signbit(x*x) vs -fsignaling-nans
2024-05-02 10:09 ` Richard Biener
@ 2024-05-02 13:48 ` Roger Sayle
2024-05-03 10:22 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Roger Sayle @ 2024-05-02 13:48 UTC (permalink / raw)
To: 'Richard Biener'; +Cc: 'Joseph Myers', gcc-patches
> From: Richard Biener <richard.guenther@gmail.com>
> On Thu, May 2, 2024 at 11:34 AM Roger Sayle <roger@nextmovesoftware.com>
> wrote:
> >
> >
> > > From: Richard Biener <richard.guenther@gmail.com> On Fri, Apr 26,
> > > 2024 at 10:19 AM Roger Sayle <roger@nextmovesoftware.com>
> > > 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=unix{-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.
>
> 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).
>
> 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).
>
> With the patch the multiplication is only preserved because __builtin_signbit still
> uses it. A plain
>
> void foo(double x)
> {
> x*x;
> }
>
> has the multiplication elided during gimplification already (even at -O0).
void foo(double x)
{
double t = 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?
> 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 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 <roger@nextmovesoftware.com>
> > > >
> > > > gcc/ChangeLog
> > > > PR middle-end/111701
> > > > * fold-const.cc (tree_binary_nonnegative_warnv_p) <case
> 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.
> > > >
> >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PR middle-end/111701: signbit(x*x) vs -fsignaling-nans
2024-05-02 13:48 ` Roger Sayle
@ 2024-05-03 10:22 ` Richard Biener
0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2024-05-03 10:22 UTC (permalink / raw)
To: Roger Sayle; +Cc: Joseph Myers, gcc-patches
On Thu, May 2, 2024 at 3:48 PM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> > From: Richard Biener <richard.guenther@gmail.com>
> > On Thu, May 2, 2024 at 11:34 AM Roger Sayle <roger@nextmovesoftware.com>
> > wrote:
> > >
> > >
> > > > From: Richard Biener <richard.guenther@gmail.com> On Fri, Apr 26,
> > > > 2024 at 10:19 AM Roger Sayle <roger@nextmovesoftware.com>
> > > > 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=unix{-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.
> >
> > 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).
> >
> > 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).
> >
> > With the patch the multiplication is only preserved because __builtin_signbit still
> > uses it. A plain
> >
> > void foo(double x)
> > {
> > x*x;
> > }
> >
> > has the multiplication elided during gimplification already (even at -O0).
>
> void foo(double x)
> {
> double t = 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?
With optimization you need -fno-delete-dead-exceptions to preserve the
multiply. Btw, the observable trap is there even without -fnon-call-exceptions
and a trap isn't an exception.
So what I do not necessarily agree with is that we need to preserve
the multiplication with -fsignaling-nans. Do we consider a program doing
handler() { exit(0); }
x = sNaN;
...
sigaction(SIGFPE, ... handler)
x*x;
format_hard_drive();
and expecting the program to exit(0) rather than formating the hard-disk
to be expecting something the C standard guarantees? And is it enough
for the program to enable -fsignaling-nans for this?
If so then the first and foremost bug is that 'x*x' doesn't have
TREE_SIDE_EFFECTS
set and thus we do not preserve it when optimizing __builtin_signbit () of it.
Richard.
>
> > 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 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 <roger@nextmovesoftware.com>
> > > > >
> > > > > gcc/ChangeLog
> > > > > PR middle-end/111701
> > > > > * fold-const.cc (tree_binary_nonnegative_warnv_p) <case
> > 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.
> > > > >
> > >
> > >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-05-03 10:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-26 8:17 [PATCH] PR middle-end/111701: signbit(x*x) vs -fsignaling-nans Roger Sayle
2024-05-02 9:19 ` Richard Biener
2024-05-02 9:34 ` Roger Sayle
2024-05-02 10:09 ` Richard Biener
2024-05-02 13:48 ` Roger Sayle
2024-05-03 10:22 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).