Message ID | 20221110150345.157116-1-aldyh@redhat.com |
---|---|
State | New |
Headers | show |
Series | Do not specify NAN sign in frange::set_nonnegative. | expand |
On Thu, Nov 10, 2022 at 04:03:46PM +0100, Aldy Hernandez wrote: > [Jakub, how's this? Do you agree?] > > After further reading of the IEEE 754 standard, it has become clear > that there are no guarantees with regards to the sign of a NAN when it > comes to any operation other than copy, copysign, abs, and negate. > > Currently, set_nonnegative() is only used in one place in ranger > applicable to floating point values, when expanding unknown calls. > Since we already specially handle copy, copysign, abs, and negate, all > the calls to set_nonnegative() must be NAN-sign agnostic. > > The cleanest solution is to leave the sign unspecificied in > frange::set_nonnegative(). Any special case, must be handled by the > caller. > > gcc/ChangeLog: > > * value-range.cc (frange::set_nonnegative): Remove NAN sign handling. > (range_tests_signed_zeros): Adjust test. LGTM, thanks. Jakub
diff --git a/gcc/value-range.cc b/gcc/value-range.cc index 852ac09f2c4..d55d85846c1 100644 --- a/gcc/value-range.cc +++ b/gcc/value-range.cc @@ -797,14 +797,17 @@ frange::zero_p () const && real_iszero (&m_max)); } +// Set the range to non-negative numbers, that is [+0.0, +INF]. +// +// The NAN in the resulting range (if HONOR_NANS) has a varying sign +// as there are no guarantees in IEEE 754 wrt to the sign of a NAN, +// except for copy, abs, and copysign. It is the responsibility of +// the caller to set the NAN's sign if desired. + void frange::set_nonnegative (tree type) { set (type, dconst0, frange_val_max (type)); - - // Set +NAN as the only possibility. - if (HONOR_NANS (type)) - update_nan (/*sign=*/false); } // Here we copy between any two irange's. The ranges can be legacy or @@ -3923,7 +3926,6 @@ range_tests_signed_zeros () ASSERT_TRUE (r0.undefined_p ()); r0.set_nonnegative (float_type_node); - ASSERT_TRUE (r0.signbit_p (signbit) && !signbit); if (HONOR_NANS (float_type_node)) ASSERT_TRUE (r0.maybe_isnan ()); }