Hi Andrea (and Jeff),
Yes, I agree these patches collide though they're really completely different
optimizations.
The use of ranger in my patch is purely to enable optimizations without the
need for -Ofast.
(float) x == 1.5 can never be true, but can't be optimized to false if the
conversion to float
can raise a inexact floating point exception. This is safe with either
-fno-trapping-math
or range information. This explains the comment in the code touched by
Eikansh's
patch:
/* TODO: allow non-fitting itype and SNaNs when
-fno-trapping-math. */
I'd recommend/suggest you approve/commit Eikansh's patch, and I'll respin my
patch on top
of those changes. This has been waiting for a couple of years, a few more days
won't hurt.
Thanks for averting the mid-air collision.
Cheers,
Roger
--
> -----Original Message-----
> From: Andrea Pinski <[email protected]>
> Sent: 19 June 2026 07:32
> To: Andrew Pinski <[email protected]>
> Cc: Roger Sayle <[email protected]>; GCC Patches <gcc-
> [email protected]>; Jeffrey Law <[email protected]>
> Subject: Re: [PATCH] PR tree-opt/57371: Improved comparison of integers to FP
> constants.
>
> On Mon, Jun 15, 2026 at 1:21 AM Andrew Pinski <[email protected]> wrote:
> >
> >
> >
> > On Mon, Jun 15, 2026, 1:09 AM Roger Sayle <[email protected]>
> wrote:
> >>
> >>
> >> Hi Jeff,
> >> I'd like to ping/progress a patch of mine from July 2024:
> >> https://gcc.gnu.org/pipermail/gcc-patches/2024-July/658482.html
> >> which was initially (positively) reviewed here:
> >> https://gcc.gnu.org/pipermail/gcc-patches/2024-August/659433.html
> >>
> >> The original motivation for this patch was Claudiu's issue/analysis
> >> at
> >> https://github.com/foss-for-synopsys-dwc-arc-processors/gcc/issues/11
> >> 8 which predicted a dramatic +26% performance improvement on EEMBC's
> >> iirflt01 benchmark.
> >>
> >> A quick summary is that GCC currently fails to optimize:
> >> int foo(long long x) { return x == 1.0; } to use an integral
> >> comparison (and avoid a floating point conversion) because all the
> >> values of long long can't be precisely represented as a (IEEE)
> >> double. This is too strict a condition, as only one value of a "long
> >> long" gets converted to 1.0 (even with overflow and rounding).
> >>
> >> The code review above agreed the implementation itself is reasonable
> >> and both machine and floating-point independent, the challenge is in
> >> writing test cases for the code, which unfortunately must assume a
> >> particular floating point format, and are therefore non-portable.
> >>
> >> To respond to Jeff's previous request/question, my preferred solution
> >> is rather than try to add "-mieee" to these new test cases, which
> >> should still pass even without strict IEEE (i.e. NaN) support, and
> >> therefore we should still be testing with -O2 default flags, but
> >> instead it makes sense for various backends to add additional
> >> tests/copies to their gcc.target directories including "-mieee",
> >> "-msoft-float", "-md-float", "-mg-float", "-mfpmath=387" etc. if they
> >> so wish. A failure of these tests does not indicate an issue with
> >> the implementation, which simply uses the current compile-time
> >> integer to FP (real) conversion support, but that that test hasn't
> >> been ported to the new/native floating point format (say FP8 or FP4).
> >>
> >> This patch has been retested 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
> >> (cleaning up any testsuite wrinkles if/as they're reported)?
> >>
> >>
> >> 2026-06-15 Roger Sayle <[email protected]>
> >>
> >> gcc/ChangeLog
> >> PR tree-optimization/57371
> >> * fold-const.cc (fold_cmp_float_cst_p): New helper function.
> >> * fold-const.h (fold_cmp_float_cst_p): Prototype here.
> >> * match.pd ((FTYPE) N CMP CST): Use ranger to determine
> >> whether value is exactly representable by floating point type,
> >> and check flag_trapping_math if not. Use the new helper
> >> fold_cmp_float_cst_p to check that transformation to an integer
> >> comparison is safe.
> >
> >
> > Hmm, I have someone working on a related patch and this will conflict.
> > We are changing (cmp (convert int) (float_cst)) to use the ranger and
> > adding a
> new function to the format helper to check if the range of the integer can
> fit into
> the floating point exactly.
> > The person working on this should be posting today or tomorrow.
> > Since I helped with that patch I can review this one too.
>
> See https://gcc.gnu.org/pipermail/gcc-patches/2026-June/720633.html .
> I do know there is some overlap between the 2 patches.
> Like for an example in your patch you do:
> ```
>
> + if (!vr0.undefined_p ())
> + {
> + lo = vr0.lower_bound ();
> + hi = vr0.upper_bound ();
> + int prec = (isign == SIGNED)
> + ? MAX (wi::min_precision (hi, SIGNED),
> + wi::min_precision (lo, SIGNED)) - 1
> + : wi::min_precision (hi, UNSIGNED);
> + wide_int nz = vr0.get_nonzero_bits ();
> + if (nz != 0)
> + prec -= wi::ctz (nz);
> + exact_p = prec <= significand_size (fmt); }
> ``
>
> While Eikansh does:
>
> +#if GIMPLE
> + int_range_max vr;
> + if (!value_ok
> + && gimple_match_range_of_expr (vr, @0, @2)) value_ok =
> +fmt.can_represent_integral_value_p (&vr); #endif
>
> And then can_represent_integral_value_p calls range_fits_type_p which is
> basically what you do here (but better):
> + int prec = (isign == SIGNED)
> + ? MAX (wi::min_precision (hi, SIGNED),
> + wi::min_precision (lo, SIGNED)) - 1
> + : wi::min_precision (hi, UNSIGNED);
> + wide_int nz = vr0.get_nonzero_bits ();
> + if (nz != 0)
> + prec -= wi::ctz (nz);
> + exact_p = prec <= significand_size (fmt);
>
>
>
>
> >
> > Thanks,
> > Andrea
> >
> >
> >>
> >> gcc/testsuite/ChangeLog
> >> PR tree-optimization/57371
> >> * c-c++-common/pr57371-6.c: New test case.
> >> * c-c++-common/pr57371-7.c: Likewise.
> >> * c-c++-common/pr57371-8.c: Likewise.
> >> * c-c++-common/pr57371-9.c: Likewise.
> >> * c-c++-common/pr57371-10.c: Likewise.
> >>
> >>
> >> Thanks in advance,
> >> Roger
> >> --
> >>
