Re: [PATCH] range-op-float, value-range, v3: Fix up float_widen_lhs_range [PR126641]
Richard Biener <[email protected]>
| Newsgroups | gmane.comp.gcc.patches |
|---|---|
| Message-ID | <[email protected]> |
On Fri, 7 Aug 2026, Jakub Jelinek wrote:
> On Fri, Aug 07, 2026 at 12:14:53AM +0200, Jakub Jelinek wrote:
> > On Thu, Aug 06, 2026 at 10:39:56PM +0200, Aldy Hernandez wrote:
> > > 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.
> >
> > Unfortunately it doesn't work :(.
>
> Here is an updated patch that passed bootstrap/regtest on x86_64-linux and
> i686-linux. It handles it the way you've originally suggested, so
> frange wlhs = lhs; wlhs.widen (type);
> which is longer in the callers, but has the advantage that it doesn't need
> to copy the range. I think in most places it actually wasn't copied in
> the previous patch either due to NRV, i.e... frange wlhs = lhs.widen (type);
> was fine, but there was one spot where it did frange wlhs; if (...) wlhs =
> lhs; else { ... wlhs = lhs.widen (type); ... } and that had to invoke
> copy assignment.
>
> Ok for trunk?
LGTM.
Thanks,
Richard.
> 2026-08-07 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
> X = float_widen_lhs_range (T, R) with X = R; X.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
> m_pair and 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 22:36:24.657659928 +0200
> +++ gcc/value-range.h 2026-08-07 00:16:16.921841171 +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;
> +
> + void widen (tree);
> 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 22:36:24.640660133 +0200
> +++ gcc/range-op-float.cc 2026-08-07 00:18:10.685471752 +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,8 @@ 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;
> + wlhs.widen (type);
> return float_binary_op_range_finish (minus.fold_range (r, type, wlhs, op2),
> r, type, wlhs);
> }
> @@ -2492,7 +2424,8 @@ operator_minus::op1_range (frange &r, tr
> {
> if (lhs.undefined_p ())
> return false;
> - frange wlhs = float_widen_lhs_range (type, lhs);
> + frange wlhs = lhs;
> + wlhs.widen (type);
> return float_binary_op_range_finish (
> range_op_handler (PLUS_EXPR).fold_range (r, type, wlhs, op2),
> r, type, wlhs);
> @@ -2505,7 +2438,8 @@ operator_minus::op2_range (frange &r, tr
> {
> if (lhs.undefined_p ())
> return false;
> - frange wlhs = float_widen_lhs_range (type, lhs);
> + frange wlhs = lhs;
> + wlhs.widen (type);
> return float_binary_op_range_finish (fold_range (r, type, op1, wlhs),
> r, type, wlhs);
> }
> @@ -2581,7 +2515,8 @@ 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;
> + wlhs.widen (type);
> bool ret = rdiv.fold_range (r, type, wlhs, op2);
> if (ret == false)
> return false;
> @@ -2746,7 +2681,8 @@ public:
> {
> if (lhs.undefined_p ())
> return false;
> - frange wlhs = float_widen_lhs_range (type, lhs);
> + frange wlhs = lhs;
> + wlhs.widen (type);
> bool ret = range_op_handler (MULT_EXPR).fold_range (r, type, wlhs, op2);
> if (!ret)
> return ret;
> @@ -2778,7 +2714,8 @@ public:
> {
> if (lhs.undefined_p ())
> return false;
> - frange wlhs = float_widen_lhs_range (type, lhs);
> + frange wlhs = lhs;
> + wlhs.widen (type);
> bool ret = fold_range (r, type, op1, wlhs);
> if (!ret)
> return ret;
> @@ -3015,7 +2952,8 @@ operator_cast::op1_range (frange &r, tre
> else
> {
> rm = true;
> - wlhs = float_widen_lhs_range (lhs_type, lhs);
> + wlhs = lhs;
> + wlhs.widen (lhs_type);
> }
> auto save_flag_rounding_math = flag_rounding_math;
> flag_rounding_math = rm;
> @@ -3257,14 +3195,15 @@ 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
> + // r.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;
> + r.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 22:36:24.657659928 +0200
> +++ gcc/value-range.cc 2026-08-07 00:21:00.984421789 +0200
> @@ -1750,6 +1750,92 @@ 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. */
> +
> +void
> +frange::widen (tree type)
> +{
> + if (known_isnan ())
> + return;
> + /* 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 (&m_pairs[j - 1].max, &lb))
> + m_pairs[j - 1].max = ub;
> + else
> + {
> + m_pairs[j].min = lb;
> + m_pairs[j].max = ub;
> + ++j;
> + }
> + }
> + m_num_ranges = j;
> + flag_finite_math_only = save_flag_finite_math_only;
> +}
> +
> // Here we copy between any two irange's.
>
> irange &
> --- gcc/real.cc.jj 2026-06-27 21:32:19.216110544 +0200
> +++ gcc/real.cc 2026-08-06 22:36:58.651249260 +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 22:36:58.651733766 +0200
> +++ gcc/testsuite/gcc.dg/pr126641.c 2026-08-06 22:36:58.651733766 +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
>
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)