| Message ID | 20221023145633.501586-1-aldyh@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PR,tree-optimization/107365] Check HONOR_NANS instead of flag_finite_math_only in frange:verify_range. | expand |
On Sun, Oct 23, 2022 at 4:57 PM Aldy Hernandez via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > [Jakub and other FP experts, would this be OK, or am I missing > something?] > > Vax does not seem to have !flag_finite_math_only, but float_type_node > does not HONOR_NANS. The check in frange::verify_range dependend on > flag_finite_math_only, which is technically not correct since > frange::set_varying() checks HONOR_NANS instead of > flag_finite_math_only. > > I'm actually getting tired of flag_finite_math_only and > !flag_finite_math_only discrepancies in the selftests (Vax and rx-elf > come to mind). I think we should just test both alternatives in the > selftests as in this patch. > > We could also check flag_finite_math_only=0 with a float_type_node > that does not HONOR_NANs, but I have no idea how to twiddle > FLOAT_MODE_FORMAT temporarily, and that may be over thinking it. > > How does this look? OK. As said elsewhere checking flag_* is never correct for any of the FP type features, always use HONOR_*. Thanks, Richard. > PR tree-optimization/107365 > > gcc/ChangeLog: > > * value-range.cc (frange::verify_range): Predicate NAN check in > VARYING range on HONOR_NANS instead of flag_finite_math_only. > (range_tests_floats): Same. > (range_tests_floats_various): New. > (range_tests): Call range_tests_floats_various. > --- > gcc/value-range.cc | 33 +++++++++++++++++++++++++-------- > 1 file changed, 25 insertions(+), 8 deletions(-) > > diff --git a/gcc/value-range.cc b/gcc/value-range.cc > index d779e9819e2..d8ee6ec0d0f 100644 > --- a/gcc/value-range.cc > +++ b/gcc/value-range.cc > @@ -720,13 +720,13 @@ frange::verify_range () > gcc_checking_assert (!m_type); > return; > case VR_VARYING: > - if (flag_finite_math_only) > - gcc_checking_assert (!m_pos_nan && !m_neg_nan); > - else > - gcc_checking_assert (m_pos_nan && m_neg_nan); > gcc_checking_assert (m_type); > gcc_checking_assert (frange_val_is_min (m_min, m_type)); > gcc_checking_assert (frange_val_is_max (m_max, m_type)); > + if (HONOR_NANS (m_type)) > + gcc_checking_assert (m_pos_nan && m_neg_nan); > + else > + gcc_checking_assert (!m_pos_nan && !m_neg_nan); > return; > case VR_RANGE: > gcc_checking_assert (m_type); > @@ -3957,10 +3957,9 @@ range_tests_floats () > // A range of [-INF,+INF] is actually VARYING if no other properties > // are set. > r0 = frange_float ("-Inf", "+Inf"); > - if (r0.maybe_isnan ()) > - ASSERT_TRUE (r0.varying_p ()); > + ASSERT_TRUE (r0.varying_p ()); > // ...unless it has some special property... > - if (!flag_finite_math_only) > + if (HONOR_NANS (r0.type ())) > { > r0.clear_nan (); > ASSERT_FALSE (r0.varying_p ()); > @@ -4041,6 +4040,24 @@ range_tests_floats () > } > } > > +// Run floating range tests for various combinations of NAN and INF > +// support. > + > +static void > +range_tests_floats_various () > +{ > + int save_finite_math_only = flag_finite_math_only; > + > + // Test -ffinite-math-only. > + flag_finite_math_only = 1; > + range_tests_floats (); > + // Test -fno-finite-math-only. > + flag_finite_math_only = 0; > + range_tests_floats (); > + > + flag_finite_math_only = save_finite_math_only; > +} > + > void > range_tests () > { > @@ -4049,7 +4066,7 @@ range_tests () > range_tests_int_range_max (); > range_tests_strict_enum (); > range_tests_nonzero_bits (); > - range_tests_floats (); > + range_tests_floats_various (); > range_tests_misc (); > } > > -- > 2.37.3 >
diff --git a/gcc/value-range.cc b/gcc/value-range.cc index d779e9819e2..d8ee6ec0d0f 100644 --- a/gcc/value-range.cc +++ b/gcc/value-range.cc @@ -720,13 +720,13 @@ frange::verify_range () gcc_checking_assert (!m_type); return; case VR_VARYING: - if (flag_finite_math_only) - gcc_checking_assert (!m_pos_nan && !m_neg_nan); - else - gcc_checking_assert (m_pos_nan && m_neg_nan); gcc_checking_assert (m_type); gcc_checking_assert (frange_val_is_min (m_min, m_type)); gcc_checking_assert (frange_val_is_max (m_max, m_type)); + if (HONOR_NANS (m_type)) + gcc_checking_assert (m_pos_nan && m_neg_nan); + else + gcc_checking_assert (!m_pos_nan && !m_neg_nan); return; case VR_RANGE: gcc_checking_assert (m_type); @@ -3957,10 +3957,9 @@ range_tests_floats () // A range of [-INF,+INF] is actually VARYING if no other properties // are set. r0 = frange_float ("-Inf", "+Inf"); - if (r0.maybe_isnan ()) - ASSERT_TRUE (r0.varying_p ()); + ASSERT_TRUE (r0.varying_p ()); // ...unless it has some special property... - if (!flag_finite_math_only) + if (HONOR_NANS (r0.type ())) { r0.clear_nan (); ASSERT_FALSE (r0.varying_p ()); @@ -4041,6 +4040,24 @@ range_tests_floats () } } +// Run floating range tests for various combinations of NAN and INF +// support. + +static void +range_tests_floats_various () +{ + int save_finite_math_only = flag_finite_math_only; + + // Test -ffinite-math-only. + flag_finite_math_only = 1; + range_tests_floats (); + // Test -fno-finite-math-only. + flag_finite_math_only = 0; + range_tests_floats (); + + flag_finite_math_only = save_finite_math_only; +} + void range_tests () { @@ -4049,7 +4066,7 @@ range_tests () range_tests_int_range_max (); range_tests_strict_enum (); range_tests_nonzero_bits (); - range_tests_floats (); + range_tests_floats_various (); range_tests_misc (); }