Rewrite NAN and sign handling in frange

Message ID 20220915054026.1359564-1-aldyh@redhat.com
State Accepted, archived
Headers
Series Rewrite NAN and sign handling in frange |

Checks

Context Check Description
snail/gcc-patches-check success Github commit url

Commit Message

Aldy Hernandez Sept. 15, 2022, 5:40 a.m. UTC
  Hi Richard.  Hi all.

The attatched patch rewrites the NAN and sign handling, dropping both
tristates in favor of a pair of boolean flags for NANs, and nothing at
all for signs.  The signs are tracked in the range itself, so now it's
possible to describe things like [-0.0, +0.0] +NAN, [+0, +0], [-5, +0],
[+0, 3] -NAN, etc.

There are a lot of changes, as the tristate was quite pervasive.  I
could use another pair of eyes.  The code IMO is cleaner and handles
all the cases we discussed.

Here is an example of the various ranges and how they are displayed:

    [frange] float VARYING NAN ;; Varying includes NAN
    [frange] UNDEFINED                      ;; Empty set as always
    [frange] float [] NAN                   ;; Unknown sign NAN
    [frange] float [] -NAN                  ;; -NAN
    [frange] float [] +NAN                  ;; +NAN
    [frange] float [-0.0, 0.0]              ;; All zeros.
    [frange] float [-0.0, -0.0] NAN         ;; -0 or NAN.
    [frange] float [-5.0e+0, -1.0e+0] +NAN  ;; [-5, -1] or +NAN
    [frange] float [-5.0e+0, -0.0] NAN      ;; [-5, -0] or +-NAN
    [frange] float [-5.0e+0, -0.0]          ;; [-5, -0]
    [frange] float [5.0e+0, 1.0e+1]         ;; [5, 10]

We could represent an unknown sign with +NAN -NAN if preferred.

Notice the NAN signs are decoupled from the range, so we can represent
a negative range with a positive NAN.  For this range,
frange::known_bit() would return false, as only when the signs of the
NANs and range agree can we be certain.

There is no longer any pessimization of ranges for intersects
involving NANs.  Also, union and intersect work with signed zeros:

//   [-0,  x] U [+0,  x] => [-0,  x]
//   [ x, -0] U [ x, +0] => [ x, +0]
//   [-0,  x] ^ [+0,  x] => [+0,  x]
//   [ x, -0] ^ [ x, +0] => [ x, -0]

The special casing for signed zeros in the singleton code is gone in
favor of just making sure the signs in the range agree, that is
[-0, -0] for example.

I have removed the idea that a known NAN is a "range", so a NAN is no
longer in the endpoints itself.  Requesting the bound of a known NAN
is a hard fail.  For that matter, we don't store the actual NAN in the
range.  The only information we have are the set of boolean flags.
This way we make sure nothing seeps into the frange.  This also means
it's explicit that we don't track anything but the sign in NANs.  We
can revisit this if we desire to track signalling or whatever
concoction y'all can imagine.

All in all, I'm quite happy with this.  It does look better, and we
handle all the corner cases we couldn't before.  Thanks for the
suggestion.

Regstrapped with mpfr tests on x86-64 and ppc64le Linux.  Selftests
were also run with -ffinite-math-only on x86-64.

At Jakub's suggestion, I built lapack with associated tests.  They
pass on x86-64 and ppc64le Linux with no regressions from mainline.
As a sanity check, I also ran them for -ffinite-math-only on x86 which
(as expected) returned:

	NaN arithmetic did not perform per the ieee spec

Otherwise, all tests pass for -ffinite-math-only.

How does this look?

gcc/ChangeLog:

	* range-op-float.cc (frange_add_zeros): Replace set_signbit with
	union of zero.
	* value-query.cc (range_query::get_tree_range): Remove set_signbit
	use.
	* value-range-pretty-print.cc (vrange_printer::print_frange_prop):
	Remove.
	(vrange_printer::print_frange_nan): New.
	* value-range-pretty-print.h (print_frange_prop): Remove.
	(print_frange_nan): New.
	* value-range-storage.cc (frange_storage_slot::set_frange): Set
	kind and NAN fields.
	(frange_storage_slot::get_frange): Restore kind and NAN fields.
	* value-range-storage.h (class frange_storage_slot): Add kind and
	NAN fields.
	* value-range.cc (frange::update_nan): Remove.
	(frange::set_signbit): Remove.
	(frange::set): Adjust for NAN fields.
	(frange::normalize_kind): Remove m_props.
	(frange::combine_zeros): New.
	(frange::union_nans): New.
	(frange::union_): Handle new NAN fields.
	(frange::intersect_nans): New.
	(frange::intersect): Handle new NAN fields.
	(frange::operator=): Same.
	(frange::operator==): Same.
	(frange::contains_p): Same.
	(frange::singleton_p): Remove special case for signed zeros.
	(frange::verify_range): Adjust for new NAN fields.
	(frange::set_zero): Handle signed zeros.
	(frange::set_nonnegative): Same.
	(range_tests_nan): Adjust tests.
	(range_tests_signed_zeros): Same.
	(range_tests_signbit): Same.
	(range_tests_floats): Same.
	* value-range.h (class fp_prop): Remove.
	(FP_PROP_ACCESSOR): Remove.
	(class frange_props): Remove
	(frange::lower_bound): NANs don't have endpoints.
	(frange::upper_bound): Same.
	(frange_props::operator==): Remove.
	(frange_props::union_): Remove.
	(frange_props::intersect): Remove.
	(frange::update_nan): New.
	(frange::clear_nan): New.
	(frange::undefined_p): New.
	(frange::set_nan): New.
	(frange::known_finite): Adjust for new NAN representation.
	(frange::maybe_nan): Same.
	(frange::known_nan): Same.
	(frange::known_signbit): Same.
---
 gcc/range-op-float.cc           |   6 +-
 gcc/value-query.cc              |  11 +-
 gcc/value-range-pretty-print.cc |  45 +--
 gcc/value-range-pretty-print.h  |   2 +-
 gcc/value-range-storage.cc      |   9 +-
 gcc/value-range-storage.h       |   7 +-
 gcc/value-range.cc              | 554 ++++++++++++++++----------------
 gcc/value-range.h               | 212 ++++++------
 8 files changed, 415 insertions(+), 431 deletions(-)
  

Comments

Richard Biener Sept. 15, 2022, 7:06 a.m. UTC | #1
On Thu, Sep 15, 2022 at 7:41 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> Hi Richard.  Hi all.
>
> The attatched patch rewrites the NAN and sign handling, dropping both
> tristates in favor of a pair of boolean flags for NANs, and nothing at
> all for signs.  The signs are tracked in the range itself, so now it's
> possible to describe things like [-0.0, +0.0] +NAN, [+0, +0], [-5, +0],
> [+0, 3] -NAN, etc.
>
> There are a lot of changes, as the tristate was quite pervasive.  I
> could use another pair of eyes.  The code IMO is cleaner and handles
> all the cases we discussed.
>
> Here is an example of the various ranges and how they are displayed:
>
>     [frange] float VARYING NAN ;; Varying includes NAN
>     [frange] UNDEFINED                      ;; Empty set as always
>     [frange] float [] NAN                   ;; Unknown sign NAN
>     [frange] float [] -NAN                  ;; -NAN
>     [frange] float [] +NAN                  ;; +NAN
>     [frange] float [-0.0, 0.0]              ;; All zeros.
>     [frange] float [-0.0, -0.0] NAN         ;; -0 or NAN.
>     [frange] float [-5.0e+0, -1.0e+0] +NAN  ;; [-5, -1] or +NAN
>     [frange] float [-5.0e+0, -0.0] NAN      ;; [-5, -0] or +-NAN
>     [frange] float [-5.0e+0, -0.0]          ;; [-5, -0]
>     [frange] float [5.0e+0, 1.0e+1]         ;; [5, 10]
>
> We could represent an unknown sign with +NAN -NAN if preferred.

maybe -+NAN or +-NAN?  I prefer to somehow show both signs for clarity

>
> Notice the NAN signs are decoupled from the range, so we can represent
> a negative range with a positive NAN.  For this range,
> frange::known_bit() would return false, as only when the signs of the
> NANs and range agree can we be certain.
>
> There is no longer any pessimization of ranges for intersects
> involving NANs.  Also, union and intersect work with signed zeros:
>
> //   [-0,  x] U [+0,  x] => [-0,  x]
> //   [ x, -0] U [ x, +0] => [ x, +0]
> //   [-0,  x] ^ [+0,  x] => [+0,  x]
> //   [ x, -0] ^ [ x, +0] => [ x, -0]
>
> The special casing for signed zeros in the singleton code is gone in
> favor of just making sure the signs in the range agree, that is
> [-0, -0] for example.
>
> I have removed the idea that a known NAN is a "range", so a NAN is no
> longer in the endpoints itself.  Requesting the bound of a known NAN
> is a hard fail.  For that matter, we don't store the actual NAN in the
> range.  The only information we have are the set of boolean flags.
> This way we make sure nothing seeps into the frange.  This also means
> it's explicit that we don't track anything but the sign in NANs.  We
> can revisit this if we desire to track signalling or whatever
> concoction y'all can imagine.
>
> All in all, I'm quite happy with this.  It does look better, and we
> handle all the corner cases we couldn't before.  Thanks for the
> suggestion.
>
> Regstrapped with mpfr tests on x86-64 and ppc64le Linux.  Selftests
> were also run with -ffinite-math-only on x86-64.
>
> At Jakub's suggestion, I built lapack with associated tests.  They
> pass on x86-64 and ppc64le Linux with no regressions from mainline.
> As a sanity check, I also ran them for -ffinite-math-only on x86 which
> (as expected) returned:
>
>         NaN arithmetic did not perform per the ieee spec
>
> Otherwise, all tests pass for -ffinite-math-only.
>
> How does this look?

Overall it looks good.

Reading ::intersect and ::union I find it less clear to spread out the _nan
cases into separate functions.

Can you add a comment to frange that its representation is
a single value-range specified by m_type, m_min, m_max
unioned with the set of { -NaN, +NaN }?  Because somehow
the ::undefined_p vs. m_type == VR_UNDEFINED checks are
a bit confusing to the occasional reader can we instead use
::nan_p to complement ::undefined_p?

