On Fri, Jul 17, 2026 at 11:30 AM Roger Sayle <[email protected]> wrote:
>
>
> > > Am 17.07.2026 um 17:40 schrieb Roger Sayle
> > <[email protected]>:
> > > Hi Richard,
> > >
> > >> From: Richard Biener <[email protected]>
> > >> Sent: 17 July 2026 10:46
> > >> To: Roger Sayle <[email protected]>
> > >> Cc: GCC Patches <[email protected]>; Andrew Pinski
> > >> <[email protected]>
> > >> Subject: Re: [PATCH] PR tree-optimization/126242: Check range is
> > >> defined to avoid ICE.
> > >>
> > >>> On Wed, Jul 15, 2026 at 10:05 PM Roger Sayle
> > >>> <[email protected]>
> > >>> wrote:
> > >>>
> > >>>
> > >>> This patch resolves PR tree-opt/126242, an unanticipated interaction
> > >>> between the two recent (float)i == 1.0 patches to match.pd. The
> > >>> issue is that value range information is getting queried in
> > >>> circumstances (on paths) where we've failed to initialize the range
> > >>> and/or ranger has failed to bound the value. The correction below
> > >>> fixes this in two
> > >>> ways: initialize the range information in more cases, and check that
> > >>> the range has been successfully initialized before using it.
> > >>>
> > >>> The motivation/benefit for the first approach is seen in the example:
> > >>>
> > >>> unsigned char t = x & 63;
> > >>> return (float)t > 100.0;
> > >>>
> > >>> Previously, because unsigned char can be safely represented in a
> > >>> float we'd use the bounds [0,255], and transform this to t > 100.
> > >>> Obviously, there's benefit in using ranger to reduce the range to
> > >>> [0,63], even when the integer type fits the floating point type,
> > >>> allowing the above expression to be simplified even further to false.
> > >>> [Admittedly, this gets cleaned up in later passes/optimizations, but
> > >>> this shows a potential benefit rather than just an inefficiency for
> > >>> safety's sake].
> > >>>
> > >>>
> > >>> This patch has been tested on x86_64-pc-linux-gnu with make
> > >>> bootstrap and make -k check, both with and without
> > >>> --target_board=unix{-m32} with no new failures. Ok for mainline?
> > >>
> > >> I'm trying to follow the logic of itype_ok and value_ok. It might be
> > >> easier to follow if hoisting i{min,max}_val out of the if
> > >> (!exception_p) like
> > >>
> > >> wide_int imin_val = wi::min_value (itype);
> > >> wide_int imax_val = wi::max_value (itype);
> > >> bool value_ok = fmt.can_represent_integral_type_p (itype); #if GIMPLE
> > >> if (!value_ok
> > >> && gimple_match_range_of_expr (vr, @0, @2)
> > >> && fmt.can_represent_range_value_p (&vr))
> > >> {
> > >> imin_val = vr.lower_bound ();
> > >> imax_val = vr.upper_bound ();
> > >> value_ok = true;
> > >> }
> > >> #endif
> > >>
> > >> and elide itype_ok?
> > >>
> > >> OK with that change.
> > >
> > > No, the original fix is better. In your suggestion above, the range
> > > is only calculated if the type fits, and then only used if the range fits.
> >
> > I read it the opposite way (but maybe I am missing a UNDEFINED check on the
> > range)
>
> The underlying problem is with the clause:
>
> >> if (!value_ok
> >> && gimple_match_range_of_expr (vr, @0, @2)
> >> && fmt.can_represent_range_value_p (&vr))
> >> ...
>
> Here gimple_match_range_of_expr isn't a simple predicate, but
> has side-effects, in fact this is the place where vr gets initialized.
>
> Hence, in the idiom above, vr is only initialized if value_ok is false.
> Likewise, imin_val and imax_val only get updated to make use of
> the calculated range information if fmt.can_represent_range_value_p
> returns true.
>
> This is why Eikansh's patch used both itype_ok and value_ok, so that
> one could be (ab)used to keep track of whether the range had been
> initialized.
>
> So a better idiom looks like:
>
> // Always initialize vr (if possible)
> if (gimple_match_range_of_expr (vr, @0, @2)) {
> // Use improved bounds now we have them.
> imin_val = vr.lower();
> imax_val = vr.upper();
> // Carry on with the original logic
> if (!value_ok && fmt.can_represent_range_value_p (&vr))
> value_ok = true;
>
> It's the use of C++ pass by reference, that makes it unclear when/where
> vr ever gets initialized.
>
> static inline bool
> gimple_match_range_of_expr (vrange &r, ...)
>
>
> Of course, the very minimal fix is to just use !vr.undefined_p () to confirm
> the range is valid. Making sure the range's validity is not conditional on
> value_ok (or -Ofast), makes the logic easier to follow, as well as catching
> a missed optimization. No-one in future needs to puzzle why/how is this
> variable getting set with one set of options, but not another. It's only
> because gfortran can make gimple_match_range_of_expr return false,
> i.e. not initialize the range even though value_ok/itype_ok is false, that
> we can see the flawed/fragile logic.
Once place in match.pd does:
if (!gimple_match_range_of_expr (r, @0, @4))
r.set_varying (TREE_TYPE (@0));
Which solves the uninitialized issue.
>
> > > See the text of my explanation (especially the example) for how things
> > > can be improved (for -Ofast where value_ok is irrelevant).
> > >
> > > But I agree there's no point in having both itype_ok and value_ok.
> > >
> > > My original patch from 2024 didn’t have any of these problems.
> > >
> > >>> 2026-07-15 Roger Sayle <[email protected]>
> > >>>
> > >>> gcc/ChangeLog
> > >>> PR tree-optimization/126242
> > >>> * match.pd ((FTYPE) N CMP CST): Always attempt to initialize
> > >>> value range information. Check undefined_p before using range
> > >>> bounds.
> > >>>
> > >>> gcc/testsuite/ChangeLog
> > >>> PR tree-optimization/126242
> > >>> * gfortran.dg/pr126242.f90: New reduced test case.
> > >>> * gfortran.dg/pr41928-2.f90: Also compile pr41928.f90 with
> > >>> -Ofast.
> > >>>
> > >>>
> > >>> Thanks in advance (and my apologies for any inconvenience), Roger
> > >>> --
>
