On 7/28/24 6:01 AM, Roger Sayle wrote:
This patch improves the tree-level optimization of (fptype)ivar != CST
in match.pd (historically tracked under PR 57371). Joseph Myers'
description in comment #1 provides an excellent overview of the issues,
that historically it's the trapping behaviour of (fptype)ivar conversion
that is the primary concern, which is why the current code in match.pd
checks fmt.can_represent_integral_type_p (itype). The first of the
improvements with this patch is to check flag_trapping_math to control
whether FP_OVERFLOW/FP_INEXACT needs to be preserved, and to use
ranger to determine whether the bounds on ivar confirm that these
traps aren't possible. For example, the expression (int)var & 15
can't overflow conversion to IEEE float, even though the type of a
32-bit int could potentially overflow the significant of a 32-bit
float.
The next of the optimizations concern checking whether the comparison
against CST is unambiguous allowing it to be replaced with a integer
comparison. For reference, consider the table below which shows the
default conversion of integers to IEEE 32-bit float.
(float)16777211 => 16777211.0f;
(float)16777212 => 16777212.0f;
(float)16777213 => 16777213.0f;
(float)16777214 => 16777214.0f;
(float)16777215 => 16777215.0f;
(float)16777216 => 16777216.0f;
(float)16777217 => 16777216.0f; // rounded
(float)16777218 => 16777218.0f;
(float)16777219 => 16777220.0f; // rounded
(float)16777220 => 16777220.0f;
(float)16777221 => 16777220.0f; // rounded
(float)16777222 => 16777222.0f;
(float)16777223 => 16777224.0f; // rounded
(float)16777224 => 16777224.0f;
(float)16777225 => 16777224.0f; // rounded
(float)16777226 => 16777226.0f;
Observe that it's safe to optimize (float)i == 16777212.0f to the
equivalent i == 16777212 (as this is the only integer that can
convert to that floating point constant), but that it's unsafe to
optimize (float)i == 16777220.0f, as with default rounding there
are three possible integer values that FLOAT_EXPR to 16777220.0f.
The pragmatic check used in this patch is to confirm that (float)(i-1)
and (float)(i+1) are both distinct from (float)i before simplifying
the comparison to an integer-typed comparison.
Finally, this patch also handles non-default rounding modes.
In the table above, it's safe to optimize (float)i == 16777222.0f
in IEEE's default rounding mode, but not in all FP rounding modes.
This eventuality is handled by testing whether the (float)i, the
(float)(i-1) and the (float)(i+1) are all exactly rounded when
-frounding-math is specified.
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? If the testcases need to be
tweaked for non-IEEE targets (the transformations themselves should
be portable to VAX and IBM floating point formats) hopefully that
can be done as follow-up patches by folks with the appropriate
effective-targets?
2024-07-28 Roger Sayle <ro...@nextmovesoftware.com>
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.
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.
Nice. I was a bit concerned about using Ranger in match.pd as match.pd
can be used for GENERIC as well as GIMPLE. But it looks like you
handled that reasonably. Similarly for the other FP formats.
As you note, I wouldn't be terribly surprised if the other FP formats
need testsuite adjustments. I'd hoped we had a target selector, but we
don't.
Does it make sense to use "add_options_for_ieee" to conditionally add
the necessary target options in the tests? It only affects alpha, sh &
rx, so it's unlikely to have been needed in your testing.
Jeff