Brain dump: maybe having a NaN-less frange with m_type, m_min, m_max
and then frange_with_nan having a frange member plus the nan bits
would make a better distinction?  Maybe we can use m_type == VR_RANGE
when the actual range is empty but we have NaNs somehow?  That we
need m_type to represent an empty range and VR_VARYING for the full
range is somehow duplicate - ]0,0[ would be an empty range as well,
but then we'd need inclusive/exclusive ranges.  NULL m_min/max might
be another (bad) representation.  Having m_type makes for efficient
checking as well, so that's a pro.  Maybe have m_type == VR_NAN for
the case of empty range but NaNs, leaving VR_UNDEFINED to the
true empty set?

Anyway, I think the patch is OK as-is with the NaN printing adjusted
and maybe avoiding the bare m_type == VR_UNDEFINED checks
(in all but the abstraction).

Thanks,
Richard.

>
> gcc/ChangeLog:
>
>         * range-op-float.cc (frange_add_zeros): Replace set_signbit with
>         union of zero.
>         * value-query.cc (range_query::get_tree_range): Remove set_signbit
>         use.
>         * value-range-pretty-print.cc (vrange_printer::print_frange_prop):
>         Remove.
>         (vrange_printer::print_frange_nan): New.
>         * value-range-pretty-print.h (print_frange_prop): Remove.
>         (print_frange_nan): New.
>         * value-range-storage.cc (frange_storage_slot::set_frange): Set
>         kind and NAN fields.
>         (frange_storage_slot::get_frange): Restore kind and NAN fields.
>         * value-range-storage.h (class frange_storage_slot): Add kind and
>         NAN fields.
>         * value-range.cc (frange::update_nan): Remove.
>         (frange::set_signbit): Remove.
>         (frange::set): Adjust for NAN fields.
>         (frange::normalize_kind): Remove m_props.
>         (frange::combine_zeros): New.
>         (frange::union_nans): New.
>         (frange::union_): Handle new NAN fields.
>         (frange::intersect_nans): New.
>         (frange::intersect): Handle new NAN fields.
>         (frange::operator=): Same.
>         (frange::operator==): Same.
>         (frange::contains_p): Same.
>         (frange::singleton_p): Remove special case for signed zeros.
>         (frange::verify_range): Adjust for new NAN fields.
>         (frange::set_zero): Handle signed zeros.
>         (frange::set_nonnegative): Same.
>         (range_tests_nan): Adjust tests.
>         (range_tests_signed_zeros): Same.
>         (range_tests_signbit): Same.
>         (range_tests_floats): Same.
>         * value-range.h (class fp_prop): Remove.
>         (FP_PROP_ACCESSOR): Remove.
>         (class frange_props): Remove
>         (frange::lower_bound): NANs don't have endpoints.
>         (frange::upper_bound): Same.
>         (frange_props::operator==): Remove.
>         (frange_props::union_): Remove.
>         (frange_props::intersect): Remove.
>         (frange::update_nan): New.
>         (frange::clear_nan): New.
>         (frange::undefined_p): New.
>         (frange::set_nan): New.
>         (frange::known_finite): Adjust for new NAN representation.
>         (frange::maybe_nan): Same.
>         (frange::known_nan): Same.
>         (frange::known_signbit): Same.
> ---
>  gcc/range-op-float.cc           |   6 +-
>  gcc/value-query.cc              |  11 +-
>  gcc/value-range-pretty-print.cc |  45 +--
>  gcc/value-range-pretty-print.h  |   2 +-
>  gcc/value-range-storage.cc      |   9 +-
>  gcc/value-range-storage.h       |   7 +-
>  gcc/value-range.cc              | 554 ++++++++++++++++----------------
>  gcc/value-range.h               | 212 ++++++------
>  8 files changed, 415 insertions(+), 431 deletions(-)
>
> diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
> index fbc14a730ad..270490010e2 100644
> --- a/gcc/range-op-float.cc
> +++ b/gcc/range-op-float.cc
> @@ -218,7 +218,11 @@ frange_add_zeros (frange &r, tree type)
>
>    if (HONOR_SIGNED_ZEROS (type)
>        && (real_iszero (&r.lower_bound ()) || real_iszero (&r.upper_bound ())))
> -    r.set_signbit (fp_prop::VARYING);
> +    {
> +      frange zero;
> +      zero.set_zero (type);
> +      r.union_ (zero);
> +    }
>  }
>
>  // Build a range that is <= VAL and store it in R.
> diff --git a/gcc/value-query.cc b/gcc/value-query.cc
> index ea6e4b979ad..0bdd670982b 100644
> --- a/gcc/value-query.cc
> +++ b/gcc/value-query.cc
> @@ -219,17 +219,8 @@ range_query::get_tree_range (vrange &r, tree expr, gimple *stmt)
>        {
>         frange &f = as_a <frange> (r);
>         f.set (expr, expr);
> -
> -       // Singletons from the tree world have known properties.
> -       REAL_VALUE_TYPE *rv = TREE_REAL_CST_PTR (expr);
> -       if (real_isnan (rv))
> -         f.update_nan (fp_prop::YES);
> -       else
> +       if (!real_isnan (TREE_REAL_CST_PTR (expr)))
>           f.clear_nan ();
> -       if (real_isneg (rv))
> -         f.set_signbit (fp_prop::YES);
> -       else
> -         f.set_signbit (fp_prop::NO);
>         return true;
>        }
>
> diff --git a/gcc/value-range-pretty-print.cc b/gcc/value-range-pretty-print.cc
> index b124e46cb9e..49b16d6a5b1 100644
> --- a/gcc/value-range-pretty-print.cc
> +++ b/gcc/value-range-pretty-print.cc
> @@ -134,34 +134,39 @@ vrange_printer::visit (const frange &r) const
>    if (r.varying_p ())
>      {
>        pp_string (pp, "VARYING");
> +      print_frange_nan (r);
>        return;
>      }
>    pp_character (pp, '[');
> -  dump_generic_node (pp,
> -                    build_real (type, r.lower_bound ()), 0, TDF_NONE, false);
> -  pp_string (pp, ", ");
> -  dump_generic_node (pp,
> -                    build_real (type, r.upper_bound ()), 0, TDF_NONE, false);
> -  pp_string (pp, "] ");
> -
> -  print_frange_prop ("NAN", r.get_nan ());
> -  print_frange_prop ("SIGN", r.get_signbit ());
> +  bool has_endpoints = !r.known_nan ();
> +  if (has_endpoints)
> +    {
> +      dump_generic_node (pp,
> +                        build_real (type, r.lower_bound ()), 0, TDF_NONE, false);
> +      pp_string (pp, ", ");
> +      dump_generic_node (pp,
> +                        build_real (type, r.upper_bound ()), 0, TDF_NONE, false);
> +    }
> +  pp_character (pp, ']');
> +  print_frange_nan (r);
>  }
>
> -// Print the FP properties in an frange.
> +// Print the NAN info for an frange.
>
>  void
> -vrange_printer::print_frange_prop (const char *str, const fp_prop &prop) const
> +vrange_printer::print_frange_nan (const frange &r) const
>  {
> -  if (prop.varying_p ())
> -    return;
> -
> -  if (prop.yes_p ())
> -    pp_string (pp, str);
> -  else if (prop.no_p ())
> +  if (r.maybe_nan ())
>      {
> -      pp_character (pp, '!');
> -      pp_string (pp, str);
> +      if (r.m_pos_nan && r.m_neg_nan)
> +       {
> +         pp_string (pp, " NAN");
> +         return;
> +       }
> +      bool nan_sign = r.m_neg_nan;
> +      if (nan_sign)
> +       pp_string (pp, " -NAN");
> +      else
> +       pp_string (pp, " +NAN");
>      }
> -  pp_character (pp, ' ');
>  }
> diff --git a/gcc/value-range-pretty-print.h b/gcc/value-range-pretty-print.h
> index ad06c93c044..20c26598fe7 100644
> --- a/gcc/value-range-pretty-print.h
> +++ b/gcc/value-range-pretty-print.h
> @@ -31,7 +31,7 @@ public:
>  private:
>    void print_irange_bound (const wide_int &w, tree type) const;
>    void print_irange_bitmasks (const irange &) const;
> -  void print_frange_prop (const char *str, const fp_prop &) const;
> +  void print_frange_nan (const frange &) const;
>
>    pretty_printer *pp;
>  };
> diff --git a/gcc/value-range-storage.cc b/gcc/value-range-storage.cc
> index b7a23fa9825..de7575ed48d 100644
> --- a/gcc/value-range-storage.cc
> +++ b/gcc/value-range-storage.cc
> @@ -253,9 +253,11 @@ frange_storage_slot::set_frange (const frange &r)
>    gcc_checking_assert (fits_p (r));
>    gcc_checking_assert (!r.undefined_p ());
>
> +  m_kind = r.m_kind;
>    m_min = r.m_min;
>    m_max = r.m_max;
> -  m_props = r.m_props;
> +  m_pos_nan = r.m_pos_nan;
> +  m_neg_nan = r.m_neg_nan;
>  }
>
>  void
> @@ -264,11 +266,12 @@ frange_storage_slot::get_frange (frange &r, tree type) const
>    gcc_checking_assert (r.supports_type_p (type));
>
>    r.set_undefined ();
> -  r.m_kind = VR_RANGE;
> -  r.m_props = m_props;
> +  r.m_kind = m_kind;
>    r.m_type = type;
>    r.m_min = m_min;
>    r.m_max = m_max;
> +  r.m_pos_nan = m_pos_nan;
> +  r.m_neg_nan = m_neg_nan;
>    r.normalize_kind ();
>
>    if (flag_checking)
> diff --git a/gcc/value-range-storage.h b/gcc/value-range-storage.h
> index f506789f3d1..0cf95ebf7c1 100644
> --- a/gcc/value-range-storage.h
> +++ b/gcc/value-range-storage.h
> @@ -113,12 +113,11 @@ class GTY (()) frange_storage_slot
>    frange_storage_slot (const frange &r) { set_frange (r); }
>    DISABLE_COPY_AND_ASSIGN (frange_storage_slot);
>
> -  // We can get away with just storing the properties and the
> -  // endpoints because the type can be gotten from the SSA, and
> -  // UNDEFINED is unsupported, so it can only be a VR_RANGE.
> +  enum value_range_kind m_kind;
>    REAL_VALUE_TYPE m_min;
>    REAL_VALUE_TYPE m_max;
> -  frange_props m_props;
> +  bool m_pos_nan;
> +  bool m_neg_nan;
>  };
>
>  class obstack_vrange_allocator final: public vrange_allocator
> diff --git a/gcc/value-range.cc b/gcc/value-range.cc
> index d759fcf178c..1c6061649b5 100644
> --- a/gcc/value-range.cc
> +++ b/gcc/value-range.cc
> @@ -267,91 +267,6 @@ tree_compare (tree_code code, tree op1, tree op2)
>    return !integer_zerop (fold_build2 (code, integer_type_node, op1, op2));
>  }
>
> -// Set the NAN property.  Adjust the range if appopriate.
> -
> -void
> -frange::update_nan (fp_prop::kind k)
> -{
> -  if (k == fp_prop::YES)
> -    {
> -      if (!maybe_nan ())
> -       {
> -         set_undefined ();
> -         return;
> -       }
> -      gcc_checking_assert (!undefined_p ());
> -      set_nan (m_type);
> -      return;
> -    }
> -
> -  if (k == fp_prop::NO && known_nan ())
> -    {
> -      set_undefined ();
> -      return;
> -    }
> -
> -  // Setting VARYING on an obviously NAN range is a no-op.
> -  if (k == fp_prop::VARYING && real_isnan (&m_min))
> -    return;
> -
> -  m_props.set_nan (k);
> -  normalize_kind ();
> -  if (flag_checking)
> -    verify_range ();
> -}
> -
> -// Set the SIGNBIT property.  Adjust the range if appropriate.
> -
> -void
> -frange::set_signbit (fp_prop::kind k)
> -{
> -  gcc_checking_assert (m_type);
> -
> -  // No additional adjustments are needed for a NAN.
> -  if (known_nan ())
> -    {
> -      m_props.set_signbit (k);
> -      return;
> -    }
> -  // Ignore sign changes when they're set correctly.
> -  if (!maybe_nan ())
> -    {
> -      // It's negative and we're trying to make it negative or varying.
> -      if (real_less (&m_max, &dconst0) && (k == fp_prop::YES
> -                                          || k == fp_prop::VARYING))
> -       return;
> -      // It's positive and we're trying to make it positive or varying.
> -      if (real_less (&dconst0, &m_min) && (k == fp_prop::NO
> -                                          || k == fp_prop::VARYING))
> -       return;
> -    }
> -  // Adjust the range depending on the sign bit.
> -  if (k == fp_prop::YES)
> -    {
> -      // Crop the range to [-INF, 0].
> -      frange crop (m_type, dconstninf, dconst0);
> -      intersect (crop);
> -      if (!undefined_p ())
> -       m_props.set_signbit (fp_prop::YES);
> -    }
> -  else if (k == fp_prop::NO)
> -    {
> -      // Crop the range to [0, +INF].
> -      frange crop (m_type, dconst0, dconstinf);
> -      intersect (crop);
> -      if (!undefined_p ())
> -       m_props.set_signbit (fp_prop::NO);
> -    }
> -  else
> -    {
> -      m_props.set_signbit (fp_prop::VARYING);
> -      normalize_kind ();
> -    }
> -
> -  if (flag_checking)
> -    verify_range ();
> -}
> -
>  // Setter for franges.
>
>  void
> @@ -375,24 +290,23 @@ frange::set (tree min, tree max, value_range_kind kind)
>        gcc_checking_assert (real_identical (TREE_REAL_CST_PTR (min),
>                                            TREE_REAL_CST_PTR (max)));
>        tree type = TREE_TYPE (min);
> -      set_nan (type);
> +      bool sign = real_isneg (TREE_REAL_CST_PTR (min));
> +      set_nan (type, sign);
>        return;
>      }
>
>    m_kind = kind;
>    m_type = TREE_TYPE (min);
> -  m_props.set_varying ();
>    m_min = *TREE_REAL_CST_PTR (min);
>    m_max = *TREE_REAL_CST_PTR (max);
> -
> -  // Set SIGNBIT property for positive and negative ranges.
> -  if (real_less (&m_max, &dconst0))
> -    m_props.signbit_set_yes ();
> -  else if (real_less (&dconst0, &m_min))
> -    m_props.signbit_set_no ();
> +  m_pos_nan = true;
> +  m_neg_nan = true;
>
>    if (!HONOR_NANS (m_type))
> -    m_props.nan_set_no ();
> +    {
> +      m_pos_nan = false;
> +      m_neg_nan = false;
> +    }
>
>    // Check for swapped ranges.
>    gcc_checking_assert (tree_compare (LE_EXPR, min, max));
> @@ -423,18 +337,11 @@ frange::set (tree type,
>  bool
>  frange::normalize_kind ()
>  {
> -  // Undefined is viral.
> -  if (m_props.nan_undefined_p () || m_props.signbit_undefined_p ())
> -    {
> -      set_undefined ();
> -      return true;
> -    }
>    if (m_kind == VR_RANGE
>        && real_isinf (&m_min, 1)
>        && real_isinf (&m_max, 0))
>      {
> -      // No FP properties set means varying.
> -      if (m_props.varying_p ())
> +      if (m_pos_nan && m_neg_nan)
>         {
>           set_varying (m_type);
>           return true;
> @@ -442,8 +349,7 @@ frange::normalize_kind ()
>      }
>    else if (m_kind == VR_VARYING)
>      {
> -      // If a VARYING has any FP properties, it's no longer VARYING.
> -      if (!m_props.varying_p ())
> +      if (!m_pos_nan || !m_neg_nan)
>         {
>           m_kind = VR_RANGE;
>           m_min = dconstninf;
> @@ -454,6 +360,60 @@ frange::normalize_kind ()
>    return false;
>  }
>
> +// Union or intersect the zero endpoints of two ranges.  For example:
> +//   [-0,  x] U [+0,  x] => [-0,  x]
> +//   [ x, -0] U [ x, +0] => [ x, +0]
> +//   [-0,  x] ^ [+0,  x] => [+0,  x]
> +//   [ x, -0] ^ [ x, +0] => [ x, -0]
> +//
> +// UNION_P is true when performing a union, or false when intersecting.
> +
> +bool
> +frange::combine_zeros (const frange &r, bool union_p)
> +{
> +  bool changed = false;
> +  if (real_iszero (&m_min) && real_iszero (&r.m_min)
> +      && real_isneg (&m_min) != real_isneg (&r.m_min))
> +    {
> +      m_min.sign = union_p;
> +      changed = true;
> +    }
> +  if (real_iszero (&m_max) && real_iszero (&r.m_max)
> +      && real_isneg (&m_max) != real_isneg (&r.m_max))
> +    {
> +      m_max.sign = !union_p;
> +      changed = true;
> +    }
> +  // If the signs are swapped, the resulting range is empty.
> +  if (m_min.sign == 0 && m_max.sign == 1)
> +    {
> +      m_kind = VR_UNDEFINED;
> +      changed = true;
> +    }
> +  return changed;
> +}
> +
> +// Union two ranges when one is known to be a NAN.
> +
> +bool
> +frange::union_nans (const frange &r)
> +{
> +  gcc_checking_assert (known_nan () || r.known_nan ());
> +
> +  if (known_nan ())
> +    {
> +      m_kind = r.m_kind;
> +      m_min = r.m_min;
> +      m_max = r.m_max;
> +    }
> +  m_pos_nan |= r.m_pos_nan;
> +  m_neg_nan |= r.m_neg_nan;
> +  normalize_kind ();
> +  if (flag_checking)
> +    verify_range ();
> +  return true;
> +}
> +
>  bool
>  frange::union_ (const vrange &v)
>  {
> @@ -467,29 +427,18 @@ frange::union_ (const vrange &v)
>        return true;
>      }
>
> -  // If one side has a NAN, the union is the other side, plus the union
> -  // of the properties and the possibility of a NAN.
> -  if (known_nan ())
> -    {
> -      frange_props save = m_props;
> -      *this = r;
> -      m_props = save;
> -      m_props.union_ (r.m_props);
> -      update_nan (fp_prop::VARYING);
> -      if (flag_checking)
> -       verify_range ();
> -      return true;
> -    }
> -  if (r.known_nan ())
> +  // Combine NAN info.
> +  if (known_nan () || r.known_nan ())
> +    return union_nans (r);
> +  bool changed = false;
> +  if (m_pos_nan != r.m_pos_nan || m_neg_nan != r.m_neg_nan)
>      {
> -      m_props.union_ (r.m_props);
> -      update_nan (fp_prop::VARYING);
> -      if (flag_checking)
> -       verify_range ();
> -      return true;
> +      m_pos_nan |= r.m_pos_nan;
> +      m_neg_nan |= r.m_neg_nan;
> +      changed = true;
>      }
>
> -  bool changed = m_props.union_ (r.m_props);
> +  // Combine endpoints.
>    if (real_less (&r.m_min, &m_min))
>      {
>        m_min = r.m_min;
> @@ -500,13 +449,38 @@ frange::union_ (const vrange &v)
>        m_max = r.m_max;
>        changed = true;
>      }
> -  changed |= normalize_kind ();
>
> +  if (HONOR_SIGNED_ZEROS (m_type))
> +    changed |= combine_zeros (r, true);
> +
> +  changed |= normalize_kind ();
>    if (flag_checking)
>      verify_range ();
>    return changed;
>  }
>
> +// Intersect two ranges when one is known to be a NAN.
> +
> +bool
> +frange::intersect_nans (const frange &r)
> +{
> +  gcc_checking_assert (known_nan () || r.known_nan ());
> +
> +  m_kind = VR_UNDEFINED;
> +  m_pos_nan &= r.m_pos_nan;
> +  m_neg_nan &= r.m_neg_nan;
> +  if (!maybe_nan ())
> +    {
> +      // If the NAN was intersected out, the resulting range is empty.
> +      set_undefined ();
> +      return true;
> +    }
> +  normalize_kind ();
> +  if (flag_checking)
> +    verify_range ();
> +  return true;
> +}
> +
>  bool
>  frange::intersect (const vrange &v)
>  {
> @@ -525,25 +499,18 @@ frange::intersect (const vrange &v)
>        return true;
>      }
>
> -  // If two NANs are not exactly the same, drop to an unknown NAN,
> -  // otherwise there's nothing to do.
> -  if (known_nan () && r.known_nan ())
> -    {
> -      if (m_props == r.m_props)
> -       return false;
> -
> -      set_nan (m_type);
> -      return true;
> -    }
> -  // ?? Perhaps the intersection of a NAN and anything is a NAN ??.
> +  // Combine NAN info.
>    if (known_nan () || r.known_nan ())
> +    return intersect_nans (r);
> +  bool changed = false;
> +  if (m_pos_nan != r.m_pos_nan || m_neg_nan != r.m_neg_nan)
>      {
> -      set_varying (m_type);
> -      return true;
> +      m_pos_nan &= r.m_pos_nan;
> +      m_neg_nan &= r.m_neg_nan;
> +      changed = true;
>      }
>
> -  bool changed = m_props.intersect (r.m_props);
> -
> +  // Combine endpoints.
>    if (real_less (&m_min, &r.m_min))
>      {
>        m_min = r.m_min;
> @@ -554,14 +521,25 @@ frange::intersect (const vrange &v)
>        m_max = r.m_max;
>        changed = true;
>      }
> -  // If the endpoints are swapped, the ranges are disjoint.
>    if (real_less (&m_max, &m_min))
>      {
> +      // If the endpoints are swapped, the resulting range is empty.
> +      if (maybe_nan ())
> +       {
> +         // An empty range with a NAN is just a NAN.
> +         m_kind = VR_UNDEFINED;
> +         if (flag_checking)
> +           verify_range ();
> +         return true;
> +       }
>        set_undefined ();
>        return true;
>      }
> -  changed |= normalize_kind ();
>
> +  if (HONOR_SIGNED_ZEROS (m_type))
> +    changed |= combine_zeros (r, false);
> +
> +  changed |= normalize_kind ();
>    if (flag_checking)
>      verify_range ();
>    return changed;
> @@ -574,7 +552,8 @@ frange::operator= (const frange &src)
>    m_type = src.m_type;
>    m_min = src.m_min;
>    m_max = src.m_max;
> -  m_props = src.m_props;
> +  m_pos_nan = src.m_pos_nan;
> +  m_neg_nan = src.m_neg_nan;
>
>    if (flag_checking)
>      verify_range ();
> @@ -597,7 +576,8 @@ frange::operator== (const frange &src) const
>
>        return (real_identical (&m_min, &src.m_min)
>               && real_identical (&m_max, &src.m_max)
> -             && m_props == src.m_props
> +             && m_pos_nan == src.m_pos_nan
> +             && m_neg_nan == src.m_neg_nan
>               && types_compatible_p (m_type, src.m_type));
>      }
>    return false;
> @@ -617,21 +597,24 @@ frange::contains_p (tree cst) const
>    if (varying_p ())
>      return true;
>
> +  if (real_isnan (rv))
> +    {
> +      // No NAN in range.
> +      if (!m_pos_nan && !m_neg_nan)
> +       return false;
> +      // Both +NAN and -NAN are present.
> +      if (m_pos_nan && m_neg_nan)
> +       return true;
> +      return m_neg_nan == rv->sign;
> +    }
> +  if (known_nan ())
> +    return false;
>
>    if (real_compare (GE_EXPR, rv, &m_min) && real_compare (LE_EXPR, rv, &m_max))
>      {
> +      // Make sure the signs are equal for signed zeros.
>        if (HONOR_SIGNED_ZEROS (m_type) && real_iszero (rv))
> -       {
> -         // FIXME: This is still using get_signbit() instead of
> -         // known_signbit() because the latter bails on possible NANs
> -         // (for now).
> -         if (get_signbit ().yes_p ())
> -           return real_isneg (rv);
> -         else if (get_signbit ().no_p ())
> -           return !real_isneg (rv);
> -         else
> -           return true;
> -       }
> +       return m_min.sign == m_max.sign && m_min.sign == rv->sign;
>        return true;
>      }
>    return false;
> @@ -651,26 +634,6 @@ frange::singleton_p (tree *result) const
>        if (HONOR_NANS (m_type) && maybe_nan ())
>         return false;
>
> -      // Return the appropriate zero if known.
> -      if (HONOR_SIGNED_ZEROS (m_type) && zero_p ())
> -       {
> -         bool signbit;
> -         if (known_signbit (signbit))
> -           {
> -             if (signbit)
> -               {
> -                 if (result)
> -                   *result = build_real (m_type, real_value_negate (&dconst0));
> -               }
> -             else
> -               {
> -                 if (result)
> -                   *result = build_real (m_type, dconst0);
> -               }
> -             return true;
> -           }
> -         return false;
> -       }
>        if (result)
>         *result = build_real (m_type, m_min);
>        return true;
> @@ -687,57 +650,31 @@ frange::supports_type_p (const_tree type) const
>  void
>  frange::verify_range ()
>  {
> -  if (undefined_p ())
> -    {
> -      gcc_checking_assert (m_props.undefined_p ());
> -      return;
> -    }
> -  gcc_checking_assert (!m_props.undefined_p ());
> -
> +  if (m_kind == VR_UNDEFINED)
> +    return;
>    if (varying_p ())
>      {
> -      gcc_checking_assert (m_props.varying_p ());
> +      gcc_checking_assert (m_pos_nan && m_neg_nan);
> +      gcc_checking_assert (real_isinf (&m_min, 1));
> +      gcc_checking_assert (real_isinf (&m_max, 0));
>        return;
>      }
>
> +  // NANs cannot appear in the endpoints of a range.
> +  gcc_checking_assert (!real_isnan (&m_min) && !real_isnan (&m_max));
> +
>    // We don't support the inverse of an frange (yet).
>    gcc_checking_assert (m_kind == VR_RANGE);
>
> -  bool is_nan = real_isnan (&m_min) || real_isnan (&m_max);
> -  if (is_nan)
> -    {
> -      // If either is a NAN, both must be a NAN.
> -      gcc_checking_assert (real_identical (&m_min, &m_max));
> -      gcc_checking_assert (known_nan ());
> -    }
> -  else
> -    // Make sure we don't have swapped ranges.
> -    gcc_checking_assert (!real_less (&m_max, &m_min));
> +  // Make sure we don't have swapped ranges.
> +  gcc_checking_assert (!real_less (&m_max, &m_min));
>
> -  // If we're absolutely sure we have a NAN, the endpoints should
> -  // reflect this, otherwise we'd have more than one way to represent
> -  // a NAN.
> -  if (known_nan ())
> -    {
> -      gcc_checking_assert (real_isnan (&m_min));
> -      gcc_checking_assert (real_isnan (&m_max));
> -    }
> -  else
> -    {
> -      // Make sure the signbit and range agree.
> -      bool signbit;
> -      if (known_signbit (signbit))
> -       {
> -         if (signbit)
> -           gcc_checking_assert (real_compare (LE_EXPR, &m_max, &dconst0));
> -         else
> -           gcc_checking_assert (real_compare (GE_EXPR, &m_min, &dconst0));
> -       }
> -    }
> +  // [ +0.0, -0.0 ] is nonsensical.
> +  gcc_checking_assert (!(real_iszero (&m_min, 0) && real_iszero (&m_max, 1)));
>
>    // If all the properties are clear, we better not span the entire
>    // domain, because that would make us varying.
> -  if (m_props.varying_p ())
> +  if (m_pos_nan && m_neg_nan)
>      gcc_checking_assert (!real_isinf (&m_min, 1) || !real_isinf (&m_max, 0));
>  }
>
> @@ -755,16 +692,24 @@ frange::nonzero_p () const
>    return false;
>  }
>
> -// Set range to [+0.0, +0.0].
> +// Set range to [+0.0, +0.0] if honoring signed zeros, or [0.0, 0.0]
> +// otherwise.
>
>  void
>  frange::set_zero (tree type)
>  {
> -  tree zero = build_zero_cst (type);
> -  set (zero, zero);
> +  if (HONOR_SIGNED_ZEROS (type))
> +    {
> +      REAL_VALUE_TYPE dconstm0 = dconst0;
> +      dconstm0.sign = 1;
> +      set (type, dconstm0, dconst0);
> +      clear_nan ();
> +    }
> +  else
> +    set (type, dconst0, dconst0);
>  }
>
> -// Return TRUE for any [0.0, 0.0] regardless of sign.
> +// Return TRUE for any zero regardless of sign.
>
>  bool
>  frange::zero_p () const
> @@ -777,9 +722,7 @@ frange::zero_p () const
>  void
>  frange::set_nonnegative (tree type)
>  {
> -  tree zero = build_zero_cst (type);
> -  tree inf = vrp_val_max (type);
> -  set (zero, inf);
> +  set (type, dconst0, dconstinf);
>  }
>
>  // Here we copy between any two irange's.  The ranges can be legacy or
> @@ -3637,8 +3580,21 @@ range_tests_nan ()
>        ASSERT_EQ (r0, r1);
>        r0.clear_nan ();
>        ASSERT_NE (r0, r1);
> +      r0.update_nan ();
> +      ASSERT_EQ (r0, r1);
> +
> +      // [10, 20] NAN ^ [30, 40] NAN = NAN.
> +      r0 = frange_float ("10", "20");
> +      r1 = frange_float ("30", "40");
> +      r0.intersect (r1);
> +      ASSERT_TRUE (r0.known_nan ());
> +
> +      // [3,5] U [5,10] NAN = ... NAN
> +      r0 = frange_float ("3", "5");
>        r0.clear_nan ();
> -      ASSERT_NE (r0, r1);
> +      r1 = frange_float ("5", "10");
> +      r0.union_ (r1);
> +      ASSERT_TRUE (r0.maybe_nan ());
>      }
>
>    // NAN ranges are not equal to each other.
> @@ -3663,15 +3619,15 @@ range_tests_nan ()
>    r0.set_nan (float_type_node);
>    r1.set_nan (float_type_node);
>    r0.union_ (r1);
> -  ASSERT_TRUE (real_isnan (&r0.lower_bound ()));
> -  ASSERT_TRUE (real_isnan (&r1.upper_bound ()));
>    ASSERT_TRUE (r0.known_nan ());
>
> -  // [INF, INF] ^ NAN = VARYING
> +  // [INF, INF] NAN ^ NAN = NAN
>    r0.set_nan (float_type_node);
>    r1 = frange_float ("+Inf", "+Inf");
> +  if (!HONOR_NANS (float_type_node))
> +    r1.update_nan ();
>    r0.intersect (r1);
> -  ASSERT_TRUE (r0.varying_p ());
> +  ASSERT_TRUE (r0.known_nan ());
>
>    // NAN ^ NAN = NAN
>    r0.set_nan (float_type_node);
> @@ -3679,18 +3635,48 @@ range_tests_nan ()
>    r0.intersect (r1);
>    ASSERT_TRUE (r0.known_nan ());
>
> +  // +NAN ^ -NAN = UNDEFINED
> +  r0.set_nan (float_type_node, false);
> +  r1.set_nan (float_type_node, true);
> +  r0.intersect (r1);
> +  ASSERT_TRUE (r0.undefined_p ());
> +
>    // VARYING ^ NAN = NAN.
>    r0.set_nan (float_type_node);
>    r1.set_varying (float_type_node);
>    r0.intersect (r1);
>    ASSERT_TRUE (r0.known_nan ());
>
> -  // Setting the NAN bit to yes, forces to range to [NAN, NAN].
> +  // [3,4] ^ NAN = UNDEFINED.
> +  r0 = frange_float ("3", "4");
> +  r0.clear_nan ();
> +  r1.set_nan (float_type_node);
> +  r0.intersect (r1);
> +  ASSERT_TRUE (r0.undefined_p ());
> +
> +  // [-3, 5] ^ NAN = UNDEFINED
> +  r0 = frange_float ("-3", "5");
> +  r0.clear_nan ();
> +  r1.set_nan (float_type_node);
> +  r0.intersect (r1);
> +  ASSERT_TRUE (r0.undefined_p ());
> +
> +  // Setting the NAN bit to yes does not make us a known NAN.
>    r0.set_varying (float_type_node);
> -  r0.update_nan (fp_prop::YES);
> -  ASSERT_TRUE (r0.known_nan ());
> -  ASSERT_TRUE (real_isnan (&r0.lower_bound ()));
> -  ASSERT_TRUE (real_isnan (&r0.upper_bound ()));
> +  r0.update_nan ();
> +  ASSERT_FALSE (r0.known_nan ());
> +
> +  // NAN is in a VARYING.
> +  r0.set_varying (float_type_node);
> +  real_nan (&r, "", 1, TYPE_MODE (float_type_node));
> +  tree nan = build_real (float_type_node, r);
> +  ASSERT_TRUE (r0.contains_p (nan));
> +
> +  // -NAN is in a VARYING.
> +  r0.set_varying (float_type_node);
> +  q = real_value_negate (&r);
> +  tree neg_nan = build_real (float_type_node, q);
> +  ASSERT_TRUE (r0.contains_p (neg_nan));
>  }
>
>  static void
> @@ -3702,49 +3688,84 @@ range_tests_signed_zeros ()
>    frange r0, r1;
>    bool signbit;
>
> -  // Since -0.0 == +0.0, a range of [-0.0, -0.0] should contain +0.0
> -  // and vice versa.
> +  // [0,0] contains [0,0] but not [-0,-0] and vice versa.
>    r0 = frange (zero, zero);
>    r1 = frange (neg_zero, neg_zero);
>    ASSERT_TRUE (r0.contains_p (zero));
> -  ASSERT_TRUE (r0.contains_p (neg_zero));
> -  ASSERT_TRUE (r1.contains_p (zero));
> +  ASSERT_TRUE (!r0.contains_p (neg_zero));
>    ASSERT_TRUE (r1.contains_p (neg_zero));
> +  ASSERT_TRUE (!r1.contains_p (zero));
>
>    // Test contains_p() when we know the sign of the zero.
> -  r0 = frange(zero, zero);
> -  r0.set_signbit (fp_prop::NO);
> +  r0 = frange (zero, zero);
>    ASSERT_TRUE (r0.contains_p (zero));
>    ASSERT_FALSE (r0.contains_p (neg_zero));
> -  r0.set_signbit (fp_prop::YES);
> +  r0 = frange (neg_zero, neg_zero);
>    ASSERT_TRUE (r0.contains_p (neg_zero));
>    ASSERT_FALSE (r0.contains_p (zero));
>
> -  // The intersection of zeros that differ in sign is the empty set.
> -  r0 = frange (zero, zero);
> -  r0.set_signbit (fp_prop::YES);
> +  // The intersection of zeros that differ in sign is a NAN (or
> +  // undefined if not honoring NANs).
> +  r0 = frange (neg_zero, neg_zero);
>    r1 = frange (zero, zero);
> -  r1.set_signbit (fp_prop::NO);
>    r0.intersect (r1);
> -  ASSERT_TRUE (r0.undefined_p ());
> +  if (HONOR_NANS (float_type_node))
> +    ASSERT_TRUE (r0.known_nan ());
> +  else
> +    ASSERT_TRUE (r0.undefined_p ());
>
>    // The union of zeros that differ in sign is a zero with unknown sign.
>    r0 = frange (zero, zero);
> -  r0.set_signbit (fp_prop::NO);
> -  r1 = frange (zero, zero);
> -  r1.set_signbit (fp_prop::YES);
> +  r1 = frange (neg_zero, neg_zero);
>    r0.union_ (r1);
>    ASSERT_TRUE (r0.zero_p () && !r0.known_signbit (signbit));
>
> -  // NAN U [5,6] should be [5,6] with no sign info.
> +  // [-0, +0] has an unknown sign.
> +  r0 = frange (neg_zero, zero);
> +  ASSERT_TRUE (r0.zero_p () && !r0.known_signbit (signbit));
> +
> +  // [-0, +0] ^ [0, 0] is [0, 0]
> +  r0 = frange (neg_zero, zero);
> +  r1 = frange (zero, zero);
> +  r0.intersect (r1);
> +  ASSERT_TRUE (r0.zero_p ());
> +
> +  // NAN U [5,6] should be [5,6] NAN.
>    r0.set_nan (float_type_node);
>    r1 = frange_float ("5", "6");
> +  r1.clear_nan ();
>    r0.union_ (r1);
>    real_from_string (&q, "5");
>    real_from_string (&r, "6");
>    ASSERT_TRUE (real_identical (&q, &r0.lower_bound ()));
>    ASSERT_TRUE (real_identical (&r, &r0.upper_bound ()));
>    ASSERT_TRUE (!r0.known_signbit (signbit));
> +  ASSERT_TRUE (r0.maybe_nan ());
> +
> +  r0 = frange_float ("+0", "5");
> +  r0.clear_nan ();
> +  ASSERT_TRUE (r0.known_signbit (signbit) && !signbit);
> +
> +  r0 = frange_float ("-0", "5");
> +  r0.clear_nan ();
> +  ASSERT_TRUE (!r0.known_signbit (signbit));
> +
> +  r0 = frange_float ("-0", "10");
> +  r1 = frange_float ("0", "5");
> +  r0.intersect (r1);
> +  ASSERT_TRUE (real_iszero (&r0.lower_bound (), false));
> +
> +  r0 = frange_float ("-0", "5");
> +  r1 = frange_float ("0", "5");
> +  r0.union_ (r1);
> +  ASSERT_TRUE (real_iszero (&r0.lower_bound (), true));
> +
> +  r0 = frange_float ("-5", "-0");
> +  r0.update_nan ();
> +  r1 = frange_float ("0", "0");
> +  r1.update_nan ();
> +  r0.intersect (r1);
> +  ASSERT_TRUE (r0.known_nan ());
>  }
>
>  static void
> @@ -3753,22 +3774,6 @@ range_tests_signbit ()
>    frange r0, r1;
>    bool signbit;
>
> -  // Setting the signbit drops the range to [-INF, 0].
> -  r0.set_varying (float_type_node);
> -  r0.set_signbit (fp_prop::YES);
> -  ASSERT_TRUE (real_isinf (&r0.lower_bound (), 1));
> -  ASSERT_TRUE (real_iszero (&r0.upper_bound ()));
> -
> -  // Setting the signbit for [-5, 10] crops the range to [-5, 0] with
> -  // the signbit property set.
> -  r0 = frange_float ("-5", "10");
> -  r0.set_signbit (fp_prop::YES);
> -  r0.clear_nan ();
> -  ASSERT_TRUE (r0.known_signbit (signbit) && signbit);
> -  r1 = frange_float ("-5", "0");
> -  ASSERT_TRUE (real_identical (&r0.lower_bound (), &r1.lower_bound ()));
> -  ASSERT_TRUE (real_identical (&r0.upper_bound (), &r1.upper_bound ()));
> -
>    // Negative numbers should have the SIGNBIT set.
>    r0 = frange_float ("-5", "-1");
>    r0.clear_nan ();
> @@ -3780,7 +3785,7 @@ range_tests_signbit ()
>    // Numbers containing zero should have an unknown SIGNBIT.
>    r0 = frange_float ("0", "10");
>    r0.clear_nan ();
> -  ASSERT_TRUE (!r0.known_signbit (signbit));
> +  ASSERT_TRUE (r0.known_signbit (signbit) && !signbit);
>    // Numbers spanning both positive and negative should have an
>    // unknown SIGNBIT.
>    r0 = frange_float ("-10", "10");
> @@ -3788,17 +3793,6 @@ range_tests_signbit ()
>    ASSERT_TRUE (!r0.known_signbit (signbit));
>    r0.set_varying (float_type_node);
>    ASSERT_TRUE (!r0.known_signbit (signbit));
> -
> -  // Ignore signbit changes when the sign bit is obviously known from
> -  // the range.
> -  r0 = frange_float ("5", "10");
> -  r0.clear_nan ();
> -  r0.set_signbit (fp_prop::VARYING);
> -  ASSERT_TRUE (r0.known_signbit (signbit) && !signbit);
> -  r0 = frange_float ("-5", "-1");
> -  r0.set_signbit (fp_prop::NO);
> -  r0.clear_nan ();
> -  ASSERT_TRUE (r0.undefined_p ());
>  }
>
>  static void
> @@ -3896,9 +3890,19 @@ range_tests_floats ()
>    r0.intersect (r1);
>    ASSERT_EQ (r0, frange_float ("15", "20"));
>
> +  // [10,20] NAN ^ [21,25] NAN = [NAN]
> +  r0 = frange_float ("10", "20");
> +  r0.update_nan ();
> +  r1 = frange_float ("21", "25");
> +  r1.update_nan ();
> +  r0.intersect (r1);
> +  ASSERT_TRUE (r0.known_nan ());
> +
>    // [10,20] ^ [21,25] = []
>    r0 = frange_float ("10", "20");
> +  r0.clear_nan ();
>    r1 = frange_float ("21", "25");
> +  r1.clear_nan ();
>    r0.intersect (r1);
>    ASSERT_TRUE (r0.undefined_p ());
>  }
> diff --git a/gcc/value-range.h b/gcc/value-range.h
> index 4392de84c8b..cbb6496f976 100644
> --- a/gcc/value-range.h
> +++ b/gcc/value-range.h
> @@ -93,7 +93,7 @@ public:
>    virtual bool fits_p (const vrange &r) const;
>
>    bool varying_p () const;
> -  bool undefined_p () const;
> +  virtual bool undefined_p () const;
>    vrange& operator= (const vrange &);
>    bool operator== (const vrange &) const;
>    bool operator!= (const vrange &r) const { return !(*this == r); }
> @@ -263,68 +263,6 @@ public:
>    virtual void accept (const vrange_visitor &v) const override;
>  };
>
> -// Floating point property to represent possible values of a NAN, INF, etc.
> -
> -class fp_prop
> -{
> -public:
> -  enum kind {
> -    UNDEFINED  = 0x0,          // Prop is impossible.
> -    YES                = 0x1,          // Prop is definitely set.
> -    NO         = 0x2,          // Prop is definitely not set.
> -    VARYING    = (YES | NO)    // Prop may hold.
> -  };
> -  fp_prop (kind f) : m_kind (f) { }
> -  bool varying_p () const { return m_kind == VARYING; }
> -  bool undefined_p () const { return m_kind == UNDEFINED; }
> -  bool yes_p () const { return m_kind == YES; }
> -  bool no_p () const { return m_kind == NO; }
> -private:
> -  unsigned char m_kind : 2;
> -};
> -
> -// Accessors for individual FP properties.
> -
> -#define FP_PROP_ACCESSOR(NAME) \
> -  void NAME##_set_varying () { u.bits.NAME = fp_prop::VARYING; }       \
> -  void NAME##_set_yes () { u.bits.NAME = fp_prop::YES; }       \
> -  void NAME##_set_no () { u.bits.NAME = fp_prop::NO; } \
> -  bool NAME##_varying_p () const { return u.bits.NAME == fp_prop::VARYING; } \
> -  bool NAME##_undefined_p () const { return u.bits.NAME == fp_prop::UNDEFINED; } \
> -  bool NAME##_yes_p () const { return u.bits.NAME == fp_prop::YES; }   \
> -  bool NAME##_no_p () const { return u.bits.NAME == fp_prop::NO; } \
> -  fp_prop get_##NAME () const                             \
> -  { return fp_prop ((fp_prop::kind) u.bits.NAME); } \
> -  void set_##NAME (fp_prop::kind f) { u.bits.NAME = f; }
> -
> -// Aggregate of all the FP properties in an frange packed into one
> -// structure to save space.  Using explicit fp_prop's in the frange,
> -// would take one byte per property because of padding.  Instead, we
> -// can save all properties into one byte.
> -
> -class frange_props
> -{
> -public:
> -  frange_props () { set_varying (); }
> -  void set_varying () { u.bytes = 0xff; }
> -  void set_undefined () { u.bytes = 0; }
> -  bool varying_p () { return u.bytes == 0xff; }
> -  bool undefined_p () { return u.bytes == 0; }
> -  bool union_ (const frange_props &other);
> -  bool intersect (const frange_props &other);
> -  bool operator== (const frange_props &other) const;
> -  FP_PROP_ACCESSOR(nan)
> -  FP_PROP_ACCESSOR(signbit)
> -private:
> -  union {
> -    struct {
> -      unsigned char nan : 2;
> -      unsigned char signbit : 2;
> -    } bits;
> -    unsigned char bytes;
> -  } u;
> -};
> -
>  // A floating point range.
>
>  class frange : public vrange
> @@ -349,8 +287,10 @@ public:
>    void set (tree type, const REAL_VALUE_TYPE &, const REAL_VALUE_TYPE &,
>             value_range_kind = VR_RANGE);
>    void set_nan (tree type);
> +  void set_nan (tree type, bool sign);
>    virtual void set_varying (tree type) override;
>    virtual void set_undefined () override;
> +  virtual bool undefined_p () const final override;
>    virtual bool union_ (const vrange &) override;
>    virtual bool intersect (const vrange &) override;
>    virtual bool contains_p (tree) const override;
> @@ -376,33 +316,33 @@ public:
>    bool known_nan () const;
>    bool known_signbit (bool &signbit) const;
>
> -  // Accessors for FP properties.
> -  void update_nan (fp_prop::kind f);
> -  void clear_nan () { update_nan (fp_prop::NO); }
> -  void set_signbit (fp_prop::kind);
> +  void update_nan ();
> +  void clear_nan ();
>  private:
> -  fp_prop get_nan () const { return m_props.get_nan (); }
> -  fp_prop get_signbit () const { return m_props.get_signbit (); }
>    void verify_range ();
>    bool normalize_kind ();
> +  bool union_nans (const frange &);
> +  bool intersect_nans (const frange &);
> +  bool combine_zeros (const frange &, bool union_p);
>
> -  frange_props m_props;
>    tree m_type;
>    REAL_VALUE_TYPE m_min;
>    REAL_VALUE_TYPE m_max;
> +  bool m_pos_nan;
> +  bool m_neg_nan;
>  };
>
>  inline const REAL_VALUE_TYPE &
>  frange::lower_bound () const
>  {
> -  gcc_checking_assert (!undefined_p ());
> +  gcc_checking_assert (!undefined_p () && !known_nan ());
>    return m_min;
>  }
>
>  inline const REAL_VALUE_TYPE &
>  frange::upper_bound () const
>  {
> -  gcc_checking_assert (!undefined_p ());
> +  gcc_checking_assert (!undefined_p () && !known_nan ());
>    return m_max;
>  }
>
> @@ -1082,30 +1022,6 @@ vrp_val_min (const_tree type)
>    return NULL_TREE;
>  }
>
> -// Supporting methods for frange.
> -
> -inline bool
> -frange_props::operator== (const frange_props &other) const
> -{
> -  return u.bytes == other.u.bytes;
> -}
> -
> -inline bool
> -frange_props::union_ (const frange_props &other)
> -{
> -  unsigned char saved = u.bytes;
> -  u.bytes |= other.u.bytes;
> -  return u.bytes != saved;
> -}
> -
> -inline bool
> -frange_props::intersect (const frange_props &other)
> -{
> -  unsigned char saved = u.bytes;
> -  u.bytes &= other.u.bytes;
> -  return u.bytes != saved;
> -}
> -
>  inline
>  frange::frange ()
>  {
> @@ -1154,15 +1070,52 @@ frange::set_varying (tree type)
>    m_type = type;
>    m_min = dconstninf;
>    m_max = dconstinf;
> -  m_props.set_varying ();
> +  m_pos_nan = true;
> +  m_neg_nan = true;
>  }
>
>  inline void
>  frange::set_undefined ()
>  {
>    m_kind = VR_UNDEFINED;
> -  m_type = NULL;
> -  m_props.set_undefined ();
> +  m_pos_nan = false;
> +  m_neg_nan = false;
> +  if (flag_checking)
> +    verify_range ();
> +}
> +
> +// Set the NAN bit and adjust the range.
> +
> +inline void
> +frange::update_nan ()
> +{
> +  gcc_checking_assert (!undefined_p ());
> +  m_pos_nan = true;
> +  m_neg_nan = true;
> +  normalize_kind ();
> +  if (flag_checking)
> +    verify_range ();
> +}
> +
> +// Clear the NAN bit and adjust the range.
> +
> +inline void
> +frange::clear_nan ()
> +{
> +  gcc_checking_assert (!undefined_p ());
> +  m_pos_nan = false;
> +  m_neg_nan = false;
> +  normalize_kind ();
> +  if (flag_checking)
> +    verify_range ();
> +}
> +
> +// Return TRUE if range is the empty set.
> +
> +inline bool
> +frange::undefined_p () const
> +{
> +  return m_kind == VR_UNDEFINED && !m_pos_nan && !m_neg_nan;
>  }
>
>  // Set R to maximum representable value for TYPE.
> @@ -1186,19 +1139,28 @@ real_min_representable (REAL_VALUE_TYPE *r, tree type)
>    *r = real_value_negate (r);
>  }
>
> -// Build a NAN of type TYPE.
> +// Build a signless NAN of type TYPE.
>
>  inline void
>  frange::set_nan (tree type)
>  {
> -  REAL_VALUE_TYPE r;
> -  gcc_assert (real_nan (&r, "", 1, TYPE_MODE (type)));
> -  m_kind = VR_RANGE;
> +  m_kind = VR_UNDEFINED;
> +  m_type = type;
> +  m_pos_nan = true;
> +  m_neg_nan = true;
> +  if (flag_checking)
> +    verify_range ();
> +}
> +
> +// Build a NAN of type TYPE with SIGN.
> +
> +inline void
> +frange::set_nan (tree type, bool sign)
> +{
> +  m_kind = VR_UNDEFINED;
>    m_type = type;
> -  m_min = r;
> -  m_max = r;
> -  m_props.set_varying ();
> -  m_props.nan_set_yes ();
> +  m_neg_nan = sign;
> +  m_pos_nan = !sign;
>    if (flag_checking)
>      verify_range ();
>  }
> @@ -1210,9 +1172,7 @@ frange::known_finite () const
>  {
>    if (undefined_p () || varying_p () || m_kind == VR_ANTI_RANGE)
>      return false;
> -  return (!real_isnan (&m_min)
> -         && !real_isinf (&m_min)
> -         && !real_isinf (&m_max));
> +  return (!maybe_nan () && !real_isinf (&m_min) && !real_isinf (&m_max));
>  }
>
>  // Return TRUE if range may be infinite.
> @@ -1242,7 +1202,7 @@ frange::known_inf () const
>  inline bool
>  frange::maybe_nan () const
>  {
> -  return !get_nan ().no_p ();
> +  return m_pos_nan || m_neg_nan;
>  }
>
>  // Return TRUE if range is a +NAN or -NAN.
> @@ -1250,7 +1210,7 @@ frange::maybe_nan () const
>  inline bool
>  frange::known_nan () const
>  {
> -  return get_nan ().yes_p ();
> +  return m_kind == VR_UNDEFINED && maybe_nan ();
>  }
>
>  // If the signbit for the range is known, set it in SIGNBIT and return
> @@ -1259,13 +1219,31 @@ frange::known_nan () const
>  inline bool
>  frange::known_signbit (bool &signbit) const
>  {
> -  // FIXME: Signed NANs are not supported yet.
> -  if (maybe_nan ())
> +  if (undefined_p ())
>      return false;
> -  if (get_signbit ().varying_p ())
> +
> +  // NAN with unknown sign.
> +  if (m_pos_nan && m_neg_nan)
>      return false;
> -  signbit = get_signbit ().yes_p ();
> -  return true;
> +  // No NAN.
> +  if (!m_pos_nan && !m_neg_nan)
> +    {
> +      if (m_min.sign == m_max.sign)
> +       {
> +         signbit = m_min.sign;
> +         return true;
> +       }
> +      return false;
> +    }
> +  // NAN with known sign.
> +  bool nan_sign = m_neg_nan;
> +  if (m_kind == VR_UNDEFINED
> +      || (nan_sign == m_min.sign && nan_sign == m_max.sign))
> +    {
> +      signbit = nan_sign;
> +      return true;
> +    }
> +  return false;
>  }
>
>  #endif // GCC_VALUE_RANGE_H
> --
> 2.37.1
>
  
