* [PATCH] range-op-float: Fix reverse ops of comparisons [PR109386]
@ 2023-04-03 19:53 Jakub Jelinek
2023-04-04 14:08 ` Aldy Hernandez
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2023-04-03 19:53 UTC (permalink / raw)
To: Andrew MacLeod, Aldy Hernandez; +Cc: gcc-patches
Hi!
I've missed one of my recent range-op-float.cc changes (likely the
r13-6967 one) caused
FAIL: libphobos.phobos/std/math/algebraic.d execution test
FAIL: libphobos.phobos_shared/std/math/algebraic.d execution test
regressions, distilled into a C testcase below.
In the testcase, we have
!(u >= v)
condition where both u and v are results of fabs*, which guards
t1 = u u<= __FLT_MAX__;
and
t2 = v u<= __FLT_MAX__;
comparisons. From false u >= v where u and v have [0.0, +Inf] NAN
ranges we (incorrectly deduce that one of them is [nextafterf (0.0, 1.0), +Inf] NAN
and the other is [0.0, nextafterf (+Inf, 0.0)] NAN and from that deduce that
one of the comparisons is always true, because UNLE_EXPR with the maximum
representable number are false only if the value is +Inf and our ranges tell
that is not possible.
The bug is that the u >= v comparison determines a sensible range only when
it is true - we then know neither operand can be NAN and it behaves
correctly. But when the comparison is false, our current code gives
sensible answers only if the other op can't be NAN. If it can be NAN,
whenever it is NAN, the comparison is always false regardless of the other
value, so the other value needs to be VARYING.
Now, foperator_unordered_lt::op1_range etc. had code to deal with that
for op?.known_nan (), but as the testcase shows, it is enough if it may be a
NAN at runtime to make it VARYING.
So, the following patch replaces for all those BRS_FALSE cases of the normal
non-equality comparisons if (opOTHER.known_isnan ()) r.set_varying (type);
to do it also if maybe_isnan ().
For the unordered or ... comparisons, it is similar for BRS_TRUE. Those
comparisons are true whenever either operand is NAN, or if neither is NAN,
the corresponding normal comparison. So, if those comparisons are true
and other operand might be a NAN, we can't tell (VARYING), if it is false,
currently handling is correct.
Bootstrapped/regtested on x86_64-linux and i686-linux, fixes those 2
D testcases and the newly added one. Ok for trunk?
2023-04-03 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/109386
* range-op-float.cc (foperator_lt::op1_range, foperator_lt::op2_range,
foperator_le::op1_range, foperator_le::op2_range,
foperator_gt::op1_range, foperator_gt::op2_range,
foperator_ge::op1_range, foperator_ge::op2_range): Make r varying for
BRS_FALSE case even if the other op is maybe_isnan, not just
known_isnan.
(foperator_unordered_lt::op1_range, foperator_unordered_lt::op2_range,
foperator_unordered_le::op1_range, foperator_unordered_le::op2_range,
foperator_unordered_gt::op1_range, foperator_unordered_gt::op2_range,
foperator_unordered_ge::op1_range, foperator_unordered_ge::op2_range):
Make r varying for BRS_TRUE case even if the other op is maybe_isnan,
not just known_isnan.
* gcc.c-torture/execute/ieee/pr109386.c: New test.
--- gcc/range-op-float.cc.jj 2023-04-03 10:42:54.000000000 +0200
+++ gcc/range-op-float.cc 2023-04-03 13:31:01.163216123 +0200
@@ -889,7 +889,7 @@ foperator_lt::op1_range (frange &r,
case BRS_FALSE:
// On the FALSE side of x < NAN, we know nothing about x.
- if (op2.known_isnan ())
+ if (op2.known_isnan () || op2.maybe_isnan ())
r.set_varying (type);
else
build_ge (r, type, op2);
@@ -926,7 +926,7 @@ foperator_lt::op2_range (frange &r,
case BRS_FALSE:
// On the FALSE side of NAN < x, we know nothing about x.
- if (op1.known_isnan ())
+ if (op1.known_isnan () || op1.maybe_isnan ())
r.set_varying (type);
else
build_le (r, type, op1);
@@ -1005,7 +1005,7 @@ foperator_le::op1_range (frange &r,
case BRS_FALSE:
// On the FALSE side of x <= NAN, we know nothing about x.
- if (op2.known_isnan ())
+ if (op2.known_isnan () || op2.maybe_isnan ())
r.set_varying (type);
else
build_gt (r, type, op2);
@@ -1038,7 +1038,7 @@ foperator_le::op2_range (frange &r,
case BRS_FALSE:
// On the FALSE side of NAN <= x, we know nothing about x.
- if (op1.known_isnan ())
+ if (op1.known_isnan () || op1.maybe_isnan ())
r.set_varying (type);
else if (op1.undefined_p ())
return false;
@@ -1122,7 +1122,7 @@ foperator_gt::op1_range (frange &r,
case BRS_FALSE:
// On the FALSE side of x > NAN, we know nothing about x.
- if (op2.known_isnan ())
+ if (op2.known_isnan () || op2.maybe_isnan ())
r.set_varying (type);
else if (op2.undefined_p ())
return false;
@@ -1161,7 +1161,7 @@ foperator_gt::op2_range (frange &r,
case BRS_FALSE:
// On The FALSE side of NAN > x, we know nothing about x.
- if (op1.known_isnan ())
+ if (op1.known_isnan () || op1.maybe_isnan ())
r.set_varying (type);
else if (op1.undefined_p ())
return false;
@@ -1241,7 +1241,7 @@ foperator_ge::op1_range (frange &r,
case BRS_FALSE:
// On the FALSE side of x >= NAN, we know nothing about x.
- if (op2.known_isnan ())
+ if (op2.known_isnan () || op2.maybe_isnan ())
r.set_varying (type);
else if (op2.undefined_p ())
return false;
@@ -1275,7 +1275,7 @@ foperator_ge::op2_range (frange &r, tree
case BRS_FALSE:
// On the FALSE side of NAN >= x, we know nothing about x.
- if (op1.known_isnan ())
+ if (op1.known_isnan () || op1.maybe_isnan ())
r.set_varying (type);
else if (op1.undefined_p ())
return false;
@@ -1639,7 +1639,7 @@ foperator_unordered_lt::op1_range (frang
switch (get_bool_state (r, lhs, type))
{
case BRS_TRUE:
- if (op2.known_isnan ())
+ if (op2.known_isnan () || op2.maybe_isnan ())
r.set_varying (type);
else if (op2.undefined_p ())
return false;
@@ -1673,7 +1673,7 @@ foperator_unordered_lt::op2_range (frang
switch (get_bool_state (r, lhs, type))
{
case BRS_TRUE:
- if (op1.known_isnan ())
+ if (op1.known_isnan () || op1.maybe_isnan ())
r.set_varying (type);
else if (op1.undefined_p ())
return false;
@@ -1755,7 +1755,7 @@ foperator_unordered_le::op1_range (frang
switch (get_bool_state (r, lhs, type))
{
case BRS_TRUE:
- if (op2.known_isnan ())
+ if (op2.known_isnan () || op2.maybe_isnan ())
r.set_varying (type);
else if (op2.undefined_p ())
return false;
@@ -1788,7 +1788,7 @@ foperator_unordered_le::op2_range (frang
switch (get_bool_state (r, lhs, type))
{
case BRS_TRUE:
- if (op1.known_isnan ())
+ if (op1.known_isnan () || op1.maybe_isnan ())
r.set_varying (type);
else if (op1.undefined_p ())
return false;
@@ -1872,7 +1872,7 @@ foperator_unordered_gt::op1_range (frang
switch (get_bool_state (r, lhs, type))
{
case BRS_TRUE:
- if (op2.known_isnan ())
+ if (op2.known_isnan () || op2.maybe_isnan ())
r.set_varying (type);
else if (op2.undefined_p ())
return false;
@@ -1907,7 +1907,7 @@ foperator_unordered_gt::op2_range (frang
switch (get_bool_state (r, lhs, type))
{
case BRS_TRUE:
- if (op1.known_isnan ())
+ if (op1.known_isnan () || op1.maybe_isnan ())
r.set_varying (type);
else if (op1.undefined_p ())
return false;
@@ -1991,7 +1991,7 @@ foperator_unordered_ge::op1_range (frang
switch (get_bool_state (r, lhs, type))
{
case BRS_TRUE:
- if (op2.known_isnan ())
+ if (op2.known_isnan () || op2.maybe_isnan ())
r.set_varying (type);
else if (op2.undefined_p ())
return false;
@@ -2025,7 +2025,7 @@ foperator_unordered_ge::op2_range (frang
switch (get_bool_state (r, lhs, type))
{
case BRS_TRUE:
- if (op1.known_isnan ())
+ if (op1.known_isnan () || op1.maybe_isnan ())
r.set_varying (type);
else if (op1.undefined_p ())
return false;
--- gcc/testsuite/gcc.c-torture/execute/ieee/pr109386.c.jj 2023-04-03 13:44:59.715981917 +0200
+++ gcc/testsuite/gcc.c-torture/execute/ieee/pr109386.c 2023-04-03 13:44:48.480145842 +0200
@@ -0,0 +1,21 @@
+/* PR tree-optimization/109386 */
+
+static inline float
+foo (float x, float y)
+{
+ float u = __builtin_fabsf (x);
+ float v = __builtin_fabsf (y);
+ if (!(u >= v))
+ {
+ if (__builtin_isinf (v)) return v;
+ if (__builtin_isinf (u)) return u;
+ }
+ return 42.0f;
+}
+
+int
+main ()
+{
+ if (!__builtin_isinf (foo (__builtin_inff (), __builtin_nanf (""))))
+ __builtin_abort ();
+}
Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] range-op-float: Fix reverse ops of comparisons [PR109386]
2023-04-03 19:53 [PATCH] range-op-float: Fix reverse ops of comparisons [PR109386] Jakub Jelinek
@ 2023-04-04 14:08 ` Aldy Hernandez
0 siblings, 0 replies; 2+ messages in thread
From: Aldy Hernandez @ 2023-04-04 14:08 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Andrew MacLeod, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 9230 bytes --]
OK.
On Mon, Apr 3, 2023, 21:54 Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> I've missed one of my recent range-op-float.cc changes (likely the
> r13-6967 one) caused
> FAIL: libphobos.phobos/std/math/algebraic.d execution test
> FAIL: libphobos.phobos_shared/std/math/algebraic.d execution test
> regressions, distilled into a C testcase below.
>
> In the testcase, we have
> !(u >= v)
> condition where both u and v are results of fabs*, which guards
> t1 = u u<= __FLT_MAX__;
> and
> t2 = v u<= __FLT_MAX__;
> comparisons. From false u >= v where u and v have [0.0, +Inf] NAN
> ranges we (incorrectly deduce that one of them is [nextafterf (0.0, 1.0),
> +Inf] NAN
> and the other is [0.0, nextafterf (+Inf, 0.0)] NAN and from that deduce
> that
> one of the comparisons is always true, because UNLE_EXPR with the maximum
> representable number are false only if the value is +Inf and our ranges
> tell
> that is not possible.
>
> The bug is that the u >= v comparison determines a sensible range only when
> it is true - we then know neither operand can be NAN and it behaves
> correctly. But when the comparison is false, our current code gives
> sensible answers only if the other op can't be NAN. If it can be NAN,
> whenever it is NAN, the comparison is always false regardless of the other
> value, so the other value needs to be VARYING.
> Now, foperator_unordered_lt::op1_range etc. had code to deal with that
> for op?.known_nan (), but as the testcase shows, it is enough if it may be
> a
> NAN at runtime to make it VARYING.
>
> So, the following patch replaces for all those BRS_FALSE cases of the
> normal
> non-equality comparisons if (opOTHER.known_isnan ()) r.set_varying (type);
> to do it also if maybe_isnan ().
>
> For the unordered or ... comparisons, it is similar for BRS_TRUE. Those
> comparisons are true whenever either operand is NAN, or if neither is NAN,
> the corresponding normal comparison. So, if those comparisons are true
> and other operand might be a NAN, we can't tell (VARYING), if it is false,
> currently handling is correct.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, fixes those 2
> D testcases and the newly added one. Ok for trunk?
>
> 2023-04-03 Jakub Jelinek <jakub@redhat.com>
>
> PR tree-optimization/109386
> * range-op-float.cc (foperator_lt::op1_range,
> foperator_lt::op2_range,
> foperator_le::op1_range, foperator_le::op2_range,
> foperator_gt::op1_range, foperator_gt::op2_range,
> foperator_ge::op1_range, foperator_ge::op2_range): Make r varying
> for
> BRS_FALSE case even if the other op is maybe_isnan, not just
> known_isnan.
> (foperator_unordered_lt::op1_range,
> foperator_unordered_lt::op2_range,
> foperator_unordered_le::op1_range,
> foperator_unordered_le::op2_range,
> foperator_unordered_gt::op1_range,
> foperator_unordered_gt::op2_range,
> foperator_unordered_ge::op1_range,
> foperator_unordered_ge::op2_range):
> Make r varying for BRS_TRUE case even if the other op is
> maybe_isnan,
> not just known_isnan.
>
> * gcc.c-torture/execute/ieee/pr109386.c: New test.
>
> --- gcc/range-op-float.cc.jj 2023-04-03 10:42:54.000000000 +0200
> +++ gcc/range-op-float.cc 2023-04-03 13:31:01.163216123 +0200
> @@ -889,7 +889,7 @@ foperator_lt::op1_range (frange &r,
>
> case BRS_FALSE:
> // On the FALSE side of x < NAN, we know nothing about x.
> - if (op2.known_isnan ())
> + if (op2.known_isnan () || op2.maybe_isnan ())
> r.set_varying (type);
> else
> build_ge (r, type, op2);
> @@ -926,7 +926,7 @@ foperator_lt::op2_range (frange &r,
>
> case BRS_FALSE:
> // On the FALSE side of NAN < x, we know nothing about x.
> - if (op1.known_isnan ())
> + if (op1.known_isnan () || op1.maybe_isnan ())
> r.set_varying (type);
> else
> build_le (r, type, op1);
> @@ -1005,7 +1005,7 @@ foperator_le::op1_range (frange &r,
>
> case BRS_FALSE:
> // On the FALSE side of x <= NAN, we know nothing about x.
> - if (op2.known_isnan ())
> + if (op2.known_isnan () || op2.maybe_isnan ())
> r.set_varying (type);
> else
> build_gt (r, type, op2);
> @@ -1038,7 +1038,7 @@ foperator_le::op2_range (frange &r,
>
> case BRS_FALSE:
> // On the FALSE side of NAN <= x, we know nothing about x.
> - if (op1.known_isnan ())
> + if (op1.known_isnan () || op1.maybe_isnan ())
> r.set_varying (type);
> else if (op1.undefined_p ())
> return false;
> @@ -1122,7 +1122,7 @@ foperator_gt::op1_range (frange &r,
>
> case BRS_FALSE:
> // On the FALSE side of x > NAN, we know nothing about x.
> - if (op2.known_isnan ())
> + if (op2.known_isnan () || op2.maybe_isnan ())
> r.set_varying (type);
> else if (op2.undefined_p ())
> return false;
> @@ -1161,7 +1161,7 @@ foperator_gt::op2_range (frange &r,
>
> case BRS_FALSE:
> // On The FALSE side of NAN > x, we know nothing about x.
> - if (op1.known_isnan ())
> + if (op1.known_isnan () || op1.maybe_isnan ())
> r.set_varying (type);
> else if (op1.undefined_p ())
> return false;
> @@ -1241,7 +1241,7 @@ foperator_ge::op1_range (frange &r,
>
> case BRS_FALSE:
> // On the FALSE side of x >= NAN, we know nothing about x.
> - if (op2.known_isnan ())
> + if (op2.known_isnan () || op2.maybe_isnan ())
> r.set_varying (type);
> else if (op2.undefined_p ())
> return false;
> @@ -1275,7 +1275,7 @@ foperator_ge::op2_range (frange &r, tree
>
> case BRS_FALSE:
> // On the FALSE side of NAN >= x, we know nothing about x.
> - if (op1.known_isnan ())
> + if (op1.known_isnan () || op1.maybe_isnan ())
> r.set_varying (type);
> else if (op1.undefined_p ())
> return false;
> @@ -1639,7 +1639,7 @@ foperator_unordered_lt::op1_range (frang
> switch (get_bool_state (r, lhs, type))
> {
> case BRS_TRUE:
> - if (op2.known_isnan ())
> + if (op2.known_isnan () || op2.maybe_isnan ())
> r.set_varying (type);
> else if (op2.undefined_p ())
> return false;
> @@ -1673,7 +1673,7 @@ foperator_unordered_lt::op2_range (frang
> switch (get_bool_state (r, lhs, type))
> {
> case BRS_TRUE:
> - if (op1.known_isnan ())
> + if (op1.known_isnan () || op1.maybe_isnan ())
> r.set_varying (type);
> else if (op1.undefined_p ())
> return false;
> @@ -1755,7 +1755,7 @@ foperator_unordered_le::op1_range (frang
> switch (get_bool_state (r, lhs, type))
> {
> case BRS_TRUE:
> - if (op2.known_isnan ())
> + if (op2.known_isnan () || op2.maybe_isnan ())
> r.set_varying (type);
> else if (op2.undefined_p ())
> return false;
> @@ -1788,7 +1788,7 @@ foperator_unordered_le::op2_range (frang
> switch (get_bool_state (r, lhs, type))
> {
> case BRS_TRUE:
> - if (op1.known_isnan ())
> + if (op1.known_isnan () || op1.maybe_isnan ())
> r.set_varying (type);
> else if (op1.undefined_p ())
> return false;
> @@ -1872,7 +1872,7 @@ foperator_unordered_gt::op1_range (frang
> switch (get_bool_state (r, lhs, type))
> {
> case BRS_TRUE:
> - if (op2.known_isnan ())
> + if (op2.known_isnan () || op2.maybe_isnan ())
> r.set_varying (type);
> else if (op2.undefined_p ())
> return false;
> @@ -1907,7 +1907,7 @@ foperator_unordered_gt::op2_range (frang
> switch (get_bool_state (r, lhs, type))
> {
> case BRS_TRUE:
> - if (op1.known_isnan ())
> + if (op1.known_isnan () || op1.maybe_isnan ())
> r.set_varying (type);
> else if (op1.undefined_p ())
> return false;
> @@ -1991,7 +1991,7 @@ foperator_unordered_ge::op1_range (frang
> switch (get_bool_state (r, lhs, type))
> {
> case BRS_TRUE:
> - if (op2.known_isnan ())
> + if (op2.known_isnan () || op2.maybe_isnan ())
> r.set_varying (type);
> else if (op2.undefined_p ())
> return false;
> @@ -2025,7 +2025,7 @@ foperator_unordered_ge::op2_range (frang
> switch (get_bool_state (r, lhs, type))
> {
> case BRS_TRUE:
> - if (op1.known_isnan ())
> + if (op1.known_isnan () || op1.maybe_isnan ())
> r.set_varying (type);
> else if (op1.undefined_p ())
> return false;
> --- gcc/testsuite/gcc.c-torture/execute/ieee/pr109386.c.jj 2023-04-03
> 13:44:59.715981917 +0200
> +++ gcc/testsuite/gcc.c-torture/execute/ieee/pr109386.c 2023-04-03
> 13:44:48.480145842 +0200
> @@ -0,0 +1,21 @@
> +/* PR tree-optimization/109386 */
> +
> +static inline float
> +foo (float x, float y)
> +{
> + float u = __builtin_fabsf (x);
> + float v = __builtin_fabsf (y);
> + if (!(u >= v))
> + {
> + if (__builtin_isinf (v)) return v;
> + if (__builtin_isinf (u)) return u;
> + }
> + return 42.0f;
> +}
> +
> +int
> +main ()
> +{
> + if (!__builtin_isinf (foo (__builtin_inff (), __builtin_nanf (""))))
> + __builtin_abort ();
> +}
>
> Jakub
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-04-04 14:08 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-03 19:53 [PATCH] range-op-float: Fix reverse ops of comparisons [PR109386] Jakub Jelinek
2023-04-04 14:08 ` Aldy Hernandez
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).