[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #5 from Xi Ruoyao --- After r13-3924 this brings PR95115 back. Note that Glibc has added an ugly hack for RISC-V and old compilers, but other ports may be haunted as well.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 Richard Biener changed: What|Removed |Added Keywords||missed-optimization --- Comment #6 from Richard Biener --- (In reply to Jakub Jelinek from comment #0) > ... but then > comes dom2 and happily replaces > _1 = 3.4028234663852885981170418348451692544e+38 * 2.0e+0; > return _1; > with > _1 = 3.4028234663852885981170418348451692544e+38 * 2.0e+0; > return Inf; > (I think this is still correct) Note this is also a pessimization code-generation wise since if we preserve the multiplication the result is readily available in a register but as optimized we have another constant pool entry and load. So we might want to consider not propagating constants generated by operations we cannot eliminate. If the only consumer is a compare-and-branch we can of course still end up with a seemingly dead stmt, so this would be only for the missed optimization.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #7 from Aldy Hernandez --- (In reply to Richard Biener from comment #6) > (In reply to Jakub Jelinek from comment #0) > > ... but then > > comes dom2 and happily replaces > > _1 = 3.4028234663852885981170418348451692544e+38 * 2.0e+0; > > return _1; > > with > > _1 = 3.4028234663852885981170418348451692544e+38 * 2.0e+0; > > return Inf; > > (I think this is still correct) > > Note this is also a pessimization code-generation wise since if we > preserve the multiplication the result is readily available in a > register but as optimized we have another constant pool entry and load. > > So we might want to consider not propagating constants generated by > operations > we cannot eliminate. If the only consumer is a compare-and-branch we > can of course still end up with a seemingly dead stmt, so this would be only > for the missed optimization. Up to y'all if this is the way to go, but here are some thoughts... Off the top of my head, we have VRP and DOM propagating constants. Technically also simplify_using_ranges, but it's only called from VRP/DOM, and it currently only works with integers, so we should be ok here. I think we could limit propagation in may_propagate_copy() which both VRP and DOM gate on. VRP uses it through its use of substitute_and_fold_engine and DOM uses it directly. Would this work? Would we need to pass an additional argument to may_propagate_copy() (edge or statement) or can we determine validity by only looking at: bool may_propagate_copy (tree dest, tree orig, bool dest_not_phi_arg_p)
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #8 from Richard Biener --- (In reply to Aldy Hernandez from comment #7) > (In reply to Richard Biener from comment #6) > > (In reply to Jakub Jelinek from comment #0) > > > ... but then > > > comes dom2 and happily replaces > > > _1 = 3.4028234663852885981170418348451692544e+38 * 2.0e+0; > > > return _1; > > > with > > > _1 = 3.4028234663852885981170418348451692544e+38 * 2.0e+0; > > > return Inf; > > > (I think this is still correct) > > > > Note this is also a pessimization code-generation wise since if we > > preserve the multiplication the result is readily available in a > > register but as optimized we have another constant pool entry and load. > > > > So we might want to consider not propagating constants generated by > > operations > > we cannot eliminate. If the only consumer is a compare-and-branch we > > can of course still end up with a seemingly dead stmt, so this would be only > > for the missed optimization. > > Up to y'all if this is the way to go, but here are some thoughts... > > Off the top of my head, we have VRP and DOM propagating constants. > Technically also simplify_using_ranges, but it's only called from VRP/DOM, > and it currently only works with integers, so we should be ok here. > > I think we could limit propagation in may_propagate_copy() which both VRP > and DOM gate on. VRP uses it through its use of substitute_and_fold_engine > and DOM uses it directly. Would this work? I don't think may_propagate_copy is the correct vehicle. Instead the substitute_and_fold_engine could only substitute from defs with no side-effects - IIRC it already refrains from propagating _into_ defs that will be removed after the propagation.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #9 from Aldy Hernandez --- (In reply to Richard Biener from comment #8) > (In reply to Aldy Hernandez from comment #7) > > (In reply to Richard Biener from comment #6) > > > (In reply to Jakub Jelinek from comment #0) > > > > ... but then > > > > comes dom2 and happily replaces > > > > _1 = 3.4028234663852885981170418348451692544e+38 * 2.0e+0; > > > > return _1; > > > > with > > > > _1 = 3.4028234663852885981170418348451692544e+38 * 2.0e+0; > > > > return Inf; > > > > (I think this is still correct) > > > > > > Note this is also a pessimization code-generation wise since if we > > > preserve the multiplication the result is readily available in a > > > register but as optimized we have another constant pool entry and load. > > > > > > So we might want to consider not propagating constants generated by > > > operations > > > we cannot eliminate. If the only consumer is a compare-and-branch we > > > can of course still end up with a seemingly dead stmt, so this would be > > > only > > > for the missed optimization. > > > > Up to y'all if this is the way to go, but here are some thoughts... > > > > Off the top of my head, we have VRP and DOM propagating constants. > > Technically also simplify_using_ranges, but it's only called from VRP/DOM, > > and it currently only works with integers, so we should be ok here. > > > > I think we could limit propagation in may_propagate_copy() which both VRP > > and DOM gate on. VRP uses it through its use of substitute_and_fold_engine > > and DOM uses it directly. Would this work? > > I don't think may_propagate_copy is the correct vehicle. Instead the > substitute_and_fold_engine could only substitute from defs with no > side-effects - IIRC it already refrains from propagating _into_ defs > that will be removed after the propagation. So where do you suggest we clamp this? The uses I can think of are VRP (various places in tree-ssa-propagate.cc) and DOM (cprop_operand).
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
--- Comment #10 from Jakub Jelinek ---
Some extra food for thought:
void bar (void);
void
foo (double x)
{
if (x >= -16.0 && x <= 16.0)
{
double y = x + 32.0;
double z = y * 42.5;
if (z < 600.0 || z > 3000.0)
bar ();
}
}
with the usual default -O2 aka -O2 -ftrapping-math.
frange can correctly prove that bar () will be never called and with -O2
-fno-trapping-math it is perfectly fine to optimize the whole function out, z
is known to be [680., 2040.] and not NaN.
Now, even the comparisons aren't strictly needed, comparisons trap only on NaNs
(< and > are not quiet, so any kind of them, but after all, frange doesn't
track sNaNs vs. qNaNs) and we know z is not NaN. But x comparisons can raise
invalid on NaNs (both qNaN and sNaN), the addition is known not to raise
invalid (x is not NaN), nor overflow (limited range), not sure right now if it
can raise underflow or inexact, but at least the latter quite possibly. The
multiplication I'm quite sure can raise inexact though,
so I think we need to keep everything until the z = computation, and either
replace the comparison(s) of z with some dummy (asm?) use of z, or keep the
comparisons but say turn the bar () call into __builtin_unreachable () or
__builtin_trap () and make sure we don't optimize away the former later?
The reason I want to show this is mainly that even when the actual operation
(comparisons here) we'd like to fold into constant are known not to raise any
exceptions (and we should use frange info to find that out), it might be some
intermediate calculation that might still raise exceptions.
I was considering to do some hack at least in my Fedora test mass rebuilds this
month like for flag_trapping_math pretend no floating point range is singleton,
but that still wouldn't cover comparisons.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
--- Comment #11 from Richard Biener ---
(In reply to Jakub Jelinek from comment #10)
> Some extra food for thought:
> void bar (void);
>
> void
> foo (double x)
> {
> if (x >= -16.0 && x <= 16.0)
> {
> double y = x + 32.0;
> double z = y * 42.5;
> if (z < 600.0 || z > 3000.0)
> bar ();
> }
> }
> with the usual default -O2 aka -O2 -ftrapping-math.
> frange can correctly prove that bar () will be never called and with -O2
> -fno-trapping-math it is perfectly fine to optimize the whole function out,
> z is known to be [680., 2040.] and not NaN.
> Now, even the comparisons aren't strictly needed, comparisons trap only on
> NaNs
> (< and > are not quiet, so any kind of them, but after all, frange doesn't
> track sNaNs vs. qNaNs) and we know z is not NaN. But x comparisons can
> raise invalid on NaNs (both qNaN and sNaN), the addition is known not to
> raise invalid (x is not NaN), nor overflow (limited range), not sure right
> now if it can raise underflow or inexact, but at least the latter quite
> possibly. The multiplication I'm quite sure can raise inexact though,
> so I think we need to keep everything until the z = computation, and either
> replace the comparison(s) of z with some dummy (asm?) use of z, or keep the
> comparisons but say turn the bar () call into __builtin_unreachable () or
> __builtin_trap () and make sure we don't optimize away the former later?
>
> The reason I want to show this is mainly that even when the actual operation
> (comparisons here) we'd like to fold into constant are known not to raise
> any exceptions (and we should use frange info to find that out), it might be
> some intermediate calculation that might still raise exceptions.
>
> I was considering to do some hack at least in my Fedora test mass rebuilds
> this month like for flag_trapping_math pretend no floating point range is
> singleton, but that still wouldn't cover comparisons.
For the comparisons I think we need to split them out of the GIMPLE_COND
(who do not have side-effects), so like with -fnon-call-exceptions do
_3 = z < 6.0e+2;
if (_3 != 0) goto ; else goto ;
then we can simplify the GIMPLE_CONDs just fine, we just need to keep
the comparison in some way.
For sNaN vs. qNaN I suppose we can use a tree predicate and go by
the kind of definition we see - sNaNs can only appear via a few
constrained operations (loads and init from a sNaN literal).
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #12 from Aldy Hernandez --- (In reply to Richard Biener from comment #6) > (In reply to Jakub Jelinek from comment #0) > > ... but then > > comes dom2 and happily replaces > > _1 = 3.4028234663852885981170418348451692544e+38 * 2.0e+0; > > return _1; > > with > > _1 = 3.4028234663852885981170418348451692544e+38 * 2.0e+0; > > return Inf; > > (I think this is still correct) > > Note this is also a pessimization code-generation wise since if we > preserve the multiplication the result is readily available in a > register but as optimized we have another constant pool entry and load. > > So we might want to consider not propagating constants generated by > operations > we cannot eliminate. If the only consumer is a compare-and-branch we > can of course still end up with a seemingly dead stmt, so this would be only > for the missed optimization. [Sorry for the delayed response. I've been on PTO.] For the original testcase, the propagation happens in DOM: [local count: 1073741824]: _1 = 3.4028234663852885981170418348451692544e+38 * 2.0e+0; return _1; range_of_expr gets called on the return's _1 and correctly returns Inf, which allows cprop_operand to do the replacement. If I understand correctly you're suggesting not propagating constants that were generated by an operation we can't eliminate. In this case, it'd be easy to chase the DEF back to the offending _1 definition (from cprop_operand and every other places where we do propagations based on range_of_expr's result), but ranger doesn't keep track of how it got to an answer, so we'd have to chase all operands used to generate _1?? That'd get hairy pretty fast, unless I'm misunderstanding something. It really looks like the problem here is DCE (and the gimplifier as you point out in comment #2), which is removing a needed statement. Can't this be fixed there?
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #13 from Richard Biener --- (In reply to Aldy Hernandez from comment #12) > (In reply to Richard Biener from comment #6) > > (In reply to Jakub Jelinek from comment #0) > > > ... but then > > > comes dom2 and happily replaces > > > _1 = 3.4028234663852885981170418348451692544e+38 * 2.0e+0; > > > return _1; > > > with > > > _1 = 3.4028234663852885981170418348451692544e+38 * 2.0e+0; > > > return Inf; > > > (I think this is still correct) > > > > Note this is also a pessimization code-generation wise since if we > > preserve the multiplication the result is readily available in a > > register but as optimized we have another constant pool entry and load. > > > > So we might want to consider not propagating constants generated by > > operations > > we cannot eliminate. If the only consumer is a compare-and-branch we > > can of course still end up with a seemingly dead stmt, so this would be only > > for the missed optimization. > > [Sorry for the delayed response. I've been on PTO.] > > For the original testcase, the propagation happens in DOM: > >[local count: 1073741824]: > _1 = 3.4028234663852885981170418348451692544e+38 * 2.0e+0; > return _1; > > range_of_expr gets called on the return's _1 and correctly returns Inf, > which allows cprop_operand to do the replacement. > > If I understand correctly you're suggesting not propagating constants that > were generated by an operation we can't eliminate. In this case, it'd be > easy to chase the DEF back to the offending _1 definition (from > cprop_operand and every other places where we do propagations based on > range_of_expr's result), but ranger doesn't keep track of how it got to an > answer, so we'd have to chase all operands used to generate _1?? That'd get > hairy pretty fast, unless I'm misunderstanding something. Yes, the fact that ranger doesn't operate as a usual propagator with a lattice makes things very difficult here. Note that my comment referred to code optimality, not correctness. > It really looks like the problem here is DCE (and the gimplifier as you > point out in comment #2), which is removing a needed statement. Can't this > be fixed there? Sure it can, but the expense is that we'd do constant folding all the way down and not remove dead code which will result in _tons_ of unnecessary constant pool entries and loads. The issue is also that -ftrapping-math is default on so we'd have to do this by default. Ugh. Note that the constant folding routines generally refrain from folding when that loses exceptions, it's just ranger when producing singleton ranges and propagating from them that doesn't adhere to that implicit rule.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #14 from Richard Biener --- const_binop has /* Don't constant fold this floating point operation if the result has overflowed and flag_trapping_math. */ if (flag_trapping_math && MODE_HAS_INFINITIES (mode) && REAL_VALUE_ISINF (result) && !REAL_VALUE_ISINF (d1) && !REAL_VALUE_ISINF (d2)) return NULL_TREE;
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #15 from Aldy Hernandez --- (In reply to Richard Biener from comment #13) > Note that the constant folding routines generally refrain from folding > when that loses exceptions, it's just ranger when producing singleton > ranges and propagating from them that doesn't adhere to that implicit rule. We already have similar restrictions to avoid folding relational operators when NANs are present. I suppose we could add the same restriction to the generic frange folder (range_operator_float::fold_range) as the const_binop snippet you quoted down-thread. Thanks for the explanation and the const_binop example. This makes it much clearer.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #16 from Aldy Hernandez --- Created attachment 54224 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54224&action=edit untested patch Perhaps this would work. It solves the testcase, though I think we should probably audit the operators that don't use the generic range_operator_float::fold_range to make sure they're not doing anything silly. Note that we could add similar code whenever we drop to a NAN like const_binop does: /* Don't constant fold this floating point operation if both operands are not NaN but the result is NaN, and flag_trapping_math. Such operations should raise an invalid operation exception. */ if (flag_trapping_math && MODE_HAS_NANS (mode) && REAL_VALUE_ISNAN (result) && !REAL_VALUE_ISNAN (d1) && !REAL_VALUE_ISNAN (d2)) return NULL_TREE; I avoided doing so because an frange of NAN does not count as a singleton so it should never be propagated. If this is a problem, I could add a similar tweak. What do y'all think?
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #17 from Richard Biener --- (In reply to Aldy Hernandez from comment #16) > Created attachment 54224 [details] > untested patch > > Perhaps this would work. It solves the testcase, though I think we should > probably audit the operators that don't use the generic > range_operator_float::fold_range to make sure they're not doing anything > silly. > > Note that we could add similar code whenever we drop to a NAN like > const_binop does: > > /* Don't constant fold this floating point operation if >both operands are not NaN but the result is NaN, and >flag_trapping_math. Such operations should raise an >invalid operation exception. */ > if (flag_trapping_math > && MODE_HAS_NANS (mode) > && REAL_VALUE_ISNAN (result) > && !REAL_VALUE_ISNAN (d1) > && !REAL_VALUE_ISNAN (d2)) > return NULL_TREE; > > I avoided doing so because an frange of NAN does not count as a singleton so > it should never be propagated. If this is a problem, I could add a similar > tweak. > > What do y'all think? I suppose it's a good workaround for now and consistent with what CCP would end up doing. Ideally we wouldn't restrict lattice propagation so we can perform simplification of uses (esp. in conditionals) but just prevent folding the actual computation that causes the overflow (thus catch it at propagation stage). As you say how ranger works doesn't easily allow that, unfortunately.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #18 from Jakub Jelinek --- See #c10, I think even with comparisons we need to be careful. One thing is whether we can prove one of the branches will be unreachable, we can do that and replace that branch with __builtin_unreachable, but if trapping math is on, if a comparison is possibly trapping (operands could be NAN as checked by frange) or if it is the last use of some SSA_NAME that needs to be kept live because its computation is possibly trapping, I think we need to preserve the comparison. One question is if the default -ftrapping-math should cover all exceptions or just the some subset, invalid/overflow are the worst ones, then underflow and least importance is inexact IMHO, pretty much anything can be inexact...
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
--- Comment #19 from rguenther at suse dot de ---
On Tue, 10 Jan 2023, jakub at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
>
> --- Comment #18 from Jakub Jelinek ---
> See #c10, I think even with comparisons we need to be careful. One thing is
> whether we can prove one of the branches will be unreachable, we can do that
> and replace that branch with __builtin_unreachable, but if trapping math is
> on,
> if a comparison is possibly trapping (operands could be NAN as checked by
> frange) or if it is the last use of some SSA_NAME that needs to be kept live
> because its computation is possibly trapping, I think we need to preserve the
> comparison.
> One question is if the default -ftrapping-math should cover all exceptions or
> just the some subset, invalid/overflow are the worst ones, then underflow and
> least importance is inexact IMHO, pretty much anything can be inexact...
I suppose similar to -fdelete-dead-exceptions with EH we need some
-fdelete-dead-traps (that's bad wording) and at least have that on by
default. I think the standard mandates #pragma FENV_ACCESS on if you
want to inspect the IEEE exception state, and our defaulting mix
(-ftrapping-math but -fno-rounding-math) doesn't make sense in this
respect and the documentation is not clear (or my english is bad) here:
"This command-line option
will be used along with @option{-frounding-math} to specify the
default state for @code{FENV_ACCESS}."
How does "along with" specify how the "default state for FENV_ACCESS"
is "computed"?
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
--- Comment #20 from Jakub Jelinek ---
(In reply to Aldy Hernandez from comment #16)
> Created attachment 54224 [details]
> untested patch
>
> Perhaps this would work. It solves the testcase, though I think we should
> probably audit the operators that don't use the generic
> range_operator_float::fold_range to make sure they're not doing anything
> silly.
Even as a workaround this seems to be quite a big hammer.
If we want to preserve overflow traps, all we need to arrange is that if
non-inf operands result in singleton inf we don't treat that result as
singleton.
Now, what result one gets in different rounding modes depends on the rounding
mode,
in round to nearest it should be +-inf, in round to zero +-max, in round to
+inf +inf or -max and in round to -inf -inf or +max. But right now GCC doesn't
handle the separate rounding modes, it just differentiates between
-fno-rounding-math where we assume round to nearest and -frounding-math where
we should consider any rounding mode.
I think for -frounding-math we already don't treat such results as singletons,
as we
end up with ranges like [+max, +inf] or [-inf, -max].
So, one possible way for -fno-rounding-math -ftrapping-math could be instead of
making
the result VARYING just extend the range by one ulp towards 0, i.e. instead of
singleton
[+inf, +inf] use [+max, +inf] etc.
Another would be to add some bool flag to frange which would say this is never
a singleton and just take that flag into account, though perhaps it is too
risky right now.
As for invalid exceptions, that implies result maybe or known NAN, but we don't
treat
maybe or known NAN as singletons, do we? After all, there isn't just a single
NAN and we don't know which one the result is. That doesn't mean we handle all
cases right, say
if a result of something is only used in __builtin_isnan or similar, we can
still happily optimized it away.
As for underflow exceptions, I've tried to construct a testcase but seems we
didn't care already in older GCC versions.
Say for float foo (void) { float x = __FLT_MIN__; return x * x; } without
-frounding-math we already optimized it in ccp1 in GCC 12 and older, with
-frounding-math again we should be fine because the result isn't singleton.
Again with the problem that if the result is in the end used in some comparison
or test that will not care about details, like if (__builtin_fabsf (foo ()) <
1.0f) then again we optimize away the trapping operation and we didn't
previously.
And inexact exceptions I think is something we just basically didn't care at
all before, those can happen pretty much all the time.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #21 from Andrew Macleod --- (In reply to Richard Biener from comment #13) > Yes, the fact that ranger doesn't operate as a usual propagator with a > lattice > makes things very difficult here. Note that my comment referred to code > optimality, not correctness. > > > It really looks like the problem here is DCE (and the gimplifier as you > > point out in comment #2), which is removing a needed statement. Can't this > > be fixed there? > > Sure it can, but the expense is that we'd do constant folding all the way > down and not remove dead code which will result in _tons_ of unnecessary > constant pool entries and loads. > I think I'm missing something, or not understanding what you are saying. Why is the propagation or lack of lattice a problem? Its DCE that is removing that potentially trapping stmt because its no longer used in the IL? THe change would be to not kill off dead statements that may trap? I guess this may leave a bunch of feeding statements that are not dead.. but I fail to see how thats different than not propagating and then not being able to delete those stmts either? > The issue is also that -ftrapping-math is default on so we'd have to > do this by default. Ugh. > > Note that the constant folding routines generally refrain from folding > when that loses exceptions, it's just ranger when producing singleton > ranges and propagating from them that doesn't adhere to that implicit rule. I'm also not sure what this means. I don't think ranger itself propagates singleton constants.. VRP is still using the substitute_and_fold engine, so any folding/propagation is still going through the same mechanisms we always did when a singleton result is produced. We just produce more of them now, especially with floats. I don't think ranger is doing anything different than VRP ever did regarding propagation. Its possible GCCs infrastructure for dealing with float propagation isn't mature enough perhaps?
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
--- Comment #22 from Aldy Hernandez ---
(In reply to Jakub Jelinek from comment #20)
> (In reply to Aldy Hernandez from comment #16)
> > Created attachment 54224 [details]
> > untested patch
> >
> > Perhaps this would work. It solves the testcase, though I think we should
> > probably audit the operators that don't use the generic
> > range_operator_float::fold_range to make sure they're not doing anything
> > silly.
>
> Even as a workaround this seems to be quite a big hammer.
> If we want to preserve overflow traps, all we need to arrange is that if
> non-inf operands result in singleton inf we don't treat that result as
> singleton.
> Now, what result one gets in different rounding modes depends on the
> rounding mode,
> in round to nearest it should be +-inf, in round to zero +-max, in round to
> +inf +inf or -max and in round to -inf -inf or +max. But right now GCC
> doesn't handle the separate rounding modes, it just differentiates between
> -fno-rounding-math where we assume round to nearest and -frounding-math
> where we should consider any rounding mode.
Note that we currently can't represent +-inf or +-max, as we only have two
endpoints. So that would just be represented as VARYING.
> I think for -frounding-math we already don't treat such results as
> singletons, as we
> end up with ranges like [+max, +inf] or [-inf, -max].
> So, one possible way for -fno-rounding-math -ftrapping-math could be instead
> of making
> the result VARYING just extend the range by one ulp towards 0, i.e. instead
> of singleton
> [+inf, +inf] use [+max, +inf] etc.
This seems reasonable. So instead of set_varying(), we could do [+max, +inf],
etc.
> Another would be to add some bool flag to frange which would say this is
> never a singleton and just take that flag into account, though perhaps it is
> too risky right now.
That seems easy to get wrong, especially this late in the cycle.
>
> As for invalid exceptions, that implies result maybe or known NAN, but we
> don't treat
> maybe or known NAN as singletons, do we? After all, there isn't just a
> single NAN and we don't know which one the result is. That doesn't mean we
> handle all cases right, say
> if a result of something is only used in __builtin_isnan or similar, we can
> still happily optimized it away.
NANs are never singletons, and maybe_nans either. See frange::singleton_p:
if (m_kind == VR_RANGE && real_identical (&m_min, &m_max))
{
// Return false for any singleton that may be a NAN.
if (HONOR_NANS (m_type) && maybe_isnan ())
return false;
...
}
Also, all the conditional operators in frange fail to fold if maybe_isnan. The
only things we fold for sure are:
a) One operand is a known NAN.
b) None of the operands can ever be a NAN *and* we know the answer to the
conditional.
For example, foperator_gt::fold_range:
...
...
if (op1.known_isnan () || op2.known_isnan ())
r = range_false (type);
else if (!maybe_isnan (op1, op2))
{
if (real_compare (LE_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
r = range_true (type);
else if (!real_compare (LE_EXPR, &op1.lower_bound (), &op2.upper_bound
()))
r = range_false (type);
else
r = range_true_and_false (type);
}
so... we're pretty careful about NOT folding relationals that have the
possibility of a NAN, and a singleton is only for a known range without a NAN.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
Jakub Jelinek changed:
What|Removed |Added
CC||jsm28 at gcc dot gnu.org
--- Comment #23 from Jakub Jelinek ---
Sure, VRP and DOM etc. have been replacing operations with singleton results
for quite a while, but most of integer arithmetics don't cause exceptions which
one can test or handle through signals. As has been said above, we have
-fdelete-dead-exceptions option and we might want to consider similar option
for the floating point traps.
Without VRP and similar optimizations, both have some chances to work fine, say
if I have:
void
foo (int *p, int *q)
{
int r = *p + *q;
}
and -fdelete-dead-exceptions -fnon-call-exceptions, the *p and *q reads could
cause
an exception, but as user said -fdelete-dead-exceptions and nothing uses the
result,
it is fine not to dereference those.
A different question is:
int
bar (int *p, int *q)
{
int pv = *p;
int qv = *q;
if (pv != 0) __builtin_unreachable ();
if (qv != 0) __builtin_unreachable ();
int r = pv + qv;
return r;
}
Here we actually are using what has been read from the memory, so it is much
more fuzzy
if we can still claim the exceptions are dead.
And I think the frange vs. exceptions is exactly the same thing, though much
more often happening in real-world code. So we'd need to figure out what the
definition for "contribute to the execution of the program" actually is...
With frange singleton folding, we could optimize tons of operations which each
could raise some exceptions, just because we could prove what the final result
would be. Does that count as a use of those values or not?
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
--- Comment #24 from Aldy Hernandez ---
(In reply to Andrew Macleod from comment #21)
> (In reply to Richard Biener from comment #13)
>
> > Yes, the fact that ranger doesn't operate as a usual propagator with a
> > lattice
> > makes things very difficult here. Note that my comment referred to code
> > optimality, not correctness.
> >
> > > It really looks like the problem here is DCE (and the gimplifier as you
> > > point out in comment #2), which is removing a needed statement. Can't
> > > this
> > > be fixed there?
> >
> > Sure it can, but the expense is that we'd do constant folding all the way
> > down and not remove dead code which will result in _tons_ of unnecessary
> > constant pool entries and loads.
> >
>
> I think I'm missing something, or not understanding what you are saying.
>
> Why is the propagation or lack of lattice a problem? Its DCE that is
> removing that potentially trapping stmt because its no longer used in the
> IL? THe change would be to not kill off dead statements that may trap? I
> guess this may leave a bunch of feeding statements that are not dead.. but I
> fail to see how thats different than not propagating and then not being able
> to delete those stmts either?
>
>
> > The issue is also that -ftrapping-math is default on so we'd have to
> > do this by default. Ugh.
> >
> > Note that the constant folding routines generally refrain from folding
> > when that loses exceptions, it's just ranger when producing singleton
> > ranges and propagating from them that doesn't adhere to that implicit rule.
>
> I'm also not sure what this means. I don't think ranger itself propagates
> singleton constants.. VRP is still using the substitute_and_fold engine, so
> any folding/propagation is still going through the same mechanisms we always
> did when a singleton result is produced. We just produce more of them now,
> especially with floats. I don't think ranger is doing anything different
> than VRP ever did regarding propagation. Its possible GCCs infrastructure
> for dealing with float propagation isn't mature enough perhaps?
Right, ranger isn't propagating anything. It's the substitute_and_fold engine,
as it always has, but only if value_of_expr() is non-NULL. Currently this
function will only return non-NULL for singletons:
tree
range_query::value_of_expr (tree expr, gimple *stmt)
{
tree t;
if (!Value_Range::supports_type_p (TREE_TYPE (expr)))
return NULL_TREE;
Value_Range r (TREE_TYPE (expr));
if (range_of_expr (r, expr, stmt))
{
// A constant used in an unreachable block oftens returns as UNDEFINED.
// If the result is undefined, check the global value for a constant.
if (r.undefined_p ())
range_of_expr (r, expr);
if (r.singleton_p (&t))
return t;
}
return NULL_TREE;
}
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #25 from Jakub Jelinek --- (In reply to Aldy Hernandez from comment #22) > Note that we currently can't represent +-inf or +-max, as we only have two > endpoints. So that would just be represented as VARYING. By +-inf I meant either [+inf, +inf] or [-inf, -inf], sorry for the shorthand. Sure, if we don't know if it is positive or negative infinity, right now it will be VARYING or close to VARYING and so nothing close to singleton.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 Aldy Hernandez changed: What|Removed |Added Attachment #54224|0 |1 is obsolete|| --- Comment #26 from Aldy Hernandez --- Created attachment 54253 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54253&action=edit proposed patch So where are we on this? Do we want to do something like this for this release, or something else entirely? This is a variation of the previous patch that instead of setting varying, avoids the singleton by returning: [INF, INF] => [+MAX, +INF] [-INF, -INF] => [-INF, -MIN]
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #27 from Jakub Jelinek --- "elide an overflow" should be probably "elide an overflow or division by zero" I think, because finite / 0.0 returns infinity and raises FE_DIVBYZERO rather than FE_OVERFLOW, even when it returns infinity from finite operands. Seems for infinity / 0.0 no exception is raised, so the finite operands infinite result condition seems to be sufficient. For GCC 13, I think it is important that we e.g. don't miscompile glibc libm, so the libm testsuite should be clean. PR107967 fixed some of the failures, and some were claimed to be dups of this PR. So, would be nice to test GCC with your patch on glibc + libm testsuite. Just CC=/path/to/patched-gcc-trunk/gcc CXX=/path/to/patched-gcc-trunk/g++ ../configure --prefix=/usr CC=/path/to/patched-gcc-trunk/gcc CXX=/path/to/patched-gcc-trunk/g++ make -jN CC=/path/to/patched-gcc-trunk/gcc CXX=/path/to/patched-gcc-trunk/g++ make -jN check should be enough in latest glibc (and perhaps compare that to GCC 12).
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #28 from Xi Ruoyao --- (In reply to Jakub Jelinek from comment #27) > For GCC 13, I think it is important that we e.g. don't miscompile glibc > libm, so > the libm testsuite should be clean. PR107967 fixed some of the failures, > and some were claimed to be dups of this PR. So, would be nice to test GCC > with your patch on glibc + libm testsuite. Some bugs are hidden on x86/x86_64 because glibc uses more assembly for FP than oother ports. It's better to try on RISC-V etc. > Just > CC=/path/to/patched-gcc-trunk/gcc CXX=/path/to/patched-gcc-trunk/g++ > ../configure --prefix=/usr > CC=/path/to/patched-gcc-trunk/gcc CXX=/path/to/patched-gcc-trunk/g++ make -jN > CC=/path/to/patched-gcc-trunk/gcc CXX=/path/to/patched-gcc-trunk/g++ make > -jN check > should be enough in latest glibc (and perhaps compare that to GCC 12).
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #29 from Aldy Hernandez --- (In reply to Jakub Jelinek from comment #27) > "elide an overflow" should be probably "elide an overflow or division by > zero" I think, > because finite / 0.0 returns infinity and raises FE_DIVBYZERO rather than > FE_OVERFLOW, > even when it returns infinity from finite operands. > Seems for infinity / 0.0 no exception is raised, so the finite operands > infinite result condition seems to be sufficient. Fixed. > > For GCC 13, I think it is important that we e.g. don't miscompile glibc > libm, so > the libm testsuite should be clean. PR107967 fixed some of the failures, > and some were claimed to be dups of this PR. So, would be nice to test GCC > with your patch on glibc + libm testsuite. > Just > CC=/path/to/patched-gcc-trunk/gcc CXX=/path/to/patched-gcc-trunk/g++ > ../configure --prefix=/usr > CC=/path/to/patched-gcc-trunk/gcc CXX=/path/to/patched-gcc-trunk/g++ make -jN > CC=/path/to/patched-gcc-trunk/gcc CXX=/path/to/patched-gcc-trunk/g++ make > -jN check > should be enough in latest glibc (and perhaps compare that to GCC 12). I ran tests on glibc from git sources and compared the tests.sum files left behind from a "make check -k -jN". There don't seem to be any regressions. For that matter, it looks like a handful of tests get fixed by the proposed patch: -FAIL: math/test-double-lgamma -FAIL: math/test-double-log1p -FAIL: math/test-float-lgamma -FAIL: math/test-float-log1p -FAIL: math/test-float128-catan -FAIL: math/test-float128-catanh -FAIL: math/test-float128-lgamma -FAIL: math/test-float128-log -FAIL: math/test-float128-log1p -FAIL: math/test-float128-y0 -FAIL: math/test-float128-y1 -FAIL: math/test-float32-lgamma -FAIL: math/test-float32-log1p -FAIL: math/test-float32x-lgamma -FAIL: math/test-float32x-log1p -FAIL: math/test-float64-lgamma -FAIL: math/test-float64-log1p -FAIL: math/test-float64x-lgamma -FAIL: math/test-ldouble-lgamma
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 Aldy Hernandez changed: What|Removed |Added Attachment #54253|0 |1 is obsolete|| --- Comment #30 from Aldy Hernandez --- Created attachment 54266 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54266&action=edit proposed patch There seem to be some nonsensical (??unrelated??) regressions on a GCC testrun: > FAIL: c-c++-common/diagnostic-format-sarif-file-4.c -std=gnu++14 > scan-sarif-file "text": " int u6587u5b57u5316u3051 = > FAIL: c-c++-common/diagnostic-format-sarif-file-4.c -std=gnu++17 > scan-sarif-file "text": " int u6587u5b57u5316u3051 = > FAIL: c-c++-common/diagnostic-format-sarif-file-4.c -std=gnu++20 > scan-sarif-file "text": " int u6587u5b57u5316u3051 = > FAIL: c-c++-common/diagnostic-format-sarif-file-4.c -std=gnu++98 > scan-sarif-file "text": " int u6587u5b57u5316u3051 = > FAIL: g++.dg/pr71488.C (test for excess errors) > FAIL: g++.dg/guality/pr55665.C -O2 -flto -fno-use-linker-plugin > -flto-partition=none line 23 p == 40 < FAIL: gcc.dg/fold-overflow-1.c scan-assembler-times 2139095040 2 > FAIL: c-c++-common/diagnostic-format-sarif-file-4.c -Wc++-compat > scan-sarif-file "text": " int u6587u5b57u5316u3051 = < FAIL: g++.dg/pr71488.C (test for excess errors) < FAIL: g++.dg/guality/pr55665.C -O2 -flto -fno-use-linker-plugin -flto-partition=none line 23 p == 40 > FAIL: ./index0-out.go execution, -O0 -g -fno-var-tracking-assignments > FAIL: go.test/test/fixedbugs/issue27836.dir/Äfoo.go -O -I. (test for excess > errors) > FAIL: go.test/test/fixedbugs/issue27836.dir/Ämain.go -O -I. (test for > excess errors) The g++.dg/pr71488.C results look exactly the same when examining the g++.log file. go always acts up for me, so I have no idea if this is legitimate: spawn -ignore SIGHUP /opt/notnfs/aldyh/bld/autobuilds/4081bed04fc/bld/gcc/testsuite/go3/../../gccgo -B/opt/notnfs/aldyh/bld/autobuilds/4081bed04fc/bld/gcc/testsuite/go3/../../ -fdiagnostics-plain-output -I/opt/notnfs/aldyh/bld/autobuilds/4081bed04fc/bld/x86_64-pc-linux-gnu/./libgo -O -I. -w -pedantic-errors -c -o Äfoo.o /home/aldyh/bld/autobuilds/4081bed04fc/src/gcc/testsuite/go.test/test/fixedbugs/issue27836.dir/Äfoo.go go1: fatal error: cannot open /home/aldyh/bld/autobuilds/4081bed04fc/src/gcc/testsuite/go.test/test/fixedbugs/issue27836.dir/Ãfoo.go: No such file or directory compilation terminated. compiler exited with status 1 FAIL: go.test/test/fixedbugs/issue27836.dir/Äfoo.go -O -I. (test for excess errors) Excess errors: go1: fatal error: cannot open /home/aldyh/bld/autobuilds/4081bed04fc/src/gcc/testsuite/go.test/test/fixedbugs/issue27836.dir/Ãfoo.go: No such file or directory compilation terminated. I think this is all testsuite noise, but I'm not sure. As mentioned, there are no glibc regressions, only fixes.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #31 from CVS Commits --- The master branch has been updated by Aldy Hernandez : https://gcc.gnu.org/g:844190af178c210a6aff6b7eb4dd8c6a49210ff9 commit r13-5179-g844190af178c210a6aff6b7eb4dd8c6a49210ff9 Author: Aldy Hernandez Date: Tue Jan 10 10:57:16 2023 +0100 [PR107608] [range-ops] Avoid folding into INF when flag_trapping_math. As discussed in the PR, for trapping math, do not fold overflowing operations into +-INF as doing so could elide a trap. There is a minor adjustment to known_isinf() where it was mistakenly returning true for an [infinity U NAN], whereas it should only return true when the range is exclusively +INF or -INF. This is benign, as there were no users of known_isinf up to now. Tested on x86-64 Linux. I also ran the glibc testsuite (git sources) on x86-64 and this patch fixes: -FAIL: math/test-double-lgamma -FAIL: math/test-double-log1p -FAIL: math/test-float-lgamma -FAIL: math/test-float-log1p -FAIL: math/test-float128-catan -FAIL: math/test-float128-catanh -FAIL: math/test-float128-lgamma -FAIL: math/test-float128-log -FAIL: math/test-float128-log1p -FAIL: math/test-float128-y0 -FAIL: math/test-float128-y1 -FAIL: math/test-float32-lgamma -FAIL: math/test-float32-log1p -FAIL: math/test-float32x-lgamma -FAIL: math/test-float32x-log1p -FAIL: math/test-float64-lgamma -FAIL: math/test-float64-log1p -FAIL: math/test-float64x-lgamma -FAIL: math/test-ldouble-lgamma PR tree-optimization/107608 gcc/ChangeLog: * range-op-float.cc (range_operator_float::fold_range): Avoid folding into INF when flag_trapping_math. * value-range.h (frange::known_isinf): Return false for possible NANs.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #32 from Romain Geissler --- Hi, Thanks for the fix, indeed it has fixed quite some glibc maths tests ;) FYI, most likely it's totally unrelated to this bug, for right now with latest gcc trunk and glibc trunk on x86-64, I still see the following iseqsig errors: FAIL: math/test-double-iseqsig FAIL: math/test-float-iseqsig FAIL: math/test-float128-iseqsig FAIL: math/test-float32-iseqsig FAIL: math/test-float32x-iseqsig FAIL: math/test-float64-iseqsig FAIL: math/test-float64x-iseqsig FAIL: math/test-ldouble-iseqsig Cheers, Romain
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #33 from Jakub Jelinek --- Isn't that PR106805 ? More importantly, do those FAIL also with GCC 12 or just the trunk?
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #34 from Romain Geissler --- >From what I wrote here https://sourceware.org/pipermail/libc-alpha/2022-November/143633.html apparently I already tried gcc 12 back in end of november 2022 and all float issues in glibc testsuite were gone. I didn't test gcc 12 since then.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #35 from Florian Weimer --- I backported the fixes for building glibc to 2.34 last week, I really expect the testsuite to be clean there (on x86-64), and on later releases as well.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #36 from Aldy Hernandez --- Can we close this PR?
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #37 from Jakub Jelinek --- I guess so. But we should look at the glibc math failures/PR106805.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 Aldy Hernandez changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #38 from Aldy Hernandez --- fixed in trunk
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #39 from Richard Biener --- But I still see FAIL: gcc.dg/pr95115.c execution test as of r13-5237-gaaf29ae6cdbaad
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #40 from Richard Biener --- (In reply to Richard Biener from comment #39) > But I still see > > FAIL: gcc.dg/pr95115.c execution test > > as of r13-5237-gaaf29ae6cdbaad That's removing the use of _3 = Inf / Inf; (probably OK) and then DCE eliminating the stmt, eliding the exception.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
--- Comment #41 from Richard Biener ---
We could fix the testcase with
diff --git a/gcc/testsuite/gcc.dg/pr95115.c b/gcc/testsuite/gcc.dg/pr95115.c
index 69c4f83250c..09273e445d2 100644
--- a/gcc/testsuite/gcc.dg/pr95115.c
+++ b/gcc/testsuite/gcc.dg/pr95115.c
@@ -17,6 +17,7 @@ int
main (void)
{
double r = x ();
+ volatile double rr = r;
if (!__builtin_isnan (r))
abort ();
if (!fetestexcept (FE_INVALID))
that preserves optimizing the isnan check but also preserves the computation
and checks the non-propagation of a NaN.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
--- Comment #42 from Xi Ruoyao ---
(In reply to Richard Biener from comment #41)
> We could fix the testcase with
>
> diff --git a/gcc/testsuite/gcc.dg/pr95115.c b/gcc/testsuite/gcc.dg/pr95115.c
> index 69c4f83250c..09273e445d2 100644
> --- a/gcc/testsuite/gcc.dg/pr95115.c
> +++ b/gcc/testsuite/gcc.dg/pr95115.c
> @@ -17,6 +17,7 @@ int
> main (void)
> {
>double r = x ();
> + volatile double rr = r;
>if (!__builtin_isnan (r))
> abort ();
>if (!fetestexcept (FE_INVALID))
>
> that preserves optimizing the isnan check but also preserves the computation
> and checks the non-propagation of a NaN.
Hmm, so it means we cannot rely on Inf / Inf to raise an exception? Then we
need to fix Glibc...
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
--- Comment #43 from rguenther at suse dot de ---
On Thu, 19 Jan 2023, xry111 at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
>
> --- Comment #42 from Xi Ruoyao ---
> (In reply to Richard Biener from comment #41)
> > We could fix the testcase with
> >
> > diff --git a/gcc/testsuite/gcc.dg/pr95115.c b/gcc/testsuite/gcc.dg/pr95115.c
> > index 69c4f83250c..09273e445d2 100644
> > --- a/gcc/testsuite/gcc.dg/pr95115.c
> > +++ b/gcc/testsuite/gcc.dg/pr95115.c
> > @@ -17,6 +17,7 @@ int
> > main (void)
> > {
> >double r = x ();
> > + volatile double rr = r;
> >if (!__builtin_isnan (r))
> > abort ();
> >if (!fetestexcept (FE_INVALID))
> >
> > that preserves optimizing the isnan check but also preserves the computation
> > and checks the non-propagation of a NaN.
>
> Hmm, so it means we cannot rely on Inf / Inf to raise an exception? Then we
> need to fix Glibc...
If the result is unused then no, GCC will happily elide exceptions from
unused computations like Inexact from the statement
1./3.;
but this has been done before. What's new is that GCC can now elide
some uses (in this case the isnan check is the only use)
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
--- Comment #44 from Xi Ruoyao ---
(In reply to rguent...@suse.de from comment #43)
> On Thu, 19 Jan 2023, xry111 at gcc dot gnu.org wrote:
>
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
> >
> > --- Comment #42 from Xi Ruoyao ---
> > (In reply to Richard Biener from comment #41)
> > > We could fix the testcase with
> > >
> > > diff --git a/gcc/testsuite/gcc.dg/pr95115.c
> > > b/gcc/testsuite/gcc.dg/pr95115.c
> > > index 69c4f83250c..09273e445d2 100644
> > > --- a/gcc/testsuite/gcc.dg/pr95115.c
> > > +++ b/gcc/testsuite/gcc.dg/pr95115.c
> > > @@ -17,6 +17,7 @@ int
> > > main (void)
> > > {
> > >double r = x ();
> > > + volatile double rr = r;
> > >if (!__builtin_isnan (r))
> > > abort ();
> > >if (!fetestexcept (FE_INVALID))
> > >
> > > that preserves optimizing the isnan check but also preserves the
> > > computation
> > > and checks the non-propagation of a NaN.
> >
> > Hmm, so it means we cannot rely on Inf / Inf to raise an exception? Then we
> > need to fix Glibc...
>
> If the result is unused then no, GCC will happily elide exceptions from
> unused computations like Inexact from the statement
>
> 1./3.;
>
> but this has been done before. What's new is that GCC can now elide
> some uses (in this case the isnan check is the only use)
The should we just change PR95115 to "INVALID" and remove the test case, and
fix any regression on Glibc side?
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
--- Comment #45 from Richard Biener ---
(In reply to Xi Ruoyao from comment #44)
> (In reply to rguent...@suse.de from comment #43)
> > On Thu, 19 Jan 2023, xry111 at gcc dot gnu.org wrote:
> >
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
> > >
> > > --- Comment #42 from Xi Ruoyao ---
> > > (In reply to Richard Biener from comment #41)
> > > > We could fix the testcase with
> > > >
> > > > diff --git a/gcc/testsuite/gcc.dg/pr95115.c
> > > > b/gcc/testsuite/gcc.dg/pr95115.c
> > > > index 69c4f83250c..09273e445d2 100644
> > > > --- a/gcc/testsuite/gcc.dg/pr95115.c
> > > > +++ b/gcc/testsuite/gcc.dg/pr95115.c
> > > > @@ -17,6 +17,7 @@ int
> > > > main (void)
> > > > {
> > > >double r = x ();
> > > > + volatile double rr = r;
> > > >if (!__builtin_isnan (r))
> > > > abort ();
> > > >if (!fetestexcept (FE_INVALID))
> > > >
> > > > that preserves optimizing the isnan check but also preserves the
> > > > computation
> > > > and checks the non-propagation of a NaN.
> > >
> > > Hmm, so it means we cannot rely on Inf / Inf to raise an exception? Then
> > > we
> > > need to fix Glibc...
> >
> > If the result is unused then no, GCC will happily elide exceptions from
> > unused computations like Inexact from the statement
> >
> > 1./3.;
> >
> > but this has been done before. What's new is that GCC can now elide
> > some uses (in this case the isnan check is the only use)
>
> The should we just change PR95115 to "INVALID" and remove the test case, and
> fix any regression on Glibc side?
I think we should adjust the testcase with a volatile like I suggested above
so we verify that we don't eliminate the computation with a "constant" NaN.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608 --- Comment #46 from Xi Ruoyao --- (In reply to Richard Biener from comment #45) > (In reply to Xi Ruoyao from comment #44) > > (In reply to rguent...@suse.de from comment #43) > > > If the result is unused then no, GCC will happily elide exceptions from > > > unused computations like Inexact from the statement > > > > > > 1./3.; > > > > > > but this has been done before. What's new is that GCC can now elide > > > some uses (in this case the isnan check is the only use) > > > > The should we just change PR95115 to "INVALID" and remove the test case, and > > fix any regression on Glibc side? > > I think we should adjust the testcase with a volatile like I suggested above > so we verify that we don't eliminate the computation with a "constant" NaN. Ok, I'll post a patch. Glibc already changed the code from Inf/Inf to (x - x) / (x - x) where x is not a constant, but I'm wandering if the compiler will attempt to optimize out (x - x) / (x - x) later... Is it possible to provide a "__builtin_feraiseexcept" so we'd be able to use it instead of the nasty (x - x) / (x - x) to raise the exception?
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
--- Comment #47 from rguenther at suse dot de ---
On Fri, 27 Jan 2023, xry111 at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
>
> --- Comment #46 from Xi Ruoyao ---
> (In reply to Richard Biener from comment #45)
> > (In reply to Xi Ruoyao from comment #44)
> > > (In reply to rguent...@suse.de from comment #43)
>
> > > > If the result is unused then no, GCC will happily elide exceptions from
> > > > unused computations like Inexact from the statement
> > > >
> > > > 1./3.;
> > > >
> > > > but this has been done before. What's new is that GCC can now elide
> > > > some uses (in this case the isnan check is the only use)
> > >
> > > The should we just change PR95115 to "INVALID" and remove the test case,
> > > and
> > > fix any regression on Glibc side?
> >
> > I think we should adjust the testcase with a volatile like I suggested above
> > so we verify that we don't eliminate the computation with a "constant" NaN.
>
> Ok, I'll post a patch.
>
> Glibc already changed the code from Inf/Inf to (x - x) / (x - x) where x
> is not a constant, but I'm wandering if the compiler will attempt to
> optimize out (x - x) / (x - x) later... Is it possible to provide a
> "__builtin_feraiseexcept" so we'd be able to use it instead of the nasty
> (x - x) / (x - x) to raise the exception?
Not trivially. I'd suggest glibc uses a volatile use, like for example
tem = Inf/Inf;
__asm__ volatile ("" : : "g" (tem));
or so to preserve the computation and avoid an actual store to a volatile
variable. Though I see at least GCC 7 optimizing the above division
to a constant, lacking a fix we deployed later.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
--- Comment #48 from Xi Ruoyao ---
(In reply to rguent...@suse.de from comment #47)
> On Fri, 27 Jan 2023, xry111 at gcc dot gnu.org wrote:
>
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
> >
> > --- Comment #46 from Xi Ruoyao ---
> > (In reply to Richard Biener from comment #45)
> > > (In reply to Xi Ruoyao from comment #44)
> > > > (In reply to rguent...@suse.de from comment #43)
> >
> > > > > If the result is unused then no, GCC will happily elide exceptions
> > > > > from
> > > > > unused computations like Inexact from the statement
> > > > >
> > > > > 1./3.;
> > > > >
> > > > > but this has been done before. What's new is that GCC can now elide
> > > > > some uses (in this case the isnan check is the only use)
> > > >
> > > > The should we just change PR95115 to "INVALID" and remove the test
> > > > case, and
> > > > fix any regression on Glibc side?
> > >
> > > I think we should adjust the testcase with a volatile like I suggested
> > > above
> > > so we verify that we don't eliminate the computation with a "constant"
> > > NaN.
> >
> > Ok, I'll post a patch.
> >
> > Glibc already changed the code from Inf/Inf to (x - x) / (x - x) where x
> > is not a constant, but I'm wandering if the compiler will attempt to
> > optimize out (x - x) / (x - x) later... Is it possible to provide a
> > "__builtin_feraiseexcept" so we'd be able to use it instead of the nasty
> > (x - x) / (x - x) to raise the exception?
>
> Not trivially. I'd suggest glibc uses a volatile use, like for example
>
> tem = Inf/Inf;
> __asm__ volatile ("" : : "g" (tem));
>
> or so to preserve the computation and avoid an actual store to a volatile
> variable. Though I see at least GCC 7 optimizing the above division
> to a constant, lacking a fix we deployed later.
Currently Glibc returns the produced NaN (and there will be no IPA involved as
the TU only contains one function, and AFAIK Glibc just does not support
building with LTO). Is it enough to preserve the computation?
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
--- Comment #49 from Jakub Jelinek ---
(In reply to rguent...@suse.de from comment #47)
> > Glibc already changed the code from Inf/Inf to (x - x) / (x - x) where x
(x - x) / (x - x) is 0 / 0, not Inf / Inf.
Anyway, for frange potential in GCC 14, I'd hope we do figure out that
x - x has [0, 0] range (never -0.0 even, unless -frounding-math where it could
be -0.0 when rounding to -Inf) provided x is known to be finite -
all of Inf - Inf, (-Inf) - (-Inf) and NaN - NaN are NaN.
And frange already has an infrastructure for that, foperator_minus::rv_fold is
passed relation_kind between op1 and op2, so if it is VREL_EQ and we can check
that Inf or NaN isn't possible in the range, we should yield [0, 0].
Or for -ffast-math do it always and yield [-0., 0] as Inf/NaN aren't expected
but signed zeros are present but are insignificant.
Shall we file a PR for that?
> > is not a constant, but I'm wandering if the compiler will attempt to
> > optimize out (x - x) / (x - x) later... Is it possible to provide a
> > "__builtin_feraiseexcept" so we'd be able to use it instead of the nasty
> > (x - x) / (x - x) to raise the exception?
>
> Not trivially. I'd suggest glibc uses a volatile use, like for example
>
> tem = Inf/Inf;
> __asm__ volatile ("" : : "g" (tem));
In this case I guess that is at least right now fine (and glibc I think even
has a macro for that, some math_*). The thing is that the result is NaN and we
don't treat NaN as singleton (because there are many representations of NaN).
Similarly the workaround for fold-overflow-1.c added in this PR will not treat
for now
operations from finite operands yielding singleton Inf or -Inf as singleton.
But if it is something else, say finite + finite and the expectation is that
inexact is raised, then the above wouldn't help, because we'd just turn it into
"g" (constant)
in the asm.
[Bug tree-optimization/107608] [13 Regression] Failure on fold-overflow-1.c and pr95115.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
--- Comment #50 from rguenther at suse dot de ---
On Fri, 27 Jan 2023, jakub at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107608
>
> --- Comment #49 from Jakub Jelinek ---
> (In reply to rguent...@suse.de from comment #47)
> > > Glibc already changed the code from Inf/Inf to (x - x) / (x - x) where x
>
> (x - x) / (x - x) is 0 / 0, not Inf / Inf.
> Anyway, for frange potential in GCC 14, I'd hope we do figure out that
> x - x has [0, 0] range (never -0.0 even, unless -frounding-math where it could
> be -0.0 when rounding to -Inf) provided x is known to be finite -
> all of Inf - Inf, (-Inf) - (-Inf) and NaN - NaN are NaN.
> And frange already has an infrastructure for that, foperator_minus::rv_fold is
> passed relation_kind between op1 and op2, so if it is VREL_EQ and we can check
> that Inf or NaN isn't possible in the range, we should yield [0, 0].
> Or for -ffast-math do it always and yield [-0., 0] as Inf/NaN aren't expected
> but signed zeros are present but are insignificant.
> Shall we file a PR for that?
>
> > > is not a constant, but I'm wandering if the compiler will attempt to
> > > optimize out (x - x) / (x - x) later... Is it possible to provide a
> > > "__builtin_feraiseexcept" so we'd be able to use it instead of the nasty
> > > (x - x) / (x - x) to raise the exception?
> >
> > Not trivially. I'd suggest glibc uses a volatile use, like for example
> >
> > tem = Inf/Inf;
> > __asm__ volatile ("" : : "g" (tem));
>
> In this case I guess that is at least right now fine (and glibc I think even
> has a macro for that, some math_*). The thing is that the result is NaN and
> we
> don't treat NaN as singleton (because there are many representations of NaN).
> Similarly the workaround for fold-overflow-1.c added in this PR will not treat
> for now
> operations from finite operands yielding singleton Inf or -Inf as singleton.
> But if it is something else, say finite + finite and the expectation is that
> inexact is raised, then the above wouldn't help, because we'd just turn it
> into
> "g" (constant)
> in the asm.
For inexact yes, but we do refrain from constant folding when that loses
exceptions (in some cases at least), not just when it produces a NaN.