Aldy Hernandez Sept. 15, 2022, 8:44 p.m. UTC | #2
On Thu, Sep 15, 2022 at 9:06 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Thu, Sep 15, 2022 at 7:41 AM Aldy Hernandez <aldyh@redhat.com> wrote:
> >
> > Hi Richard.  Hi all.
> >
> > The attatched patch rewrites the NAN and sign handling, dropping both
> > tristates in favor of a pair of boolean flags for NANs, and nothing at
> > all for signs.  The signs are tracked in the range itself, so now it's
> > possible to describe things like [-0.0, +0.0] +NAN, [+0, +0], [-5, +0],
> > [+0, 3] -NAN, etc.
> >
> > There are a lot of changes, as the tristate was quite pervasive.  I
> > could use another pair of eyes.  The code IMO is cleaner and handles
> > all the cases we discussed.
> >
> > Here is an example of the various ranges and how they are displayed:
> >
> >     [frange] float VARYING NAN ;; Varying includes NAN
> >     [frange] UNDEFINED                      ;; Empty set as always
> >     [frange] float [] NAN                   ;; Unknown sign NAN
> >     [frange] float [] -NAN                  ;; -NAN
> >     [frange] float [] +NAN                  ;; +NAN
> >     [frange] float [-0.0, 0.0]              ;; All zeros.
> >     [frange] float [-0.0, -0.0] NAN         ;; -0 or NAN.
> >     [frange] float [-5.0e+0, -1.0e+0] +NAN  ;; [-5, -1] or +NAN
> >     [frange] float [-5.0e+0, -0.0] NAN      ;; [-5, -0] or +-NAN
> >     [frange] float [-5.0e+0, -0.0]          ;; [-5, -0]
> >     [frange] float [5.0e+0, 1.0e+1]         ;; [5, 10]
> >
> > We could represent an unknown sign with +NAN -NAN if preferred.
>
> maybe -+NAN or +-NAN?  I prefer to somehow show both signs for clarity

Sure.

>
> >
> > Notice the NAN signs are decoupled from the range, so we can represent
> > a negative range with a positive NAN.  For this range,
> > frange::known_bit() would return false, as only when the signs of the
> > NANs and range agree can we be certain.
> >
> > There is no longer any pessimization of ranges for intersects
> > involving NANs.  Also, union and intersect work with signed zeros:
> >
> > //   [-0,  x] U [+0,  x] => [-0,  x]
> > //   [ x, -0] U [ x, +0] => [ x, +0]
> > //   [-0,  x] ^ [+0,  x] => [+0,  x]
> > //   [ x, -0] ^ [ x, +0] => [ x, -0]
> >
> > The special casing for signed zeros in the singleton code is gone in
> > favor of just making sure the signs in the range agree, that is
> > [-0, -0] for example.
> >
> > I have removed the idea that a known NAN is a "range", so a NAN is no
> > longer in the endpoints itself.  Requesting the bound of a known NAN
> > is a hard fail.  For that matter, we don't store the actual NAN in the
> > range.  The only information we have are the set of boolean flags.
> > This way we make sure nothing seeps into the frange.  This also means
> > it's explicit that we don't track anything but the sign in NANs.  We
> > can revisit this if we desire to track signalling or whatever
> > concoction y'all can imagine.
> >
> > All in all, I'm quite happy with this.  It does look better, and we
> > handle all the corner cases we couldn't before.  Thanks for the
> > suggestion.
> >
> > Regstrapped with mpfr tests on x86-64 and ppc64le Linux.  Selftests
> > were also run with -ffinite-math-only on x86-64.
> >
> > At Jakub's suggestion, I built lapack with associated tests.  They
> > pass on x86-64 and ppc64le Linux with no regressions from mainline.
> > As a sanity check, I also ran them for -ffinite-math-only on x86 which
> > (as expected) returned:
> >
> >         NaN arithmetic did not perform per the ieee spec
> >
> > Otherwise, all tests pass for -ffinite-math-only.
> >
> > How does this look?
>
> Overall it looks good.
>
> Reading ::intersect and ::union I find it less clear to spread out the _nan
> cases into separate functions.

OK, will inline them.

>
> Can you add a comment to frange that its representation is
> a single value-range specified by m_type, m_min, m_max
> unioned with the set of { -NaN, +NaN }?  Because somehow
> the ::undefined_p vs. m_type == VR_UNDEFINED checks are
> a bit confusing to the occasional reader can we instead use
> ::nan_p to complement ::undefined_p?

Wouldn't that just make nan_p the same as known_nan?  Speaking of
which, I'm not a big fan of known_nan.  Perhaps we should rename all
the known_foo variants to foo_p variants?  Or...maybe even:

  // fpclassify like API
  bool isfinite () const;
  bool isinf () const;
  bool maybe_isinf () const;
  bool isnan () const;
  bool maybe_isnan () const;
  bool signbit_p (bool &signbit) const;

That would make it clear how they map to the fpclassify API.  And the
signbit_p() follows what we do for singleton_p(tree *).

isnan() would be your nan_p suggestion.

