public inbox for gcc-regression@sourceware.org
help / color / mirror / Atom feed
* [TCWG CI] Failure after basepoints/gcc-13-4492-g4500baaccb6: range-op-float: Fix up multiplication and division reverse operation [PR107879]
@ 2022-12-05 16:39 ci_notify
  0 siblings, 0 replies; 2+ messages in thread
From: ci_notify @ 2022-12-05 16:39 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-regression

[-- Attachment #1: Type: text/plain, Size: 17603 bytes --]

Failure after basepoints/gcc-13-4492-g4500baaccb6: range-op-float: Fix up multiplication and division reverse operation [PR107879]:

Results changed to
-10
# true:
0
# build_abe binutils:
1
# build_abe gcc:
2
# build_abe linux:
4
# build_abe glibc:
# FAILED
# First few build errors in logs:
# 00:02:14 ../sysdeps/ieee754/ldbl-128/s_f64xdivf128.c:39:1: internal compiler error: in lower_bound, at value-range.h:350
# 00:02:14 ../sysdeps/ieee754/dbl-64/s_f32xdivf64.c:30:1: internal compiler error: in lower_bound, at value-range.h:350
# 00:02:14 ../sysdeps/ieee754/dbl-64/s_f32xmulf64.c:30:1: internal compiler error: in lower_bound, at value-range.h:350
# 00:02:14 ../sysdeps/ieee754/ldbl-128/s_f64xmulf128.c:39:1: internal compiler error: in lower_bound, at value-range.h:350
# 00:02:15 make[2]: *** [/home/tcwg-buildslave/workspace/tcwg_gnu_13/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/glibc-glibc.git~master/sysd-rules:553: /home/tcwg-buildslave/workspace/tcwg_gnu_13/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/glibc-glibc.git~master/math/s_f64xdivf128.o] Error 1
# 00:02:15 make[2]: *** [/home/tcwg-buildslave/workspace/tcwg_gnu_13/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/glibc-glibc.git~master/sysd-rules:587: /home/tcwg-buildslave/workspace/tcwg_gnu_13/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/glibc-glibc.git~master/math/s_f32xmulf64.o] Error 1
# 00:02:15 make[2]: *** [/home/tcwg-buildslave/workspace/tcwg_gnu_13/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/glibc-glibc.git~master/sysd-rules:553: /home/tcwg-buildslave/workspace/tcwg_gnu_13/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/glibc-glibc.git~master/math/s_f64xmulf128.o] Error 1
# 00:02:15 make[2]: *** [/home/tcwg-buildslave/workspace/tcwg_gnu_13/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/glibc-glibc.git~master/sysd-rules:587: /home/tcwg-buildslave/workspace/tcwg_gnu_13/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/glibc-glibc.git~master/math/s_f32xdivf64.o] Error 1
# 00:02:15 make[1]: *** [Makefile:484: math/others] Error 2
# 00:02:15 make: *** [Makefile:9: all] Error 2

from
-10
# true:
0
# build_abe binutils:
1
# build_abe gcc:
2
# build_abe linux:
4
# build_abe glibc:
5
# build_abe gdb:
6

THIS IS THE END OF INTERESTING STUFF.  BELOW ARE LINKS TO BUILDS, REPRODUCTION INSTRUCTIONS, AND THE RAW COMMIT.

For latest status see comments in https://linaro.atlassian.net/browse/GNU-692 .
Status of basepoints/gcc-13-4492-g4500baaccb6 commit for tcwg_gnu_native_build:
commit 4500baaccb6e4d696e223c338bbdf7705c3646dd
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Dec 5 11:17:42 2022 +0100

    range-op-float: Fix up multiplication and division reverse operation [PR107879]
    
    While for the normal cases it seems to be correct to implement
    reverse multiplication (op1_range/op2_range) through division
    with float_binary_op_range_finish, reverse division (op1_range)
    through multiplication with float_binary_op_range_finish or
    (op2_range) through division with float_binary_op_range_finish,
    as e.g. following testcase shows for the corner cases it is
    incorrect.
    Say on the testcase we are doing reverse multiplication, we
    have [-0., 0.] range (no NAN) on lhs and VARYING on op1 (or op2).
    We implement that through division, because x from
    lhs = x * op2
    is
    x = lhs / op2
    For the division, [-0., 0.] / VARYING is computed (IMHO correctly)
    as [-0., 0.] +-NAN, because 0 / anything but 0 or NAN is still
    0 and 0 / 0 is NAN and ditto 0 / NAN.  And then we just
    float_binary_op_range_finish, which figures out that because lhs
    can't be NAN, neither operand can be NAN.  So, the end range is
    [-0., 0.].  But that is not correct for the reverse multiplication.
    When the result is 0, if op2 can be zero, then x can be anything
    (VARYING), to be precise anything but INF (unless result can be NAN),
    because anything * 0 is 0 (or NAN for INF).  While if op2 must be
    non-zero, then x must be 0.  Of course the sign logic
    (signbit(x) = signbit(lhs) ^ signbit(op2)) still holds, so it actually
    isn't full VARYING if both lhs and op2 have known sign bits.
    And going through other corner cases one by one shows other differences
    between what we compute for the corresponding forward operation and
    what we should compute for the reverse operations.
    The following patch is slightly conservative and includes INF
    (in case of result including 0 and not NAN) in the ranges or
    0 in the ranges (in case of result including INF and not NAN).
    The latter is what happens anyway because we flush denormals to 0,
    and the former just not to deal with all the corner cases.
    So, the end test is that for reverse multiplication and division
    op2_range the cases we need to adjust to VARYING or VARYING positive
    or VARYING negative are if lhs and op? ranges both contain 0,
    or both contain some infinity, while for division op1_range the
    corner case is if lhs range contains 0 and op2 range contains INF or vice
    versa.  Otherwise I believe ranges from the corresponding operation
    are ok, or could be slightly more conservative (e.g. for
    reverse multiplication, if op? range is singleton INF and lhs
    range doesn't include any INF, then x's range should be UNDEFINED or
    known NAN (depending on if lhs can be NAN), while the division computes
    [-0., 0.] +-NAN; or similarly if op? range is only 0 and lhs range
    doesn't include 0, division would compute +INF +-NAN, or -INF +-NAN,
    or (for lack of multipart franges -INF +INF +-NAN just VARYING +-NAN),
    while again it is UNDEFINED or known NAN.
    
    Oh, and I found by code inspection wrong condition for the division's
    known NAN result, due to thinko it would trigger not just when
    both operands are known to be 0 or both are known to be INF, but
    when either both are known to be 0, or at least one is known to be INF.
    
    2022-12-05  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/107879
            * range-op-float.cc (foperator_mult::op1_range): If both
            lhs and op2 ranges contain zero or both ranges contain
            some infinity, set r range to zero_to_inf_range depending on
            signbit_known_p.
            (foperator_div::op2_range): Similarly for lhs and op1 ranges.
            (foperator_div::op1_range): If lhs range contains zero and op2
            range contains some infinity or vice versa, set r range to
            zero_to_inf_range depending on signbit_known_p.
            (foperator_div::rv_fold): Fix up condition for returning known NAN.
* master-aarch64
** Failure after basepoints/gcc-13-4492-g4500baaccb6: range-op-float: Fix up multiplication and division reverse operation [PR107879]:
** https://ci.linaro.org/job/tcwg_gnu_native_build-build-master-aarch64/552/

Bad  build: https://ci.linaro.org/job/tcwg_gnu_native_build-build-master-aarch64/552/artifact/artifacts
Good build: https://ci.linaro.org/job/tcwg_gnu_native_build-build-master-aarch64/551/artifact/artifacts

Reproduce current build:
<cut>
mkdir -p investigate-gcc-4500baaccb6e4d696e223c338bbdf7705c3646dd
cd investigate-gcc-4500baaccb6e4d696e223c338bbdf7705c3646dd

# Fetch scripts
git clone https://git.linaro.org/toolchain/jenkins-scripts

# Fetch manifests for bad and good builds
mkdir -p bad/artifacts good/artifacts
curl -o bad/artifacts/manifest.sh https://ci.linaro.org/job/tcwg_gnu_native_build-build-master-aarch64/552/artifact/artifacts/manifest.sh --fail
curl -o good/artifacts/manifest.sh https://ci.linaro.org/job/tcwg_gnu_native_build-build-master-aarch64/551/artifact/artifacts/manifest.sh --fail

# Reproduce bad build
(cd bad; ../jenkins-scripts/tcwg_gnu-build.sh ^^ true %%rr[top_artifacts] artifacts)
# Reproduce good build
(cd good; ../jenkins-scripts/tcwg_gnu-build.sh ^^ true %%rr[top_artifacts] artifacts)
</cut>

Full commit (up to 1000 lines):
<cut>
commit 4500baaccb6e4d696e223c338bbdf7705c3646dd
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Dec 5 11:17:42 2022 +0100

    range-op-float: Fix up multiplication and division reverse operation [PR107879]
    
    While for the normal cases it seems to be correct to implement
    reverse multiplication (op1_range/op2_range) through division
    with float_binary_op_range_finish, reverse division (op1_range)
    through multiplication with float_binary_op_range_finish or
    (op2_range) through division with float_binary_op_range_finish,
    as e.g. following testcase shows for the corner cases it is
    incorrect.
    Say on the testcase we are doing reverse multiplication, we
    have [-0., 0.] range (no NAN) on lhs and VARYING on op1 (or op2).
    We implement that through division, because x from
    lhs = x * op2
    is
    x = lhs / op2
    For the division, [-0., 0.] / VARYING is computed (IMHO correctly)
    as [-0., 0.] +-NAN, because 0 / anything but 0 or NAN is still
    0 and 0 / 0 is NAN and ditto 0 / NAN.  And then we just
    float_binary_op_range_finish, which figures out that because lhs
    can't be NAN, neither operand can be NAN.  So, the end range is
    [-0., 0.].  But that is not correct for the reverse multiplication.
    When the result is 0, if op2 can be zero, then x can be anything
    (VARYING), to be precise anything but INF (unless result can be NAN),
    because anything * 0 is 0 (or NAN for INF).  While if op2 must be
    non-zero, then x must be 0.  Of course the sign logic
    (signbit(x) = signbit(lhs) ^ signbit(op2)) still holds, so it actually
    isn't full VARYING if both lhs and op2 have known sign bits.
    And going through other corner cases one by one shows other differences
    between what we compute for the corresponding forward operation and
    what we should compute for the reverse operations.
    The following patch is slightly conservative and includes INF
    (in case of result including 0 and not NAN) in the ranges or
    0 in the ranges (in case of result including INF and not NAN).
    The latter is what happens anyway because we flush denormals to 0,
    and the former just not to deal with all the corner cases.
    So, the end test is that for reverse multiplication and division
    op2_range the cases we need to adjust to VARYING or VARYING positive
    or VARYING negative are if lhs and op? ranges both contain 0,
    or both contain some infinity, while for division op1_range the
    corner case is if lhs range contains 0 and op2 range contains INF or vice
    versa.  Otherwise I believe ranges from the corresponding operation
    are ok, or could be slightly more conservative (e.g. for
    reverse multiplication, if op? range is singleton INF and lhs
    range doesn't include any INF, then x's range should be UNDEFINED or
    known NAN (depending on if lhs can be NAN), while the division computes
    [-0., 0.] +-NAN; or similarly if op? range is only 0 and lhs range
    doesn't include 0, division would compute +INF +-NAN, or -INF +-NAN,
    or (for lack of multipart franges -INF +INF +-NAN just VARYING +-NAN),
    while again it is UNDEFINED or known NAN.
    
    Oh, and I found by code inspection wrong condition for the division's
    known NAN result, due to thinko it would trigger not just when
    both operands are known to be 0 or both are known to be INF, but
    when either both are known to be 0, or at least one is known to be INF.
    
    2022-12-05  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/107879
            * range-op-float.cc (foperator_mult::op1_range): If both
            lhs and op2 ranges contain zero or both ranges contain
            some infinity, set r range to zero_to_inf_range depending on
            signbit_known_p.
            (foperator_div::op2_range): Similarly for lhs and op1 ranges.
            (foperator_div::op1_range): If lhs range contains zero and op2
            range contains some infinity or vice versa, set r range to
            zero_to_inf_range depending on signbit_known_p.
            (foperator_div::rv_fold): Fix up condition for returning known NAN.
---
 gcc/range-op-float.cc                          | 74 +++++++++++++++++++++++---
 gcc/testsuite/gcc.c-torture/execute/pr107879.c | 25 +++++++++
 2 files changed, 91 insertions(+), 8 deletions(-)

diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index ee88511eba0..e9455a929ae 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -2143,8 +2143,30 @@ public:
     range_op_handler rdiv (RDIV_EXPR, type);
     if (!rdiv)
       return false;
-    return float_binary_op_range_finish (rdiv.fold_range (r, type, lhs, op2),
-					 r, type, lhs);
+    bool ret = rdiv.fold_range (r, type, lhs, op2);
+    if (ret == false)
+      return false;
+    const REAL_VALUE_TYPE &lhs_lb = lhs.lower_bound ();
+    const REAL_VALUE_TYPE &lhs_ub = lhs.upper_bound ();
+    const REAL_VALUE_TYPE &op2_lb = op2.lower_bound ();
+    const REAL_VALUE_TYPE &op2_ub = op2.upper_bound ();
+    if ((contains_zero_p (lhs_lb, lhs_ub) && contains_zero_p (op2_lb, op2_ub))
+	|| ((real_isinf (&lhs_lb) || real_isinf (&lhs_ub))
+	    && (real_isinf (&op2_lb) || real_isinf (&op2_ub))))
+      {
+	// If both lhs and op2 could be zeros or both could be infinities,
+	// we don't know anything about op1 except maybe for the sign
+	// and perhaps if it can be NAN or not.
+	REAL_VALUE_TYPE lb, ub;
+	int signbit_known = signbit_known_p (lhs_lb, lhs_ub, op2_lb, op2_ub);
+	zero_to_inf_range (lb, ub, signbit_known);
+	r.set (type, lb, ub);
+      }
+    // Otherwise, if op2 is a singleton INF and lhs doesn't include INF,
+    // or if lhs must be zero and op2 doesn't include zero, it would be
+    // UNDEFINED, while rdiv.fold_range computes a zero or singleton INF
+    // range.  Those are supersets of UNDEFINED, so let's keep that way.
+    return float_binary_op_range_finish (ret, r, type, lhs);
   }
   virtual bool op2_range (frange &r, tree type,
 			  const frange &lhs,
@@ -2271,9 +2293,27 @@ public:
   {
     if (lhs.undefined_p ())
       return false;
-    return float_binary_op_range_finish (fop_mult.fold_range (r, type, lhs,
-							      op2),
-					 r, type, lhs);
+    bool ret = fop_mult.fold_range (r, type, lhs, op2);
+    if (!ret)
+      return ret;
+    const REAL_VALUE_TYPE &lhs_lb = lhs.lower_bound ();
+    const REAL_VALUE_TYPE &lhs_ub = lhs.upper_bound ();
+    const REAL_VALUE_TYPE &op2_lb = op2.lower_bound ();
+    const REAL_VALUE_TYPE &op2_ub = op2.upper_bound ();
+    if ((contains_zero_p (lhs_lb, lhs_ub)
+	 && (real_isinf (&op2_lb) || real_isinf (&op2_ub)))
+	|| ((contains_zero_p (op2_lb, op2_ub))
+	    && (real_isinf (&lhs_lb) || real_isinf (&lhs_ub))))
+      {
+	// If both lhs could be zero and op2 infinity or vice versa,
+	// we don't know anything about op1 except maybe for the sign
+	// and perhaps if it can be NAN or not.
+	REAL_VALUE_TYPE lb, ub;
+	int signbit_known = signbit_known_p (lhs_lb, lhs_ub, op2_lb, op2_ub);
+	zero_to_inf_range (lb, ub, signbit_known);
+	r.set (type, lb, ub);
+      }
+    return float_binary_op_range_finish (ret, r, type, lhs);
   }
   virtual bool op2_range (frange &r, tree type,
 			  const frange &lhs,
@@ -2282,8 +2322,26 @@ public:
   {
     if (lhs.undefined_p ())
       return false;
-    return float_binary_op_range_finish (fold_range (r, type, op1, lhs),
-					 r, type, lhs);
+    bool ret = fold_range (r, type, op1, lhs);
+    if (!ret)
+      return ret;
+    const REAL_VALUE_TYPE &lhs_lb = lhs.lower_bound ();
+    const REAL_VALUE_TYPE &lhs_ub = lhs.upper_bound ();
+    const REAL_VALUE_TYPE &op1_lb = op1.lower_bound ();
+    const REAL_VALUE_TYPE &op1_ub = op1.upper_bound ();
+    if ((contains_zero_p (lhs_lb, lhs_ub) && contains_zero_p (op1_lb, op1_ub))
+	|| ((real_isinf (&lhs_lb) || real_isinf (&lhs_ub))
+	    && (real_isinf (&op1_lb) || real_isinf (&op1_ub))))
+      {
+	// If both lhs and op1 could be zeros or both could be infinities,
+	// we don't know anything about op2 except maybe for the sign
+	// and perhaps if it can be NAN or not.
+	REAL_VALUE_TYPE lb, ub;
+	int signbit_known = signbit_known_p (lhs_lb, lhs_ub, op1_lb, op1_ub);
+	zero_to_inf_range (lb, ub, signbit_known);
+	r.set (type, lb, ub);
+      }
+    return float_binary_op_range_finish (ret, r, type, lhs);
   }
 private:
   void rv_fold (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, bool &maybe_nan,
@@ -2296,7 +2354,7 @@ private:
   {
     // +-0.0 / +-0.0 or +-INF / +-INF is a known NAN.
     if ((zero_p (lh_lb, lh_ub) && zero_p (rh_lb, rh_ub))
-	|| (singleton_inf_p (lh_lb, lh_ub) || singleton_inf_p (rh_lb, rh_ub)))
+	|| (singleton_inf_p (lh_lb, lh_ub) && singleton_inf_p (rh_lb, rh_ub)))
       {
 	real_nan (&lb, "", 0, TYPE_MODE (type));
 	ub = lb;
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr107879.c b/gcc/testsuite/gcc.c-torture/execute/pr107879.c
new file mode 100644
index 00000000000..2aabe8e3a0c
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr107879.c
@@ -0,0 +1,25 @@
+/* PR tree-optimization/107879 */
+
+__attribute__((noipa)) static double
+foo (double *y)
+{
+  volatile int ph = 0;
+  volatile double vf = 1.0;
+  double factor = vf;
+  double x = - (double) ph * factor;
+  if (x == 0)
+    *y = 1.0;
+  else
+    *y = 1.0 / x;
+  double w = 2.0 * x / factor;
+  double omww = 1 - w;
+  return omww > 0.0 ? omww : 0.0;
+}
+
+int
+main ()
+{
+  double y = 42.0;
+  if (foo (&y) != 1.0)
+    __builtin_abort ();
+}
</cut>

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [TCWG CI] Failure after basepoints/gcc-13-4492-g4500baaccb6: range-op-float: Fix up multiplication and division reverse operation [PR107879]
@ 2022-12-05 22:53 ci_notify
  0 siblings, 0 replies; 2+ messages in thread
From: ci_notify @ 2022-12-05 22:53 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-regression

[-- Attachment #1: Type: text/plain, Size: 16653 bytes --]

Failure after basepoints/gcc-13-4492-g4500baaccb6: range-op-float: Fix up multiplication and division reverse operation [PR107879]:

Results changed to
-10
# true:
0
# build_abe binutils:
1
# build_abe stage1:
2
# build_abe linux:
3
# build_abe glibc:
# FAILED
# First few build errors in logs:
# 00:02:31 ../sysdeps/ieee754/dbl-64/s_f32xdivf64.c:30:1: internal compiler error: in lower_bound, at value-range.h:350
# 00:02:31 ../sysdeps/ieee754/dbl-64/s_f32xmulf64.c:30:1: internal compiler error: in lower_bound, at value-range.h:350
# 00:02:32 make[2]: *** [/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/builds/x86_64-pc-linux-gnu/arm-linux-gnueabihf/glibc-glibc.git~master/sysd-rules:757: /home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/builds/x86_64-pc-linux-gnu/arm-linux-gnueabihf/glibc-glibc.git~master/math/s_f32xdivf64.o] Error 1
# 00:02:32 make[2]: *** [/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/builds/x86_64-pc-linux-gnu/arm-linux-gnueabihf/glibc-glibc.git~master/sysd-rules:757: /home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/builds/x86_64-pc-linux-gnu/arm-linux-gnueabihf/glibc-glibc.git~master/math/s_f32xmulf64.o] Error 1
# 00:02:33 make[1]: *** [Makefile:484: math/others] Error 2
# 00:02:33 make: *** [Makefile:9: all] Error 2

from
-10
# true:
0
# build_abe binutils:
1
# build_abe stage1:
2
# build_abe linux:
3
# build_abe glibc:
4
# build_abe stage2:
5
# build_abe gdb:
# FAILED

THIS IS THE END OF INTERESTING STUFF.  BELOW ARE LINKS TO BUILDS, REPRODUCTION INSTRUCTIONS, AND THE RAW COMMIT.

For latest status see comments in https://linaro.atlassian.net/browse/GNU-692 .
Status of basepoints/gcc-13-4492-g4500baaccb6 commit for tcwg_gnu_cross_build:
commit 4500baaccb6e4d696e223c338bbdf7705c3646dd
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Dec 5 11:17:42 2022 +0100

    range-op-float: Fix up multiplication and division reverse operation [PR107879]
    
    While for the normal cases it seems to be correct to implement
    reverse multiplication (op1_range/op2_range) through division
    with float_binary_op_range_finish, reverse division (op1_range)
    through multiplication with float_binary_op_range_finish or
    (op2_range) through division with float_binary_op_range_finish,
    as e.g. following testcase shows for the corner cases it is
    incorrect.
    Say on the testcase we are doing reverse multiplication, we
    have [-0., 0.] range (no NAN) on lhs and VARYING on op1 (or op2).
    We implement that through division, because x from
    lhs = x * op2
    is
    x = lhs / op2
    For the division, [-0., 0.] / VARYING is computed (IMHO correctly)
    as [-0., 0.] +-NAN, because 0 / anything but 0 or NAN is still
    0 and 0 / 0 is NAN and ditto 0 / NAN.  And then we just
    float_binary_op_range_finish, which figures out that because lhs
    can't be NAN, neither operand can be NAN.  So, the end range is
    [-0., 0.].  But that is not correct for the reverse multiplication.
    When the result is 0, if op2 can be zero, then x can be anything
    (VARYING), to be precise anything but INF (unless result can be NAN),
    because anything * 0 is 0 (or NAN for INF).  While if op2 must be
    non-zero, then x must be 0.  Of course the sign logic
    (signbit(x) = signbit(lhs) ^ signbit(op2)) still holds, so it actually
    isn't full VARYING if both lhs and op2 have known sign bits.
    And going through other corner cases one by one shows other differences
    between what we compute for the corresponding forward operation and
    what we should compute for the reverse operations.
    The following patch is slightly conservative and includes INF
    (in case of result including 0 and not NAN) in the ranges or
    0 in the ranges (in case of result including INF and not NAN).
    The latter is what happens anyway because we flush denormals to 0,
    and the former just not to deal with all the corner cases.
    So, the end test is that for reverse multiplication and division
    op2_range the cases we need to adjust to VARYING or VARYING positive
    or VARYING negative are if lhs and op? ranges both contain 0,
    or both contain some infinity, while for division op1_range the
    corner case is if lhs range contains 0 and op2 range contains INF or vice
    versa.  Otherwise I believe ranges from the corresponding operation
    are ok, or could be slightly more conservative (e.g. for
    reverse multiplication, if op? range is singleton INF and lhs
    range doesn't include any INF, then x's range should be UNDEFINED or
    known NAN (depending on if lhs can be NAN), while the division computes
    [-0., 0.] +-NAN; or similarly if op? range is only 0 and lhs range
    doesn't include 0, division would compute +INF +-NAN, or -INF +-NAN,
    or (for lack of multipart franges -INF +INF +-NAN just VARYING +-NAN),
    while again it is UNDEFINED or known NAN.
    
    Oh, and I found by code inspection wrong condition for the division's
    known NAN result, due to thinko it would trigger not just when
    both operands are known to be 0 or both are known to be INF, but
    when either both are known to be 0, or at least one is known to be INF.
    
    2022-12-05  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/107879
            * range-op-float.cc (foperator_mult::op1_range): If both
            lhs and op2 ranges contain zero or both ranges contain
            some infinity, set r range to zero_to_inf_range depending on
            signbit_known_p.
            (foperator_div::op2_range): Similarly for lhs and op1 ranges.
            (foperator_div::op1_range): If lhs range contains zero and op2
            range contains some infinity or vice versa, set r range to
            zero_to_inf_range depending on signbit_known_p.
            (foperator_div::rv_fold): Fix up condition for returning known NAN.
* master-arm
** Failure after basepoints/gcc-13-4492-g4500baaccb6: range-op-float: Fix up multiplication and division reverse operation [PR107879]:
** https://ci.linaro.org/job/tcwg_gnu_cross_build-build-master-arm/1949/

Bad  build: https://ci.linaro.org/job/tcwg_gnu_cross_build-build-master-arm/1949/artifact/artifacts
Good build: https://ci.linaro.org/job/tcwg_gnu_cross_build-build-master-arm/1948/artifact/artifacts

Reproduce current build:
<cut>
mkdir -p investigate-gcc-4500baaccb6e4d696e223c338bbdf7705c3646dd
cd investigate-gcc-4500baaccb6e4d696e223c338bbdf7705c3646dd

# Fetch scripts
git clone https://git.linaro.org/toolchain/jenkins-scripts

# Fetch manifests for bad and good builds
mkdir -p bad/artifacts good/artifacts
curl -o bad/artifacts/manifest.sh https://ci.linaro.org/job/tcwg_gnu_cross_build-build-master-arm/1949/artifact/artifacts/manifest.sh --fail
curl -o good/artifacts/manifest.sh https://ci.linaro.org/job/tcwg_gnu_cross_build-build-master-arm/1948/artifact/artifacts/manifest.sh --fail

# Reproduce bad build
(cd bad; ../jenkins-scripts/tcwg_gnu-build.sh ^^ true %%rr[top_artifacts] artifacts)
# Reproduce good build
(cd good; ../jenkins-scripts/tcwg_gnu-build.sh ^^ true %%rr[top_artifacts] artifacts)
</cut>

Full commit (up to 1000 lines):
<cut>
commit 4500baaccb6e4d696e223c338bbdf7705c3646dd
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Dec 5 11:17:42 2022 +0100

    range-op-float: Fix up multiplication and division reverse operation [PR107879]
    
    While for the normal cases it seems to be correct to implement
    reverse multiplication (op1_range/op2_range) through division
    with float_binary_op_range_finish, reverse division (op1_range)
    through multiplication with float_binary_op_range_finish or
    (op2_range) through division with float_binary_op_range_finish,
    as e.g. following testcase shows for the corner cases it is
    incorrect.
    Say on the testcase we are doing reverse multiplication, we
    have [-0., 0.] range (no NAN) on lhs and VARYING on op1 (or op2).
    We implement that through division, because x from
    lhs = x * op2
    is
    x = lhs / op2
    For the division, [-0., 0.] / VARYING is computed (IMHO correctly)
    as [-0., 0.] +-NAN, because 0 / anything but 0 or NAN is still
    0 and 0 / 0 is NAN and ditto 0 / NAN.  And then we just
    float_binary_op_range_finish, which figures out that because lhs
    can't be NAN, neither operand can be NAN.  So, the end range is
    [-0., 0.].  But that is not correct for the reverse multiplication.
    When the result is 0, if op2 can be zero, then x can be anything
    (VARYING), to be precise anything but INF (unless result can be NAN),
    because anything * 0 is 0 (or NAN for INF).  While if op2 must be
    non-zero, then x must be 0.  Of course the sign logic
    (signbit(x) = signbit(lhs) ^ signbit(op2)) still holds, so it actually
    isn't full VARYING if both lhs and op2 have known sign bits.
    And going through other corner cases one by one shows other differences
    between what we compute for the corresponding forward operation and
    what we should compute for the reverse operations.
    The following patch is slightly conservative and includes INF
    (in case of result including 0 and not NAN) in the ranges or
    0 in the ranges (in case of result including INF and not NAN).
    The latter is what happens anyway because we flush denormals to 0,
    and the former just not to deal with all the corner cases.
    So, the end test is that for reverse multiplication and division
    op2_range the cases we need to adjust to VARYING or VARYING positive
    or VARYING negative are if lhs and op? ranges both contain 0,
    or both contain some infinity, while for division op1_range the
    corner case is if lhs range contains 0 and op2 range contains INF or vice
    versa.  Otherwise I believe ranges from the corresponding operation
    are ok, or could be slightly more conservative (e.g. for
    reverse multiplication, if op? range is singleton INF and lhs
    range doesn't include any INF, then x's range should be UNDEFINED or
    known NAN (depending on if lhs can be NAN), while the division computes
    [-0., 0.] +-NAN; or similarly if op? range is only 0 and lhs range
    doesn't include 0, division would compute +INF +-NAN, or -INF +-NAN,
    or (for lack of multipart franges -INF +INF +-NAN just VARYING +-NAN),
    while again it is UNDEFINED or known NAN.
    
    Oh, and I found by code inspection wrong condition for the division's
    known NAN result, due to thinko it would trigger not just when
    both operands are known to be 0 or both are known to be INF, but
    when either both are known to be 0, or at least one is known to be INF.
    
    2022-12-05  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/107879
            * range-op-float.cc (foperator_mult::op1_range): If both
            lhs and op2 ranges contain zero or both ranges contain
            some infinity, set r range to zero_to_inf_range depending on
            signbit_known_p.
            (foperator_div::op2_range): Similarly for lhs and op1 ranges.
            (foperator_div::op1_range): If lhs range contains zero and op2
            range contains some infinity or vice versa, set r range to
            zero_to_inf_range depending on signbit_known_p.
            (foperator_div::rv_fold): Fix up condition for returning known NAN.
---
 gcc/range-op-float.cc                          | 74 +++++++++++++++++++++++---
 gcc/testsuite/gcc.c-torture/execute/pr107879.c | 25 +++++++++
 2 files changed, 91 insertions(+), 8 deletions(-)

diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index ee88511eba0..e9455a929ae 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -2143,8 +2143,30 @@ public:
     range_op_handler rdiv (RDIV_EXPR, type);
     if (!rdiv)
       return false;
-    return float_binary_op_range_finish (rdiv.fold_range (r, type, lhs, op2),
-					 r, type, lhs);
+    bool ret = rdiv.fold_range (r, type, lhs, op2);
+    if (ret == false)
+      return false;
+    const REAL_VALUE_TYPE &lhs_lb = lhs.lower_bound ();
+    const REAL_VALUE_TYPE &lhs_ub = lhs.upper_bound ();
+    const REAL_VALUE_TYPE &op2_lb = op2.lower_bound ();
+    const REAL_VALUE_TYPE &op2_ub = op2.upper_bound ();
+    if ((contains_zero_p (lhs_lb, lhs_ub) && contains_zero_p (op2_lb, op2_ub))
+	|| ((real_isinf (&lhs_lb) || real_isinf (&lhs_ub))
+	    && (real_isinf (&op2_lb) || real_isinf (&op2_ub))))
+      {
+	// If both lhs and op2 could be zeros or both could be infinities,
+	// we don't know anything about op1 except maybe for the sign
+	// and perhaps if it can be NAN or not.
+	REAL_VALUE_TYPE lb, ub;
+	int signbit_known = signbit_known_p (lhs_lb, lhs_ub, op2_lb, op2_ub);
+	zero_to_inf_range (lb, ub, signbit_known);
+	r.set (type, lb, ub);
+      }
+    // Otherwise, if op2 is a singleton INF and lhs doesn't include INF,
+    // or if lhs must be zero and op2 doesn't include zero, it would be
+    // UNDEFINED, while rdiv.fold_range computes a zero or singleton INF
+    // range.  Those are supersets of UNDEFINED, so let's keep that way.
+    return float_binary_op_range_finish (ret, r, type, lhs);
   }
   virtual bool op2_range (frange &r, tree type,
 			  const frange &lhs,
@@ -2271,9 +2293,27 @@ public:
   {
     if (lhs.undefined_p ())
       return false;
-    return float_binary_op_range_finish (fop_mult.fold_range (r, type, lhs,
-							      op2),
-					 r, type, lhs);
+    bool ret = fop_mult.fold_range (r, type, lhs, op2);
+    if (!ret)
+      return ret;
+    const REAL_VALUE_TYPE &lhs_lb = lhs.lower_bound ();
+    const REAL_VALUE_TYPE &lhs_ub = lhs.upper_bound ();
+    const REAL_VALUE_TYPE &op2_lb = op2.lower_bound ();
+    const REAL_VALUE_TYPE &op2_ub = op2.upper_bound ();
+    if ((contains_zero_p (lhs_lb, lhs_ub)
+	 && (real_isinf (&op2_lb) || real_isinf (&op2_ub)))
+	|| ((contains_zero_p (op2_lb, op2_ub))
+	    && (real_isinf (&lhs_lb) || real_isinf (&lhs_ub))))
+      {
+	// If both lhs could be zero and op2 infinity or vice versa,
+	// we don't know anything about op1 except maybe for the sign
+	// and perhaps if it can be NAN or not.
+	REAL_VALUE_TYPE lb, ub;
+	int signbit_known = signbit_known_p (lhs_lb, lhs_ub, op2_lb, op2_ub);
+	zero_to_inf_range (lb, ub, signbit_known);
+	r.set (type, lb, ub);
+      }
+    return float_binary_op_range_finish (ret, r, type, lhs);
   }
   virtual bool op2_range (frange &r, tree type,
 			  const frange &lhs,
@@ -2282,8 +2322,26 @@ public:
   {
     if (lhs.undefined_p ())
       return false;
-    return float_binary_op_range_finish (fold_range (r, type, op1, lhs),
-					 r, type, lhs);
+    bool ret = fold_range (r, type, op1, lhs);
+    if (!ret)
+      return ret;
+    const REAL_VALUE_TYPE &lhs_lb = lhs.lower_bound ();
+    const REAL_VALUE_TYPE &lhs_ub = lhs.upper_bound ();
+    const REAL_VALUE_TYPE &op1_lb = op1.lower_bound ();
+    const REAL_VALUE_TYPE &op1_ub = op1.upper_bound ();
+    if ((contains_zero_p (lhs_lb, lhs_ub) && contains_zero_p (op1_lb, op1_ub))
+	|| ((real_isinf (&lhs_lb) || real_isinf (&lhs_ub))
+	    && (real_isinf (&op1_lb) || real_isinf (&op1_ub))))
+      {
+	// If both lhs and op1 could be zeros or both could be infinities,
+	// we don't know anything about op2 except maybe for the sign
+	// and perhaps if it can be NAN or not.
+	REAL_VALUE_TYPE lb, ub;
+	int signbit_known = signbit_known_p (lhs_lb, lhs_ub, op1_lb, op1_ub);
+	zero_to_inf_range (lb, ub, signbit_known);
+	r.set (type, lb, ub);
+      }
+    return float_binary_op_range_finish (ret, r, type, lhs);
   }
 private:
   void rv_fold (REAL_VALUE_TYPE &lb, REAL_VALUE_TYPE &ub, bool &maybe_nan,
@@ -2296,7 +2354,7 @@ private:
   {
     // +-0.0 / +-0.0 or +-INF / +-INF is a known NAN.
     if ((zero_p (lh_lb, lh_ub) && zero_p (rh_lb, rh_ub))
-	|| (singleton_inf_p (lh_lb, lh_ub) || singleton_inf_p (rh_lb, rh_ub)))
+	|| (singleton_inf_p (lh_lb, lh_ub) && singleton_inf_p (rh_lb, rh_ub)))
       {
 	real_nan (&lb, "", 0, TYPE_MODE (type));
 	ub = lb;
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr107879.c b/gcc/testsuite/gcc.c-torture/execute/pr107879.c
new file mode 100644
index 00000000000..2aabe8e3a0c
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr107879.c
@@ -0,0 +1,25 @@
+/* PR tree-optimization/107879 */
+
+__attribute__((noipa)) static double
+foo (double *y)
+{
+  volatile int ph = 0;
+  volatile double vf = 1.0;
+  double factor = vf;
+  double x = - (double) ph * factor;
+  if (x == 0)
+    *y = 1.0;
+  else
+    *y = 1.0 / x;
+  double w = 2.0 * x / factor;
+  double omww = 1 - w;
+  return omww > 0.0 ? omww : 0.0;
+}
+
+int
+main ()
+{
+  double y = 42.0;
+  if (foo (&y) != 1.0)
+    __builtin_abort ();
+}
</cut>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-12-05 22:53 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-05 16:39 [TCWG CI] Failure after basepoints/gcc-13-4492-g4500baaccb6: range-op-float: Fix up multiplication and division reverse operation [PR107879] ci_notify
2022-12-05 22:53 ci_notify

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).