On Mon, Sep 5, 2022 at 8:24 AM Aldy Hernandez via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Intersecting two ranges where one is a NAN is keeping the sign bit of
> the NAN range. This is not correct as the sign bits may not match.
>
> I think the only time we're absolutely sure about the intersection of
> a NAN and something else, is when both are a NAN with exactly the same
> properties (sign bit). If we're intersecting two NANs of differing
> sign, we can decide later whether that's undefined or just a NAN with
> no known sign. For now I've done the latter.
>
> I'm still mentally working on intersections involving NANs, especially
> if we want to keep track of signbits. For now, let's be extra careful
> and only do things we're absolutely sure about.
>
> Later we may want to fold the intersect of [NAN,NAN] and say [3,5]
> with the posibility of NAN, to a NAN, but I'm not 100% sure.
The intersection of [NAN, NAN] and [3, 5] is empty. The intersection
of [NAN, NAN] and VARYING is [NAN, NAN].
Not sure why you think NAN is somehow special? I suppose that
[NAN, NAN] is actually implemented as ][ U NAN (empty range
plus aside NaN bit)
Richard.
> As I've
> said before, setting varying is always a safe choice, because it means
> we know nothing and ranger won't attempt to optimize anything.
>
> gcc/ChangeLog:
>
> * value-range.cc (early_nan_resolve): Remove.
> (frange::intersect): Handle NANs.
> ---
> gcc/value-range.cc | 35 ++++++++++++++++-------------------
> 1 file changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/gcc/value-range.cc b/gcc/value-range.cc
> index c9f42fe272c..9c561415971 100644
> --- a/gcc/value-range.cc
> +++ b/gcc/value-range.cc
> @@ -444,24 +444,6 @@ frange::normalize_kind ()
> return false;
> }
>
> -// If both operands are definitely NAN, do nothing as they combine
> -// perfectly. If OTOH, only one is a NAN, set R to VARYING as they
> -// can't be neither unioned nor intersected. Return TRUE if we
> -// changed anything.
> -
> -static inline bool
> -early_nan_resolve (frange &r, const frange &other)
> -{
> - gcc_checking_assert (r.get_nan ().yes_p () || other.get_nan ().yes_p ());
> -
> - // There's nothing to do for both NANs.
> - if (r.get_nan ().yes_p () == other.get_nan ().yes_p ())
> - return false;
> - // ?? Perhaps the intersection of a NAN and anything is a NAN ??.
> - r.set_varying (r.type ());
> - return true;
> -}
> -
> bool
> frange::union_ (const vrange &v)
> {
> @@ -532,8 +514,23 @@ frange::intersect (const vrange &v)
> *this = r;
> return true;
> }
> +
> + // If two NANs are not exactly the same, drop to an unknown NAN,
> + // otherwise there's nothing to do.
> + if (get_nan ().yes_p () && r.get_nan ().yes_p ())
> + {
> + if (m_props == r.m_props)
> + return false;
> +
> + *this = frange_nan (m_type);
> + return true;
> + }
> + // ?? Perhaps the intersection of a NAN and anything is a NAN ??.
> if (get_nan ().yes_p () || r.get_nan ().yes_p ())
> - return early_nan_resolve (*this, r);
> + {
> + set_varying (m_type);
> + return true;
> + }
>
> bool changed = m_props.intersect (r.m_props);
>
> --
> 2.37.1
>