>
> Brain dump: maybe having a NaN-less frange with m_type, m_min, m_max
> and then frange_with_nan having a frange member plus the nan bits
> would make a better distinction?  Maybe we can use m_type == VR_RANGE
> when the actual range is empty but we have NaNs somehow?  That we
> need m_type to represent an empty range and VR_VARYING for the full
> range is somehow duplicate - ]0,0[ would be an empty range as well,
> but then we'd need inclusive/exclusive ranges.  NULL m_min/max might
> be another (bad) representation.  Having m_type makes for efficient
> checking as well, so that's a pro.  Maybe have m_type == VR_NAN for
> the case of empty range but NaNs, leaving VR_UNDEFINED to the
> true empty set?

Hmm, you may be onto something.

// NANless frange
class frange_base : public vrange
{
  tree m_type
  REAL_VALUE_TYPE m_min;
  REAL_VALUE_TYPE m_max;
};

// frange with NAN
class frange : public frange_base
{
  // m_kind == VR_NAN for nan_p / isnan
  bool m_pos_nan;
  bool m_neg_nan;
};

This would make the code pretty clean (and obvious).  The frange class
would handle the VR_NAN cases, and the other cases could be passed to
the base class with a final adjustment for the m_*nan flags.

BTW, I think you mean m_kind, not m_type.  Yes, I know it's confusing,
but I was running out of terms.  m_type is the tree type (undefined
for undefined_p ()).  Whereas m_kind is the "type" of VR_*.

Also, m_min == m_max == NULL is a no-go, because AFAICT, there's no
way to represent "nothing" in REAL_VALUE_TYPE.  I believe all zeros is
just a +0.0.

Aldy

>
> Anyway, I think the patch is OK as-is with the NaN printing adjusted
> and maybe avoiding the bare m_type == VR_UNDEFINED checks
> (in all but the abstraction).
>
> Thanks,
> Richard.
>
> >
> > gcc/ChangeLog:
> >
> >         * range-op-float.cc (frange_add_zeros): Replace set_signbit with
> >         union of zero.
> >         * value-query.cc (range_query::get_tree_range): Remove set_signbit
> >         use.
> >         * value-range-pretty-print.cc (vrange_printer::print_frange_prop):
> >         Remove.
> >         (vrange_printer::print_frange_nan): New.
> >         * value-range-pretty-print.h (print_frange_prop): Remove.
> >         (print_frange_nan): New.
> >         * value-range-storage.cc (frange_storage_slot::set_frange): Set
> >         kind and NAN fields.
> >         (frange_storage_slot::get_frange): Restore kind and NAN fields.
> >         * value-range-storage.h (class frange_storage_slot): Add kind and
> >         NAN fields.
> >         * value-range.cc (frange::update_nan): Remove.
> >         (frange::set_signbit): Remove.
> >         (frange::set): Adjust for NAN fields.
> >         (frange::normalize_kind): Remove m_props.
> >         (frange::combine_zeros): New.
> >         (frange::union_nans): New.
> >         (frange::union_): Handle new NAN fields.
> >         (frange::intersect_nans): New.
> >         (frange::intersect): Handle new NAN fields.
> >         (frange::operator=): Same.
> >         (frange::operator==): Same.
> >         (frange::contains_p): Same.
> >         (frange::singleton_p): Remove special case for signed zeros.
> >         (frange::verify_range): Adjust for new NAN fields.
> >         (frange::set_zero): Handle signed zeros.
> >         (frange::set_nonnegative): Same.
> >         (range_tests_nan): Adjust tests.
> >         (range_tests_signed_zeros): Same.
> >         (range_tests_signbit): Same.
> >         (range_tests_floats): Same.
> >         * value-range.h (class fp_prop): Remove.
> >         (FP_PROP_ACCESSOR): Remove.
> >         (class frange_props): Remove
> >         (frange::lower_bound): NANs don't have endpoints.
> >         (frange::upper_bound): Same.
> >         (frange_props::operator==): Remove.
> >         (frange_props::union_): Remove.
> >         (frange_props::intersect): Remove.
> >         (frange::update_nan): New.
> >         (frange::clear_nan): New.
> >         (frange::undefined_p): New.
> >         (frange::set_nan): New.
> >         (frange::known_finite): Adjust for new NAN representation.
> >         (frange::maybe_nan): Same.
> >         (frange::known_nan): Same.
> >         (frange::known_signbit): Same.
> > ---
> >  gcc/range-op-float.cc           |   6 +-
> >  gcc/value-query.cc              |  11 +-
> >  gcc/value-range-pretty-print.cc |  45 +--
> >  gcc/value-range-pretty-print.h  |   2 +-
> >  gcc/value-range-storage.cc      |   9 +-
> >  gcc/value-range-storage.h       |   7 +-
> >  gcc/value-range.cc              | 554 ++++++++++++++++----------------
> >  gcc/value-range.h               | 212 ++++++------
> >  8 files changed, 415 insertions(+), 431 deletions(-)
> >
> > diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
> > index fbc14a730ad..270490010e2 100644
> > --- a/gcc/range-op-float.cc
> > +++ b/gcc/range-op-float.cc
> > @@ -218,7 +218,11 @@ frange_add_zeros (frange &r, tree type)
> >
> >    if (HONOR_SIGNED_ZEROS (type)
> >        && (real_iszero (&r.lower_bound ()) || real_iszero (&r.upper_bound ())))
> > -    r.set_signbit (fp_prop::VARYING);
> > +    {
> > +      frange zero;
> > +      zero.set_zero (type);
> > +      r.union_ (zero);
> > +    }
> >  }
> >
> >  // Build a range that is <= VAL and store it in R.
> > diff --git a/gcc/value-query.cc b/gcc/value-query.cc
> > index ea6e4b979ad..0bdd670982b 100644
> > --- a/gcc/value-query.cc
> > +++ b/gcc/value-query.cc
> > @@ -219,17 +219,8 @@ range_query::get_tree_range (vrange &r, tree expr, gimple *stmt)
> >        {
> >         frange &f = as_a <frange> (r);
> >         f.set (expr, expr);
> > -
> > -       // Singletons from the tree world have known properties.
> > -       REAL_VALUE_TYPE *rv = TREE_REAL_CST_PTR (expr);
> > -       if (real_isnan (rv))
> > -         f.update_nan (fp_prop::YES);
> > -       else
> > +       if (!real_isnan (TREE_REAL_CST_PTR (expr)))
> >           f.clear_nan ();
> > -       if (real_isneg (rv))
> > -         f.set_signbit (fp_prop::YES);
> > -       else
> > -         f.set_signbit (fp_prop::NO);
> >         return true;
> >        }
> >
> > diff --git a/gcc/value-range-pretty-print.cc b/gcc/value-range-pretty-print.cc
> > index b124e46cb9e..49b16d6a5b1 100644
> > --- a/gcc/value-range-pretty-print.cc
> > +++ b/gcc/value-range-pretty-print.cc
> > @@ -134,34 +134,39 @@ vrange_printer::visit (const frange &r) const
> >    if (r.varying_p ())
> >      {
> >        pp_string (pp, "VARYING");
> > +      print_frange_nan (r);
> >        return;
> >      }
> >    pp_character (pp, '[');
> > -  dump_generic_node (pp,
> > -                    build_real (type, r.lower_bound ()), 0, TDF_NONE, false);
> > -  pp_string (pp, ", ");
> > -  dump_generic_node (pp,
> > -                    build_real (type, r.upper_bound ()), 0, TDF_NONE, false);
> > -  pp_string (pp, "] ");
> > -
> > -  print_frange_prop ("NAN", r.get_nan ());
> > -  print_frange_prop ("SIGN", r.get_signbit ());
> > +  bool has_endpoints = !r.known_nan ();
> > +  if (has_endpoints)
> > +    {
> > +      dump_generic_node (pp,
> > +                        build_real (type, r.lower_bound ()), 0, TDF_NONE, false);
> > +      pp_string (pp, ", ");
> > +      dump_generic_node (pp,
> > +                        build_real (type, r.upper_bound ()), 0, TDF_NONE, false);
> > +    }
> > +  pp_character (pp, ']');
> > +  print_frange_nan (r);
> >  }
> >
> > -// Print the FP properties in an frange.
> > +// Print the NAN info for an frange.
> >
> >  void
> > -vrange_printer::print_frange_prop (const char *str, const fp_prop &prop) const
> > +vrange_printer::print_frange_nan (const frange &r) const
> >  {
> > -  if (prop.varying_p ())
> > -    return;
> > -
> > -  if (prop.yes_p ())
> > -    pp_string (pp, str);
> > -  else if (prop.no_p ())
> > +  if (r.maybe_nan ())
> >      {
> > -      pp_character (pp, '!');
> > -      pp_string (pp, str);
> > +      if (r.m_pos_nan && r.m_neg_nan)
> > +       {
> > +         pp_string (pp, " NAN");
> > +         return;
> > +       }
> > +      bool nan_sign = r.m_neg_nan;
> > +      if (nan_sign)
> > +       pp_string (pp, " -NAN");
> > +      else
> > +       pp_string (pp, " +NAN");
> >      }
> > -  pp_character (pp, ' ');
> >  }
> > diff --git a/gcc/value-range-pretty-print.h b/gcc/value-range-pretty-print.h
> > index ad06c93c044..20c26598fe7 100644
> > --- a/gcc/value-range-pretty-print.h
> > +++ b/gcc/value-range-pretty-print.h
> > @@ -31,7 +31,7 @@ public:
> >  private:
> >    void print_irange_bound (const wide_int &w, tree type) const;
> >    void print_irange_bitmasks (const irange &) const;
> > -  void print_frange_prop (const char *str, const fp_prop &) const;
> > +  void print_frange_nan (const frange &) const;
> >
> >    pretty_printer *pp;
> >  };
> > diff --git a/gcc/value-range-storage.cc b/gcc/value-range-storage.cc
> > index b7a23fa9825..de7575ed48d 100644
> > --- a/gcc/value-range-storage.cc
> > +++ b/gcc/value-range-storage.cc
> > @@ -253,9 +253,11 @@ frange_storage_slot::set_frange (const frange &r)
> >    gcc_checking_assert (fits_p (r));
> >    gcc_checking_assert (!r.undefined_p ());
> >
> > +  m_kind = r.m_kind;
> >    m_min = r.m_min;
> >    m_max = r.m_max;
> > -  m_props = r.m_props;
> > +  m_pos_nan = r.m_pos_nan;
> > +  m_neg_nan = r.m_neg_nan;
> >  }
> >
> >  void
> > @@ -264,11 +266,12 @@ frange_storage_slot::get_frange (frange &r, tree type) const
> >    gcc_checking_assert (r.supports_type_p (type));
> >
> >    r.set_undefined ();
> > -  r.m_kind = VR_RANGE;
> > -  r.m_props = m_props;
> > +  r.m_kind = m_kind;
> >    r.m_type = type;
> >    r.m_min = m_min;
> >    r.m_max = m_max;
> > +  r.m_pos_nan = m_pos_nan;
> > +  r.m_neg_nan = m_neg_nan;
> >    r.normalize_kind ();
> >
> >    if (flag_checking)
> > diff --git a/gcc/value-range-storage.h b/gcc/value-range-storage.h
> > index f506789f3d1..0cf95ebf7c1 100644
> > --- a/gcc/value-range-storage.h
> > +++ b/gcc/value-range-storage.h
> > @@ -113,12 +113,11 @@ class GTY (()) frange_storage_slot
> >    frange_storage_slot (const frange &r) { set_frange (r); }
> >    DISABLE_COPY_AND_ASSIGN (frange_storage_slot);
> >
> > -  // We can get away with just storing the properties and the
> > -  // endpoints because the type can be gotten from the SSA, and
> > -  // UNDEFINED is unsupported, so it can only be a VR_RANGE.
> > +  enum value_range_kind m_kind;
> >    REAL_VALUE_TYPE m_min;
> >    REAL_VALUE_TYPE m_max;
> > -  frange_props m_props;
> > +  bool m_pos_nan;
> > +  bool m_neg_nan;
> >  };
> >
> >  class obstack_vrange_allocator final: public vrange_allocator
> > diff --git a/gcc/value-range.cc b/gcc/value-range.cc
> > index d759fcf178c..1c6061649b5 100644
> > --- a/gcc/value-range.cc
> > +++ b/gcc/value-range.cc
> > @@ -267,91 +267,6 @@ tree_compare (tree_code code, tree op1, tree op2)
> >    return !integer_zerop (fold_build2 (code, integer_type_node, op1, op2));
> >  }
> >
> > -// Set the NAN property.  Adjust the range if appopriate.
> > -
> > -void
> > -frange::update_nan (fp_prop::kind k)
> > -{
> > -  if (k == fp_prop::YES)
> > -    {
> > -      if (!maybe_nan ())
> > -       {
> > -         set_undefined ();
> > -         return;
> > -       }
> > -      gcc_checking_assert (!undefined_p ());
> > -      set_nan (m_type);
> > -      return;
> > -    }
> > -
> > -  if (k == fp_prop::NO && known_nan ())
> > -    {
> > -      set_undefined ();
> > -      return;
> > -    }
> > -
> > -  // Setting VARYING on an obviously NAN range is a no-op.
> > -  if (k == fp_prop::VARYING && real_isnan (&m_min))
> > -    return;
> > -
> > -  m_props.set_nan (k);
> > -  normalize_kind ();
> > -  if (flag_checking)
> > -    verify_range ();
> > -}
> > -
> > -// Set the SIGNBIT property.  Adjust the range if appropriate.
> > -
> > -void
> > -frange::set_signbit (fp_prop::kind k)
> > -{
> > -  gcc_checking_assert (m_type);
> > -
> > -  // No additional adjustments are needed for a NAN.
> > -  if (known_nan ())
> > -    {
> > -      m_props.set_signbit (k);
> > -      return;
> > -    }
> > -  // Ignore sign changes when they're set correctly.
> > -  if (!maybe_nan ())
> > -    {
> > -      // It's negative and we're trying to make it negative or varying.
> > -      if (real_less (&m_max, &dconst0) && (k == fp_prop::YES
> > -                                          || k == fp_prop::VARYING))
> > -       return;
> > -      // It's positive and we're trying to make it positive or varying.
> > -      if (real_less (&dconst0, &m_min) && (k == fp_prop::NO
> > -                                          || k == fp_prop::VARYING))
> > -       return;
> > -    }
> > -  // Adjust the range depending on the sign bit.
> > -  if (k == fp_prop::YES)
> > -    {
> > -      // Crop the range to [-INF, 0].
> > -      frange crop (m_type, dconstninf, dconst0);
> > -      intersect (crop);
> > -      if (!undefined_p ())
> > -       m_props.set_signbit (fp_prop::YES);
> > -    }
> > -  else if (k == fp_prop::NO)
> > -    {
> > -      // Crop the range to [0, +INF].
> > -      frange crop (m_type, dconst0, dconstinf);
> > -      intersect (crop);
> > -      if (!undefined_p ())
> > -       m_props.set_signbit (fp_prop::NO);
> > -    }
> > -  else
> > -    {
> > -      m_props.set_signbit (fp_prop::VARYING);
> > -      normalize_kind ();
> > -    }
> > -
> > -  if (flag_checking)
> > -    verify_range ();
> > -}
> > -
> >  // Setter for franges.
> >
> >  void
> > @@ -375,24 +290,23 @@ frange::set (tree min, tree max, value_range_kind kind)
> >        gcc_checking_assert (real_identical (TREE_REAL_CST_PTR (min),
> >                                            TREE_REAL_CST_PTR (max)));
> >        tree type = TREE_TYPE (min);
> > -      set_nan (type);
> > +      bool sign = real_isneg (TREE_REAL_CST_PTR (min));
> > +      set_nan (type, sign);
> >        return;
> >      }
> >
> >    m_kind = kind;
> >    m_type = TREE_TYPE (min);
> > -  m_props.set_varying ();
> >    m_min = *TREE_REAL_CST_PTR (min);
> >    m_max = *TREE_REAL_CST_PTR (max);
> > -
> > -  // Set SIGNBIT property for positive and negative ranges.
> > -  if (real_less (&m_max, &dconst0))
> > -    m_props.signbit_set_yes ();
> > -  else if (real_less (&dconst0, &m_min))
> > -    m_props.signbit_set_no ();
> > +  m_pos_nan = true;
> > +  m_neg_nan = true;
> >
> >    if (!HONOR_NANS (m_type))
> > -    m_props.nan_set_no ();
> > +    {
> > +      m_pos_nan = false;
> > +      m_neg_nan = false;
> > +    }
> >
> >    // Check for swapped ranges.
> >    gcc_checking_assert (tree_compare (LE_EXPR, min, max));
> > @@ -423,18 +337,11 @@ frange::set (tree type,
> >  bool
> >  frange::normalize_kind ()
> >  {
> > -  // Undefined is viral.
> > -  if (m_props.nan_undefined_p () || m_props.signbit_undefined_p ())
> > -    {
> > -      set_undefined ();
> > -      return true;
> > -    }
> >    if (m_kind == VR_RANGE
> >        && real_isinf (&m_min, 1)
> >        && real_isinf (&m_max, 0))
> >      {
> > -      // No FP properties set means varying.
> > -      if (m_props.varying_p ())
> > +      if (m_pos_nan && m_neg_nan)
> >         {
> >           set_varying (m_type);
> >           return true;
> > @@ -442,8 +349,7 @@ frange::normalize_kind ()
> >      }
> >    else if (m_kind == VR_VARYING)
> >      {
> > -      // If a VARYING has any FP properties, it's no longer VARYING.
> > -      if (!m_props.varying_p ())
> > +      if (!m_pos_nan || !m_neg_nan)
> >         {
> >           m_kind = VR_RANGE;
> >           m_min = dconstninf;
> > @@ -454,6 +360,60 @@ frange::normalize_kind ()
> >    return false;
> >  }
> >
> > +// Union or intersect the zero endpoints of two ranges.  For example:
> > +//   [-0,  x] U [+0,  x] => [-0,  x]
> > +//   [ x, -0] U [ x, +0] => [ x, +0]
> > +//   [-0,  x] ^ [+0,  x] => [+0,  x]
> > +//   [ x, -0] ^ [ x, +0] => [ x, -0]
> > +//
> > +// UNION_P is true when performing a union, or false when intersecting.
> > +
> > +bool
> > +frange::combine_zeros (const frange &r, bool union_p)
> > +{
> > +  bool changed = false;
> > +  if (real_iszero (&m_min) && real_iszero (&r.m_min)
> > +      && real_isneg (&m_min) != real_isneg (&r.m_min))
> > +    {
> > +      m_min.sign = union_p;
> > +      changed = true;
> > +    }
> > +  if (real_iszero (&m_max) && real_iszero (&r.m_max)
> > +      && real_isneg (&m_max) != real_isneg (&r.m_max))
> > +    {
> > +      m_max.sign = !union_p;
> > +      changed = true;
> > +    }
> > +  // If the signs are swapped, the resulting range is empty.
> > +  if (m_min.sign == 0 && m_max.sign == 1)
> > +    {
> > +      m_kind = VR_UNDEFINED;
> > +      changed = true;
> > +    }
> > +  return changed;
> > +}
> > +
> > +// Union two ranges when one is known to be a NAN.
> > +
> > +bool
> > +frange::union_nans (const frange &r)
> > +{
> > +  gcc_checking_assert (known_nan () || r.known_nan ());
> > +
> > +  if (known_nan ())
> > +    {
> > +      m_kind = r.m_kind;
> > +      m_min = r.m_min;
> > +      m_max = r.m_max;
> > +    }
> > +  m_pos_nan |= r.m_pos_nan;
> > +  m_neg_nan |= r.m_neg_nan;
> > +  normalize_kind ();
> > +  if (flag_checking)
> > +    verify_range ();
> > +  return true;
> > +}
> > +
> >  bool
> >  frange::union_ (const vrange &v)
> >  {
> > @@ -467,29 +427,18 @@ frange::union_ (const vrange &v)
> >        return true;
> >      }
> >
> > -  // If one side has a NAN, the union is the other side, plus the union
> > -  // of the properties and the possibility of a NAN.
> > -  if (known_nan ())
> > -    {
> > -      frange_props save = m_props;
> > -      *this = r;
> > -      m_props = save;
> > -      m_props.union_ (r.m_props);
> > -      update_nan (fp_prop::VARYING);
> > -      if (flag_checking)
> > -       verify_range ();
> > -      return true;
> > -    }
> > -  if (r.known_nan ())
> > +  // Combine NAN info.
> > +  if (known_nan () || r.known_nan ())
> > +    return union_nans (r);
> > +  bool changed = false;
> > +  if (m_pos_nan != r.m_pos_nan || m_neg_nan != r.m_neg_nan)
> >      {
> > -      m_props.union_ (r.m_props);
> > -      update_nan (fp_prop::VARYING);
> > -      if (flag_checking)
> > -       verify_range ();
> > -      return true;
> > +      m_pos_nan |= r.m_pos_nan;
> > +      m_neg_nan |= r.m_neg_nan;
> > +      changed = true;
> >      }
> >
> > -  bool changed = m_props.union_ (r.m_props);
> > +  // Combine endpoints.
> >    if (real_less (&r.m_min, &m_min))
> >      {
> >        m_min = r.m_min;
> > @@ -500,13 +449,38 @@ frange::union_ (const vrange &v)
> >        m_max = r.m_max;
> >        changed = true;
> >      }
> > -  changed |= normalize_kind ();
> >
> > +  if (HONOR_SIGNED_ZEROS (m_type))
> > +    changed |= combine_zeros (r, true);
> > +
> > +  changed |= normalize_kind ();
> >    if (flag_checking)
> >      verify_range ();
> >    return changed;
> >  }
> >
> > +// Intersect two ranges when one is known to be a NAN.
> > +
> > +bool
> > +frange::intersect_nans (const frange &r)
> > +{
> > +  gcc_checking_assert (known_nan () || r.known_nan ());
> > +
> > +  m_kind = VR_UNDEFINED;
> > +  m_pos_nan &= r.m_pos_nan;
> > +  m_neg_nan &= r.m_neg_nan;
> > +  if (!maybe_nan ())
> > +    {
> > +      // If the NAN was intersected out, the resulting range is empty.
> > +      set_undefined ();
> > +      return true;
> > +    }
> > +  normalize_kind ();
> > +  if (flag_checking)
> > +    verify_range ();
> > +  return true;
> > +}
> > +
> >  bool
> >  frange::intersect (const vrange &v)
> >  {
> > @@ -525,25 +499,18 @@ frange::intersect (const vrange &v)
> >        return true;
> >      }
> >
> > -  // If two NANs are not exactly the same, drop to an unknown NAN,
> > -  // otherwise there's nothing to do.
> > -  if (known_nan () && r.known_nan ())
> > -    {
> > -      if (m_props == r.m_props)
> > -       return false;
> > -
> > -      set_nan (m_type);
> > -      return true;
> > -    }
> > -  // ?? Perhaps the intersection of a NAN and anything is a NAN ??.
> > +  // Combine NAN info.
> >    if (known_nan () || r.known_nan ())
> > +    return intersect_nans (r);
> > +  bool changed = false;
> > +  if (m_pos_nan != r.m_pos_nan || m_neg_nan != r.m_neg_nan)
> >      {
> > -      set_varying (m_type);
> > -      return true;
> > +      m_pos_nan &= r.m_pos_nan;
> > +      m_neg_nan &= r.m_neg_nan;
> > +      changed = true;
> >      }
> >
> > -  bool changed = m_props.intersect (r.m_props);
> > -
> > +  // Combine endpoints.
> >    if (real_less (&m_min, &r.m_min))
> >      {
> >        m_min = r.m_min;
> > @@ -554,14 +521,25 @@ frange::intersect (const vrange &v)
> >        m_max = r.m_max;
> >        changed = true;
> >      }
> > -  // If the endpoints are swapped, the ranges are disjoint.
> >    if (real_less (&m_max, &m_min))
> >      {
> > +      // If the endpoints are swapped, the resulting range is empty.
> > +      if (maybe_nan ())
> > +       {
> > +         // An empty range with a NAN is just a NAN.
> > +         m_kind = VR_UNDEFINED;
> > +         if (flag_checking)
> > +           verify_range ();
> > +         return true;
> > +       }
> >        set_undefined ();
> >        return true;
> >      }
> > -  changed |= normalize_kind ();
> >
> > +  if (HONOR_SIGNED_ZEROS (m_type))
> > +    changed |= combine_zeros (r, false);
> > +
> > +  changed |= normalize_kind ();
> >    if (flag_checking)
> >      verify_range ();
> >    return changed;
> > @@ -574,7 +552,8 @@ frange::operator= (const frange &src)
> >    m_type = src.m_type;
> >    m_min = src.m_min;
> >    m_max = src.m_max;
> > -  m_props = src.m_props;
> > +  m_pos_nan = src.m_pos_nan;
> > +  m_neg_nan = src.m_neg_nan;
> >
> >    if (flag_checking)
> >      verify_range ();
> > @@ -597,7 +576,8 @@ frange::operator== (const frange &src) const
> >
> >        return (real_identical (&m_min, &src.m_min)
> >               && real_identical (&m_max, &src.m_max)
> > -             && m_props == src.m_props
> > +             && m_pos_nan == src.m_pos_nan
> > +             && m_neg_nan == src.m_neg_nan
> >               && types_compatible_p (m_type, src.m_type));
> >      }
> >    return false;
> > @@ -617,21 +597,24 @@ frange::contains_p (tree cst) const
> >    if (varying_p ())
> >      return true;
> >
> > +  if (real_isnan (rv))
> > +    {
> > +      // No NAN in range.
> > +      if (!m_pos_nan && !m_neg_nan)
> > +       return false;
> > +      // Both +NAN and -NAN are present.
> > +      if (m_pos_nan && m_neg_nan)
> > +       return true;
> > +      return m_neg_nan == rv->sign;
> > +    }
> > +  if (known_nan ())
> > +    return false;
> >
> >    if (real_compare (GE_EXPR, rv, &m_min) && real_compare (LE_EXPR, rv, &m_max))
> >      {
> > +      // Make sure the signs are equal for signed zeros.
> >        if (HONOR_SIGNED_ZEROS (m_type) && real_iszero (rv))
> > -       {
> > -         // FIXME: This is still using get_signbit() instead of
> > -         // known_signbit() because the latter bails on possible NANs
> > -         // (for now).
> > -         if (get_signbit ().yes_p ())
> > -           return real_isneg (rv);
> > -         else if (get_signbit ().no_p ())
> > -           return !real_isneg (rv);
> > -         else
> > -           return true;
> > -       }
> > +       return m_min.sign == m_max.sign && m_min.sign == rv->sign;
> >        return true;
> >      }
> >    return false;
> > @@ -651,26 +634,6 @@ frange::singleton_p (tree *result) const
> >        if (HONOR_NANS (m_type) && maybe_nan ())
> >         return false;
> >
> > -      // Return the appropriate zero if known.
> > -      if (HONOR_SIGNED_ZEROS (m_type) && zero_p ())
> > -       {
> > -         bool signbit;
> > -         if (known_signbit (signbit))
> > -           {
> > -             if (signbit)
> > -               {
> > -                 if (result)
> > -                   *result = build_real (m_type, real_value_negate (&dconst0));
> > -               }
> > -             else
> > -               {
> > -                 if (result)
> > -                   *result = build_real (m_type, dconst0);
> > -               }
> > -             return true;
> > -           }
> > -         return false;
> > -       }
> >        if (result)
> >         *result = build_real (m_type, m_min);
> >        return true;
> > @@ -687,57 +650,31 @@ frange::supports_type_p (const_tree type) const
> >  void
> >  frange::verify_range ()
> >  {
> > -  if (undefined_p ())
> > -    {
> > -      gcc_checking_assert (m_props.undefined_p ());
> > -      return;
> > -    }
> > -  gcc_checking_assert (!m_props.undefined_p ());
> > -
> > +  if (m_kind == VR_UNDEFINED)
> > +    return;
> >    if (varying_p ())
> >      {
> > -      gcc_checking_assert (m_props.varying_p ());
> > +      gcc_checking_assert (m_pos_nan && m_neg_nan);
> > +      gcc_checking_assert (real_isinf (&m_min, 1));
> > +      gcc_checking_assert (real_isinf (&m_max, 0));
> >        return;
> >      }
> >
> > +  // NANs cannot appear in the endpoints of a range.
> > +  gcc_checking_assert (!real_isnan (&m_min) && !real_isnan (&m_max));
> > +
> >    // We don't support the inverse of an frange (yet).
> >    gcc_checking_assert (m_kind == VR_RANGE);
> >
> > -  bool is_nan = real_isnan (&m_min) || real_isnan (&m_max);
> > -  if (is_nan)
> > -    {
> > -      // If either is a NAN, both must be a NAN.
> > -      gcc_checking_assert (real_identical (&m_min, &m_max));
> > -      gcc_checking_assert (known_nan ());
> > -    }
> > -  else
> > -    // Make sure we don't have swapped ranges.
> > -    gcc_checking_assert (!real_less (&m_max, &m_min));
> > +  // Make sure we don't have swapped ranges.
> > +  gcc_checking_assert (!real_less (&m_max, &m_min));
> >
> > -  // If we're absolutely sure we have a NAN, the endpoints should
> > -  // reflect this, otherwise we'd have more than one way to represent
> > -  // a NAN.
> > -  if (known_nan ())
> > -    {
> > -      gcc_checking_assert (real_isnan (&m_min));
> > -      gcc_checking_assert (real_isnan (&m_max));
> > -    }
> > -  else
> > -    {
> > -      // Make sure the signbit and range agree.
> > -      bool signbit;
> > -      if (known_signbit (signbit))
> > -       {
> > -         if (signbit)
> > -           gcc_checking_assert (real_compare (LE_EXPR, &m_max, &dconst0));
> > -         else
> > -           gcc_checking_assert (real_compare (GE_EXPR, &m_min, &dconst0));
> > -       }
> > -    }
> > +  // [ +0.0, -0.0 ] is nonsensical.
> > +  gcc_checking_assert (!(real_iszero (&m_min, 0) && real_iszero (&m_max, 1)));
> >
> >    // If all the properties are clear, we better not span the entire
> >    // domain, because that would make us varying.
> > -  if (m_props.varying_p ())
> > +  if (m_pos_nan && m_neg_nan)
> >      gcc_checking_assert (!real_isinf (&m_min, 1) || !real_isinf (&m_max, 0));
> >  }
> >
> > @@ -755,16 +692,24 @@ frange::nonzero_p () const
> >    return false;
> >  }
> >
> > -// Set range to [+0.0, +0.0].
> > +// Set range to [+0.0, +0.0] if honoring signed zeros, or [0.0, 0.0]
> > +// otherwise.
> >
> >  void
> >  frange::set_zero (tree type)
> >  {
> > -  tree zero = build_zero_cst (type);
> > -  set (zero, zero);
> > +  if (HONOR_SIGNED_ZEROS (type))
> > +    {
> > +      REAL_VALUE_TYPE dconstm0 = dconst0;
> > +      dconstm0.sign = 1;
> > +      set (type, dconstm0, dconst0);
> > +      clear_nan ();
> > +    }
> > +  else
> > +    set (type, dconst0, dconst0);
> >  }
> >
> > -// Return TRUE for any [0.0, 0.0] regardless of sign.
> > +// Return TRUE for any zero regardless of sign.
> >
> >  bool
> >  frange::zero_p () const
> > @@ -777,9 +722,7 @@ frange::zero_p () const
> >  void
> >  frange::set_nonnegative (tree type)
> >  {
> > -  tree zero = build_zero_cst (type);
> > -  tree inf = vrp_val_max (type);
> > -  set (zero, inf);
> > +  set (type, dconst0, dconstinf);
> >  }
> >
> >  // Here we copy between any two irange's.  The ranges can be legacy or
> > @@ -3637,8 +3580,21 @@ range_tests_nan ()
> >        ASSERT_EQ (r0, r1);
> >        r0.clear_nan ();
> >        ASSERT_NE (r0, r1);
> > +      r0.update_nan ();
> > +      ASSERT_EQ (r0, r1);
> > +
> > +      // [10, 20] NAN ^ [30, 40] NAN = NAN.
> > +      r0 = frange_float ("10", "20");
> > +      r1 = frange_float ("30", "40");
> > +      r0.intersect (r1);
> > +      ASSERT_TRUE (r0.known_nan ());
> > +
> > +      // [3,5] U [5,10] NAN = ... NAN
> > +      r0 = frange_float ("3", "5");
> >        r0.clear_nan ();
> > -      ASSERT_NE (r0, r1);
> > +      r1 = frange_float ("5", "10");
> > +      r0.union_ (r1);
> > +      ASSERT_TRUE (r0.maybe_nan ());
> >      }
> >
> >    // NAN ranges are not equal to each other.
> > @@ -3663,15 +3619,15 @@ range_tests_nan ()
> >    r0.set_nan (float_type_node);
> >    r1.set_nan (float_type_node);
> >    r0.union_ (r1);
> > -  ASSERT_TRUE (real_isnan (&r0.lower_bound ()));
> > -  ASSERT_TRUE (real_isnan (&r1.upper_bound ()));
> >    ASSERT_TRUE (r0.known_nan ());
> >
> > -  // [INF, INF] ^ NAN = VARYING
> > +  // [INF, INF] NAN ^ NAN = NAN
> >    r0.set_nan (float_type_node);
> >    r1 = frange_float ("+Inf", "+Inf");
> > +  if (!HONOR_NANS (float_type_node))
> > +    r1.update_nan ();
> >    r0.intersect (r1);
> > -  ASSERT_TRUE (r0.varying_p ());
> > +  ASSERT_TRUE (r0.known_nan ());
> >
> >    // NAN ^ NAN = NAN
> >    r0.set_nan (float_type_node);
> > @@ -3679,18 +3635,48 @@ range_tests_nan ()
> >    r0.intersect (r1);
> >    ASSERT_TRUE (r0.known_nan ());
> >
> > +  // +NAN ^ -NAN = UNDEFINED
> > +  r0.set_nan (float_type_node, false);
> > +  r1.set_nan (float_type_node, true);
> > +  r0.intersect (r1);
> > +  ASSERT_TRUE (r0.undefined_p ());
> > +
> >    // VARYING ^ NAN = NAN.
> >    r0.set_nan (float_type_node);
> >    r1.set_varying (float_type_node);
> >    r0.intersect (r1);
> >    ASSERT_TRUE (r0.known_nan ());
> >
> > -  // Setting the NAN bit to yes, forces to range to [NAN, NAN].
> > +  // [3,4] ^ NAN = UNDEFINED.
> > +  r0 = frange_float ("3", "4");
> > +  r0.clear_nan ();
> > +  r1.set_nan (float_type_node);
> > +  r0.intersect (r1);
> > +  ASSERT_TRUE (r0.undefined_p ());
> > +
> > +  // [-3, 5] ^ NAN = UNDEFINED
> > +  r0 = frange_float ("-3", "5");
> > +  r0.clear_nan ();
> > +  r1.set_nan (float_type_node);
> > +  r0.intersect (r1);
> > +  ASSERT_TRUE (r0.undefined_p ());
> > +
> > +  // Setting the NAN bit to yes does not make us a known NAN.
> >    r0.set_varying (float_type_node);
> > -  r0.update_nan (fp_prop::YES);
> > -  ASSERT_TRUE (r0.known_nan ());
> > -  ASSERT_TRUE (real_isnan (&r0.lower_bound ()));
> > -  ASSERT_TRUE (real_isnan (&r0.upper_bound ()));
> > +  r0.update_nan ();
> > +  ASSERT_FALSE (r0.known_nan ());
> > +
> > +  // NAN is in a VARYING.
> > +  r0.set_varying (float_type_node);
> > +  real_nan (&r, "", 1, TYPE_MODE (float_type_node));
> > +  tree nan = build_real (float_type_node, r);
> > +  ASSERT_TRUE (r0.contains_p (nan));
> > +
> > +  // -NAN is in a VARYING.
> > +  r0.set_varying (float_type_node);
> > +  q = real_value_negate (&r);
> > +  tree neg_nan = build_real (float_type_node, q);
> > +  ASSERT_TRUE (r0.contains_p (neg_nan));
> >  }
> >
> >  static void
> > @@ -3702,49 +3688,84 @@ range_tests_signed_zeros ()
> >    frange r0, r1;
> >    bool signbit;
> >
> > -  // Since -0.0 == +0.0, a range of [-0.0, -0.0] should contain +0.0
> > -  // and vice versa.
> > +  // [0,0] contains [0,0] but not [-0,-0] and vice versa.
> >    r0 = frange (zero, zero);
> >    r1 = frange (neg_zero, neg_zero);
> >    ASSERT_TRUE (r0.contains_p (zero));
> > -  ASSERT_TRUE (r0.contains_p (neg_zero));
> > -  ASSERT_TRUE (r1.contains_p (zero));
> > +  ASSERT_TRUE (!r0.contains_p (neg_zero));
> >    ASSERT_TRUE (r1.contains_p (neg_zero));
> > +  ASSERT_TRUE (!r1.contains_p (zero));
> >
> >    // Test contains_p() when we know the sign of the zero.
> > -  r0 = frange(zero, zero);
> > -  r0.set_signbit (fp_prop::NO);
> > +  r0 = frange (zero, zero);
> >    ASSERT_TRUE (r0.contains_p (zero));
> >    ASSERT_FALSE (r0.contains_p (neg_zero));
> > -  r0.set_signbit (fp_prop::YES);
> > +  r0 = frange (neg_zero, neg_zero);
> >    ASSERT_TRUE (r0.contains_p (neg_zero));
> >    ASSERT_FALSE (r0.contains_p (zero));
> >
> > -  // The intersection of zeros that differ in sign is the empty set.
> > -  r0 = frange (zero, zero);
> > -  r0.set_signbit (fp_prop::YES);
> > +  // The intersection of zeros that differ in sign is a NAN (or
> > +  // undefined if not honoring NANs).
> > +  r0 = frange (neg_zero, neg_zero);
> >    r1 = frange (zero, zero);
> > -  r1.set_signbit (fp_prop::NO);
> >    r0.intersect (r1);
> > -  ASSERT_TRUE (r0.undefined_p ());
> > +  if (HONOR_NANS (float_type_node))
> > +    ASSERT_TRUE (r0.known_nan ());
> > +  else
> > +    ASSERT_TRUE (r0.undefined_p ());
> >
> >    // The union of zeros that differ in sign is a zero with unknown sign.
> >    r0 = frange (zero, zero);
> > -  r0.set_signbit (fp_prop::NO);
> > -  r1 = frange (zero, zero);
> > -  r1.set_signbit (fp_prop::YES);
> > +  r1 = frange (neg_zero, neg_zero);
> >    r0.union_ (r1);
> >    ASSERT_TRUE (r0.zero_p () && !r0.known_signbit (signbit));
> >
> > -  // NAN U [5,6] should be [5,6] with no sign info.
> > +  // [-0, +0] has an unknown sign.
> > +  r0 = frange (neg_zero, zero);
> > +  ASSERT_TRUE (r0.zero_p () && !r0.known_signbit (signbit));
> > +
> > +  // [-0, +0] ^ [0, 0] is [0, 0]
> > +  r0 = frange (neg_zero, zero);
> > +  r1 = frange (zero, zero);
> > +  r0.intersect (r1);
> > +  ASSERT_TRUE (r0.zero_p ());
> > +
> > +  // NAN U [5,6] should be [5,6] NAN.
> >    r0.set_nan (float_type_node);
> >    r1 = frange_float ("5", "6");
> > +  r1.clear_nan ();
> >    r0.union_ (r1);
> >    real_from_string (&q, "5");
> >    real_from_string (&r, "6");
> >    ASSERT_TRUE (real_identical (&q, &r0.lower_bound ()));
> >    ASSERT_TRUE (real_identical (&r, &r0.upper_bound ()));
> >    ASSERT_TRUE (!r0.known_signbit (signbit));
> > +  ASSERT_TRUE (r0.maybe_nan ());
> > +
> > +  r0 = frange_float ("+0", "5");
> > +  r0.clear_nan ();
> > +  ASSERT_TRUE (r0.known_signbit (signbit) && !signbit);
> > +
> > +  r0 = frange_float ("-0", "5");
> > +  r0.clear_nan ();
> > +  ASSERT_TRUE (!r0.known_signbit (signbit));
> > +
> > +  r0 = frange_float ("-0", "10");
> > +  r1 = frange_float ("0", "5");
> > +  r0.intersect (r1);
> > +  ASSERT_TRUE (real_iszero (&r0.lower_bound (), false));
> > +
> > +  r0 = frange_float ("-0", "5");
> > +  r1 = frange_float ("0", "5");
> > +  r0.union_ (r1);
> > +  ASSERT_TRUE (real_iszero (&r0.lower_bound (), true));
> > +
> > +  r0 = frange_float ("-5", "-0");
> > +  r0.update_nan ();
> > +  r1 = frange_float ("0", "0");
> > +  r1.update_nan ();
> > +  r0.intersect (r1);
> > +  ASSERT_TRUE (r0.known_nan ());
> >  }
> >
> >  static void
> > @@ -3753,22 +3774,6 @@ range_tests_signbit ()
> >    frange r0, r1;
> >    bool signbit;
> >
> > -  // Setting the signbit drops the range to [-INF, 0].
> > -  r0.set_varying (float_type_node);
> > -  r0.set_signbit (fp_prop::YES);
> > -  ASSERT_TRUE (real_isinf (&r0.lower_bound (), 1));
> > -  ASSERT_TRUE (real_iszero (&r0.upper_bound ()));
> > -
> > -  // Setting the signbit for [-5, 10] crops the range to [-5, 0] with
> > -  // the signbit property set.
> > -  r0 = frange_float ("-5", "10");
> > -  r0.set_signbit (fp_prop::YES);
> > -  r0.clear_nan ();
> > -  ASSERT_TRUE (r0.known_signbit (signbit) && signbit);
> > -  r1 = frange_float ("-5", "0");
> > -  ASSERT_TRUE (real_identical (&r0.lower_bound (), &r1.lower_bound ()));
> > -  ASSERT_TRUE (real_identical (&r0.upper_bound (), &r1.upper_bound ()));
> > -
> >    // Negative numbers should have the SIGNBIT set.
> >    r0 = frange_float ("-5", "-1");
> >    r0.clear_nan ();
> > @@ -3780,7 +3785,7 @@ range_tests_signbit ()
> >    // Numbers containing zero should have an unknown SIGNBIT.
> >    r0 = frange_float ("0", "10");
> >    r0.clear_nan ();
> > -  ASSERT_TRUE (!r0.known_signbit (signbit));
> > +  ASSERT_TRUE (r0.known_signbit (signbit) && !signbit);
> >    // Numbers spanning both positive and negative should have an
> >    // unknown SIGNBIT.
> >    r0 = frange_float ("-10", "10");
> > @@ -3788,17 +3793,6 @@ range_tests_signbit ()
> >    ASSERT_TRUE (!r0.known_signbit (signbit));
> >    r0.set_varying (float_type_node);
> >    ASSERT_TRUE (!r0.known_signbit (signbit));
> > -
> > -  // Ignore signbit changes when the sign bit is obviously known from
> > -  // the range.
> > -  r0 = frange_float ("5", "10");
> > -  r0.clear_nan ();
> > -  r0.set_signbit (fp_prop::VARYING);
> > -  ASSERT_TRUE (r0.known_signbit (signbit) && !signbit);
> > -  r0 = frange_float ("-5", "-1");
> > -  r0.set_signbit (fp_prop::NO);
> > -  r0.clear_nan ();
> > -  ASSERT_TRUE (r0.undefined_p ());
> >  }
> >
> >  static void
> > @@ -3896,9 +3890,19 @@ range_tests_floats ()
> >    r0.intersect (r1);
> >    ASSERT_EQ (r0, frange_float ("15", "20"));
> >
> > +  // [10,20] NAN ^ [21,25] NAN = [NAN]
> > +  r0 = frange_float ("10", "20");
> > +  r0.update_nan ();
> > +  r1 = frange_float ("21", "25");
> > +  r1.update_nan ();
> > +  r0.intersect (r1);
> > +  ASSERT_TRUE (r0.known_nan ());
> > +
> >    // [10,20] ^ [21,25] = []
> >    r0 = frange_float ("10", "20");
> > +  r0.clear_nan ();
> >    r1 = frange_float ("21", "25");
> > +  r1.clear_nan ();
> >    r0.intersect (r1);
> >    ASSERT_TRUE (r0.undefined_p ());
> >  }
> > diff --git a/gcc/value-range.h b/gcc/value-range.h
> > index 4392de84c8b..cbb6496f976 100644
> > --- a/gcc/value-range.h
> > +++ b/gcc/value-range.h
> > @@ -93,7 +93,7 @@ public:
> >    virtual bool fits_p (const vrange &r) const;
> >
> >    bool varying_p () const;
> > -  bool undefined_p () const;
> > +  virtual bool undefined_p () const;
> >    vrange& operator= (const vrange &);
> >    bool operator== (const vrange &) const;
> >    bool operator!= (const vrange &r) const { return !(*this == r); }
> > @@ -263,68 +263,6 @@ public:
> >    virtual void accept (const vrange_visitor &v) const override;
> >  };
> >
> > -// Floating point property to represent possible values of a NAN, INF, etc.
> > -
> > -class fp_prop
> > -{
> > -public:
> > -  enum kind {
> > -    UNDEFINED  = 0x0,          // Prop is impossible.
> > -    YES                = 0x1,          // Prop is definitely set.
> > -    NO         = 0x2,          // Prop is definitely not set.
> > -    VARYING    = (YES | NO)    // Prop may hold.
> > -  };
> > -  fp_prop (kind f) : m_kind (f) { }
> > -  bool varying_p () const { return m_kind == VARYING; }
> > -  bool undefined_p () const { return m_kind == UNDEFINED; }
> > -  bool yes_p () const { return m_kind == YES; }
> > -  bool no_p () const { return m_kind == NO; }
> > -private:
> > -  unsigned char m_kind : 2;
> > -};
> > -
> > -// Accessors for individual FP properties.
> > -
> > -#define FP_PROP_ACCESSOR(NAME) \
> > -  void NAME##_set_varying () { u.bits.NAME = fp_prop::VARYING; }       \
> > -  void NAME##_set_yes () { u.bits.NAME = fp_prop::YES; }       \
> > -  void NAME##_set_no () { u.bits.NAME = fp_prop::NO; } \
> > -  bool NAME##_varying_p () const { return u.bits.NAME == fp_prop::VARYING; } \
> > -  bool NAME##_undefined_p () const { return u.bits.NAME == fp_prop::UNDEFINED; } \
> > -  bool NAME##_yes_p () const { return u.bits.NAME == fp_prop::YES; }   \
> > -  bool NAME##_no_p () const { return u.bits.NAME == fp_prop::NO; } \
> > -  fp_prop get_##NAME () const                             \
> > -  { return fp_prop ((fp_prop::kind) u.bits.NAME); } \
> > -  void set_##NAME (fp_prop::kind f) { u.bits.NAME = f; }
> > -
> > -// Aggregate of all the FP properties in an frange packed into one
> > -// structure to save space.  Using explicit fp_prop's in the frange,
> > -// would take one byte per property because of padding.  Instead, we
> > -// can save all properties into one byte.
> > -
> > -class frange_props
> > -{
> > -public:
> > -  frange_props () { set_varying (); }
> > -  void set_varying () { u.bytes = 0xff; }
> > -  void set_undefined () { u.bytes = 0; }
> > -  bool varying_p () { return u.bytes == 0xff; }
> > -  bool undefined_p () { return u.bytes == 0; }
> > -  bool union_ (const frange_props &other);
> > -  bool intersect (const frange_props &other);
> > -  bool operator== (const frange_props &other) const;
> > -  FP_PROP_ACCESSOR(nan)
> > -  FP_PROP_ACCESSOR(signbit)
> > -private:
> > -  union {
> > -    struct {
> > -      unsigned char nan : 2;
> > -      unsigned char signbit : 2;
> > -    } bits;
> > -    unsigned char bytes;
> > -  } u;
> > -};
> > -
> >  // A floating point range.
> >
> >  class frange : public vrange
> > @@ -349,8 +287,10 @@ public:
> >    void set (tree type, const REAL_VALUE_TYPE &, const REAL_VALUE_TYPE &,
> >             value_range_kind = VR_RANGE);
> >    void set_nan (tree type);
> > +  void set_nan (tree type, bool sign);
> >    virtual void set_varying (tree type) override;
> >    virtual void set_undefined () override;
> > +  virtual bool undefined_p () const final override;
> >    virtual bool union_ (const vrange &) override;
> >    virtual bool intersect (const vrange &) override;
> >    virtual bool contains_p (tree) const override;
> > @@ -376,33 +316,33 @@ public:
> >    bool known_nan () const;
> >    bool known_signbit (bool &signbit) const;
> >
> > -  // Accessors for FP properties.
> > -  void update_nan (fp_prop::kind f);
> > -  void clear_nan () { update_nan (fp_prop::NO); }
> > -  void set_signbit (fp_prop::kind);
> > +  void update_nan ();
> > +  void clear_nan ();
> >  private:
> > -  fp_prop get_nan () const { return m_props.get_nan (); }
> > -  fp_prop get_signbit () const { return m_props.get_signbit (); }
> >    void verify_range ();
> >    bool normalize_kind ();
> > +  bool union_nans (const frange &);
> > +  bool intersect_nans (const frange &);
> > +  bool combine_zeros (const frange &, bool union_p);
> >
> > -  frange_props m_props;
> >    tree m_type;
> >    REAL_VALUE_TYPE m_min;
> >    REAL_VALUE_TYPE m_max;
> > +  bool m_pos_nan;
> > +  bool m_neg_nan;
> >  };
> >
> >  inline const REAL_VALUE_TYPE &
> >  frange::lower_bound () const
> >  {
> > -  gcc_checking_assert (!undefined_p ());
> > +  gcc_checking_assert (!undefined_p () && !known_nan ());
> >    return m_min;
> >  }
> >
> >  inline const REAL_VALUE_TYPE &
> >  frange::upper_bound () const
> >  {
> > -  gcc_checking_assert (!undefined_p ());
> > +  gcc_checking_assert (!undefined_p () && !known_nan ());
> >    return m_max;
> >  }
> >
> > @@ -1082,30 +1022,6 @@ vrp_val_min (const_tree type)
> >    return NULL_TREE;
> >  }
> >
> > -// Supporting methods for frange.
> > -
> > -inline bool
> > -frange_props::operator== (const frange_props &other) const
> > -{
> > -  return u.bytes == other.u.bytes;
> > -}
> > -
> > -inline bool
> > -frange_props::union_ (const frange_props &other)
> > -{
> > -  unsigned char saved = u.bytes;
> > -  u.bytes |= other.u.bytes;
> > -  return u.bytes != saved;
> > -}
> > -
> > -inline bool
> > -frange_props::intersect (const frange_props &other)
> > -{
> > -  unsigned char saved = u.bytes;
> > -  u.bytes &= other.u.bytes;
> > -  return u.bytes != saved;
> > -}
> > -
> >  inline
> >  frange::frange ()
> >  {
> > @@ -1154,15 +1070,52 @@ frange::set_varying (tree type)
> >    m_type = type;
> >    m_min = dconstninf;
> >    m_max = dconstinf;
> > -  m_props.set_varying ();
> > +  m_pos_nan = true;
> > +  m_neg_nan = true;
> >  }
> >
> >  inline void
> >  frange::set_undefined ()
> >  {
> >    m_kind = VR_UNDEFINED;
> > -  m_type = NULL;
> > -  m_props.set_undefined ();
> > +  m_pos_nan = false;
> > +  m_neg_nan = false;
> > +  if (flag_checking)
> > +    verify_range ();
> > +}
> > +
> > +// Set the NAN bit and adjust the range.
> > +
> > +inline void
> > +frange::update_nan ()
> > +{
> > +  gcc_checking_assert (!undefined_p ());
> > +  m_pos_nan = true;
> > +  m_neg_nan = true;
> > +  normalize_kind ();
> > +  if (flag_checking)
> > +    verify_range ();
> > +}
> > +
> > +// Clear the NAN bit and adjust the range.
> > +
> > +inline void
> > +frange::clear_nan ()
> > +{
> > +  gcc_checking_assert (!undefined_p ());
> > +  m_pos_nan = false;
> > +  m_neg_nan = false;
> > +  normalize_kind ();
> > +  if (flag_checking)
> > +    verify_range ();
> > +}
> > +
> > +// Return TRUE if range is the empty set.
> > +
> > +inline bool
> > +frange::undefined_p () const
> > +{
> > +  return m_kind == VR_UNDEFINED && !m_pos_nan && !m_neg_nan;
> >  }
> >
> >  // Set R to maximum representable value for TYPE.
> > @@ -1186,19 +1139,28 @@ real_min_representable (REAL_VALUE_TYPE *r, tree type)
> >    *r = real_value_negate (r);
> >  }
> >
> > -// Build a NAN of type TYPE.
> > +// Build a signless NAN of type TYPE.
> >
> >  inline void
> >  frange::set_nan (tree type)
> >  {
> > -  REAL_VALUE_TYPE r;
> > -  gcc_assert (real_nan (&r, "", 1, TYPE_MODE (type)));
> > -  m_kind = VR_RANGE;
> > +  m_kind = VR_UNDEFINED;
> > +  m_type = type;
> > +  m_pos_nan = true;
> > +  m_neg_nan = true;
> > +  if (flag_checking)
> > +    verify_range ();
> > +}
> > +
> > +// Build a NAN of type TYPE with SIGN.
> > +
> > +inline void
> > +frange::set_nan (tree type, bool sign)
> > +{
> > +  m_kind = VR_UNDEFINED;
> >    m_type = type;
> > -  m_min = r;
> > -  m_max = r;
> > -  m_props.set_varying ();
> > -  m_props.nan_set_yes ();
> > +  m_neg_nan = sign;
> > +  m_pos_nan = !sign;
> >    if (flag_checking)
> >      verify_range ();
> >  }
> > @@ -1210,9 +1172,7 @@ frange::known_finite () const
> >  {
> >    if (undefined_p () || varying_p () || m_kind == VR_ANTI_RANGE)
> >      return false;
> > -  return (!real_isnan (&m_min)
> > -         && !real_isinf (&m_min)
> > -         && !real_isinf (&m_max));
> > +  return (!maybe_nan () && !real_isinf (&m_min) && !real_isinf (&m_max));
> >  }
> >
> >  // Return TRUE if range may be infinite.
> > @@ -1242,7 +1202,7 @@ frange::known_inf () const
> >  inline bool
> >  frange::maybe_nan () const
> >  {
> > -  return !get_nan ().no_p ();
> > +  return m_pos_nan || m_neg_nan;
> >  }
> >
> >  // Return TRUE if range is a +NAN or -NAN.
> > @@ -1250,7 +1210,7 @@ frange::maybe_nan () const
> >  inline bool
> >  frange::known_nan () const
> >  {
> > -  return get_nan ().yes_p ();
> > +  return m_kind == VR_UNDEFINED && maybe_nan ();
> >  }
> >
> >  // If the signbit for the range is known, set it in SIGNBIT and return
> > @@ -1259,13 +1219,31 @@ frange::known_nan () const
> >  inline bool
> >  frange::known_signbit (bool &signbit) const
> >  {
> > -  // FIXME: Signed NANs are not supported yet.
> > -  if (maybe_nan ())
> > +  if (undefined_p ())
> >      return false;
> > -  if (get_signbit ().varying_p ())
> > +
> > +  // NAN with unknown sign.
> > +  if (m_pos_nan && m_neg_nan)
> >      return false;
> > -  signbit = get_signbit ().yes_p ();
> > -  return true;
> > +  // No NAN.
> > +  if (!m_pos_nan && !m_neg_nan)
> > +    {
> > +      if (m_min.sign == m_max.sign)
> > +       {
> > +         signbit = m_min.sign;
> > +         return true;
> > +       }
> > +      return false;
> > +    }
> > +  // NAN with known sign.
> > +  bool nan_sign = m_neg_nan;
> > +  if (m_kind == VR_UNDEFINED
> > +      || (nan_sign == m_min.sign && nan_sign == m_max.sign))
> > +    {
> > +      signbit = nan_sign;
> > +      return true;
> > +    }
> > +  return false;
> >  }
> >
> >  #endif // GCC_VALUE_RANGE_H
> > --
> > 2.37.1
> >
>
  
