Re: [PATCH] range-op-float, value-range, v2: Fix up float_widen_lhs_range [PR126641]

On Thu, Aug 06, 2026 at 07:32:17PM +0200, Jakub Jelinek wrote:
> Hi!
> 
> On Thu, Aug 06, 2026 at 06:25:35PM +0200, Aldy Hernandez wrote:
> > > Not sure I completely follow exactly, but can you simply make
> > > float_widen_lhs_range a public method of frange? it always seems to be a
> > > copy that is been adjusted anyway.. so instead of
> > > 
> > > frange wlhs = float_widen_lhs_range (type, lhs); you'd do something like
> > > frange wlhs = lhs; wlhs.widen (type);
> > 
> > Exactly, that's what I meant.
> 
> So like this?

LGTM.

Thanks.
Aldy

> 
> Instead of modifying *this this patch returns a different frange, so
> I've been changing
> frange wlhs = float_widen_lhs_range (type, lhs);
> to
> frange wlhs = lhs.widen (type);
> because that is less code in all the callers compared to
> frange wlhs = lhs;
> wlhs.widen (type);
> 
> 2026-08-06  Jakub Jelinek  <[email protected]>
> 
> 	PR tree-optimization/126641
> 	* value-range.h (class frange): Declare widen method.
> 	* range-op-float.cc (float_widen_bound): Move to value-range.cc.
> 	(float_widen_lhs_range): Remove.
> 	(operator_plus::op1_range, operator_minus::op1_range,
> 	operator_minus::op2_range, operator_mult::op1_range,
> 	foperator_div::op1_range, foperator_div::op2_range,
> 	operator_cast::op1_range, range_op_float_tests): Replace
> 	float_widen_lhs_range (T, R) with R.widen (T).
> 	* value-range.cc (float_widen_bound): New, moved from
> 	range-op-float.cc.
> 	(frange::widen): New method, partially based on
> 	float_widen_lhs_range, but replace lhs with *this and
> 	avoid using set/union_ to construct range, instead modify
> 	ret.m_pair and ret.m_num_ranges directly.
> 	* real.cc (set_significand_bit, clear_significand_bit,
> 	test_significand_bit): Add
> 	gcc_checking_assert (n < SIGNIFICAND_BITS).
> 	(clear_significand_below): Add
> 	gcc_checking_assert (n <= SIGNIFICAND_BITS).
> 
> 	* gcc.dg/pr126641.c: New test.
> 
> --- gcc/value-range.h.jj	2026-08-06 17:46:47.582463890 +0200
> +++ gcc/value-range.h	2026-08-06 18:52:00.475570460 +0200
> @@ -672,6 +672,8 @@ public:
>    unsigned num_pairs () const { return m_num_ranges; }
>    const REAL_VALUE_TYPE &lower_bound (unsigned pair) const;
>    const REAL_VALUE_TYPE &upper_bound (unsigned pair) const;
> +
> +  frange widen (tree) const;
>  protected:
>    virtual bool contains_p (tree cst) const override;
>    virtual void set (tree, tree, value_range_kind = VR_RANGE) override;
> --- gcc/range-op-float.cc.jj	2026-08-06 17:46:47.580463914 +0200
> +++ gcc/range-op-float.cc	2026-08-06 18:53:17.091646640 +0200
> @@ -2356,75 +2356,6 @@ zero_to_inf_range (REAL_VALUE_TYPE &lb,
>      }
>  }
>  
> -/* Widen a single bound of a sub-range by 1ulp (or 0.5ulp) in the direction of
> -   DIR.  */
> -
> -static REAL_VALUE_TYPE
> -float_widen_bound (tree type, const REAL_VALUE_TYPE &bound,
> -		   const REAL_VALUE_TYPE &dir)
> -{
> -  REAL_VALUE_TYPE res = bound;
> -  if (!real_isfinite (&bound) && real_isneg (&bound) == real_isneg (&dir))
> -    return res;
> -  frange_nextafter (TYPE_MODE (type), res, dir);
> -  if (real_isinf (&res))
> -    {
> -      /* For +-DBL_MAX, instead of +-Inf use nexttoward (+-DBL_MAX, +-LDBL_MAX)
> -	 in a hypothetical wider type with the same mantissa precision but
> -	 larger exponent range; it is outside of range of double values, but
> -	 makes it clear it is just one ulp larger rather than infinite amount
> -	 larger.  */
> -      res = real_isneg (&dir) ? dconstm1 : dconst1;
> -      SET_REAL_EXP (&res, FLOAT_MODE_FORMAT (TYPE_MODE (type))->emax + 1);
> -    }
> -  if (!flag_rounding_math
> -      && !MODE_COMPOSITE_P (TYPE_MODE (type))
> -      && real_isfinite (&bound))
> -    {
> -      /* If not -frounding-math nor IBM double double, actually widen
> -	 just by 0.5ulp rather than 1ulp.  */
> -      REAL_VALUE_TYPE tem;
> -      real_arithmetic (&tem, PLUS_EXPR, &bound, &res);
> -      real_arithmetic (&res, RDIV_EXPR, &tem, &dconst2);
> -    }
> -  return res;
> -}
> -
> -/* Extend the LHS range by 1ulp in each direction.  For op1_range
> -   or op2_range of binary operations just computing the inverse
> -   operation on ranges isn't sufficient.  Consider e.g.
> -   [1., 1.] = op1 + [1., 1.].  op1's range is not [0., 0.], but
> -   [-0x1.0p-54, 0x1.0p-53] (when not -frounding-math), any value for
> -   which adding 1. to it results in 1. after rounding to nearest.
> -   So, for op1_range/op2_range extend the lhs range by 1ulp (or 0.5ulp)
> -   in each direction.  See PR109008 for more details.  */
> -
> -static frange
> -float_widen_lhs_range (tree type, const frange &lhs)
> -{
> -  frange ret = lhs;
> -  if (lhs.known_isnan ())
> -    return ret;
> -  /* Temporarily disable -ffinite-math-only, so that frange::set doesn't
> -     reduce the range back to real_min_representable (type) as lower bound
> -     or real_max_representable (type) as upper bound.  */
> -  bool save_flag_finite_math_only = flag_finite_math_only;
> -  flag_finite_math_only = false;
> -  ret.set_undefined ();
> -  for (unsigned i = 0; i < lhs.num_pairs (); ++i)
> -    {
> -      REAL_VALUE_TYPE lb = float_widen_bound (type, lhs.lower_bound (i),
> -					      dconstninf);
> -      REAL_VALUE_TYPE ub = float_widen_bound (type, lhs.upper_bound (i),
> -					      dconstinf);
> -      frange tmp;
> -      tmp.set (type, lb, ub, lhs.get_nan_state ());
> -      ret.union_ (tmp);
> -    }
> -  flag_finite_math_only = save_flag_finite_math_only;
> -  return ret;
> -}
> -
>  bool
>  operator_plus::op1_range (frange &r, tree type, const frange &lhs,
>  			  const frange &op2, relation_trio) const
> @@ -2434,7 +2365,7 @@ operator_plus::op1_range (frange &r, tre
>    range_op_handler minus (MINUS_EXPR);
>    if (!minus)
>      return false;
> -  frange wlhs = float_widen_lhs_range (type, lhs);
> +  frange wlhs = lhs.widen (type);
>    return float_binary_op_range_finish (minus.fold_range (r, type, wlhs, op2),
>  				       r, type, wlhs);
>  }
> @@ -2492,7 +2423,7 @@ operator_minus::op1_range (frange &r, tr
>  {
>    if (lhs.undefined_p ())
>      return false;
> -  frange wlhs = float_widen_lhs_range (type, lhs);
> +  frange wlhs = lhs.widen (type);
>    return float_binary_op_range_finish (
>  	      range_op_handler (PLUS_EXPR).fold_range (r, type, wlhs, op2),
>  	      r, type, wlhs);
> @@ -2505,7 +2436,7 @@ operator_minus::op2_range (frange &r, tr
>  {
>    if (lhs.undefined_p ())
>      return false;
> -  frange wlhs = float_widen_lhs_range (type, lhs);
> +  frange wlhs = lhs.widen (type);
>    return float_binary_op_range_finish (fold_range (r, type, op1, wlhs),
>  				       r, type, wlhs);
>  }
> @@ -2581,7 +2512,7 @@ operator_mult::op1_range (frange &r, tre
>    range_op_handler rdiv (RDIV_EXPR);
>    if (!rdiv)
>      return false;
> -  frange wlhs = float_widen_lhs_range (type, lhs);
> +  frange wlhs = lhs.widen (type);
>    bool ret = rdiv.fold_range (r, type, wlhs, op2);
>    if (ret == false)
>      return false;
> @@ -2746,7 +2677,7 @@ public:
>    {
>      if (lhs.undefined_p ())
>        return false;
> -    frange wlhs = float_widen_lhs_range (type, lhs);
> +    frange wlhs = lhs.widen (type);
>      bool ret = range_op_handler (MULT_EXPR).fold_range (r, type, wlhs, op2);
>      if (!ret)
>        return ret;
> @@ -2778,7 +2709,7 @@ public:
>    {
>      if (lhs.undefined_p ())
>        return false;
> -    frange wlhs = float_widen_lhs_range (type, lhs);
> +    frange wlhs = lhs.widen (type);
>      bool ret = fold_range (r, type, op1, wlhs);
>      if (!ret)
>        return ret;
> @@ -3015,7 +2946,7 @@ operator_cast::op1_range (frange &r, tre
>    else
>      {
>        rm = true;
> -      wlhs = float_widen_lhs_range (lhs_type, lhs);
> +      wlhs = lhs.widen (lhs_type);
>      }
>    auto save_flag_rounding_math = flag_rounding_math;
>    flag_rounding_math = rm;
> @@ -3257,14 +3188,14 @@ range_op_float_tests ()
>    if (HONOR_NANS (float_type_node))
>      ASSERT_TRUE (r.maybe_isnan ());
>  
> -  // float_widen_lhs_range widens each sub-range and keeps the gap between
> +  // r0.widen widens each sub-range and keeps the gap between
>    // them.
>    r0 = frange_float ("1.0", "2.0");
>    r1 = frange_float ("10.0", "11.0");
>    r0.union_ (r1);
>    r0.clear_nan ();
>    ASSERT_EQ (r0.num_pairs (), 2);
> -  r = float_widen_lhs_range (float_type_node, r0);
> +  r = r0.widen (float_type_node);
>    ASSERT_EQ (r.num_pairs (), 2);
>    REAL_VALUE_TYPE five;
>    real_from_string (&five, "5.0");
> --- gcc/value-range.cc.jj	2026-08-06 13:18:50.486508218 +0200
> +++ gcc/value-range.cc	2026-08-06 18:59:55.771847613 +0200
> @@ -1750,6 +1750,94 @@ frange::ubound () const
>    return build_real (type (), upper_bound ());
>  }
>  
> +/* Widen a single bound of a sub-range by 1ulp (or 0.5ulp) in the direction of
> +   DIR.  */
> +
> +static REAL_VALUE_TYPE
> +float_widen_bound (tree type, const REAL_VALUE_TYPE &bound,
> +		   const REAL_VALUE_TYPE &dir)
> +{
> +  REAL_VALUE_TYPE res = bound;
> +  if (!real_isfinite (&bound) && real_isneg (&bound) == real_isneg (&dir))
> +    return res;
> +  frange_nextafter (TYPE_MODE (type), res, dir);
> +  if (real_isinf (&res))
> +    {
> +      /* For +-DBL_MAX, instead of +-Inf use nexttoward (+-DBL_MAX, +-LDBL_MAX)
> +	 in a hypothetical wider type with the same mantissa precision but
> +	 larger exponent range; it is outside of range of double values, but
> +	 makes it clear it is just one ulp larger rather than infinite amount
> +	 larger.  */
> +      res = real_isneg (&dir) ? dconstm1 : dconst1;
> +      SET_REAL_EXP (&res, FLOAT_MODE_FORMAT (TYPE_MODE (type))->emax + 1);
> +    }
> +  if (!flag_rounding_math
> +      && !MODE_COMPOSITE_P (TYPE_MODE (type))
> +      && real_isfinite (&bound))
> +    {
> +      /* If not -frounding-math nor IBM double double, actually widen
> +	 just by 0.5ulp rather than 1ulp.  */
> +      REAL_VALUE_TYPE tem;
> +      real_arithmetic (&tem, PLUS_EXPR, &bound, &res);
> +      real_arithmetic (&res, RDIV_EXPR, &tem, &dconst2);
> +    }
> +  return res;
> +}
> +
> +/* Extend the *this range by 1ulp in each direction.  For op1_range
> +   or op2_range of binary operations just computing the inverse
> +   operation on ranges isn't sufficient.  Consider e.g.
> +   [1., 1.] = op1 + [1., 1.].  op1's range is not [0., 0.], but
> +   [-0x1.0p-54, 0x1.0p-53] (when not -frounding-math), any value for
> +   which adding 1. to it results in 1. after rounding to nearest.
> +   So, for op1_range/op2_range extend the lhs range by 1ulp (or 0.5ulp)
> +   in each direction.  See PR109008 for more details.  */
> +
> +frange
> +frange::widen (tree type) const
> +{
> +  frange ret = *this;
> +  if (known_isnan ())
> +    return ret;
> +  /* Temporarily disable -ffinite-math-only, so that frange::set doesn't
> +     reduce the range back to real_min_representable (type) as lower bound
> +     or real_max_representable (type) as upper bound.  */
> +  bool save_flag_finite_math_only = flag_finite_math_only;
> +  flag_finite_math_only = false;
> +  unsigned j = 0;
> +  for (unsigned i = 0; i < num_pairs (); ++i)
> +    {
> +      REAL_VALUE_TYPE lb = float_widen_bound (type, lower_bound (i),
> +					      dconstninf);
> +      REAL_VALUE_TYPE ub = float_widen_bound (type, upper_bound (i),
> +					      dconstinf);
> +      /* The result of float_widen_bound is often not representable in
> +	 type (could be smaller by 1ulp from representable finite minimum,
> +	 0.5ulp from some representable finite value or 1ulp larger than
> +	 representable finite maximum).  On such values calling e.g.
> +	 frange_nextafter doesn't work properly, so avoid merging the
> +	 pairs with union_ because that calls frange_fusible_p etc.
> +	 This range is often just something that should have the
> +	 real values passed to frange_arithmetic etc. and have the result
> +	 of that converted to something actually representable in the
> +	 type.  See PR126641 and PR109008.  As lhs should have been
> +	 canonicalized before, the slightly adjusted range should have
> +	 similar properties, just merge pairs where max would be >= than
> +	 min of the next pair.  */
> +      if (j && !real_less (&ret.m_pairs[j - 1].max, &lb))
> +	ret.m_pairs[j - 1].max = ub;
> +      else
> +	{
> +	  ret.m_pairs[j].min = lb;
> +	  ret.m_pairs[j].max = ub;
> +	  ++j;
> +	}
> +    }
> +  ret.m_num_ranges = j;
> +  flag_finite_math_only = save_flag_finite_math_only;
> +  return ret;
> +}
> +
>  // Here we copy between any two irange's.
>  
>  irange &
> --- gcc/real.cc.jj	2026-06-30 09:20:44.116805362 +0200
> +++ gcc/real.cc	2026-08-06 18:57:58.149262436 +0200
> @@ -399,6 +399,7 @@ cmp_significand_0 (const REAL_VALUE_TYPE
>  static inline void
>  set_significand_bit (REAL_VALUE_TYPE *r, unsigned int n)
>  {
> +  gcc_checking_assert (n < SIGNIFICAND_BITS);
>    r->sig[n / HOST_BITS_PER_LONG]
>      |= (unsigned long)1 << (n % HOST_BITS_PER_LONG);
>  }
> @@ -408,6 +409,7 @@ set_significand_bit (REAL_VALUE_TYPE *r,
>  static inline void
>  clear_significand_bit (REAL_VALUE_TYPE *r, unsigned int n)
>  {
> +  gcc_checking_assert (n < SIGNIFICAND_BITS);
>    r->sig[n / HOST_BITS_PER_LONG]
>      &= ~((unsigned long)1 << (n % HOST_BITS_PER_LONG));
>  }
> @@ -420,6 +422,7 @@ test_significand_bit (REAL_VALUE_TYPE *r
>    /* ??? Compiler bug here if we return this expression directly.
>       The conversion to bool strips the "&1" and we wind up testing
>       e.g. 2 != 0 -> true.  Seen in gcc version 3.2 20020520.  */
> +  gcc_checking_assert (n < SIGNIFICAND_BITS);
>    int t = (r->sig[n / HOST_BITS_PER_LONG] >> (n % HOST_BITS_PER_LONG)) & 1;
>    return t;
>  }
> @@ -431,6 +434,7 @@ clear_significand_below (REAL_VALUE_TYPE
>  {
>    int i, w = n / HOST_BITS_PER_LONG;
>  
> +  gcc_checking_assert (n <= SIGNIFICAND_BITS);
>    for (i = 0; i < w; ++i)
>      r->sig[i] = 0;
>  
> --- gcc/testsuite/gcc.dg/pr126641.c.jj	2026-08-06 18:47:51.255671582 +0200
> +++ gcc/testsuite/gcc.dg/pr126641.c	2026-08-06 18:47:51.255671582 +0200
> @@ -0,0 +1,12 @@
> +/* PR tree-optimization/126641 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +extern double x;
> +extern int n;
> +
> +int
> +foo ()
> +{
> +  return n - x && x * 0;
> +}
> 
> 
> 	Jakub
>