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

On Thu, Aug 06, 2026 at 02:50:56PM +0200, Jakub Jelinek wrote:
> Hi!
> 
> The recent change to make float_widen_lhs_range work on multiple
> subrange pairs broke e.g. the following testcase or various other
> things, set_significand is often called with SIGNIFICANT_BITS
> as the second argument and overflows the storage (in my case 3 elt
> array, if 192 is passed that stores [3]).
> 
> The culprit is in the PR109008 optimization, float_widen_lhs_range
> produces something that isn't really a valid range for the corresponding
> type, it is the passed in valid range slightly extended if needed.
> The extension can be +-1ulp from the largest representable finite values
> (negative/positive) or +-0.5ulp in between some representable finite values,
> or in some cases +-1ulp.
> 
> The reason for this hack is that extending by full representable 1ulp
> in all cases is simply too much, especially if that extension means going
> from the largest representable finite value to +-infinity.  The function
> is used in various frange reverse ops and the intent is that this adjusted
> range doesn't really survive after the reverse operation handler, we should
> call some frange_arithmetic etc. call and that should handle even the not
> exactly representable in type (but representable in the gcc internal format,
> 160 bit precision, wide range of exponents) values, we do range_arithmetic
> which produces something in the gcc internal format and then round it to
> the mode of the desired type.
> 
> Now, the r17-2929 change broke this, because it uses union_ ->
> frange_fusible_p -> frange_nextafter on those not exactly representable
> values and frange_nextafter -> real_nextafter quite understandably doesn't
> work properly on those values, to find the next after value it assumes the
> input is representable.
> 
> The following patch fixes this in admittedly hacky way by copying the lhs
> range to ret and tweaking the m_pairs in it directly.
> Because lhs should be a canonicalized range, I think the widening shouldn't
> change the range canonicalization properties (like m_kind etc.), the
> extension is always just a tiny bit, shouldn't jump from finite to infinite
> or to NaN etc.  The only thing that can happen is that e.g. with
> -frounding-math if the original range has 1ulp hole in betweenn pairs (say
> ~[0.5, 0.5] range that the widening of 0.5-1ulp could result in 0.5 and
> 0.5+1ulp on the other bound also to 0.5 could result in the same value,
> so the code just merges pairs in such rare cases.
> 
> Ok for trunk if it passes full bootstrap/regtest?

I still think we should put in an assert in the real.cc code that dies
hard if we try to blow past the significant size, like I had in the
PR.  That way we can at least catch problems in this space without
having to go through the hell of memory corruption.

Overall, I'm not a big fan of having to expose the internals of frange
in this manner.  The fact that we have to do this, means we're either
doing something wrong, or the API is missing something.  What could we
provide to make this cleaner?

That being said, I think someone else should review this patch.  It's
way beyond my circle of competence and I'd just be making stuff up :).
If no one steps in, then I could take a stab at it.

Thanks for working on this.  I know it's been painful.

Aldy

p.s. Feel free to commit as is, to get everyone's builds back to
sanity, and you can tackle any review as a follow-up.

> 
> 2026-08-06  Jakub Jelinek  <[email protected]>
> 
> 	PR tree-optimization/126641
> 	* value-range.h (class frange): Add float_widen_lhs_range
> 	function as friend.
> 	* range-op-float.cc (float_widen_lhs_range): No longer static.
> 	Don't use set and union_ to merge adjusted pairs, instead
> 	manipulate m_pairs directly.
> 
> 	* gcc.dg/pr126641.c: New test.
> 
> --- gcc/value-range.h.jj	2026-08-06 13:18:50.487508206 +0200
> +++ gcc/value-range.h	2026-08-06 14:09:39.803450331 +0200
> @@ -603,6 +603,7 @@ class frange final : public vrange
>  {
>    friend class frange_storage;
>    friend class vrange_printer;
> +  friend frange float_widen_lhs_range (tree, const frange &);
>  public:
>    frange ();
>    frange (const frange &);
> --- gcc/range-op-float.cc.jj	2026-08-04 19:03:00.172862604 +0200
> +++ gcc/range-op-float.cc	2026-08-06 14:29:14.214187427 +0200
> @@ -2407,7 +2407,7 @@ float_widen_bound (tree type, const REAL
>     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
> +frange
>  float_widen_lhs_range (tree type, const frange &lhs)
>  {
>    frange ret = lhs;
> @@ -2418,17 +2418,37 @@ float_widen_lhs_range (tree type, const
>       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 ();
> +  ret = lhs;
> +  unsigned j = 0;
>    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);
> +      /* 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;
>  }
> --- gcc/testsuite/gcc.dg/pr126641.c.jj	2026-08-06 14:32:45.653623872 +0200
> +++ gcc/testsuite/gcc.dg/pr126641.c	2026-08-06 14:32:30.085812620 +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
>