Richard Sandiford Sept. 16, 2022, 8:33 a.m. UTC | #3
Aldy Hernandez via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> On Thu, Sep 15, 2022 at 9:06 AM Richard Biener
> <richard.guenther@gmail.com> wrote:
>>
>> On Thu, Sep 15, 2022 at 7:41 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>> >
>> > Hi Richard.  Hi all.
>> >
>> > The attatched patch rewrites the NAN and sign handling, dropping both
>> > tristates in favor of a pair of boolean flags for NANs, and nothing at
>> > all for signs.  The signs are tracked in the range itself, so now it's
>> > possible to describe things like [-0.0, +0.0] +NAN, [+0, +0], [-5, +0],
>> > [+0, 3] -NAN, etc.
>> >
>> > There are a lot of changes, as the tristate was quite pervasive.  I
>> > could use another pair of eyes.  The code IMO is cleaner and handles
>> > all the cases we discussed.
>> >
>> > Here is an example of the various ranges and how they are displayed:
>> >
>> >     [frange] float VARYING NAN ;; Varying includes NAN
>> >     [frange] UNDEFINED                      ;; Empty set as always
>> >     [frange] float [] NAN                   ;; Unknown sign NAN
>> >     [frange] float [] -NAN                  ;; -NAN
>> >     [frange] float [] +NAN                  ;; +NAN
>> >     [frange] float [-0.0, 0.0]              ;; All zeros.
>> >     [frange] float [-0.0, -0.0] NAN         ;; -0 or NAN.
>> >     [frange] float [-5.0e+0, -1.0e+0] +NAN  ;; [-5, -1] or +NAN
>> >     [frange] float [-5.0e+0, -0.0] NAN      ;; [-5, -0] or +-NAN
>> >     [frange] float [-5.0e+0, -0.0]          ;; [-5, -0]
>> >     [frange] float [5.0e+0, 1.0e+1]         ;; [5, 10]
>> >
>> > We could represent an unknown sign with +NAN -NAN if preferred.
>>
>> maybe -+NAN or +-NAN?  I prefer to somehow show both signs for clarity
>
> Sure.
>
>>
>> >
>> > Notice the NAN signs are decoupled from the range, so we can represent
>> > a negative range with a positive NAN.  For this range,
>> > frange::known_bit() would return false, as only when the signs of the
>> > NANs and range agree can we be certain.
>> >
>> > There is no longer any pessimization of ranges for intersects
>> > involving NANs.  Also, union and intersect work with signed zeros:
>> >
>> > //   [-0,  x] U [+0,  x] => [-0,  x]
>> > //   [ x, -0] U [ x, +0] => [ x, +0]
>> > //   [-0,  x] ^ [+0,  x] => [+0,  x]
>> > //   [ x, -0] ^ [ x, +0] => [ x, -0]
>> >
>> > The special casing for signed zeros in the singleton code is gone in
>> > favor of just making sure the signs in the range agree, that is
>> > [-0, -0] for example.
>> >
>> > I have removed the idea that a known NAN is a "range", so a NAN is no
>> > longer in the endpoints itself.  Requesting the bound of a known NAN
>> > is a hard fail.  For that matter, we don't store the actual NAN in the
>> > range.  The only information we have are the set of boolean flags.
>> > This way we make sure nothing seeps into the frange.  This also means
>> > it's explicit that we don't track anything but the sign in NANs.  We
>> > can revisit this if we desire to track signalling or whatever
>> > concoction y'all can imagine.
>> >
>> > All in all, I'm quite happy with this.  It does look better, and we
>> > handle all the corner cases we couldn't before.  Thanks for the
>> > suggestion.
>> >
>> > Regstrapped with mpfr tests on x86-64 and ppc64le Linux.  Selftests
>> > were also run with -ffinite-math-only on x86-64.
>> >
>> > At Jakub's suggestion, I built lapack with associated tests.  They
>> > pass on x86-64 and ppc64le Linux with no regressions from mainline.
>> > As a sanity check, I also ran them for -ffinite-math-only on x86 which
>> > (as expected) returned:
>> >
>> >         NaN arithmetic did not perform per the ieee spec
>> >
>> > Otherwise, all tests pass for -ffinite-math-only.
>> >
>> > How does this look?
>>
>> Overall it looks good.
>>
>> Reading ::intersect and ::union I find it less clear to spread out the _nan
>> cases into separate functions.
>
> OK, will inline them.
>
>>
>> Can you add a comment to frange that its representation is
>> a single value-range specified by m_type, m_min, m_max
>> unioned with the set of { -NaN, +NaN }?  Because somehow
>> the ::undefined_p vs. m_type == VR_UNDEFINED checks are
>> a bit confusing to the occasional reader can we instead use
>> ::nan_p to complement ::undefined_p?
>
> Wouldn't that just make nan_p the same as known_nan?  Speaking of
> which, I'm not a big fan of known_nan.  Perhaps we should rename all
> the known_foo variants to foo_p variants?  Or...maybe even:
>
>   // fpclassify like API
>   bool isfinite () const;
>   bool isinf () const;
>   bool maybe_isinf () const;
>   bool isnan () const;
>   bool maybe_isnan () const;
>   bool signbit_p (bool &signbit) const;
>
> That would make it clear how they map to the fpclassify API.  And the
> signbit_p() follows what we do for singleton_p(tree *).
>
> isnan() would be your nan_p suggestion.

