* [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] @ 2023-03-10 8:07 Jakub Jelinek 2023-03-10 8:53 ` Richard Biener 0 siblings, 1 reply; 19+ messages in thread From: Jakub Jelinek @ 2023-03-10 8:07 UTC (permalink / raw) To: Richard Biener, Aldy Hernandez; +Cc: gcc-patches Hi! The following patch does two things (both related to range extension around the boundaries). The first part (in the 2 real_isfinite blocks) is to make the ranges narrower when the old boundaries are minimum and/or maximum representable finite number. In that case frange_nextafter gives -Inf or +Inf, but then the resulting computed reverse range is very far from the actually needed range, usually extends up to infinity or could even result in NaNs. While infinities are really the next representable numbers in the corresponding mode, REAL_VALUE_TYPE is actually a type with wider range for exponent and 160 bit precision, so the patch instead uses nextafter number in a hypothetical floating point format with the same mantissa precision but wider range of exponents. This significantly improves the actual ranges of the reverse operations, while still making them conservatively correct. The second part is a fix for miscompilation of the new testcase below. For -ffinite-math-only, without this patch we extend the minimum and/or maximum representable finite number to -Inf or +Inf, with the patch to some number outside of the normal exponent range of the mode, but then we use set which canonicalizes it and turns the boundaries back to the minimum and/or maximum representable finite numbers, but because in say [__DBL_MAX__, __DBL_MAX__] = op1 + [__DBL_MAX__, __DBL_MAX__] op1 can be larger than 0, up to the largest number which rounds to even down back to __DBL_MAX__ and there are still no infinities involved, it needs to work even with -ffinite-math-only. So, we really need to widen the lhs range a little bit even in that case. The patch does that through temporarily clearing -ffinite-math-only, such that the value with infinities or the outside of bounds values passes the setting and verification (the VR_VARYING case is needed because we get ICEs otherwise, but when lhs is VR_VARYING in -ffast-math, i.e. minimum to maximum representable finite and both signs of NaN, then set does all we need, we don't need to or in a NaN range). We don't really later use the range in a way that would become a problem that it is wider than varying, we actually just perform maths on the two boundaries. As I said in the PR, this doesn't fix the !MODE_HAS_INFINITIES case, I believe we actually need to treat the boundary values as infinities in that case because they (probably) work like that, but it is unclear if it is just the reverse operation lhs widening that is a problem there, or whether it is a general problem. I have zero experience with floating points without infinities (PDP11, some ARM half type?, what else?). Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2023-03-10 Jakub Jelinek <jakub@redhat.com> PR tree-optimization/109008 * range-op-float.cc (float_widen_lhs_range): If lb is minimum representable finite number or ub is maximum representable finite number, instead of widening it to -inf or inf widen it to negative or positive 0x0.8p+(EMAX+1). Temporarily clear flag_finite_math_only when canonicalizing the widened range. * gcc.dg/pr109008.c: New test. --- gcc/range-op-float.cc.jj 2023-03-09 09:54:53.880453046 +0100 +++ gcc/range-op-float.cc 2023-03-09 20:52:07.456284507 +0100 @@ -2217,12 +2217,42 @@ float_widen_lhs_range (tree type, const REAL_VALUE_TYPE lb = lhs.lower_bound (); REAL_VALUE_TYPE ub = lhs.upper_bound (); if (real_isfinite (&lb)) - frange_nextafter (TYPE_MODE (type), lb, dconstninf); + { + frange_nextafter (TYPE_MODE (type), lb, dconstninf); + if (real_isinf (&lb)) + { + /* For -DBL_MAX, instead of -Inf use + nexttoward (-DBL_MAX, -LDBL_MAX) in a hypothetical + wider type with the same mantissa precision but larger + exponent range; it is outside of range of double values, + but makes it clear it is just one ulp larger rather than + infinite amount larger. */ + lb = dconstm1; + SET_REAL_EXP (&lb, FLOAT_MODE_FORMAT (TYPE_MODE (type))->emax + 1); + } + } if (real_isfinite (&ub)) - frange_nextafter (TYPE_MODE (type), ub, dconstinf); + { + frange_nextafter (TYPE_MODE (type), ub, dconstinf); + if (real_isinf (&ub)) + { + /* For DBL_MAX similarly. */ + ub = dconst1; + SET_REAL_EXP (&ub, FLOAT_MODE_FORMAT (TYPE_MODE (type))->emax + 1); + } + } + /* Temporarily disable -ffinite-math-only, so that frange::set doesn't + reduce the range back to real_min_representable (type) as lower bound + 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 (type, lb, ub); - ret.clear_nan (); - ret.union_ (lhs); + if (lhs.kind () != VR_VARYING) + { + ret.clear_nan (); + ret.union_ (lhs); + } + flag_finite_math_only = save_flag_finite_math_only; return ret; } --- gcc/testsuite/gcc.dg/pr109008.c.jj 2023-03-09 12:25:11.507955698 +0100 +++ gcc/testsuite/gcc.dg/pr109008.c 2023-03-09 12:33:35.795598344 +0100 @@ -0,0 +1,26 @@ +/* PR tree-optimization/109008 */ +/* { dg-do run } */ +/* { dg-options "-O2 -ffinite-math-only -fexcess-precision=standard" } */ + +__attribute__((noipa)) double +foo (double eps) +{ + double d = __DBL_MAX__ + eps; + if (d == __DBL_MAX__) + if (eps > 16.0) + return eps; + return 0.0; +} + +int +main () +{ +#if __DBL_MANT_DIG__ == 53 && __DBL_MAX_EXP__ == 1024 && __DBL_MIN_EXP__ == -1021 \ + && __FLT_EVAL_METHOD__ == 0 + if (foo (0x0.8p+970) == 0.0) + __builtin_abort (); + if (foo (32.0) == 0.0) + __builtin_abort (); +#endif + return 0; +} Jakub ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] 2023-03-10 8:07 [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] Jakub Jelinek @ 2023-03-10 8:53 ` Richard Biener 2023-03-10 10:29 ` Jakub Jelinek 2023-03-21 13:28 ` Aldy Hernandez 0 siblings, 2 replies; 19+ messages in thread From: Richard Biener @ 2023-03-10 8:53 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Aldy Hernandez, gcc-patches On Fri, 10 Mar 2023, Jakub Jelinek wrote: > Hi! > > The following patch does two things (both related to range extension > around the boundaries). > > The first part (in the 2 real_isfinite blocks) is to make the ranges > narrower when the old boundaries are minimum and/or maximum representable > finite number. In that case frange_nextafter gives -Inf or +Inf, > but then the resulting computed reverse range is very far from the actually > needed range, usually extends up to infinity or could even result in NaNs. > While infinities are really the next representable numbers in the > corresponding mode, REAL_VALUE_TYPE is actually a type with wider range > for exponent and 160 bit precision, so the patch instead uses > nextafter number in a hypothetical floating point format with the same > mantissa precision but wider range of exponents. This significantly > improves the actual ranges of the reverse operations, while still making > them conservatively correct. > > The second part is a fix for miscompilation of the new testcase below. > For -ffinite-math-only, without this patch we extend the minimum and/or > maximum representable finite number to -Inf or +Inf, with the patch to > some number outside of the normal exponent range of the mode, but then > we use set which canonicalizes it and turns the boundaries back to > the minimum and/or maximum representable finite numbers, but because > in say [__DBL_MAX__, __DBL_MAX__] = op1 + [__DBL_MAX__, __DBL_MAX__] > op1 can be larger than 0, up to the largest number which rounds to even > down back to __DBL_MAX__ and there are still no infinities involved, > it needs to work even with -ffinite-math-only. So, we really need to > widen the lhs range a little bit even in that case. The patch does > that through temporarily clearing -ffinite-math-only, such that the > value with infinities or the outside of bounds values passes the > setting and verification (the VR_VARYING case is needed because > we get ICEs otherwise, but when lhs is VR_VARYING in -ffast-math, > i.e. minimum to maximum representable finite and both signs of NaN, > then set does all we need, we don't need to or in a NaN range). > We don't really later use the range in a way that would become a problem > that it is wider than varying, we actually just perform maths on the > two boundaries. > > As I said in the PR, this doesn't fix the !MODE_HAS_INFINITIES case, > I believe we actually need to treat the boundary values as infinities > in that case because they (probably) work like that, but it is unclear > if it is just the reverse operation lhs widening that is a problem there, > or whether it is a general problem. I have zero experience with > floating points without infinities (PDP11, some ARM half type?, > what else?). > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2023-03-10 Jakub Jelinek <jakub@redhat.com> > > PR tree-optimization/109008 > * range-op-float.cc (float_widen_lhs_range): If lb is > minimum representable finite number or ub is maximum > representable finite number, instead of widening it to > -inf or inf widen it to negative or positive 0x0.8p+(EMAX+1). > Temporarily clear flag_finite_math_only when canonicalizing > the widened range. > > * gcc.dg/pr109008.c: New test. > > --- gcc/range-op-float.cc.jj 2023-03-09 09:54:53.880453046 +0100 > +++ gcc/range-op-float.cc 2023-03-09 20:52:07.456284507 +0100 > @@ -2217,12 +2217,42 @@ float_widen_lhs_range (tree type, const > REAL_VALUE_TYPE lb = lhs.lower_bound (); > REAL_VALUE_TYPE ub = lhs.upper_bound (); > if (real_isfinite (&lb)) > - frange_nextafter (TYPE_MODE (type), lb, dconstninf); > + { > + frange_nextafter (TYPE_MODE (type), lb, dconstninf); > + if (real_isinf (&lb)) > + { > + /* For -DBL_MAX, instead of -Inf use > + nexttoward (-DBL_MAX, -LDBL_MAX) in a hypothetical > + wider type with the same mantissa precision but larger > + exponent range; it is outside of range of double values, > + but makes it clear it is just one ulp larger rather than > + infinite amount larger. */ > + lb = dconstm1; > + SET_REAL_EXP (&lb, FLOAT_MODE_FORMAT (TYPE_MODE (type))->emax + 1); > + } > + } > if (real_isfinite (&ub)) > - frange_nextafter (TYPE_MODE (type), ub, dconstinf); > + { > + frange_nextafter (TYPE_MODE (type), ub, dconstinf); > + if (real_isinf (&ub)) > + { > + /* For DBL_MAX similarly. */ > + ub = dconst1; > + SET_REAL_EXP (&ub, FLOAT_MODE_FORMAT (TYPE_MODE (type))->emax + 1); > + } > + } > + /* Temporarily disable -ffinite-math-only, so that frange::set doesn't > + reduce the range back to real_min_representable (type) as lower bound > + 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 (type, lb, ub); > - ret.clear_nan (); > - ret.union_ (lhs); > + if (lhs.kind () != VR_VARYING) > + { > + ret.clear_nan (); > + ret.union_ (lhs); > + } > + flag_finite_math_only = save_flag_finite_math_only; Meh - I wonder if we can avoid all this by making float_widen_lhs_range friend of frange and simply access m_min/m_max directly and use the copy-CTOR to copy bounds and nan state ... after all verify_range will likely fail after you restore flag_finite_math_only ... But OK for the moment. Thanks, Richard. > return ret; > } > > --- gcc/testsuite/gcc.dg/pr109008.c.jj 2023-03-09 12:25:11.507955698 +0100 > +++ gcc/testsuite/gcc.dg/pr109008.c 2023-03-09 12:33:35.795598344 +0100 > @@ -0,0 +1,26 @@ > +/* PR tree-optimization/109008 */ > +/* { dg-do run } */ > +/* { dg-options "-O2 -ffinite-math-only -fexcess-precision=standard" } */ > + > +__attribute__((noipa)) double > +foo (double eps) > +{ > + double d = __DBL_MAX__ + eps; > + if (d == __DBL_MAX__) > + if (eps > 16.0) > + return eps; > + return 0.0; > +} > + > +int > +main () > +{ > +#if __DBL_MANT_DIG__ == 53 && __DBL_MAX_EXP__ == 1024 && __DBL_MIN_EXP__ == -1021 \ > + && __FLT_EVAL_METHOD__ == 0 > + if (foo (0x0.8p+970) == 0.0) > + __builtin_abort (); > + if (foo (32.0) == 0.0) > + __builtin_abort (); > +#endif > + return 0; > +} > > Jakub > > -- Richard Biener <rguenther@suse.de> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] 2023-03-10 8:53 ` Richard Biener @ 2023-03-10 10:29 ` Jakub Jelinek 2023-03-13 7:18 ` Aldy Hernandez 2023-03-21 13:28 ` Aldy Hernandez 1 sibling, 1 reply; 19+ messages in thread From: Jakub Jelinek @ 2023-03-10 10:29 UTC (permalink / raw) To: Richard Biener, Aldy Hernandez; +Cc: gcc-patches On Fri, Mar 10, 2023 at 08:53:37AM +0000, Richard Biener wrote: > Meh - I wonder if we can avoid all this by making float_widen_lhs_range > friend of frange and simply access m_min/m_max directly and use the > copy-CTOR to copy bounds and nan state ... after all verify_range > will likely fail after you restore flag_finite_math_only ... I'll defer such changes to Aldy. As for verification, I think verify_range will not fail on it, it mainly checks whether it is normalized (e.g. if minimum is frange_val_min and maximum is frange_val_max and NaNs are possible with both signs (if NaNs are supported) then it is VR_VARYING etc.). It doesn't check if the actual VR_RANGE bounds are smaller or larger than the VR_VARYING bounds, there is just equality check. Of course, behavior of wider than varying ranges is still unexpected in many ways, say the union_ of such a range and VR_VARYING will ICE etc. Now, I guess another possibility for the reverse ops over these wider ranges would be avoid calling fold_range in the reverse ops, but call rv_fold directly or have fold_range variant which would instead of the op1, op2 argument have 2 triplets, op1, op1lb, op1ub, op2, op2lb, op2ub, and it would use those const REAL_VALUE_TYPE &op??b in preference to op?.{lower,upper}_bound () or perhaps normal fold_range be implemented in terms of this extended fold_range. Then we wouldn't need to bother with these non-standard franges... > But OK for the moment. Thanks, committed. Jakub ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] 2023-03-10 10:29 ` Jakub Jelinek @ 2023-03-13 7:18 ` Aldy Hernandez 2023-03-13 7:50 ` Richard Biener 0 siblings, 1 reply; 19+ messages in thread From: Aldy Hernandez @ 2023-03-13 7:18 UTC (permalink / raw) To: Jakub Jelinek, Richard Biener; +Cc: gcc-patches, Andrew MacLeod On 3/10/23 11:29, Jakub Jelinek wrote: > On Fri, Mar 10, 2023 at 08:53:37AM +0000, Richard Biener wrote: >> Meh - I wonder if we can avoid all this by making float_widen_lhs_range >> friend of frange and simply access m_min/m_max directly and use the >> copy-CTOR to copy bounds and nan state ... after all verify_range >> will likely fail after you restore flag_finite_math_only ... > > I'll defer such changes to Aldy. > > As for verification, I think verify_range will not fail on it, it mainly > checks whether it is normalized (e.g. if minimum is frange_val_min and > maximum is frange_val_max and NaNs are possible with both signs (if NaNs > are supported) then it is VR_VARYING etc.). It doesn't check if the actual > VR_RANGE bounds are smaller or larger than the VR_VARYING bounds, there is > just equality check. > Of course, behavior of wider than varying ranges is still unexpected in > many ways, say the union_ of such a range and VR_VARYING will ICE etc. > > Now, I guess another possibility for the reverse ops over these wider ranges > would be avoid calling fold_range in the reverse ops, but call rv_fold > directly or have fold_range variant which would instead of the op1, op2 > argument have 2 triplets, op1, op1lb, op1ub, op2, op2lb, op2ub, and it > would use those const REAL_VALUE_TYPE &op??b in preference to > op?.{lower,upper}_bound () or perhaps normal fold_range be implemented > in terms of this extended fold_range. Then we wouldn't need to bother with > these non-standard franges... > >> But OK for the moment. > > Thanks, committed. I'm not a big fan of constructing ranges that break all our internal consistency checks. I'd also prefer to add another constructor (or add a flag to the current constructor) instead of making range-op-float routines friends. We also have bits in the vrange (or frange) that we could use for other semantics, especially since frange_storage can be streamlined when stored in GC/etc. I'm on PTO this week. Could we revisit this next week? And if worse comes to worse, leave it as is, and fix it properly next release? Thanks for your work on this. Aldy ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] 2023-03-13 7:18 ` Aldy Hernandez @ 2023-03-13 7:50 ` Richard Biener 2023-03-13 7:59 ` Aldy Hernandez 0 siblings, 1 reply; 19+ messages in thread From: Richard Biener @ 2023-03-13 7:50 UTC (permalink / raw) To: Aldy Hernandez; +Cc: Jakub Jelinek, gcc-patches, Andrew MacLeod On Mon, 13 Mar 2023, Aldy Hernandez wrote: > > > On 3/10/23 11:29, Jakub Jelinek wrote: > > On Fri, Mar 10, 2023 at 08:53:37AM +0000, Richard Biener wrote: > >> Meh - I wonder if we can avoid all this by making float_widen_lhs_range > >> friend of frange and simply access m_min/m_max directly and use the > >> copy-CTOR to copy bounds and nan state ... after all verify_range > >> will likely fail after you restore flag_finite_math_only ... > > > > I'll defer such changes to Aldy. > > > > As for verification, I think verify_range will not fail on it, it mainly > > checks whether it is normalized (e.g. if minimum is frange_val_min and > > maximum is frange_val_max and NaNs are possible with both signs (if NaNs > > are supported) then it is VR_VARYING etc.). It doesn't check if the actual > > VR_RANGE bounds are smaller or larger than the VR_VARYING bounds, there is > > just equality check. > > Of course, behavior of wider than varying ranges is still unexpected in > > many ways, say the union_ of such a range and VR_VARYING will ICE etc. > > > > Now, I guess another possibility for the reverse ops over these wider ranges > > would be avoid calling fold_range in the reverse ops, but call rv_fold > > directly or have fold_range variant which would instead of the op1, op2 > > argument have 2 triplets, op1, op1lb, op1ub, op2, op2lb, op2ub, and it > > would use those const REAL_VALUE_TYPE &op??b in preference to > > op?.{lower,upper}_bound () or perhaps normal fold_range be implemented > > in terms of this extended fold_range. Then we wouldn't need to bother with > > these non-standard franges... > > > >> But OK for the moment. > > > > Thanks, committed. > > I'm not a big fan of constructing ranges that break all our internal > consistency checks. I'd also prefer to add another constructor (or add a flag > to the current constructor) instead of making range-op-float routines friends. > We also have bits in the vrange (or frange) that we could use for other > semantics, especially since frange_storage can be streamlined when stored in > GC/etc. > > I'm on PTO this week. Could we revisit this next week? And if worse comes to > worse, leave it as is, and fix it properly next release? Yes, sure - I just noticed that we're forced to use high-level API for something that's quite low-level and should be internal (a range "breaking" internal consistency checks). Richard. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] 2023-03-13 7:50 ` Richard Biener @ 2023-03-13 7:59 ` Aldy Hernandez 2023-03-13 8:06 ` Jakub Jelinek 0 siblings, 1 reply; 19+ messages in thread From: Aldy Hernandez @ 2023-03-13 7:59 UTC (permalink / raw) To: Richard Biener; +Cc: Jakub Jelinek, gcc-patches, Andrew MacLeod On 3/13/23 08:50, Richard Biener wrote: > On Mon, 13 Mar 2023, Aldy Hernandez wrote: > >> >> >> On 3/10/23 11:29, Jakub Jelinek wrote: >>> On Fri, Mar 10, 2023 at 08:53:37AM +0000, Richard Biener wrote: >>>> Meh - I wonder if we can avoid all this by making float_widen_lhs_range >>>> friend of frange and simply access m_min/m_max directly and use the >>>> copy-CTOR to copy bounds and nan state ... after all verify_range >>>> will likely fail after you restore flag_finite_math_only ... >>> >>> I'll defer such changes to Aldy. >>> >>> As for verification, I think verify_range will not fail on it, it mainly >>> checks whether it is normalized (e.g. if minimum is frange_val_min and >>> maximum is frange_val_max and NaNs are possible with both signs (if NaNs >>> are supported) then it is VR_VARYING etc.). It doesn't check if the actual >>> VR_RANGE bounds are smaller or larger than the VR_VARYING bounds, there is >>> just equality check. >>> Of course, behavior of wider than varying ranges is still unexpected in >>> many ways, say the union_ of such a range and VR_VARYING will ICE etc. >>> >>> Now, I guess another possibility for the reverse ops over these wider ranges >>> would be avoid calling fold_range in the reverse ops, but call rv_fold >>> directly or have fold_range variant which would instead of the op1, op2 >>> argument have 2 triplets, op1, op1lb, op1ub, op2, op2lb, op2ub, and it >>> would use those const REAL_VALUE_TYPE &op??b in preference to >>> op?.{lower,upper}_bound () or perhaps normal fold_range be implemented >>> in terms of this extended fold_range. Then we wouldn't need to bother with >>> these non-standard franges... >>> >>>> But OK for the moment. >>> >>> Thanks, committed. >> >> I'm not a big fan of constructing ranges that break all our internal >> consistency checks. I'd also prefer to add another constructor (or add a flag >> to the current constructor) instead of making range-op-float routines friends. >> We also have bits in the vrange (or frange) that we could use for other >> semantics, especially since frange_storage can be streamlined when stored in >> GC/etc. >> >> I'm on PTO this week. Could we revisit this next week? And if worse comes to >> worse, leave it as is, and fix it properly next release? > > Yes, sure - I just noticed that we're forced to use high-level API for > something that's quite low-level and should be internal (a range > "breaking" internal consistency checks). Yeah, let's fix the API. No sense hacking around things if what we need is to tweak the design. I don't like hacking around things. It always comes back to bite me ;-). Aldy ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] 2023-03-13 7:59 ` Aldy Hernandez @ 2023-03-13 8:06 ` Jakub Jelinek 2023-03-13 8:41 ` Aldy Hernandez 0 siblings, 1 reply; 19+ messages in thread From: Jakub Jelinek @ 2023-03-13 8:06 UTC (permalink / raw) To: Aldy Hernandez; +Cc: Richard Biener, gcc-patches, Andrew MacLeod On Mon, Mar 13, 2023 at 08:59:15AM +0100, Aldy Hernandez wrote: > > Yes, sure - I just noticed that we're forced to use high-level API for > > something that's quite low-level and should be internal (a range > > "breaking" internal consistency checks). > > Yeah, let's fix the API. No sense hacking around things if what we need is > to tweak the design. > > I don't like hacking around things. It always comes back to bite me ;-). Sure. The current state is that I think the actual bugs are fixed except for the !MODE_HAS_INFINITIES case which people rarely use, so fixing up the API can wait even to next release. For !MODE_HAS_INFINITIES, I wonder if the best fix wouldn't be to change set and a few other spots, so that if the boundaries are real_min_representable/real_max_representable, we widen them to -inf and inf and change frange_val_min/max to also be dconstninf/dconstinf for !MODE_HAS_INFINITIES, because the min/max for that case (probably) really work as infinities. Whenever we actually round that value to mode, it will become real_min_representable/real_max_representable again. But that can also wait a week. Jakub ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] 2023-03-13 8:06 ` Jakub Jelinek @ 2023-03-13 8:41 ` Aldy Hernandez 2023-03-20 16:14 ` Jakub Jelinek 0 siblings, 1 reply; 19+ messages in thread From: Aldy Hernandez @ 2023-03-13 8:41 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches, Andrew MacLeod On 3/13/23 09:06, Jakub Jelinek wrote: > On Mon, Mar 13, 2023 at 08:59:15AM +0100, Aldy Hernandez wrote: >>> Yes, sure - I just noticed that we're forced to use high-level API for >>> something that's quite low-level and should be internal (a range >>> "breaking" internal consistency checks). >> >> Yeah, let's fix the API. No sense hacking around things if what we need is >> to tweak the design. >> >> I don't like hacking around things. It always comes back to bite me ;-). > > Sure. The current state is that I think the actual bugs are fixed except > for the !MODE_HAS_INFINITIES case which people rarely use, so fixing up the > API can wait even to next release. > > For !MODE_HAS_INFINITIES, I wonder if the best fix wouldn't be to change > set and a few other spots, so that if the boundaries are > real_min_representable/real_max_representable, we widen them to -inf and inf > and change frange_val_min/max to also be dconstninf/dconstinf for > !MODE_HAS_INFINITIES, because the min/max for that case (probably) really work as > infinities. Whenever we actually round that value to mode, it will become > real_min_representable/real_max_representable again. > But that can also wait a week. That sounds very reasonable. It would remove special casing and would make the code easier to read. For that matter, that was what I had in the original implementation. Aldy ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] 2023-03-13 8:41 ` Aldy Hernandez @ 2023-03-20 16:14 ` Jakub Jelinek 2023-03-21 12:56 ` Aldy Hernandez 0 siblings, 1 reply; 19+ messages in thread From: Jakub Jelinek @ 2023-03-20 16:14 UTC (permalink / raw) To: Aldy Hernandez; +Cc: Richard Biener, gcc-patches, Andrew MacLeod On Mon, Mar 13, 2023 at 09:41:47AM +0100, Aldy Hernandez wrote: > On 3/13/23 09:06, Jakub Jelinek wrote: > > On Mon, Mar 13, 2023 at 08:59:15AM +0100, Aldy Hernandez wrote: > > > > Yes, sure - I just noticed that we're forced to use high-level API for > > > > something that's quite low-level and should be internal (a range > > > > "breaking" internal consistency checks). > > > > > > Yeah, let's fix the API. No sense hacking around things if what we need is > > > to tweak the design. > > > > > > I don't like hacking around things. It always comes back to bite me ;-). > > > > Sure. The current state is that I think the actual bugs are fixed except > > for the !MODE_HAS_INFINITIES case which people rarely use, so fixing up the > > API can wait even to next release. > > > > For !MODE_HAS_INFINITIES, I wonder if the best fix wouldn't be to change > > set and a few other spots, so that if the boundaries are > > real_min_representable/real_max_representable, we widen them to -inf and inf > > and change frange_val_min/max to also be dconstninf/dconstinf for > > !MODE_HAS_INFINITIES, because the min/max for that case (probably) really work as > > infinities. Whenever we actually round that value to mode, it will become > > real_min_representable/real_max_representable again. > > But that can also wait a week. > > That sounds very reasonable. It would remove special casing and would make > the code easier to read. For that matter, that was what I had in the > original implementation. I think we don't want to remove the special casing for -ffinite-math-only on types which do support infinities. Thinking further on it, perhaps for !MODE_HAS_INFINITIES a better fix would be to do something like the patch below. Consider say having a range of VAX float type: #define M0 -FLT_MAX #define M1 nextafterf (F0, FLT_MAX) #define M2 nextafterf (M1, FLT_MAX) [M2, M2] - [M0, M1] Or perhaps if one or both of the operands are in such a case a min and max, perform real_arithmetic recurse on the argument replaced with dconstninf/dconstinf and then depending on inf pick the mininum or maximum of the two results (and carefully think about what to do if both operands are min/max). 2023-03-20 Jakub Jelinek <jakub@redhat.com> * range-op-float.cc (frange_arithmetic): For !MODE_HAS_INFINITIES types, pretend operands with minimum or maximum values are actually infinities. --- gcc/range-op-float.cc.jj 2023-03-10 12:40:19.673108938 +0100 +++ gcc/range-op-float.cc 2023-03-20 16:58:36.604981486 +0100 @@ -313,8 +313,26 @@ frange_arithmetic (enum tree_code code, REAL_VALUE_TYPE value; enum machine_mode mode = TYPE_MODE (type); bool mode_composite = MODE_COMPOSITE_P (mode); + const REAL_VALUE_TYPE *pop1 = &op1; + const REAL_VALUE_TYPE *pop2 = &op2; - bool inexact = real_arithmetic (&value, code, &op1, &op2); + if (!MODE_HAS_INFINITIES (mode)) + { + // If mode doesn't have infinities, the minimum and maximum + // values are saturating. Pretend for real_arithmetic such + // values are actual infinities. real_convert will then + // canonicalize the result not to be an infinity. + if (frange_val_is_min (op1, type)) + pop1 = &dconstninf; + else if (frange_val_is_max (op1, type)) + pop1 = &dconstinf; + if (frange_val_is_min (op2, type)) + pop2 = &dconstninf; + else if (frange_val_is_max (op2, type)) + pop2 = &dconstinf; + } + + bool inexact = real_arithmetic (&value, code, pop1, pop2); real_convert (&result, mode, &value); // Be extra careful if there may be discrepancies between the Jakub ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] 2023-03-20 16:14 ` Jakub Jelinek @ 2023-03-21 12:56 ` Aldy Hernandez 0 siblings, 0 replies; 19+ messages in thread From: Aldy Hernandez @ 2023-03-21 12:56 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches, Andrew MacLeod On Mon, Mar 20, 2023 at 5:14 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Mon, Mar 13, 2023 at 09:41:47AM +0100, Aldy Hernandez wrote: > > On 3/13/23 09:06, Jakub Jelinek wrote: > > > On Mon, Mar 13, 2023 at 08:59:15AM +0100, Aldy Hernandez wrote: > > > > > Yes, sure - I just noticed that we're forced to use high-level API for > > > > > something that's quite low-level and should be internal (a range > > > > > "breaking" internal consistency checks). > > > > > > > > Yeah, let's fix the API. No sense hacking around things if what we need is > > > > to tweak the design. > > > > > > > > I don't like hacking around things. It always comes back to bite me ;-). > > > > > > Sure. The current state is that I think the actual bugs are fixed except > > > for the !MODE_HAS_INFINITIES case which people rarely use, so fixing up the > > > API can wait even to next release. > > > > > > For !MODE_HAS_INFINITIES, I wonder if the best fix wouldn't be to change > > > set and a few other spots, so that if the boundaries are > > > real_min_representable/real_max_representable, we widen them to -inf and inf > > > and change frange_val_min/max to also be dconstninf/dconstinf for > > > !MODE_HAS_INFINITIES, because the min/max for that case (probably) really work as > > > infinities. Whenever we actually round that value to mode, it will become > > > real_min_representable/real_max_representable again. > > > But that can also wait a week. > > > > That sounds very reasonable. It would remove special casing and would make > > the code easier to read. For that matter, that was what I had in the > > original implementation. > > I think we don't want to remove the special casing for -ffinite-math-only > on types which do support infinities. > Thinking further on it, perhaps for !MODE_HAS_INFINITIES a better fix would > be to do something like the patch below. > Consider say having a range of VAX float type: > #define M0 -FLT_MAX > #define M1 nextafterf (F0, FLT_MAX) > #define M2 nextafterf (M1, FLT_MAX) > [M2, M2] - [M0, M1] > Or perhaps if one or both of the operands are in such a case a min and max, > perform real_arithmetic recurse on the argument replaced with > dconstninf/dconstinf and then depending on inf pick the mininum or maximum > of the two results (and carefully think about what to do if both operands > are min/max). LGTM. Aldy > > 2023-03-20 Jakub Jelinek <jakub@redhat.com> > > * range-op-float.cc (frange_arithmetic): For !MODE_HAS_INFINITIES > types, pretend operands with minimum or maximum values are actually > infinities. > > --- gcc/range-op-float.cc.jj 2023-03-10 12:40:19.673108938 +0100 > +++ gcc/range-op-float.cc 2023-03-20 16:58:36.604981486 +0100 > @@ -313,8 +313,26 @@ frange_arithmetic (enum tree_code code, > REAL_VALUE_TYPE value; > enum machine_mode mode = TYPE_MODE (type); > bool mode_composite = MODE_COMPOSITE_P (mode); > + const REAL_VALUE_TYPE *pop1 = &op1; > + const REAL_VALUE_TYPE *pop2 = &op2; > > - bool inexact = real_arithmetic (&value, code, &op1, &op2); > + if (!MODE_HAS_INFINITIES (mode)) > + { > + // If mode doesn't have infinities, the minimum and maximum > + // values are saturating. Pretend for real_arithmetic such > + // values are actual infinities. real_convert will then > + // canonicalize the result not to be an infinity. > + if (frange_val_is_min (op1, type)) > + pop1 = &dconstninf; > + else if (frange_val_is_max (op1, type)) > + pop1 = &dconstinf; > + if (frange_val_is_min (op2, type)) > + pop2 = &dconstninf; > + else if (frange_val_is_max (op2, type)) > + pop2 = &dconstinf; > + } > + > + bool inexact = real_arithmetic (&value, code, pop1, pop2); > real_convert (&result, mode, &value); > > // Be extra careful if there may be discrepancies between the > > > Jakub > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] 2023-03-10 8:53 ` Richard Biener 2023-03-10 10:29 ` Jakub Jelinek @ 2023-03-21 13:28 ` Aldy Hernandez 2023-03-21 13:39 ` Jakub Jelinek 1 sibling, 1 reply; 19+ messages in thread From: Aldy Hernandez @ 2023-03-21 13:28 UTC (permalink / raw) To: Richard Biener, Jakub Jelinek; +Cc: gcc-patches, Andrew MacLeod On 3/10/23 09:53, Richard Biener wrote: > On Fri, 10 Mar 2023, Jakub Jelinek wrote: Coming back to this... > /* Temporarily disable -ffinite-math-only, so that frange::set doesn't > reduce the range back to real_min_representable (type) as lower bound > 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 (type, lb, ub); > if (lhs.kind () != VR_VARYING) > { > ret.clear_nan (); > ret.union_ (lhs); > } > flag_finite_math_only = save_flag_finite_math_only; It looks like what you want to do is be able to create a range with a known NAN state, but without the setter reducing the range to min/max_representable. How about we enhance the API to provide: 1. Constructor with a known NAN state. 2. Setter with a flag to keep it from canonicalizing into min/max_representable. The flag in 2 could in the future be saved in the frange object to keep union and friends from further canonicalization. So the above could be written as: // Construct [lb, ub] with a known NAN state. frange tmp (lb, ub, lhs.get_nan_state ()); // Set RET without dropping/reducing the range to MIN/MAX. ret.set (tmp, FRANGE_FLAG_NO_REPRESENTABLE_REDUCTION); An alternative is to allow the setter to set everything: ret.set (type, lb, ub, lhs.get_nan_state (), FRANGE_FLAG_NO_REPRESENTABLE_REDUCTION); Would this work? I'd be happy to whip up something this week, or if preferred, leave it to the next release. Aldy ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] 2023-03-21 13:28 ` Aldy Hernandez @ 2023-03-21 13:39 ` Jakub Jelinek 2023-03-21 13:49 ` Aldy Hernandez 0 siblings, 1 reply; 19+ messages in thread From: Jakub Jelinek @ 2023-03-21 13:39 UTC (permalink / raw) To: Aldy Hernandez; +Cc: Richard Biener, gcc-patches, Andrew MacLeod On Tue, Mar 21, 2023 at 02:28:31PM +0100, Aldy Hernandez wrote: > > /* Temporarily disable -ffinite-math-only, so that frange::set doesn't > > reduce the range back to real_min_representable (type) as lower bound > > 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 (type, lb, ub); > > if (lhs.kind () != VR_VARYING) > > { > > ret.clear_nan (); > > ret.union_ (lhs); > > } > > flag_finite_math_only = save_flag_finite_math_only; > > It looks like what you want to do is be able to create a range with a known > NAN state, but without the setter reducing the range to > min/max_representable. > > How about we enhance the API to provide: > > 1. Constructor with a known NAN state. > 2. Setter with a flag to keep it from canonicalizing into > min/max_representable. > > The flag in 2 could in the future be saved in the frange object to keep > union and friends from further canonicalization. > > So the above could be written as: > > // Construct [lb, ub] with a known NAN state. > frange tmp (lb, ub, lhs.get_nan_state ()); > > // Set RET without dropping/reducing the range to MIN/MAX. > ret.set (tmp, FRANGE_FLAG_NO_REPRESENTABLE_REDUCTION); > > An alternative is to allow the setter to set everything: > > ret.set (type, lb, ub, > lhs.get_nan_state (), > FRANGE_FLAG_NO_REPRESENTABLE_REDUCTION); > > Would this work? I'd be happy to whip up something this week, or if > preferred, leave it to the next release. The latter would be better, I really don't need a temporary range in that spot, the union_ is only to copy the NaN state. Though, I think right now set actually doesn't do reduction to representable at all, all it does is equality compare bounds against the applicable boundaries and if both are equal and NaN state is appropriate, change it into VR_VARYING. So maybe for now all we need is the 4 argument set. Jakub ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] 2023-03-21 13:39 ` Jakub Jelinek @ 2023-03-21 13:49 ` Aldy Hernandez 2023-03-21 13:56 ` Jakub Jelinek 0 siblings, 1 reply; 19+ messages in thread From: Aldy Hernandez @ 2023-03-21 13:49 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches, Andrew MacLeod On 3/21/23 14:39, Jakub Jelinek wrote: > On Tue, Mar 21, 2023 at 02:28:31PM +0100, Aldy Hernandez wrote: >>> /* Temporarily disable -ffinite-math-only, so that frange::set doesn't >>> reduce the range back to real_min_representable (type) as lower bound >>> 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 (type, lb, ub); >>> if (lhs.kind () != VR_VARYING) >>> { >>> ret.clear_nan (); >>> ret.union_ (lhs); >>> } >>> flag_finite_math_only = save_flag_finite_math_only; >> >> It looks like what you want to do is be able to create a range with a known >> NAN state, but without the setter reducing the range to >> min/max_representable. >> >> How about we enhance the API to provide: >> >> 1. Constructor with a known NAN state. >> 2. Setter with a flag to keep it from canonicalizing into >> min/max_representable. >> >> The flag in 2 could in the future be saved in the frange object to keep >> union and friends from further canonicalization. >> >> So the above could be written as: >> >> // Construct [lb, ub] with a known NAN state. >> frange tmp (lb, ub, lhs.get_nan_state ()); >> >> // Set RET without dropping/reducing the range to MIN/MAX. >> ret.set (tmp, FRANGE_FLAG_NO_REPRESENTABLE_REDUCTION); >> >> An alternative is to allow the setter to set everything: >> >> ret.set (type, lb, ub, >> lhs.get_nan_state (), >> FRANGE_FLAG_NO_REPRESENTABLE_REDUCTION); >> >> Would this work? I'd be happy to whip up something this week, or if >> preferred, leave it to the next release. > > The latter would be better, I really don't need a temporary range in that > spot, the union_ is only to copy the NaN state. > Though, I think right now set actually doesn't do reduction to representable > at all, all it does is equality compare bounds against the applicable > boundaries and if both are equal and NaN state is appropriate, change it > into VR_VARYING. > So maybe for now all we need is the 4 argument set. So, this? frange::set (tree type, const REAL_VALUE_TYPE &min, const REAL_VALUE_TYPE &max, const nan_state &, value_range_kind kind = VR_RANGE) If so, I'll start poking at it. Aldy ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] 2023-03-21 13:49 ` Aldy Hernandez @ 2023-03-21 13:56 ` Jakub Jelinek 2023-03-22 6:32 ` Aldy Hernandez 0 siblings, 1 reply; 19+ messages in thread From: Jakub Jelinek @ 2023-03-21 13:56 UTC (permalink / raw) To: Aldy Hernandez; +Cc: Richard Biener, gcc-patches, Andrew MacLeod On Tue, Mar 21, 2023 at 02:49:49PM +0100, Aldy Hernandez wrote: > So, this? > > frange::set (tree type, > const REAL_VALUE_TYPE &min, > const REAL_VALUE_TYPE &max, > const nan_state &, > value_range_kind kind = VR_RANGE) > > If so, I'll start poking at it. Yes. Jakub ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] 2023-03-21 13:56 ` Jakub Jelinek @ 2023-03-22 6:32 ` Aldy Hernandez 2023-03-22 8:35 ` Jakub Jelinek 2023-03-28 7:54 ` [PATCH] range-op-float: Use get_nan_state in float_widen_lhs_range Jakub Jelinek 0 siblings, 2 replies; 19+ messages in thread From: Aldy Hernandez @ 2023-03-22 6:32 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches, Andrew MacLeod [-- Attachment #1: Type: text/plain, Size: 372 bytes --] On 3/21/23 14:56, Jakub Jelinek wrote: > On Tue, Mar 21, 2023 at 02:49:49PM +0100, Aldy Hernandez wrote: >> So, this? >> >> frange::set (tree type, >> const REAL_VALUE_TYPE &min, >> const REAL_VALUE_TYPE &max, >> const nan_state &, >> value_range_kind kind = VR_RANGE) >> >> If so, I'll start poking at it. > > Yes. > > Jakub > Tested on x86-64 Linux. OK? Aldy [-- Attachment #2: 0001-frange-Implement-nan_state-class-PR109008.patch --] [-- Type: text/x-patch, Size: 4446 bytes --] From 64b9fabc0f31c661bb72029440227fe319566654 Mon Sep 17 00:00:00 2001 From: Aldy Hernandez <aldyh@redhat.com> Date: Wed, 8 Mar 2023 10:58:01 +0100 Subject: [PATCH] frange: Implement nan_state class [PR109008] This patch implements a nan_state class, that allows us to query or pass around the NANness of an frange. We can store +NAN, -NAN, +-NAN, or not-a-NAN with it. I tried to touch as little as possible, leaving other cleanups to the next release. For example, we should replace the m_*_nan fields in frange with nan_state, and provide relevant accessors to nan_state (isnan, etc). gcc/ChangeLog: * value-range.cc (frange::set): Add nan_state argument. * value-range.h (class nan_state): New. (frange::get_nan_state): New. --- gcc/value-range.cc | 18 +++++++++++--- gcc/value-range.h | 62 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/gcc/value-range.cc b/gcc/value-range.cc index a535337c47a..f71554b7d7c 100644 --- a/gcc/value-range.cc +++ b/gcc/value-range.cc @@ -284,7 +284,7 @@ frange::flush_denormals_to_zero () void frange::set (tree type, const REAL_VALUE_TYPE &min, const REAL_VALUE_TYPE &max, - value_range_kind kind) + const nan_state &nan, value_range_kind kind) { switch (kind) { @@ -316,8 +316,8 @@ frange::set (tree type, m_max = max; if (HONOR_NANS (m_type)) { - m_pos_nan = true; - m_neg_nan = true; + m_pos_nan = nan.pos_p (); + m_neg_nan = nan.neg_p (); } else { @@ -367,6 +367,18 @@ frange::set (tree type, verify_range (); } +// Setter for an frange defaulting the NAN possibility to +-NAN when +// HONOR_NANS. + +void +frange::set (tree type, + const REAL_VALUE_TYPE &min, const REAL_VALUE_TYPE &max, + value_range_kind kind) +{ + nan_state nan; + set (type, min, max, nan, kind); +} + void frange::set (tree min, tree max, value_range_kind kind) { diff --git a/gcc/value-range.h b/gcc/value-range.h index f4ac73b499f..ec50346826c 100644 --- a/gcc/value-range.h +++ b/gcc/value-range.h @@ -268,6 +268,56 @@ public: virtual void accept (const vrange_visitor &v) const override; }; +// The NAN state as an opaque object. The default constructor is +-NAN. + +class nan_state +{ +public: + nan_state (); + nan_state (bool pos_nan, bool neg_nan); + bool neg_p () const; + bool pos_p () const; +private: + bool m_pos_nan; + bool m_neg_nan; +}; + +// Default constructor initializing the object to +-NAN. + +inline +nan_state::nan_state () +{ + m_pos_nan = true; + m_neg_nan = true; +} + +// Constructor initializing the object to +NAN if POS_NAN is set, -NAN +// if NEG_NAN is set, or +-NAN if both are set. Otherwise POS_NAN and +// NEG_NAN are clear, and the object cannot be a NAN. + +inline +nan_state::nan_state (bool pos_nan, bool neg_nan) +{ + m_pos_nan = pos_nan; + m_neg_nan = neg_nan; +} + +// Return if +NAN is possible. + +inline bool +nan_state::pos_p () const +{ + return m_pos_nan; +} + +// Return if -NAN is possible. + +inline bool +nan_state::neg_p () const +{ + return m_neg_nan; +} + // A floating point range. // // The representation is a type with a couple of endpoints, unioned @@ -295,6 +345,8 @@ public: virtual void set (tree, tree, value_range_kind = VR_RANGE) override; void set (tree type, const REAL_VALUE_TYPE &, const REAL_VALUE_TYPE &, value_range_kind = VR_RANGE); + void set (tree type, const REAL_VALUE_TYPE &, const REAL_VALUE_TYPE &, + const nan_state &, value_range_kind = VR_RANGE); void set_nan (tree type); void set_nan (tree type, bool sign); virtual void set_varying (tree type) override; @@ -315,9 +367,11 @@ public: bool operator!= (const frange &r) const { return !(*this == r); } const REAL_VALUE_TYPE &lower_bound () const; const REAL_VALUE_TYPE &upper_bound () const; + nan_state get_nan_state () const; void update_nan (); void update_nan (bool sign); void update_nan (tree) = delete; // Disallow silent conversion to bool. + void update_nan (const nan_state &); void clear_nan (); // fpclassify like API @@ -358,6 +412,14 @@ frange::upper_bound () const return m_max; } +// Return the NAN state. + +inline nan_state +frange::get_nan_state () const +{ + return nan_state (m_pos_nan, m_neg_nan); +} + // is_a<> and as_a<> implementation for vrange. // Anything we haven't specialized is a hard fail. -- 2.39.2 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] 2023-03-22 6:32 ` Aldy Hernandez @ 2023-03-22 8:35 ` Jakub Jelinek 2023-03-28 7:54 ` [PATCH] range-op-float: Use get_nan_state in float_widen_lhs_range Jakub Jelinek 1 sibling, 0 replies; 19+ messages in thread From: Jakub Jelinek @ 2023-03-22 8:35 UTC (permalink / raw) To: Aldy Hernandez; +Cc: Richard Biener, gcc-patches, Andrew MacLeod On Wed, Mar 22, 2023 at 07:32:44AM +0100, Aldy Hernandez wrote: > On 3/21/23 14:56, Jakub Jelinek wrote: > > On Tue, Mar 21, 2023 at 02:49:49PM +0100, Aldy Hernandez wrote: > > > So, this? > > > > > > frange::set (tree type, > > > const REAL_VALUE_TYPE &min, > > > const REAL_VALUE_TYPE &max, > > > const nan_state &, > > > value_range_kind kind = VR_RANGE) > > > > > > If so, I'll start poking at it. > > > > Yes. > > > > Jakub > > > > Tested on x86-64 Linux. > > OK? Yes, thanks. Jakub ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] range-op-float: Use get_nan_state in float_widen_lhs_range 2023-03-22 6:32 ` Aldy Hernandez 2023-03-22 8:35 ` Jakub Jelinek @ 2023-03-28 7:54 ` Jakub Jelinek 2023-03-28 8:50 ` Aldy Hernandez 1 sibling, 1 reply; 19+ messages in thread From: Jakub Jelinek @ 2023-03-28 7:54 UTC (permalink / raw) To: Aldy Hernandez; +Cc: gcc-patches, Andrew MacLeod Hi! On Wed, Mar 22, 2023 at 07:32:44AM +0100, Aldy Hernandez wrote: > * value-range.cc (frange::set): Add nan_state argument. > * value-range.h (class nan_state): New. > (frange::get_nan_state): New. The following patch makes use of those changes in float_widen_lhs_range. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2023-03-28 Jakub Jelinek <jakub@redhat.com> * range-op-float.cc (float_widen_lhs_range): Use pass get_nan_state as 4th argument to set to avoid clear_nan and union_ calls. --- gcc/range-op-float.cc.jj 2023-03-23 15:25:47.119740274 +0100 +++ gcc/range-op-float.cc 2023-03-27 13:28:18.847264635 +0200 @@ -2262,12 +2262,7 @@ 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 (type, lb, ub); - if (lhs.kind () != VR_VARYING) - { - ret.clear_nan (); - ret.union_ (lhs); - } + ret.set (type, lb, ub, lhs.get_nan_state ()); flag_finite_math_only = save_flag_finite_math_only; return ret; } Jakub ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Use get_nan_state in float_widen_lhs_range 2023-03-28 7:54 ` [PATCH] range-op-float: Use get_nan_state in float_widen_lhs_range Jakub Jelinek @ 2023-03-28 8:50 ` Aldy Hernandez 2023-03-29 9:39 ` Aldy Hernandez 0 siblings, 1 reply; 19+ messages in thread From: Aldy Hernandez @ 2023-03-28 8:50 UTC (permalink / raw) To: Jakub Jelinek; +Cc: gcc-patches, Andrew MacLeod On 3/28/23 09:54, Jakub Jelinek wrote: > Hi! > > On Wed, Mar 22, 2023 at 07:32:44AM +0100, Aldy Hernandez wrote: >> * value-range.cc (frange::set): Add nan_state argument. >> * value-range.h (class nan_state): New. >> (frange::get_nan_state): New. > > The following patch makes use of those changes in float_widen_lhs_range. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? LGTM. Aldy > > 2023-03-28 Jakub Jelinek <jakub@redhat.com> > > * range-op-float.cc (float_widen_lhs_range): Use pass get_nan_state > as 4th argument to set to avoid clear_nan and union_ calls. > > --- gcc/range-op-float.cc.jj 2023-03-23 15:25:47.119740274 +0100 > +++ gcc/range-op-float.cc 2023-03-27 13:28:18.847264635 +0200 > @@ -2262,12 +2262,7 @@ 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 (type, lb, ub); > - if (lhs.kind () != VR_VARYING) > - { > - ret.clear_nan (); > - ret.union_ (lhs); > - } > + ret.set (type, lb, ub, lhs.get_nan_state ()); > flag_finite_math_only = save_flag_finite_math_only; > return ret; > } > > > Jakub > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] range-op-float: Use get_nan_state in float_widen_lhs_range 2023-03-28 8:50 ` Aldy Hernandez @ 2023-03-29 9:39 ` Aldy Hernandez 0 siblings, 0 replies; 19+ messages in thread From: Aldy Hernandez @ 2023-03-29 9:39 UTC (permalink / raw) To: Jakub Jelinek; +Cc: gcc-patches, Andrew MacLeod On 3/28/23 10:50, Aldy Hernandez wrote: > > > On 3/28/23 09:54, Jakub Jelinek wrote: >> Hi! >> >> On Wed, Mar 22, 2023 at 07:32:44AM +0100, Aldy Hernandez wrote: >>> * value-range.cc (frange::set): Add nan_state argument. >>> * value-range.h (class nan_state): New. >>> (frange::get_nan_state): New. >> >> The following patch makes use of those changes in float_widen_lhs_range. >> >> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > LGTM. > Aldy >> >> 2023-03-28 Jakub Jelinek <jakub@redhat.com> >> >> * range-op-float.cc (float_widen_lhs_range): Use pass get_nan_state >> as 4th argument to set to avoid clear_nan and union_ calls. >> >> --- gcc/range-op-float.cc.jj 2023-03-23 15:25:47.119740274 +0100 >> +++ gcc/range-op-float.cc 2023-03-27 13:28:18.847264635 +0200 >> @@ -2262,12 +2262,7 @@ 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 (type, lb, ub); >> - if (lhs.kind () != VR_VARYING) >> - { >> - ret.clear_nan (); >> - ret.union_ (lhs); >> - } >> + ret.set (type, lb, ub, lhs.get_nan_state ()); >> flag_finite_math_only = save_flag_finite_math_only; >> return ret; I just noticed we're still doing the flag_finite_math_only hack. We should add some sort of flag to the setter to avoid reducing the range. I suppose this can wait to the next release. Aldy ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2023-03-29 9:39 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-10 8:07 [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] Jakub Jelinek 2023-03-10 8:53 ` Richard Biener 2023-03-10 10:29 ` Jakub Jelinek 2023-03-13 7:18 ` Aldy Hernandez 2023-03-13 7:50 ` Richard Biener 2023-03-13 7:59 ` Aldy Hernandez 2023-03-13 8:06 ` Jakub Jelinek 2023-03-13 8:41 ` Aldy Hernandez 2023-03-20 16:14 ` Jakub Jelinek 2023-03-21 12:56 ` Aldy Hernandez 2023-03-21 13:28 ` Aldy Hernandez 2023-03-21 13:39 ` Jakub Jelinek 2023-03-21 13:49 ` Aldy Hernandez 2023-03-21 13:56 ` Jakub Jelinek 2023-03-22 6:32 ` Aldy Hernandez 2023-03-22 8:35 ` Jakub Jelinek 2023-03-28 7:54 ` [PATCH] range-op-float: Use get_nan_state in float_widen_lhs_range Jakub Jelinek 2023-03-28 8:50 ` Aldy Hernandez 2023-03-29 9:39 ` Aldy Hernandez
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).