FWIW, the reason I didn't do this with the poly_int stuff is that
it makes negative conditions harder to reason about.  It's easy for
tired eyes to read:

   !isfinite()

as meaning "is infinite", especially since there isn't a separate
isinfinite() query.  But if isfinite() is equivalent to known_isfinite()
then !isfinite() instead means "might be infinite".  !known_isfinite()
IMO makes that more explicit.

Thanks,
Richard
  
Aldy Hernandez Sept. 16, 2022, 1:26 p.m. UTC | #4
On Fri, Sep 16, 2022 at 10:33 AM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Aldy Hernandez via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > On Thu, Sep 15, 2022 at 9:06 AM Richard Biener
> > <richard.guenther@gmail.com> wrote:
> >>
> >> On Thu, Sep 15, 2022 at 7:41 AM Aldy Hernandez <aldyh@redhat.com> wrote:
> >> >
> >> > Hi Richard.  Hi all.
> >> >
> >> > The attatched patch rewrites the NAN and sign handling, dropping both
> >> > tristates in favor of a pair of boolean flags for NANs, and nothing at
> >> > all for signs.  The signs are tracked in the range itself, so now it's
> >> > possible to describe things like [-0.0, +0.0] +NAN, [+0, +0], [-5, +0],
> >> > [+0, 3] -NAN, etc.
> >> >
> >> > There are a lot of changes, as the tristate was quite pervasive.  I
> >> > could use another pair of eyes.  The code IMO is cleaner and handles
> >> > all the cases we discussed.
> >> >
> >> > Here is an example of the various ranges and how they are displayed:
> >> >
> >> >     [frange] float VARYING NAN ;; Varying includes NAN
> >> >     [frange] UNDEFINED                      ;; Empty set as always
> >> >     [frange] float [] NAN                   ;; Unknown sign NAN
> >> >     [frange] float [] -NAN                  ;; -NAN
> >> >     [frange] float [] +NAN                  ;; +NAN
> >> >     [frange] float [-0.0, 0.0]              ;; All zeros.
> >> >     [frange] float [-0.0, -0.0] NAN         ;; -0 or NAN.
> >> >     [frange] float [-5.0e+0, -1.0e+0] +NAN  ;; [-5, -1] or +NAN
> >> >     [frange] float [-5.0e+0, -0.0] NAN      ;; [-5, -0] or +-NAN
> >> >     [frange] float [-5.0e+0, -0.0]          ;; [-5, -0]
> >> >     [frange] float [5.0e+0, 1.0e+1]         ;; [5, 10]
> >> >
> >> > We could represent an unknown sign with +NAN -NAN if preferred.
> >>
> >> maybe -+NAN or +-NAN?  I prefer to somehow show both signs for clarity
> >
> > Sure.
> >
> >>
> >> >
> >> > Notice the NAN signs are decoupled from the range, so we can represent
> >> > a negative range with a positive NAN.  For this range,
> >> > frange::known_bit() would return false, as only when the signs of the
> >> > NANs and range agree can we be certain.
> >> >
> >> > There is no longer any pessimization of ranges for intersects
> >> > involving NANs.  Also, union and intersect work with signed zeros:
> >> >
> >> > //   [-0,  x] U [+0,  x] => [-0,  x]
> >> > //   [ x, -0] U [ x, +0] => [ x, +0]
> >> > //   [-0,  x] ^ [+0,  x] => [+0,  x]
> >> > //   [ x, -0] ^ [ x, +0] => [ x, -0]
> >> >
> >> > The special casing for signed zeros in the singleton code is gone in
> >> > favor of just making sure the signs in the range agree, that is
> >> > [-0, -0] for example.
> >> >
> >> > I have removed the idea that a known NAN is a "range", so a NAN is no
> >> > longer in the endpoints itself.  Requesting the bound of a known NAN
> >> > is a hard fail.  For that matter, we don't store the actual NAN in the
> >> > range.  The only information we have are the set of boolean flags.
> >> > This way we make sure nothing seeps into the frange.  This also means
> >> > it's explicit that we don't track anything but the sign in NANs.  We
> >> > can revisit this if we desire to track signalling or whatever
> >> > concoction y'all can imagine.
> >> >
> >> > All in all, I'm quite happy with this.  It does look better, and we
> >> > handle all the corner cases we couldn't before.  Thanks for the
> >> > suggestion.
> >> >
> >> > Regstrapped with mpfr tests on x86-64 and ppc64le Linux.  Selftests
> >> > were also run with -ffinite-math-only on x86-64.
> >> >
> >> > At Jakub's suggestion, I built lapack with associated tests.  They
> >> > pass on x86-64 and ppc64le Linux with no regressions from mainline.
> >> > As a sanity check, I also ran them for -ffinite-math-only on x86 which
> >> > (as expected) returned:
> >> >
> >> >         NaN arithmetic did not perform per the ieee spec
> >> >
> >> > Otherwise, all tests pass for -ffinite-math-only.
> >> >
> >> > How does this look?
> >>
> >> Overall it looks good.
> >>
> >> Reading ::intersect and ::union I find it less clear to spread out the _nan
> >> cases into separate functions.
> >
> > OK, will inline them.
> >
> >>
> >> Can you add a comment to frange that its representation is
> >> a single value-range specified by m_type, m_min, m_max
> >> unioned with the set of { -NaN, +NaN }?  Because somehow
> >> the ::undefined_p vs. m_type == VR_UNDEFINED checks are
> >> a bit confusing to the occasional reader can we instead use
> >> ::nan_p to complement ::undefined_p?
> >
> > Wouldn't that just make nan_p the same as known_nan?  Speaking of
> > which, I'm not a big fan of known_nan.  Perhaps we should rename all
> > the known_foo variants to foo_p variants?  Or...maybe even:
> >
> >   // fpclassify like API
> >   bool isfinite () const;
> >   bool isinf () const;
> >   bool maybe_isinf () const;
> >   bool isnan () const;
> >   bool maybe_isnan () const;
> >   bool signbit_p (bool &signbit) const;
> >
> > That would make it clear how they map to the fpclassify API.  And the
> > signbit_p() follows what we do for singleton_p(tree *).
> >
> > isnan() would be your nan_p suggestion.
>
> FWIW, the reason I didn't do this with the poly_int stuff is that
> it makes negative conditions harder to reason about.  It's easy for
> tired eyes to read:
>
>    !isfinite()
>
> as meaning "is infinite", especially since there isn't a separate
> isinfinite() query.  But if isfinite() is equivalent to known_isfinite()
> then !isfinite() instead means "might be infinite".  !known_isfinite()
> IMO makes that more explicit.

Ughh, fair enough.  I've settled on:

  bool known_isfinite () const;
  bool known_isnan () const;
  bool known_isinf () const;
  bool maybe_isnan () const;
  bool maybe_isinf () const;
  bool signbit_p (bool &signbit) const;

Let me know what you think.

In this next revision, I have addressed a few of the suggestions by
Richi, though I have left the out-of-line handling of NANs for
intersect/union because I still find them more readable (for now).
This may get shuffled again if we implement frange_base and frange, so
hang on.

Also, I opted to implement ][ NAN with m_kind == VR_NAN instead of
overloading VR_UNDEFINED.  This makes the code less confusing.

I have retested on x86-64 Linux.  Let me know what y'all think.

Thanks.
Aldy
  
Aldy Hernandez Sept. 18, 2022, 7:10 a.m. UTC | #5
Pushed.  We can address any further changes as follow-ups.

Thanks.
Aldy

On Fri, Sep 16, 2022 at 3:26 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> On Fri, Sep 16, 2022 at 10:33 AM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
> >
> > Aldy Hernandez via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > > On Thu, Sep 15, 2022 at 9:06 AM Richard Biener
> > > <richard.guenther@gmail.com> wrote:
> > >>
> > >> On Thu, Sep 15, 2022 at 7:41 AM Aldy Hernandez <aldyh@redhat.com> wrote:
> > >> >
> > >> > Hi Richard.  Hi all.
> > >> >
> > >> > The attatched patch rewrites the NAN and sign handling, dropping both
> > >> > tristates in favor of a pair of boolean flags for NANs, and nothing at
> > >> > all for signs.  The signs are tracked in the range itself, so now it's
> > >> > possible to describe things like [-0.0, +0.0] +NAN, [+0, +0], [-5, +0],
> > >> > [+0, 3] -NAN, etc.
> > >> >
> > >> > There are a lot of changes, as the tristate was quite pervasive.  I
> > >> > could use another pair of eyes.  The code IMO is cleaner and handles
> > >> > all the cases we discussed.
> > >> >
> > >> > Here is an example of the various ranges and how they are displayed:
> > >> >
> > >> >     [frange] float VARYING NAN ;; Varying includes NAN
> > >> >     [frange] UNDEFINED                      ;; Empty set as always
> > >> >     [frange] float [] NAN                   ;; Unknown sign NAN
> > >> >     [frange] float [] -NAN                  ;; -NAN
> > >> >     [frange] float [] +NAN                  ;; +NAN
> > >> >     [frange] float [-0.0, 0.0]              ;; All zeros.
> > >> >     [frange] float [-0.0, -0.0] NAN         ;; -0 or NAN.
> > >> >     [frange] float [-5.0e+0, -1.0e+0] +NAN  ;; [-5, -1] or +NAN
> > >> >     [frange] float [-5.0e+0, -0.0] NAN      ;; [-5, -0] or +-NAN
> > >> >     [frange] float [-5.0e+0, -0.0]          ;; [-5, -0]
> > >> >     [frange] float [5.0e+0, 1.0e+1]         ;; [5, 10]
> > >> >
> > >> > We could represent an unknown sign with +NAN -NAN if preferred.
> > >>
> > >> maybe -+NAN or +-NAN?  I prefer to somehow show both signs for clarity
> > >
> > > Sure.
> > >
> > >>
> > >> >
> > >> > Notice the NAN signs are decoupled from the range, so we can represent
> > >> > a negative range with a positive NAN.  For this range,
> > >> > frange::known_bit() would return false, as only when the signs of the
> > >> > NANs and range agree can we be certain.
> > >> >
> > >> > There is no longer any pessimization of ranges for intersects
> > >> > involving NANs.  Also, union and intersect work with signed zeros:
> > >> >
> > >> > //   [-0,  x] U [+0,  x] => [-0,  x]
> > >> > //   [ x, -0] U [ x, +0] => [ x, +0]
> > >> > //   [-0,  x] ^ [+0,  x] => [+0,  x]
> > >> > //   [ x, -0] ^ [ x, +0] => [ x, -0]
> > >> >
> > >> > The special casing for signed zeros in the singleton code is gone in
> > >> > favor of just making sure the signs in the range agree, that is
> > >> > [-0, -0] for example.
> > >> >
> > >> > I have removed the idea that a known NAN is a "range", so a NAN is no
> > >> > longer in the endpoints itself.  Requesting the bound of a known NAN
> > >> > is a hard fail.  For that matter, we don't store the actual NAN in the
> > >> > range.  The only information we have are the set of boolean flags.
> > >> > This way we make sure nothing seeps into the frange.  This also means
> > >> > it's explicit that we don't track anything but the sign in NANs.  We
> > >> > can revisit this if we desire to track signalling or whatever
> > >> > concoction y'all can imagine.
> > >> >
> > >> > All in all, I'm quite happy with this.  It does look better, and we
> > >> > handle all the corner cases we couldn't before.  Thanks for the
> > >> > suggestion.
> > >> >
> > >> > Regstrapped with mpfr tests on x86-64 and ppc64le Linux.  Selftests
> > >> > were also run with -ffinite-math-only on x86-64.
> > >> >
> > >> > At Jakub's suggestion, I built lapack with associated tests.  They
> > >> > pass on x86-64 and ppc64le Linux with no regressions from mainline.
> > >> > As a sanity check, I also ran them for -ffinite-math-only on x86 which
> > >> > (as expected) returned:
> > >> >
> > >> >         NaN arithmetic did not perform per the ieee spec
> > >> >
> > >> > Otherwise, all tests pass for -ffinite-math-only.
> > >> >
> > >> > How does this look?
> > >>
> > >> Overall it looks good.
> > >>
> > >> Reading ::intersect and ::union I find it less clear to spread out the _nan
> > >> cases into separate functions.
> > >
> > > OK, will inline them.
> > >
> > >>
> > >> Can you add a comment to frange that its representation is
> > >> a single value-range specified by m_type, m_min, m_max
> > >> unioned with the set of { -NaN, +NaN }?  Because somehow
> > >> the ::undefined_p vs. m_type == VR_UNDEFINED checks are
> > >> a bit confusing to the occasional reader can we instead use
> > >> ::nan_p to complement ::undefined_p?
> > >
> > > Wouldn't that just make nan_p the same as known_nan?  Speaking of
> > > which, I'm not a big fan of known_nan.  Perhaps we should rename all
> > > the known_foo variants to foo_p variants?  Or...maybe even:
> > >
> > >   // fpclassify like API
> > >   bool isfinite () const;
> > >   bool isinf () const;
> > >   bool maybe_isinf () const;
> > >   bool isnan () const;
> > >   bool maybe_isnan () const;
> > >   bool signbit_p (bool &signbit) const;
> > >
> > > That would make it clear how they map to the fpclassify API.  And the
> > > signbit_p() follows what we do for singleton_p(tree *).
> > >
> > > isnan() would be your nan_p suggestion.
> >
> > FWIW, the reason I didn't do this with the poly_int stuff is that
> > it makes negative conditions harder to reason about.  It's easy for
> > tired eyes to read:
> >
> >    !isfinite()
> >
> > as meaning "is infinite", especially since there isn't a separate
> > isinfinite() query.  But if isfinite() is equivalent to known_isfinite()
> > then !isfinite() instead means "might be infinite".  !known_isfinite()
> > IMO makes that more explicit.
>
> Ughh, fair enough.  I've settled on:
>
>   bool known_isfinite () const;
>   bool known_isnan () const;
>   bool known_isinf () const;
>   bool maybe_isnan () const;
>   bool maybe_isinf () const;
>   bool signbit_p (bool &signbit) const;
>
> Let me know what you think.
>
> In this next revision, I have addressed a few of the suggestions by
> Richi, though I have left the out-of-line handling of NANs for
> intersect/union because I still find them more readable (for now).
> This may get shuffled again if we implement frange_base and frange, so
> hang on.
>
> Also, I opted to implement ][ NAN with m_kind == VR_NAN instead of
> overloading VR_UNDEFINED.  This makes the code less confusing.
>
> I have retested on x86-64 Linux.  Let me know what y'all think.
>
> Thanks.
> Aldy
  
Mikael Morin Sept. 27, 2022, 1 p.m. UTC | #6
Hello,

Le 16/09/2022 à 15:26, Aldy Hernandez via Gcc-patches a écrit :
> diff --git a/gcc/value-range.cc b/gcc/value-range.cc
> index d759fcf178c..55a216efd8b 100644
> --- a/gcc/value-range.cc
> +++ b/gcc/value-range.cc
> @@ -617,21 +602,24 @@ frange::contains_p (tree cst) const
>    if (varying_p ())
>      return true;
>  
  (...)
>  
>    if (real_compare (GE_EXPR, rv, &m_min) && real_compare (LE_EXPR, rv, &m_max))
>      {
> +      // Make sure the signs are equal for signed zeros.
>        if (HONOR_SIGNED_ZEROS (m_type) && real_iszero (rv))
> -	{
> -	  // FIXME: This is still using get_signbit() instead of
> -	  // known_signbit() because the latter bails on possible NANs
> -	  // (for now).
> -	  if (get_signbit ().yes_p ())
> -	    return real_isneg (rv);
> -	  else if (get_signbit ().no_p ())
> -	    return !real_isneg (rv);
> -	  else
> -	    return true;
> -	}
> +	return m_min.sign == m_max.sign && m_min.sign == rv->sign;
>        return true;
>      }
>    return false;

It seems that this won't report any range with mismatching bound signs 
as containing zero.
Maybe a selftest explains it better: the following fails.

diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 9ca442478c9..8fc909171bc 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -3780,6 +3780,14 @@ range_tests_signed_zeros ()
    ASSERT_TRUE (r0.contains_p (neg_zero));
    ASSERT_FALSE (r0.contains_p (zero));

+  r0 = frange_float ("-3", "5");
+  ASSERT_TRUE (r0.contains_p (neg_zero));
+  ASSERT_TRUE (r0.contains_p (zero));
+
+  r0 = frange (neg_zero, zero);
+  ASSERT_TRUE (r0.contains_p (neg_zero));
+  ASSERT_TRUE (r0.contains_p (zero));
+
    // The intersection of zeros that differ in sign is a NAN (or
    // undefined if not honoring NANs).
    r0 = frange (neg_zero, neg_zero);
  
Aldy Hernandez Nov. 2, 2022, 1:35 p.m. UTC | #7
On 9/27/22 15:00, Mikael Morin wrote:
> Hello,
> 
> Le 16/09/2022 à 15:26, Aldy Hernandez via Gcc-patches a écrit :
>> diff --git a/gcc/value-range.cc b/gcc/value-range.cc
>> index d759fcf178c..55a216efd8b 100644
>> --- a/gcc/value-range.cc
>> +++ b/gcc/value-range.cc
>> @@ -617,21 +602,24 @@ frange::contains_p (tree cst) const
>>    if (varying_p ())
>>      return true;
>>
>   (...)
>>
>>    if (real_compare (GE_EXPR, rv, &m_min) && real_compare (LE_EXPR, 
>> rv, &m_max))
>>      {
>> +      // Make sure the signs are equal for signed zeros.
>>        if (HONOR_SIGNED_ZEROS (m_type) && real_iszero (rv))
>> -    {
>> -      // FIXME: This is still using get_signbit() instead of
>> -      // known_signbit() because the latter bails on possible NANs
>> -      // (for now).
>> -      if (get_signbit ().yes_p ())
>> -        return real_isneg (rv);
>> -      else if (get_signbit ().no_p ())
>> -        return !real_isneg (rv);
>> -      else
>> -        return true;
>> -    }
>> +    return m_min.sign == m_max.sign && m_min.sign == rv->sign;
>>        return true;
>>      }
>>    return false;
> 
> It seems that this won't report any range with mismatching bound signs 
> as containing zero.
> Maybe a selftest explains it better: the following fails.

My apologies for only seeing this now.  You did not CC me in the 
response, and it got lost amongst my other list mail.

You are absolutely right.

The attached patch fixes this problem.  It has been tested on x86-64 
Linux and pushed.

Thanks for pointing this out.
Aldy

> 
> diff --git a/gcc/value-range.cc b/gcc/value-range.cc
> index 9ca442478c9..8fc909171bc 100644
> --- a/gcc/value-range.cc
> +++ b/gcc/value-range.cc
> @@ -3780,6 +3780,14 @@ range_tests_signed_zeros ()
>     ASSERT_TRUE (r0.contains_p (neg_zero));
>     ASSERT_FALSE (r0.contains_p (zero));
> 
> +  r0 = frange_float ("-3", "5");
> +  ASSERT_TRUE (r0.contains_p (neg_zero));
> +  ASSERT_TRUE (r0.contains_p (zero));
> +
> +  r0 = frange (neg_zero, zero);
> +  ASSERT_TRUE (r0.contains_p (neg_zero));
> +  ASSERT_TRUE (r0.contains_p (zero));
> +
>     // The intersection of zeros that differ in sign is a NAN (or
>     // undefined if not honoring NANs).
>     r0 = frange (neg_zero, neg_zero);
>
  

Patch

diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index fbc14a730ad..270490010e2 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -218,7 +218,11 @@  frange_add_zeros (frange &r, tree type)
 
   if (HONOR_SIGNED_ZEROS (type)
       && (real_iszero (&r.lower_bound ()) || real_iszero (&r.upper_bound ())))
-    r.set_signbit (fp_prop::VARYING);
+    {
+      frange zero;
+      zero.set_zero (type);
+      r.union_ (zero);
+    }
 }
 
 // Build a range that is <= VAL and store it in R.
diff --git a/gcc/value-query.cc b/gcc/value-query.cc
index ea6e4b979ad..0bdd670982b 100644
--- a/gcc/value-query.cc
+++ b/gcc/value-query.cc
@@ -219,17 +219,8 @@  range_query::get_tree_range (vrange &r, tree expr, gimple *stmt)
       {
 	frange &f = as_a <frange> (r);
 	f.set (expr, expr);
-
-	// Singletons from the tree world have known properties.
-	REAL_VALUE_TYPE *rv = TREE_REAL_CST_PTR (expr);
-	if (real_isnan (rv))
-	  f.update_nan (fp_prop::YES);
-	else
+	if (!real_isnan (TREE_REAL_CST_PTR (expr)))
 	  f.clear_nan ();
-	if (real_isneg (rv))
-	  f.set_signbit (fp_prop::YES);
-	else
-	  f.set_signbit (fp_prop::NO);
 	return true;
       }
 
diff --git a/gcc/value-range-pretty-print.cc b/gcc/value-range-pretty-print.cc
index b124e46cb9e..49b16d6a5b1 100644
--- a/gcc/value-range-pretty-print.cc
+++ b/gcc/value-range-pretty-print.cc
@@ -134,34 +134,39 @@  vrange_printer::visit (const frange &r) const
   if (r.varying_p ())
     {
       pp_string (pp, "VARYING");
+      print_frange_nan (r);
       return;
     }
   pp_character (pp, '[');
-  dump_generic_node (pp,
-		     build_real (type, r.lower_bound ()), 0, TDF_NONE, false);
-  pp_string (pp, ", ");
-  dump_generic_node (pp,
-		     build_real (type, r.upper_bound ()), 0, TDF_NONE, false);
-  pp_string (pp, "] ");
-
-  print_frange_prop ("NAN", r.get_nan ());
-  print_frange_prop ("SIGN", r.get_signbit ());
+  bool has_endpoints = !r.known_nan ();
+  if (has_endpoints)
+    {
+      dump_generic_node (pp,
+			 build_real (type, r.lower_bound ()), 0, TDF_NONE, false);
+      pp_string (pp, ", ");
+      dump_generic_node (pp,
+			 build_real (type, r.upper_bound ()), 0, TDF_NONE, false);
+    }
+  pp_character (pp, ']');
+  print_frange_nan (r);
 }
 
-// Print the FP properties in an frange.
+// Print the NAN info for an frange.
 
 void
-vrange_printer::print_frange_prop (const char *str, const fp_prop &prop) const
+vrange_printer::print_frange_nan (const frange &r) const
 {
-  if (prop.varying_p ())
-    return;
-
-  if (prop.yes_p ())
-    pp_string (pp, str);
-  else if (prop.no_p ())
+  if (r.maybe_nan ())
     {
-      pp_character (pp, '!');
-      pp_string (pp, str);
+      if (r.m_pos_nan && r.m_neg_nan)
+	{
+	  pp_string (pp, " NAN");
+	  return;
+	}
+      bool nan_sign = r.m_neg_nan;
+      if (nan_sign)
+	pp_string (pp, " -NAN");
+      else
+	pp_string (pp, " +NAN");
     }
-  pp_character (pp, ' ');
 }
diff --git a/gcc/value-range-pretty-print.h b/gcc/value-range-pretty-print.h
index ad06c93c044..20c26598fe7 100644
--- a/gcc/value-range-pretty-print.h
+++ b/gcc/value-range-pretty-print.h
@@ -31,7 +31,7 @@  public:
 private:
   void print_irange_bound (const wide_int &w, tree type) const;
   void print_irange_bitmasks (const irange &) const;
-  void print_frange_prop (const char *str, const fp_prop &) const;
+  void print_frange_nan (const frange &) const;
 
   pretty_printer *pp;
 };
diff --git a/gcc/value-range-storage.cc b/gcc/value-range-storage.cc
index b7a23fa9825..de7575ed48d 100644
--- a/gcc/value-range-storage.cc
+++ b/gcc/value-range-storage.cc
@@ -253,9 +253,11 @@  frange_storage_slot::set_frange (const frange &r)
   gcc_checking_assert (fits_p (r));
   gcc_checking_assert (!r.undefined_p ());
 
+  m_kind = r.m_kind;
   m_min = r.m_min;
   m_max = r.m_max;
-  m_props = r.m_props;
+  m_pos_nan = r.m_pos_nan;
+  m_neg_nan = r.m_neg_nan;
 }
 
 void
@@ -264,11 +266,12 @@  frange_storage_slot::get_frange (frange &r, tree type) const
   gcc_checking_assert (r.supports_type_p (type));
 
   r.set_undefined ();
-  r.m_kind = VR_RANGE;
-  r.m_props = m_props;
+  r.m_kind = m_kind;
   r.m_type = type;
   r.m_min = m_min;
   r.m_max = m_max;
+  r.m_pos_nan = m_pos_nan;
+  r.m_neg_nan = m_neg_nan;
   r.normalize_kind ();
 
   if (flag_checking)
diff --git a/gcc/value-range-storage.h b/gcc/value-range-storage.h
index f506789f3d1..0cf95ebf7c1 100644
--- a/gcc/value-range-storage.h
+++ b/gcc/value-range-storage.h
@@ -113,12 +113,11 @@  class GTY (()) frange_storage_slot
   frange_storage_slot (const frange &r) { set_frange (r); }
   DISABLE_COPY_AND_ASSIGN (frange_storage_slot);
 
-  // We can get away with just storing the properties and the
-  // endpoints because the type can be gotten from the SSA, and
-  // UNDEFINED is unsupported, so it can only be a VR_RANGE.
+  enum value_range_kind m_kind;
   REAL_VALUE_TYPE m_min;
   REAL_VALUE_TYPE m_max;
-  frange_props m_props;
+  bool m_pos_nan;
+  bool m_neg_nan;
 };
 
 class obstack_vrange_allocator final: public vrange_allocator
diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index d759fcf178c..1c6061649b5 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -267,91 +267,6 @@  tree_compare (tree_code code, tree op1, tree op2)
   return !integer_zerop (fold_build2 (code, integer_type_node, op1, op2));
 }
 
-// Set the NAN property.  Adjust the range if appopriate.
-
-void
-frange::update_nan (fp_prop::kind k)
-{
-  if (k == fp_prop::YES)
-    {
-      if (!maybe_nan ())
-	{
-	  set_undefined ();
-	  return;
-	}
-      gcc_checking_assert (!undefined_p ());
-      set_nan (m_type);
-      return;
-    }
-
-  if (k == fp_prop::NO && known_nan ())
-    {
-      set_undefined ();
-      return;
-    }
-
-  // Setting VARYING on an obviously NAN range is a no-op.
-  if (k == fp_prop::VARYING && real_isnan (&m_min))
-    return;
-
-  m_props.set_nan (k);
-  normalize_kind ();
-  if (flag_checking)
-    verify_range ();
-}
-
-// Set the SIGNBIT property.  Adjust the range if appropriate.
-
-void
-frange::set_signbit (fp_prop::kind k)
-{
-  gcc_checking_assert (m_type);
-
-  // No additional adjustments are needed for a NAN.
-  if (known_nan ())
-    {
-      m_props.set_signbit (k);
-      return;
-    }
-  // Ignore sign changes when they're set correctly.
-  if (!maybe_nan ())
-    {
-      // It's negative and we're trying to make it negative or varying.
-      if (real_less (&m_max, &dconst0) && (k == fp_prop::YES
-					   || k == fp_prop::VARYING))
-	return;
-      // It's positive and we're trying to make it positive or varying.
-      if (real_less (&dconst0, &m_min) && (k == fp_prop::NO
-					   || k == fp_prop::VARYING))
-	return;
-    }
-  // Adjust the range depending on the sign bit.
-  if (k == fp_prop::YES)
-    {
-      // Crop the range to [-INF, 0].
-      frange crop (m_type, dconstninf, dconst0);
-      intersect (crop);
-      if (!undefined_p ())
-	m_props.set_signbit (fp_prop::YES);
-    }
-  else if (k == fp_prop::NO)
-    {
-      // Crop the range to [0, +INF].
-      frange crop (m_type, dconst0, dconstinf);
-      intersect (crop);
-      if (!undefined_p ())
-	m_props.set_signbit (fp_prop::NO);
-    }
-  else
-    {
-      m_props.set_signbit (fp_prop::VARYING);
-      normalize_kind ();
-    }
-
-  if (flag_checking)
-    verify_range ();
-}
-
 // Setter for franges.
 
 void
@@ -375,24 +290,23 @@  frange::set (tree min, tree max, value_range_kind kind)
       gcc_checking_assert (real_identical (TREE_REAL_CST_PTR (min),
 					   TREE_REAL_CST_PTR (max)));
       tree type = TREE_TYPE (min);
-      set_nan (type);
+      bool sign = real_isneg (TREE_REAL_CST_PTR (min));
+      set_nan (type, sign);
       return;
     }
 
   m_kind = kind;
   m_type = TREE_TYPE (min);
-  m_props.set_varying ();
   m_min = *TREE_REAL_CST_PTR (min);
   m_max = *TREE_REAL_CST_PTR (max);
-
-  // Set SIGNBIT property for positive and negative ranges.
-  if (real_less (&m_max, &dconst0))
-    m_props.signbit_set_yes ();
-  else if (real_less (&dconst0, &m_min))
-    m_props.signbit_set_no ();
+  m_pos_nan = true;
+  m_neg_nan = true;
 
   if (!HONOR_NANS (m_type))
-    m_props.nan_set_no ();
+    {
+      m_pos_nan = false;
+      m_neg_nan = false;
+    }
 
   // Check for swapped ranges.
   gcc_checking_assert (tree_compare (LE_EXPR, min, max));
@@ -423,18 +337,11 @@  frange::set (tree type,
 bool
 frange::normalize_kind ()
 {
-  // Undefined is viral.
-  if (m_props.nan_undefined_p () || m_props.signbit_undefined_p ())
-    {
-      set_undefined ();
-      return true;
-    }
   if (m_kind == VR_RANGE
       && real_isinf (&m_min, 1)
       && real_isinf (&m_max, 0))
     {
-      // No FP properties set means varying.
-      if (m_props.varying_p ())
+      if (m_pos_nan && m_neg_nan)
 	{
 	  set_varying (m_type);
 	  return true;
@@ -442,8 +349,7 @@  frange::normalize_kind ()
     }
   else if (m_kind == VR_VARYING)
     {
-      // If a VARYING has any FP properties, it's no longer VARYING.
-      if (!m_props.varying_p ())
+      if (!m_pos_nan || !m_neg_nan)
 	{
 	  m_kind = VR_RANGE;
 	  m_min = dconstninf;
@@ -454,6 +360,60 @@  frange::normalize_kind ()
   return false;
 }
 
+// Union or intersect the zero endpoints of two ranges.  For example:
+//   [-0,  x] U [+0,  x] => [-0,  x]
+//   [ x, -0] U [ x, +0] => [ x, +0]
+//   [-0,  x] ^ [+0,  x] => [+0,  x]
+//   [ x, -0] ^ [ x, +0] => [ x, -0]
+//
+// UNION_P is true when performing a union, or false when intersecting.
+
+bool
+frange::combine_zeros (const frange &r, bool union_p)
+{
+  bool changed = false;
+  if (real_iszero (&m_min) && real_iszero (&r.m_min)
+      && real_isneg (&m_min) != real_isneg (&r.m_min))
+    {
+      m_min.sign = union_p;
+      changed = true;
+    }
+  if (real_iszero (&m_max) && real_iszero (&r.m_max)
+      && real_isneg (&m_max) != real_isneg (&r.m_max))
+    {
+      m_max.sign = !union_p;
+      changed = true;
+    }
+  // If the signs are swapped, the resulting range is empty.
+  if (m_min.sign == 0 && m_max.sign == 1)
+    {
+      m_kind = VR_UNDEFINED;
+      changed = true;
+    }
+  return changed;
+}
+
+// Union two ranges when one is known to be a NAN.
+
+bool
+frange::union_nans (const frange &r)
+{
+  gcc_checking_assert (known_nan () || r.known_nan ());
+
+  if (known_nan ())
+    {
+      m_kind = r.m_kind;
+      m_min = r.m_min;
+      m_max = r.m_max;
+    }
+  m_pos_nan |= r.m_pos_nan;
+  m_neg_nan |= r.m_neg_nan;
+  normalize_kind ();
+  if (flag_checking)
+    verify_range ();
+  return true;
+}
+
 bool
 frange::union_ (const vrange &v)
 {
@@ -467,29 +427,18 @@  frange::union_ (const vrange &v)
       return true;
     }
 
-  // If one side has a NAN, the union is the other side, plus the union
-  // of the properties and the possibility of a NAN.
-  if (known_nan ())
-    {
-      frange_props save = m_props;
-      *this = r;
-      m_props = save;
-      m_props.union_ (r.m_props);
-      update_nan (fp_prop::VARYING);
-      if (flag_checking)
-	verify_range ();
-      return true;
-    }
-  if (r.known_nan ())
+  // Combine NAN info.
+  if (known_nan () || r.known_nan ())
+    return union_nans (r);
+  bool changed = false;
+  if (m_pos_nan != r.m_pos_nan || m_neg_nan != r.m_neg_nan)
     {
-      m_props.union_ (r.m_props);
-      update_nan (fp_prop::VARYING);
-      if (flag_checking)
-	verify_range ();
-      return true;
+      m_pos_nan |= r.m_pos_nan;
+      m_neg_nan |= r.m_neg_nan;
+      changed = true;
     }
 
-  bool changed = m_props.union_ (r.m_props);
+  // Combine endpoints.
   if (real_less (&r.m_min, &m_min))
     {
       m_min = r.m_min;
@@ -500,13 +449,38 @@  frange::union_ (const vrange &v)
       m_max = r.m_max;
       changed = true;
     }
-  changed |= normalize_kind ();
 
+  if (HONOR_SIGNED_ZEROS (m_type))
+    changed |= combine_zeros (r, true);
+
+  changed |= normalize_kind ();
   if (flag_checking)
     verify_range ();
   return changed;
 }
 
+// Intersect two ranges when one is known to be a NAN.
+
+bool
+frange::intersect_nans (const frange &r)
+{
+  gcc_checking_assert (known_nan () || r.known_nan ());
+
+  m_kind = VR_UNDEFINED;
+  m_pos_nan &= r.m_pos_nan;
+  m_neg_nan &= r.m_neg_nan;
+  if (!maybe_nan ())
+    {
+      // If the NAN was intersected out, the resulting range is empty.
+      set_undefined ();
+      return true;
+    }
+  normalize_kind ();
+  if (flag_checking)
+    verify_range ();
+  return true;
+}
+
 bool
 frange::intersect (const vrange &v)
 {
@@ -525,25 +499,18 @@  frange::intersect (const vrange &v)
       return true;
     }
 
-  // If two NANs are not exactly the same, drop to an unknown NAN,
-  // otherwise there's nothing to do.
-  if (known_nan () && r.known_nan ())
-    {
-      if (m_props == r.m_props)
-	return false;
-
-      set_nan (m_type);
-      return true;
-    }
-  // ?? Perhaps the intersection of a NAN and anything is a NAN ??.
+  // Combine NAN info.
   if (known_nan () || r.known_nan ())
+    return intersect_nans (r);
+  bool changed = false;
+  if (m_pos_nan != r.m_pos_nan || m_neg_nan != r.m_neg_nan)
     {
-      set_varying (m_type);
-      return true;
+      m_pos_nan &= r.m_pos_nan;
+      m_neg_nan &= r.m_neg_nan;
+      changed = true;
     }
 
-  bool changed = m_props.intersect (r.m_props);
-
+  // Combine endpoints.
   if (real_less (&m_min, &r.m_min))
     {
       m_min = r.m_min;
@@ -554,14 +521,25 @@  frange::intersect (const vrange &v)
       m_max = r.m_max;
       changed = true;
     }
-  // If the endpoints are swapped, the ranges are disjoint.
   if (real_less (&m_max, &m_min))
     {
+      // If the endpoints are swapped, the resulting range is empty.
+      if (maybe_nan ())
+	{
+	  // An empty range with a NAN is just a NAN.
+	  m_kind = VR_UNDEFINED;
+	  if (flag_checking)
+	    verify_range ();
+	  return true;
+	}
       set_undefined ();
       return true;
     }
-  changed |= normalize_kind ();
 
+  if (HONOR_SIGNED_ZEROS (m_type))
+    changed |= combine_zeros (r, false);
+
+  changed |= normalize_kind ();
   if (flag_checking)
     verify_range ();
   return changed;
@@ -574,7 +552,8 @@  frange::operator= (const frange &src)
   m_type = src.m_type;
   m_min = src.m_min;
   m_max = src.m_max;
-  m_props = src.m_props;
+  m_pos_nan = src.m_pos_nan;
+  m_neg_nan = src.m_neg_nan;
 
   if (flag_checking)
     verify_range ();
@@ -597,7 +576,8 @@  frange::operator== (const frange &src) const
 
       return (real_identical (&m_min, &src.m_min)
 	      && real_identical (&m_max, &src.m_max)
-	      && m_props == src.m_props
+	      && m_pos_nan == src.m_pos_nan
+	      && m_neg_nan == src.m_neg_nan
 	      && types_compatible_p (m_type, src.m_type));
     }
   return false;
@@ -617,21 +597,24 @@  frange::contains_p (tree cst) const
   if (varying_p ())
     return true;
 
+  if (real_isnan (rv))
+    {
+      // No NAN in range.
+      if (!m_pos_nan && !m_neg_nan)
+	return false;
+      // Both +NAN and -NAN are present.
+      if (m_pos_nan && m_neg_nan)
+	return true;
+      return m_neg_nan == rv->sign;
+    }
+  if (known_nan ())
+    return false;
 
   if (real_compare (GE_EXPR, rv, &m_min) && real_compare (LE_EXPR, rv, &m_max))
     {
+      // Make sure the signs are equal for signed zeros.
       if (HONOR_SIGNED_ZEROS (m_type) && real_iszero (rv))
-	{
-	  // FIXME: This is still using get_signbit() instead of
-	  // known_signbit() because the latter bails on possible NANs
-	  // (for now).
-	  if (get_signbit ().yes_p ())
-	    return real_isneg (rv);
-	  else if (get_signbit ().no_p ())
-	    return !real_isneg (rv);
-	  else
-	    return true;
-	}
+	return m_min.sign == m_max.sign && m_min.sign == rv->sign;
       return true;
     }
   return false;
@@ -651,26 +634,6 @@  frange::singleton_p (tree *result) const
       if (HONOR_NANS (m_type) && maybe_nan ())
 	return false;
 
-      // Return the appropriate zero if known.
-      if (HONOR_SIGNED_ZEROS (m_type) && zero_p ())
-	{
-	  bool signbit;
-	  if (known_signbit (signbit))
-	    {
-	      if (signbit)
-		{
-		  if (result)
-		    *result = build_real (m_type, real_value_negate (&dconst0));
-		}
-	      else
-		{
-		  if (result)
-		    *result = build_real (m_type, dconst0);
-		}
-	      return true;
-	    }
-	  return false;
-	}
       if (result)
 	*result = build_real (m_type, m_min);
       return true;
@@ -687,57 +650,31 @@  frange::supports_type_p (const_tree type) const
 void
 frange::verify_range ()
 {
-  if (undefined_p ())
-    {
-      gcc_checking_assert (m_props.undefined_p ());
-      return;
-    }
-  gcc_checking_assert (!m_props.undefined_p ());
-
+  if (m_kind == VR_UNDEFINED)
+    return;
   if (varying_p ())
     {
-      gcc_checking_assert (m_props.varying_p ());
+      gcc_checking_assert (m_pos_nan && m_neg_nan);
+      gcc_checking_assert (real_isinf (&m_min, 1));
+      gcc_checking_assert (real_isinf (&m_max, 0));
       return;
     }
 
+  // NANs cannot appear in the endpoints of a range.
+  gcc_checking_assert (!real_isnan (&m_min) && !real_isnan (&m_max));
+
   // We don't support the inverse of an frange (yet).
   gcc_checking_assert (m_kind == VR_RANGE);
 
-  bool is_nan = real_isnan (&m_min) || real_isnan (&m_max);
-  if (is_nan)
-    {
-      // If either is a NAN, both must be a NAN.
-      gcc_checking_assert (real_identical (&m_min, &m_max));
-      gcc_checking_assert (known_nan ());
-    }
-  else
-    // Make sure we don't have swapped ranges.
-    gcc_checking_assert (!real_less (&m_max, &m_min));
+  // Make sure we don't have swapped ranges.
+  gcc_checking_assert (!real_less (&m_max, &m_min));
 
-  // If we're absolutely sure we have a NAN, the endpoints should
-  // reflect this, otherwise we'd have more than one way to represent
-  // a NAN.
-  if (known_nan ())
-    {
-      gcc_checking_assert (real_isnan (&m_min));
-      gcc_checking_assert (real_isnan (&m_max));
-    }
-  else
-    {
-      // Make sure the signbit and range agree.
-      bool signbit;
-      if (known_signbit (signbit))
-	{
-	  if (signbit)
-	    gcc_checking_assert (real_compare (LE_EXPR, &m_max, &dconst0));
-	  else
-	    gcc_checking_assert (real_compare (GE_EXPR, &m_min, &dconst0));
-	}
-    }
+  // [ +0.0, -0.0 ] is nonsensical.
+  gcc_checking_assert (!(real_iszero (&m_min, 0) && real_iszero (&m_max, 1)));
 
   // If all the properties are clear, we better not span the entire
   // domain, because that would make us varying.
-  if (m_props.varying_p ())
+  if (m_pos_nan && m_neg_nan)
     gcc_checking_assert (!real_isinf (&m_min, 1) || !real_isinf (&m_max, 0));
 }
 
@@ -755,16 +692,24 @@  frange::nonzero_p () const
   return false;
 }
 
-// Set range to [+0.0, +0.0].
+// Set range to [+0.0, +0.0] if honoring signed zeros, or [0.0, 0.0]
+// otherwise.
 
 void
 frange::set_zero (tree type)
 {
-  tree zero = build_zero_cst (type);
-  set (zero, zero);
+  if (HONOR_SIGNED_ZEROS (type))
+    {
+      REAL_VALUE_TYPE dconstm0 = dconst0;
+      dconstm0.sign = 1;
+      set (type, dconstm0, dconst0);
+      clear_nan ();
+    }
+  else
+    set (type, dconst0, dconst0);
 }
 
-// Return TRUE for any [0.0, 0.0] regardless of sign.
+// Return TRUE for any zero regardless of sign.
 
 bool
 frange::zero_p () const
@@ -777,9 +722,7 @@  frange::zero_p () const
 void
 frange::set_nonnegative (tree type)
 {
-  tree zero = build_zero_cst (type);
-  tree inf = vrp_val_max (type);
-  set (zero, inf);
+  set (type, dconst0, dconstinf);
 }
 
 // Here we copy between any two irange's.  The ranges can be legacy or
@@ -3637,8 +3580,21 @@  range_tests_nan ()
       ASSERT_EQ (r0, r1);
       r0.clear_nan ();
       ASSERT_NE (r0, r1);
+      r0.update_nan ();
+      ASSERT_EQ (r0, r1);
+
+      // [10, 20] NAN ^ [30, 40] NAN = NAN.
+      r0 = frange_float ("10", "20");
+      r1 = frange_float ("30", "40");
+      r0.intersect (r1);
+      ASSERT_TRUE (r0.known_nan ());
+
+      // [3,5] U [5,10] NAN = ... NAN
+      r0 = frange_float ("3", "5");
       r0.clear_nan ();
-      ASSERT_NE (r0, r1);
+      r1 = frange_float ("5", "10");
+      r0.union_ (r1);
+      ASSERT_TRUE (r0.maybe_nan ());
     }
 
   // NAN ranges are not equal to each other.
@@ -3663,15 +3619,15 @@  range_tests_nan ()
   r0.set_nan (float_type_node);
   r1.set_nan (float_type_node);
   r0.union_ (r1);
-  ASSERT_TRUE (real_isnan (&r0.lower_bound ()));
-  ASSERT_TRUE (real_isnan (&r1.upper_bound ()));
   ASSERT_TRUE (r0.known_nan ());
 
-  // [INF, INF] ^ NAN = VARYING
+  // [INF, INF] NAN ^ NAN = NAN
   r0.set_nan (float_type_node);
   r1 = frange_float ("+Inf", "+Inf");
+  if (!HONOR_NANS (float_type_node))
+    r1.update_nan ();
   r0.intersect (r1);
-  ASSERT_TRUE (r0.varying_p ());
+  ASSERT_TRUE (r0.known_nan ());
 
   // NAN ^ NAN = NAN
   r0.set_nan (float_type_node);
@@ -3679,18 +3635,48 @@  range_tests_nan ()
   r0.intersect (r1);
   ASSERT_TRUE (r0.known_nan ());
 
+  // +NAN ^ -NAN = UNDEFINED
+  r0.set_nan (float_type_node, false);
+  r1.set_nan (float_type_node, true);
+  r0.intersect (r1);
+  ASSERT_TRUE (r0.undefined_p ());
+
   // VARYING ^ NAN = NAN.
   r0.set_nan (float_type_node);
   r1.set_varying (float_type_node);
   r0.intersect (r1);
   ASSERT_TRUE (r0.known_nan ());
 
-  // Setting the NAN bit to yes, forces to range to [NAN, NAN].
+  // [3,4] ^ NAN = UNDEFINED.
+  r0 = frange_float ("3", "4");
+  r0.clear_nan ();
+  r1.set_nan (float_type_node);
+  r0.intersect (r1);
+  ASSERT_TRUE (r0.undefined_p ());
+
+  // [-3, 5] ^ NAN = UNDEFINED
+  r0 = frange_float ("-3", "5");
+  r0.clear_nan ();
+  r1.set_nan (float_type_node);
+  r0.intersect (r1);
+  ASSERT_TRUE (r0.undefined_p ());
+
+  // Setting the NAN bit to yes does not make us a known NAN.
   r0.set_varying (float_type_node);
-  r0.update_nan (fp_prop::YES);
-  ASSERT_TRUE (r0.known_nan ());
-  ASSERT_TRUE (real_isnan (&r0.lower_bound ()));
-  ASSERT_TRUE (real_isnan (&r0.upper_bound ()));
+  r0.update_nan ();
+  ASSERT_FALSE (r0.known_nan ());
+
+  // NAN is in a VARYING.
+  r0.set_varying (float_type_node);
+  real_nan (&r, "", 1, TYPE_MODE (float_type_node));
+  tree nan = build_real (float_type_node, r);
+  ASSERT_TRUE (r0.contains_p (nan));
+
+  // -NAN is in a VARYING.
+  r0.set_varying (float_type_node);
+  q = real_value_negate (&r);
+  tree neg_nan = build_real (float_type_node, q);
+  ASSERT_TRUE (r0.contains_p (neg_nan));
 }
 
 static void
@@ -3702,49 +3688,84 @@  range_tests_signed_zeros ()
   frange r0, r1;
   bool signbit;
 
-  // Since -0.0 == +0.0, a range of [-0.0, -0.0] should contain +0.0
-  // and vice versa.
+  // [0,0] contains [0,0] but not [-0,-0] and vice versa.
   r0 = frange (zero, zero);
   r1 = frange (neg_zero, neg_zero);
   ASSERT_TRUE (r0.contains_p (zero));
-  ASSERT_TRUE (r0.contains_p (neg_zero));
-  ASSERT_TRUE (r1.contains_p (zero));
+  ASSERT_TRUE (!r0.contains_p (neg_zero));
   ASSERT_TRUE (r1.contains_p (neg_zero));
+  ASSERT_TRUE (!r1.contains_p (zero));
 
   // Test contains_p() when we know the sign of the zero.
-  r0 = frange(zero, zero);
-  r0.set_signbit (fp_prop::NO);
+  r0 = frange (zero, zero);
   ASSERT_TRUE (r0.contains_p (zero));
   ASSERT_FALSE (r0.contains_p (neg_zero));
-  r0.set_signbit (fp_prop::YES);
+  r0 = frange (neg_zero, neg_zero);
   ASSERT_TRUE (r0.contains_p (neg_zero));
   ASSERT_FALSE (r0.contains_p (zero));
 
-  // The intersection of zeros that differ in sign is the empty set.
-  r0 = frange (zero, zero);
-  r0.set_signbit (fp_prop::YES);
+  // The intersection of zeros that differ in sign is a NAN (or
+  // undefined if not honoring NANs).
+  r0 = frange (neg_zero, neg_zero);
   r1 = frange (zero, zero);
-  r1.set_signbit (fp_prop::NO);
   r0.intersect (r1);
-  ASSERT_TRUE (r0.undefined_p ());
+  if (HONOR_NANS (float_type_node))
+    ASSERT_TRUE (r0.known_nan ());
+  else
+    ASSERT_TRUE (r0.undefined_p ());
 
   // The union of zeros that differ in sign is a zero with unknown sign.
   r0 = frange (zero, zero);
-  r0.set_signbit (fp_prop::NO);
-  r1 = frange (zero, zero);
-  r1.set_signbit (fp_prop::YES);
+  r1 = frange (neg_zero, neg_zero);
   r0.union_ (r1);
   ASSERT_TRUE (r0.zero_p () && !r0.known_signbit (signbit));
 
-  // NAN U [5,6] should be [5,6] with no sign info.
+  // [-0, +0] has an unknown sign.
+  r0 = frange (neg_zero, zero);
+  ASSERT_TRUE (r0.zero_p () && !r0.known_signbit (signbit));
+
+  // [-0, +0] ^ [0, 0] is [0, 0]
+  r0 = frange (neg_zero, zero);
+  r1 = frange (zero, zero);
+  r0.intersect (r1);
+  ASSERT_TRUE (r0.zero_p ());
+
+  // NAN U [5,6] should be [5,6] NAN.
   r0.set_nan (float_type_node);
   r1 = frange_float ("5", "6");
+  r1.clear_nan ();
   r0.union_ (r1);
   real_from_string (&q, "5");
   real_from_string (&r, "6");
   ASSERT_TRUE (real_identical (&q, &r0.lower_bound ()));
   ASSERT_TRUE (real_identical (&r, &r0.upper_bound ()));
   ASSERT_TRUE (!r0.known_signbit (signbit));
+  ASSERT_TRUE (r0.maybe_nan ());
+
+  r0 = frange_float ("+0", "5");
+  r0.clear_nan ();
+  ASSERT_TRUE (r0.known_signbit (signbit) && !signbit);
+
+  r0 = frange_float ("-0", "5");
+  r0.clear_nan ();
+  ASSERT_TRUE (!r0.known_signbit (signbit));
+
+  r0 = frange_float ("-0", "10");
+  r1 = frange_float ("0", "5");
+  r0.intersect (r1);
+  ASSERT_TRUE (real_iszero (&r0.lower_bound (), false));
+
+  r0 = frange_float ("-0", "5");
+  r1 = frange_float ("0", "5");
+  r0.union_ (r1);
+  ASSERT_TRUE (real_iszero (&r0.lower_bound (), true));
+
+  r0 = frange_float ("-5", "-0");
+  r0.update_nan ();
+  r1 = frange_float ("0", "0");
+  r1.update_nan ();
+  r0.intersect (r1);
+  ASSERT_TRUE (r0.known_nan ());
 }
 
 static void
@@ -3753,22 +3774,6 @@  range_tests_signbit ()
   frange r0, r1;
   bool signbit;
 
-  // Setting the signbit drops the range to [-INF, 0].
-  r0.set_varying (float_type_node);
-  r0.set_signbit (fp_prop::YES);
-  ASSERT_TRUE (real_isinf (&r0.lower_bound (), 1));
-  ASSERT_TRUE (real_iszero (&r0.upper_bound ()));
-
-  // Setting the signbit for [-5, 10] crops the range to [-5, 0] with
-  // the signbit property set.
-  r0 = frange_float ("-5", "10");
-  r0.set_signbit (fp_prop::YES);
-  r0.clear_nan ();
-  ASSERT_TRUE (r0.known_signbit (signbit) && signbit);
-  r1 = frange_float ("-5", "0");
-  ASSERT_TRUE (real_identical (&r0.lower_bound (), &r1.lower_bound ()));
-  ASSERT_TRUE (real_identical (&r0.upper_bound (), &r1.upper_bound ()));
-
   // Negative numbers should have the SIGNBIT set.
   r0 = frange_float ("-5", "-1");
   r0.clear_nan ();
@@ -3780,7 +3785,7 @@  range_tests_signbit ()
   // Numbers containing zero should have an unknown SIGNBIT.
   r0 = frange_float ("0", "10");
   r0.clear_nan ();
-  ASSERT_TRUE (!r0.known_signbit (signbit));
+  ASSERT_TRUE (r0.known_signbit (signbit) && !signbit);
   // Numbers spanning both positive and negative should have an
   // unknown SIGNBIT.
   r0 = frange_float ("-10", "10");
@@ -3788,17 +3793,6 @@  range_tests_signbit ()
   ASSERT_TRUE (!r0.known_signbit (signbit));
   r0.set_varying (float_type_node);
   ASSERT_TRUE (!r0.known_signbit (signbit));
-
-  // Ignore signbit changes when the sign bit is obviously known from
-  // the range.
-  r0 = frange_float ("5", "10");
-  r0.clear_nan ();
-  r0.set_signbit (fp_prop::VARYING);
-  ASSERT_TRUE (r0.known_signbit (signbit) && !signbit);
-  r0 = frange_float ("-5", "-1");
-  r0.set_signbit (fp_prop::NO);
-  r0.clear_nan ();
-  ASSERT_TRUE (r0.undefined_p ());
 }
 
 static void
@@ -3896,9 +3890,19 @@  range_tests_floats ()
   r0.intersect (r1);
   ASSERT_EQ (r0, frange_float ("15", "20"));
 
+  // [10,20] NAN ^ [21,25] NAN = [NAN]
+  r0 = frange_float ("10", "20");
+  r0.update_nan ();
+  r1 = frange_float ("21", "25");
+  r1.update_nan ();
+  r0.intersect (r1);
+  ASSERT_TRUE (r0.known_nan ());
+
   // [10,20] ^ [21,25] = []
   r0 = frange_float ("10", "20");
+  r0.clear_nan ();
   r1 = frange_float ("21", "25");
+  r1.clear_nan ();
   r0.intersect (r1);
   ASSERT_TRUE (r0.undefined_p ());
 }
diff --git a/gcc/value-range.h b/gcc/value-range.h
index 4392de84c8b..cbb6496f976 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -93,7 +93,7 @@  public:
   virtual bool fits_p (const vrange &r) const;
 
   bool varying_p () const;
-  bool undefined_p () const;
+  virtual bool undefined_p () const;
   vrange& operator= (const vrange &);
   bool operator== (const vrange &) const;
   bool operator!= (const vrange &r) const { return !(*this == r); }
@@ -263,68 +263,6 @@  public:
   virtual void accept (const vrange_visitor &v) const override;
 };
 
-// Floating point property to represent possible values of a NAN, INF, etc.
-
-class fp_prop
-{
-public:
-  enum kind {
-    UNDEFINED	= 0x0,		// Prop is impossible.
-    YES		= 0x1,		// Prop is definitely set.
-    NO		= 0x2,		// Prop is definitely not set.
-    VARYING	= (YES | NO)	// Prop may hold.
-  };
-  fp_prop (kind f) : m_kind (f) { }
-  bool varying_p () const { return m_kind == VARYING; }
-  bool undefined_p () const { return m_kind == UNDEFINED; }
-  bool yes_p () const { return m_kind == YES; }
-  bool no_p () const { return m_kind == NO; }
-private:
-  unsigned char m_kind : 2;
-};
-
-// Accessors for individual FP properties.
-
-#define FP_PROP_ACCESSOR(NAME) \
-  void NAME##_set_varying () { u.bits.NAME = fp_prop::VARYING; }	\
-  void NAME##_set_yes () { u.bits.NAME = fp_prop::YES; }	\
-  void NAME##_set_no () { u.bits.NAME = fp_prop::NO; }	\
-  bool NAME##_varying_p () const { return u.bits.NAME == fp_prop::VARYING; } \
-  bool NAME##_undefined_p () const { return u.bits.NAME == fp_prop::UNDEFINED; } \
-  bool NAME##_yes_p () const { return u.bits.NAME == fp_prop::YES; }	\
-  bool NAME##_no_p () const { return u.bits.NAME == fp_prop::NO; } \
-  fp_prop get_##NAME () const				   \
-  { return fp_prop ((fp_prop::kind) u.bits.NAME); } \
-  void set_##NAME (fp_prop::kind f) { u.bits.NAME = f; }
-
-// Aggregate of all the FP properties in an frange packed into one
-// structure to save space.  Using explicit fp_prop's in the frange,
-// would take one byte per property because of padding.  Instead, we
-// can save all properties into one byte.
-
-class frange_props
-{
-public:
-  frange_props () { set_varying (); }
-  void set_varying () { u.bytes = 0xff; }
-  void set_undefined () { u.bytes = 0; }
-  bool varying_p () { return u.bytes == 0xff; }
-  bool undefined_p () { return u.bytes == 0; }
-  bool union_ (const frange_props &other);
-  bool intersect (const frange_props &other);
-  bool operator== (const frange_props &other) const;
-  FP_PROP_ACCESSOR(nan)
-  FP_PROP_ACCESSOR(signbit)
-private:
-  union {
-    struct {
-      unsigned char nan : 2;
-      unsigned char signbit : 2;
-    } bits;
-    unsigned char bytes;
-  } u;
-};
-
 // A floating point range.
 
 class frange : public vrange
@@ -349,8 +287,10 @@  public:
   void set (tree type, const REAL_VALUE_TYPE &, const REAL_VALUE_TYPE &,
 	    value_range_kind = VR_RANGE);
   void set_nan (tree type);
+  void set_nan (tree type, bool sign);
   virtual void set_varying (tree type) override;
   virtual void set_undefined () override;
+  virtual bool undefined_p () const final override;
   virtual bool union_ (const vrange &) override;
   virtual bool intersect (const vrange &) override;
   virtual bool contains_p (tree) const override;
@@ -376,33 +316,33 @@  public:
   bool known_nan () const;
   bool known_signbit (bool &signbit) const;
 
-  // Accessors for FP properties.
-  void update_nan (fp_prop::kind f);
-  void clear_nan () { update_nan (fp_prop::NO); }
-  void set_signbit (fp_prop::kind);
+  void update_nan ();
+  void clear_nan ();
 private:
-  fp_prop get_nan () const { return m_props.get_nan (); }
-  fp_prop get_signbit () const { return m_props.get_signbit (); }
   void verify_range ();
   bool normalize_kind ();
+  bool union_nans (const frange &);
+  bool intersect_nans (const frange &);
+  bool combine_zeros (const frange &, bool union_p);
 
-  frange_props m_props;
   tree m_type;
   REAL_VALUE_TYPE m_min;
   REAL_VALUE_TYPE m_max;
+  bool m_pos_nan;
+  bool m_neg_nan;
 };
 
 inline const REAL_VALUE_TYPE &
 frange::lower_bound () const
 {
-  gcc_checking_assert (!undefined_p ());
+  gcc_checking_assert (!undefined_p () && !known_nan ());
   return m_min;
 }
 
 inline const REAL_VALUE_TYPE &
 frange::upper_bound () const
 {
-  gcc_checking_assert (!undefined_p ());
+  gcc_checking_assert (!undefined_p () && !known_nan ());
   return m_max;
 }
 
@@ -1082,30 +1022,6 @@  vrp_val_min (const_tree type)
   return NULL_TREE;
 }
 
-// Supporting methods for frange.
-
-inline bool
-frange_props::operator== (const frange_props &other) const
-{
-  return u.bytes == other.u.bytes;
-}
-
-inline bool
-frange_props::union_ (const frange_props &other)
-{
-  unsigned char saved = u.bytes;
-  u.bytes |= other.u.bytes;
-  return u.bytes != saved;
-}
-
-inline bool
-frange_props::intersect (const frange_props &other)
-{
-  unsigned char saved = u.bytes;
-  u.bytes &= other.u.bytes;
-  return u.bytes != saved;
-}
-
 inline
 frange::frange ()
 {
@@ -1154,15 +1070,52 @@  frange::set_varying (tree type)
   m_type = type;
   m_min = dconstninf;
   m_max = dconstinf;
-  m_props.set_varying ();
+  m_pos_nan = true;
+  m_neg_nan = true;
 }
 
 inline void
 frange::set_undefined ()
 {
   m_kind = VR_UNDEFINED;
-  m_type = NULL;
-  m_props.set_undefined ();
+  m_pos_nan = false;
+  m_neg_nan = false;
+  if (flag_checking)
+    verify_range ();
+}
+
+// Set the NAN bit and adjust the range.
+
+inline void
+frange::update_nan ()
+{
+  gcc_checking_assert (!undefined_p ());
+  m_pos_nan = true;
+  m_neg_nan = true;
+  normalize_kind ();
+  if (flag_checking)
+    verify_range ();
+}
+
+// Clear the NAN bit and adjust the range.
+
+inline void
+frange::clear_nan ()
+{
+  gcc_checking_assert (!undefined_p ());
+  m_pos_nan = false;
+  m_neg_nan = false;
+  normalize_kind ();
+  if (flag_checking)
+    verify_range ();
+}
+
+// Return TRUE if range is the empty set.
+
+inline bool
+frange::undefined_p () const
+{
+  return m_kind == VR_UNDEFINED && !m_pos_nan && !m_neg_nan;
 }
 
 // Set R to maximum representable value for TYPE.
@@ -1186,19 +1139,28 @@  real_min_representable (REAL_VALUE_TYPE *r, tree type)
   *r = real_value_negate (r);
 }
 
-// Build a NAN of type TYPE.
+// Build a signless NAN of type TYPE.
 
 inline void
 frange::set_nan (tree type)
 {
-  REAL_VALUE_TYPE r;
-  gcc_assert (real_nan (&r, "", 1, TYPE_MODE (type)));
-  m_kind = VR_RANGE;
+  m_kind = VR_UNDEFINED;
+  m_type = type;
+  m_pos_nan = true;
+  m_neg_nan = true;
+  if (flag_checking)
+    verify_range ();
+}
+
+// Build a NAN of type TYPE with SIGN.
+
+inline void
+frange::set_nan (tree type, bool sign)
+{
+  m_kind = VR_UNDEFINED;
   m_type = type;
-  m_min = r;
-  m_max = r;
-  m_props.set_varying ();
-  m_props.nan_set_yes ();
+  m_neg_nan = sign;
+  m_pos_nan = !sign;
   if (flag_checking)
     verify_range ();
 }
@@ -1210,9 +1172,7 @@  frange::known_finite () const
 {
   if (undefined_p () || varying_p () || m_kind == VR_ANTI_RANGE)
     return false;
-  return (!real_isnan (&m_min)
-	  && !real_isinf (&m_min)
-	  && !real_isinf (&m_max));
+  return (!maybe_nan () && !real_isinf (&m_min) && !real_isinf (&m_max));
 }
 
 // Return TRUE if range may be infinite.
@@ -1242,7 +1202,7 @@  frange::known_inf () const
 inline bool
 frange::maybe_nan () const
 {
-  return !get_nan ().no_p ();
+  return m_pos_nan || m_neg_nan;
 }
 
 // Return TRUE if range is a +NAN or -NAN.
@@ -1250,7 +1210,7 @@  frange::maybe_nan () const
 inline bool
 frange::known_nan () const
 {
-  return get_nan ().yes_p ();
+  return m_kind == VR_UNDEFINED && maybe_nan ();
 }
 
 // If the signbit for the range is known, set it in SIGNBIT and return
@@ -1259,13 +1219,31 @@  frange::known_nan () const
 inline bool
 frange::known_signbit (bool &signbit) const
 {
-  // FIXME: Signed NANs are not supported yet.
-  if (maybe_nan ())
+  if (undefined_p ())
     return false;
-  if (get_signbit ().varying_p ())
+
+  // NAN with unknown sign.
+  if (m_pos_nan && m_neg_nan)
     return false;
-  signbit = get_signbit ().yes_p ();
-  return true;
+  // No NAN.
+  if (!m_pos_nan && !m_neg_nan)
+    {
+      if (m_min.sign == m_max.sign)
+	{
+	  signbit = m_min.sign;
+	  return true;
+	}
+      return false;
+    }
+  // NAN with known sign.
+  bool nan_sign = m_neg_nan;
+  if (m_kind == VR_UNDEFINED
+      || (nan_sign == m_min.sign && nan_sign == m_max.sign))
+    {
+      signbit = nan_sign;
+      return true;
+    }
+  return false;
 }
 
 #endif // GCC_VALUE_RANGE_H