Message ID | 20220823114224.904934-1-aldyh@redhat.com |
---|---|
State | New |
Headers | show |
Series | Add support for floating point endpoints to frange. | expand |
[pinskia: I'm CCing you as the author of the match.pd pattern.] So, as I wrap up the work here (latest patch attached), I see there's another phiopt regression (not spaceship related). I was hoping someone could either give me a hand, or offer some guidance. The failure is in gcc.dg/tree-ssa/phi-opt-24.c. We fail to transform the following into -A: /* { dg-options "-O2 -fno-signed-zeros -fdump-tree-phiopt" } */ float f0(float A) { // A == 0? A : -A same as -A if (A == 0) return A; return -A; } This is because the abs/negative match.pd pattern here: /* abs/negative simplifications moved from fold_cond_expr_with_comparison, Need to handle (A - B) case as fold_cond_expr_with_comparison does. Need to handle UN* comparisons. ... ... Sees IL that has the 0.0 propagated. Instead of: <bb 2> [local count: 1073741824]: if (A_2(D) == 0.0) goto <bb 4>; [34.00%] else goto <bb 3>; [66.00%] <bb 3> [local count: 708669601]: _3 = -A_2(D); <bb 4> [local count: 1073741824]: # _1 = PHI <A_2(D)(2), _3(3)> It now sees: <bb 4> [local count: 1073741824]: # _1 = PHI <0.0(2), _3(3)> which it leaves untouched, causing the if conditional to survive. Is this something that can be done by improving the match.pd pattern, or should be done elsewhere? Thanks. Aldy
On Fri, Aug 26, 2022 at 8:55 AM Aldy Hernandez <aldyh@redhat.com> wrote: > > [pinskia: I'm CCing you as the author of the match.pd pattern.] > > So, as I wrap up the work here (latest patch attached), I see there's > another phiopt regression (not spaceship related). I was hoping > someone could either give me a hand, or offer some guidance. > > The failure is in gcc.dg/tree-ssa/phi-opt-24.c. > > We fail to transform the following into -A: > > /* { dg-options "-O2 -fno-signed-zeros -fdump-tree-phiopt" } */ > > float f0(float A) > { > // A == 0? A : -A same as -A > if (A == 0) return A; > return -A; > } > > This is because the abs/negative match.pd pattern here: > > /* abs/negative simplifications moved from fold_cond_expr_with_comparison, > Need to handle (A - B) case as fold_cond_expr_with_comparison does. > Need to handle UN* comparisons. > ... > ... > > Sees IL that has the 0.0 propagated. > > Instead of: > > <bb 2> [local count: 1073741824]: > if (A_2(D) == 0.0) > goto <bb 4>; [34.00%] > else > goto <bb 3>; [66.00%] > > <bb 3> [local count: 708669601]: > _3 = -A_2(D); > > <bb 4> [local count: 1073741824]: > # _1 = PHI <A_2(D)(2), _3(3)> > > It now sees: > > <bb 4> [local count: 1073741824]: > # _1 = PHI <0.0(2), _3(3)> > > which it leaves untouched, causing the if conditional to survive. > > Is this something that can be done by improving the match.pd pattern, > or should be done elsewhere? Oh the pattern which is supposed to catch this does: (simplify (cnd (cmp @0 zerop) integer_zerop (negate@1 @0)) (if (!HONOR_SIGNED_ZEROS (type)) @1)) Notice the integer_zerop here. fold_cond_expr_with_comparison has integer_zerop there too. I am not 100% sure you can replace A_2 with 0.0 where you are doing it as mentioned in another thread. Thanks, Andrew Pinski > > Thanks. > Aldy
On Fri, Aug 26, 2022, 19:40 Andrew Pinski <pinskia@gmail.com> wrote: > On Fri, Aug 26, 2022 at 8:55 AM Aldy Hernandez <aldyh@redhat.com> wrote: > > > > [pinskia: I'm CCing you as the author of the match.pd pattern.] > > > > So, as I wrap up the work here (latest patch attached), I see there's > > another phiopt regression (not spaceship related). I was hoping > > someone could either give me a hand, or offer some guidance. > > > > The failure is in gcc.dg/tree-ssa/phi-opt-24.c. > > > > We fail to transform the following into -A: > > > > /* { dg-options "-O2 -fno-signed-zeros -fdump-tree-phiopt" } */ > > > > float f0(float A) > > { > > // A == 0? A : -A same as -A > > if (A == 0) return A; > > return -A; > > } > > > > This is because the abs/negative match.pd pattern here: > > > > /* abs/negative simplifications moved from > fold_cond_expr_with_comparison, > > Need to handle (A - B) case as fold_cond_expr_with_comparison does. > > Need to handle UN* comparisons. > > ... > > ... > > > > Sees IL that has the 0.0 propagated. > > > > Instead of: > > > > <bb 2> [local count: 1073741824]: > > if (A_2(D) == 0.0) > > goto <bb 4>; [34.00%] > > else > > goto <bb 3>; [66.00%] > > > > <bb 3> [local count: 708669601]: > > _3 = -A_2(D); > > > > <bb 4> [local count: 1073741824]: > > # _1 = PHI <A_2(D)(2), _3(3)> > > > > It now sees: > > > > <bb 4> [local count: 1073741824]: > > # _1 = PHI <0.0(2), _3(3)> > > > > which it leaves untouched, causing the if conditional to survive. > > > > Is this something that can be done by improving the match.pd pattern, > > or should be done elsewhere? > > Oh the pattern which is supposed to catch this does: > (simplify > (cnd (cmp @0 zerop) integer_zerop (negate@1 @0)) > (if (!HONOR_SIGNED_ZEROS (type)) > @1)) > > Notice the integer_zerop here. > fold_cond_expr_with_comparison has integer_zerop there too. > I am not 100% sure you can replace A_2 with 0.0 where you are doing it > as mentioned in another thread. > Are you sure we can't make the replacement, cause the test runs with -fno-signed-zeros? Aldy > Thanks, > Andrew Pinski > > > > > > Thanks. > > Aldy > >
On Fri, Aug 26, 2022 at 7:40 PM Andrew Pinski <pinskia@gmail.com> wrote: > > On Fri, Aug 26, 2022 at 8:55 AM Aldy Hernandez <aldyh@redhat.com> wrote: > > > > [pinskia: I'm CCing you as the author of the match.pd pattern.] > > > > So, as I wrap up the work here (latest patch attached), I see there's > > another phiopt regression (not spaceship related). I was hoping > > someone could either give me a hand, or offer some guidance. > > > > The failure is in gcc.dg/tree-ssa/phi-opt-24.c. > > > > We fail to transform the following into -A: > > > > /* { dg-options "-O2 -fno-signed-zeros -fdump-tree-phiopt" } */ > > > > float f0(float A) > > { > > // A == 0? A : -A same as -A > > if (A == 0) return A; > > return -A; > > } > > > > This is because the abs/negative match.pd pattern here: > > > > /* abs/negative simplifications moved from fold_cond_expr_with_comparison, > > Need to handle (A - B) case as fold_cond_expr_with_comparison does. > > Need to handle UN* comparisons. > > ... > > ... > > > > Sees IL that has the 0.0 propagated. > > > > Instead of: > > > > <bb 2> [local count: 1073741824]: > > if (A_2(D) == 0.0) > > goto <bb 4>; [34.00%] > > else > > goto <bb 3>; [66.00%] > > > > <bb 3> [local count: 708669601]: > > _3 = -A_2(D); > > > > <bb 4> [local count: 1073741824]: > > # _1 = PHI <A_2(D)(2), _3(3)> > > > > It now sees: > > > > <bb 4> [local count: 1073741824]: > > # _1 = PHI <0.0(2), _3(3)> > > > > which it leaves untouched, causing the if conditional to survive. > > > > Is this something that can be done by improving the match.pd pattern, > > or should be done elsewhere? > > Oh the pattern which is supposed to catch this does: > (simplify > (cnd (cmp @0 zerop) integer_zerop (negate@1 @0)) > (if (!HONOR_SIGNED_ZEROS (type)) > @1)) On trunk without any patches, for the following snippet with -O2 -fno-signed-zeros -fdump-tree-phiopt-folding... float f0(float A) { // A == 0? A : -A same as -A if (A == 0) return A; return -A; } ...the phiopt2 dump file has: Applying pattern match.pd:4805, gimple-match.cc:69291, which corresponds to the aforementioned pattern. So it looks like that was the pattern that was matching that isn't any more? Are you saying this pattern should only work with integers? Aldy
On Fri, Aug 26, 2022 at 12:16 PM Aldy Hernandez <aldyh@redhat.com> wrote: > > On Fri, Aug 26, 2022 at 7:40 PM Andrew Pinski <pinskia@gmail.com> wrote: > > > > On Fri, Aug 26, 2022 at 8:55 AM Aldy Hernandez <aldyh@redhat.com> wrote: > > > > > > [pinskia: I'm CCing you as the author of the match.pd pattern.] > > > > > > So, as I wrap up the work here (latest patch attached), I see there's > > > another phiopt regression (not spaceship related). I was hoping > > > someone could either give me a hand, or offer some guidance. > > > > > > The failure is in gcc.dg/tree-ssa/phi-opt-24.c. > > > > > > We fail to transform the following into -A: > > > > > > /* { dg-options "-O2 -fno-signed-zeros -fdump-tree-phiopt" } */ > > > > > > float f0(float A) > > > { > > > // A == 0? A : -A same as -A > > > if (A == 0) return A; > > > return -A; > > > } > > > > > > This is because the abs/negative match.pd pattern here: > > > > > > /* abs/negative simplifications moved from fold_cond_expr_with_comparison, > > > Need to handle (A - B) case as fold_cond_expr_with_comparison does. > > > Need to handle UN* comparisons. > > > ... > > > ... > > > > > > Sees IL that has the 0.0 propagated. > > > > > > Instead of: > > > > > > <bb 2> [local count: 1073741824]: > > > if (A_2(D) == 0.0) > > > goto <bb 4>; [34.00%] > > > else > > > goto <bb 3>; [66.00%] > > > > > > <bb 3> [local count: 708669601]: > > > _3 = -A_2(D); > > > > > > <bb 4> [local count: 1073741824]: > > > # _1 = PHI <A_2(D)(2), _3(3)> > > > > > > It now sees: > > > > > > <bb 4> [local count: 1073741824]: > > > # _1 = PHI <0.0(2), _3(3)> > > > > > > which it leaves untouched, causing the if conditional to survive. > > > > > > Is this something that can be done by improving the match.pd pattern, > > > or should be done elsewhere? > > > > Oh the pattern which is supposed to catch this does: > > (simplify > > (cnd (cmp @0 zerop) integer_zerop (negate@1 @0)) > > (if (!HONOR_SIGNED_ZEROS (type)) > > @1)) > > On trunk without any patches, for the following snippet with -O2 > -fno-signed-zeros -fdump-tree-phiopt-folding... > > float f0(float A) > { > // A == 0? A : -A same as -A > if (A == 0) return A; > return -A; > } > > ...the phiopt2 dump file has: > > Applying pattern match.pd:4805, gimple-match.cc:69291, which > corresponds to the aforementioned pattern. So it looks like that was > the pattern that was matching that isn't any more? > > Are you saying this pattern should only work with integers? I am saying the pattern which is right after the one that matches (without your patch) currrently works for integer only. You could change integer_zerop to zerop in that pattern but I am not 100% sure that is valid thing to do. Note there are a few other patterns in that for loop that does integer_zerop which might need to be zerop too. Thanks, Andrew Pinski > > Aldy >
Jakub, et al... here is the latest version of the frange endpoints patch addressing the signed zero problem (well treating +-0.0 ambiguously), as well as implementing all the relational operators. [Andrew M: I mostly copied our relop code from irange, while keeping track NANs, etc. It should be pleasantly familiar.] A few notes... We can now represent things like [5.0, 5.0], which is the singleton 5.0 *or* a NAN. OTOH, we could also have [5.0, 5.0] !NAN, which is 5.0 without the possibility of a NAN. This allows us to track appropriate ranges on both sides of an if, but keep track of which side (true side) we definitely know we won't have a NAN. As mentioned, frange::singleton_p([0,0]) returns false for any version of 0.0 (unless !HONOR_SIGNED_ZEROS). I'll work on zero tracking at a later time. The patch is getting pretty large as is. For convenience, singleton_p() returns false for a NAN. IMO, it makes the implementation cleaner, but I'm not wed to the idea if someone objects. This patch passes all tests for all languages, including go and ada, with the aforementioned exception of gcc.dg/tree-ssa/phi-opt-24.c which is getting confused because we have propagated (correctly) a 0.0 into the PHI. Hints? Takers? ;-). Aldy On Fri, Aug 26, 2022 at 9:44 PM Andrew Pinski <pinskia@gmail.com> wrote: > > On Fri, Aug 26, 2022 at 12:16 PM Aldy Hernandez <aldyh@redhat.com> wrote: > > > > On Fri, Aug 26, 2022 at 7:40 PM Andrew Pinski <pinskia@gmail.com> wrote: > > > > > > On Fri, Aug 26, 2022 at 8:55 AM Aldy Hernandez <aldyh@redhat.com> wrote: > > > > > > > > [pinskia: I'm CCing you as the author of the match.pd pattern.] > > > > > > > > So, as I wrap up the work here (latest patch attached), I see there's > > > > another phiopt regression (not spaceship related). I was hoping > > > > someone could either give me a hand, or offer some guidance. > > > > > > > > The failure is in gcc.dg/tree-ssa/phi-opt-24.c. > > > > > > > > We fail to transform the following into -A: > > > > > > > > /* { dg-options "-O2 -fno-signed-zeros -fdump-tree-phiopt" } */ > > > > > > > > float f0(float A) > > > > { > > > > // A == 0? A : -A same as -A > > > > if (A == 0) return A; > > > > return -A; > > > > } > > > > > > > > This is because the abs/negative match.pd pattern here: > > > > > > > > /* abs/negative simplifications moved from fold_cond_expr_with_comparison, > > > > Need to handle (A - B) case as fold_cond_expr_with_comparison does. > > > > Need to handle UN* comparisons. > > > > ... > > > > ... > > > > > > > > Sees IL that has the 0.0 propagated. > > > > > > > > Instead of: > > > > > > > > <bb 2> [local count: 1073741824]: > > > > if (A_2(D) == 0.0) > > > > goto <bb 4>; [34.00%] > > > > else > > > > goto <bb 3>; [66.00%] > > > > > > > > <bb 3> [local count: 708669601]: > > > > _3 = -A_2(D); > > > > > > > > <bb 4> [local count: 1073741824]: > > > > # _1 = PHI <A_2(D)(2), _3(3)> > > > > > > > > It now sees: > > > > > > > > <bb 4> [local count: 1073741824]: > > > > # _1 = PHI <0.0(2), _3(3)> > > > > > > > > which it leaves untouched, causing the if conditional to survive. > > > > > > > > Is this something that can be done by improving the match.pd pattern, > > > > or should be done elsewhere? > > > > > > Oh the pattern which is supposed to catch this does: > > > (simplify > > > (cnd (cmp @0 zerop) integer_zerop (negate@1 @0)) > > > (if (!HONOR_SIGNED_ZEROS (type)) > > > @1)) > > > > On trunk without any patches, for the following snippet with -O2 > > -fno-signed-zeros -fdump-tree-phiopt-folding... > > > > float f0(float A) > > { > > // A == 0? A : -A same as -A > > if (A == 0) return A; > > return -A; > > } > > > > ...the phiopt2 dump file has: > > > > Applying pattern match.pd:4805, gimple-match.cc:69291, which > > corresponds to the aforementioned pattern. So it looks like that was > > the pattern that was matching that isn't any more? > > > > Are you saying this pattern should only work with integers? > > I am saying the pattern which is right after the one that matches > (without your patch) currrently works for integer only. > You could change integer_zerop to zerop in that pattern but I am not > 100% sure that is valid thing to do. > Note there are a few other patterns in that for loop that does > integer_zerop which might need to be zerop too. > > Thanks, > Andrew Pinski > > > > > Aldy > > >
On Mon, Aug 29, 2022 at 03:45:33PM +0200, Aldy Hernandez wrote: > For convenience, singleton_p() returns false for a NAN. IMO, it makes > the implementation cleaner, but I'm not wed to the idea if someone > objects. If singleton_p() is used to decide whether one can just replace a variable with singleton range with a constant, then certainly. If MODE_HAS_SIGNED_ZEROS, zero has 2 representations (-0.0 and 0.0) and NaNs have lots of different representations (the sign bit is ignored except for stuff like copysign/signbit, there are qNaNs and sNaNs and except for the single case how Inf is represented, all other values of the mantissa mean different representations of NaN). So, unless we track which exact form of NaN can appear, NaN or any [x, x] range with NaN property set can't be a singleton. There could be programs that propagate something important in NaN mantissa and would be upset if frange kills that. Of course, one needs to take into account that when a FPU creates NaN, it will create the canonical qNaN. Jakub
On 8/29/22 15:54, Jakub Jelinek via Gcc-patches wrote: > On Mon, Aug 29, 2022 at 03:45:33PM +0200, Aldy Hernandez wrote: >> For convenience, singleton_p() returns false for a NAN. IMO, it makes >> the implementation cleaner, but I'm not wed to the idea if someone >> objects. > > If singleton_p() is used to decide whether one can just replace a variable > with singleton range with a constant, then certainly. > If MODE_HAS_SIGNED_ZEROS, zero has 2 representations (-0.0 and 0.0) and > NaNs have lots of different representations (the sign bit is ignored > except for stuff like copysign/signbit, there are qNaNs and sNaNs and > except for the single case how Inf is represented, all other values of the > mantissa mean different representations of NaN). So, unless we track which > exact form of NaN can appear, NaN or any [x, x] range with NaN property > set can't be a singleton. There could be programs that propagate something > important in NaN mantissa and would be upset if frange kills that. > Of course, one needs to take into account that when a FPU creates NaN, it > will create the canonical qNaN. > > Jakub > But the NaNs are irrelevant with -ffinite-math-only, as are the various Infs (I do not know offhand which MODE_ that is) ... Kind regards,
On Mon, Aug 29, 2022 at 3:55 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Mon, Aug 29, 2022 at 03:45:33PM +0200, Aldy Hernandez wrote: > > For convenience, singleton_p() returns false for a NAN. IMO, it makes > > the implementation cleaner, but I'm not wed to the idea if someone > > objects. > > If singleton_p() is used to decide whether one can just replace a variable > with singleton range with a constant, then certainly. > If MODE_HAS_SIGNED_ZEROS, zero has 2 representations (-0.0 and 0.0) and > NaNs have lots of different representations (the sign bit is ignored > except for stuff like copysign/signbit, there are qNaNs and sNaNs and > except for the single case how Inf is represented, all other values of the > mantissa mean different representations of NaN). So, unless we track which > exact form of NaN can appear, NaN or any [x, x] range with NaN property Ok that was more or less what I was thinking. And no, we don't keep track of the type of NANs. How does this look? bool frange::singleton_p (tree *result) const { if (m_kind == VR_RANGE && real_identical (&m_min, &m_max)) { // If we're honoring signed zeros, fail because we don't know // which zero we have. This avoids propagating the wrong zero. if (HONOR_SIGNED_ZEROS (m_type) && zero_p ()) return false; // Return false for any singleton that may be a NAN. if (!get_nan ().no_p ()) return false; if (result) *result = build_real (m_type, m_min); return true; } return false; } Thanks. Aldy > set can't be a singleton. There could be programs that propagate something > important in NaN mantissa and would be upset if frange kills that. > Of course, one needs to take into account that when a FPU creates NaN, it > will create the canonical qNaN. > > Jakub >
On Mon, Aug 29, 2022 at 4:08 PM Toon Moene <toon@moene.org> wrote: > > On 8/29/22 15:54, Jakub Jelinek via Gcc-patches wrote: > > > On Mon, Aug 29, 2022 at 03:45:33PM +0200, Aldy Hernandez wrote: > > >> For convenience, singleton_p() returns false for a NAN. IMO, it makes > >> the implementation cleaner, but I'm not wed to the idea if someone > >> objects. > > > > If singleton_p() is used to decide whether one can just replace a variable > > with singleton range with a constant, then certainly. > > If MODE_HAS_SIGNED_ZEROS, zero has 2 representations (-0.0 and 0.0) and > > NaNs have lots of different representations (the sign bit is ignored > > except for stuff like copysign/signbit, there are qNaNs and sNaNs and > > except for the single case how Inf is represented, all other values of the > > mantissa mean different representations of NaN). So, unless we track which > > exact form of NaN can appear, NaN or any [x, x] range with NaN property > > set can't be a singleton. There could be programs that propagate something > > important in NaN mantissa and would be upset if frange kills that. > > Of course, one needs to take into account that when a FPU creates NaN, it > > will create the canonical qNaN. > > > > Jakub > > > > But the NaNs are irrelevant with -ffinite-math-only, as are the various > Infs (I do not know offhand which MODE_ that is) ... But even with -ffinite-math-only, is there any benefit to propagating a known NAN? For example: if (__builtin_isnan (x)) { use(x); } or perhaps: if (x != x) { use(x); } Should we propagate NAN into the use of x? For that matter, AFAICT we don't even generate the unordered comparisons needed to distinguish a NAN for -ffinite-math-only. void foo(float f) { if (__builtin_isnan (f)) bark(); } $ cat a.c.*original ;; Function foo (null) ;; enabled by -tree-original { if (0) { bark (); } } Cheers. Aldy
On Mon, Aug 29, 2022 at 04:08:58PM +0200, Aldy Hernandez wrote: > On Mon, Aug 29, 2022 at 3:55 PM Jakub Jelinek <jakub@redhat.com> wrote: > > > > On Mon, Aug 29, 2022 at 03:45:33PM +0200, Aldy Hernandez wrote: > > > For convenience, singleton_p() returns false for a NAN. IMO, it makes > > > the implementation cleaner, but I'm not wed to the idea if someone > > > objects. > > > > If singleton_p() is used to decide whether one can just replace a variable > > with singleton range with a constant, then certainly. > > If MODE_HAS_SIGNED_ZEROS, zero has 2 representations (-0.0 and 0.0) and > > NaNs have lots of different representations (the sign bit is ignored > > except for stuff like copysign/signbit, there are qNaNs and sNaNs and > > except for the single case how Inf is represented, all other values of the > > mantissa mean different representations of NaN). So, unless we track which > > exact form of NaN can appear, NaN or any [x, x] range with NaN property > > Ok that was more or less what I was thinking. And no, we don't keep > track of the type of NANs. > > How does this look? > > bool > frange::singleton_p (tree *result) const > { > if (m_kind == VR_RANGE && real_identical (&m_min, &m_max)) > { > // If we're honoring signed zeros, fail because we don't know > // which zero we have. This avoids propagating the wrong zero. > if (HONOR_SIGNED_ZEROS (m_type) && zero_p ()) > return false; > > // Return false for any singleton that may be a NAN. > if (!get_nan ().no_p ()) > return false; Perhaps if (HONOR_NANS (m_type) && !get_nan ().no_p ()) instead? Or do you ensure the nan property is never set for -ffinite-math-only? > > if (result) > *result = build_real (m_type, m_min); > return true; > } > return false; > } Otherwise LGTM. Jakub
On Mon, Aug 29, 2022 at 4:17 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Mon, Aug 29, 2022 at 04:08:58PM +0200, Aldy Hernandez wrote: > > On Mon, Aug 29, 2022 at 3:55 PM Jakub Jelinek <jakub@redhat.com> wrote: > > > > > > On Mon, Aug 29, 2022 at 03:45:33PM +0200, Aldy Hernandez wrote: > > > > For convenience, singleton_p() returns false for a NAN. IMO, it makes > > > > the implementation cleaner, but I'm not wed to the idea if someone > > > > objects. > > > > > > If singleton_p() is used to decide whether one can just replace a variable > > > with singleton range with a constant, then certainly. > > > If MODE_HAS_SIGNED_ZEROS, zero has 2 representations (-0.0 and 0.0) and > > > NaNs have lots of different representations (the sign bit is ignored > > > except for stuff like copysign/signbit, there are qNaNs and sNaNs and > > > except for the single case how Inf is represented, all other values of the > > > mantissa mean different representations of NaN). So, unless we track which > > > exact form of NaN can appear, NaN or any [x, x] range with NaN property > > > > Ok that was more or less what I was thinking. And no, we don't keep > > track of the type of NANs. > > > > How does this look? > > > > bool > > frange::singleton_p (tree *result) const > > { > > if (m_kind == VR_RANGE && real_identical (&m_min, &m_max)) > > { > > // If we're honoring signed zeros, fail because we don't know > > // which zero we have. This avoids propagating the wrong zero. > > if (HONOR_SIGNED_ZEROS (m_type) && zero_p ()) > > return false; > > > > // Return false for any singleton that may be a NAN. > > if (!get_nan ().no_p ()) > > return false; > > Perhaps if (HONOR_NANS (m_type) && !get_nan ().no_p ()) instead? > Or do you ensure the nan property is never set for -ffinite-math-only? See followup with Tom downthread. Sure, I can add the HONOR_NANS, but can we even "see" a NAN in the IL for -ffinite-math-only? I suppose it's cleaner with HONOR_NANS.... Aldy
On Mon, Aug 29, 2022 at 04:20:16PM +0200, Aldy Hernandez wrote: > Sure, I can add the HONOR_NANS, but can we even "see" a NAN in the IL > for -ffinite-math-only? Sure, you can, e.g. __builtin_nan{,s}{,f,l} etc. would do it. It would be UB to use it at runtime in -ffinite-math-only code though. Another question is, when making a range VARYING, do you set the NAN property or not when !HONOR_NANS && MODE_HAS_NANS? > I suppose it's cleaner with HONOR_NANS.... Jakub
On 8/29/22 16:15, Aldy Hernandez wrote: > But even with -ffinite-math-only, is there any benefit to propagating > a known NAN? For example: The original intent (in 2002) for the option -ffinite-math-only was for the optimizers to ignore all the various exceptions to common optimizations because they might not work correctly when presented with a NaN or an Inf. I do not know what the effect for floating point range information would be - offhand. But in the *spirit* of this option would be to ignore that the range [5.0, 5.0] would "also" contain NaN, for instance. Kind regards,
On Mon, Aug 29, 2022 at 4:27 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Mon, Aug 29, 2022 at 04:20:16PM +0200, Aldy Hernandez wrote: > > Sure, I can add the HONOR_NANS, but can we even "see" a NAN in the IL > > for -ffinite-math-only? > > Sure, you can, e.g. __builtin_nan{,s}{,f,l} etc. would do it. > It would be UB to use it at runtime in -ffinite-math-only code though. > Another question is, when making a range VARYING, do you set the NAN > property or not when !HONOR_NANS && MODE_HAS_NANS? A range of VARYING sets the NAN property to unknown (fp_prop::VARYING). If you prefer we can set the property to fp_prop::NO for !HONOR_NANS && MODE_HAS_NANS. ?? Aldy
On Mon, Aug 29, 2022 at 4:30 PM Toon Moene <toon@moene.org> wrote: > > On 8/29/22 16:15, Aldy Hernandez wrote: > > > But even with -ffinite-math-only, is there any benefit to propagating > > a known NAN? For example: > > The original intent (in 2002) for the option -ffinite-math-only was for > the optimizers to ignore all the various exceptions to common > optimizations because they might not work correctly when presented with > a NaN or an Inf. > > I do not know what the effect for floating point range information would > be - offhand. > > But in the *spirit* of this option would be to ignore that the range > [5.0, 5.0] would "also" contain NaN, for instance. Hmm, this is somewhat similar to what Jakub suggested. Perhaps we could categorically set !NAN for !HONOR_NANS at frange construction time? For reference: bool HONOR_NANS (machine_mode m) { return MODE_HAS_NANS (m) && !flag_finite_math_only; } Thanks. Aldy
On 8/29/22 16:36, Aldy Hernandez wrote: > On Mon, Aug 29, 2022 at 4:30 PM Toon Moene <toon@moene.org> wrote: >> >> On 8/29/22 16:15, Aldy Hernandez wrote: >> >>> But even with -ffinite-math-only, is there any benefit to propagating >>> a known NAN? For example: >> >> The original intent (in 2002) for the option -ffinite-math-only was for >> the optimizers to ignore all the various exceptions to common >> optimizations because they might not work correctly when presented with >> a NaN or an Inf. >> >> I do not know what the effect for floating point range information would >> be - offhand. >> >> But in the *spirit* of this option would be to ignore that the range >> [5.0, 5.0] would "also" contain NaN, for instance. > > Hmm, this is somewhat similar to what Jakub suggested. Perhaps we > could categorically set !NAN for !HONOR_NANS at frange construction > time? > > For reference: > bool > HONOR_NANS (machine_mode m) > { > return MODE_HAS_NANS (m) && !flag_finite_math_only; > } > > Thanks. > Aldy > Yep, I think that would do it. Thanks,
OK, I'm good to go. As the patch was getting rather large, I have split it into two parts. The first is the core endpoints support to frange along with removal of the +-INF markers (since they are no longer needed). The second part is the FP relational operators. Splitting it up should help in reviewing/improving the code for readers. Also, it always helps regression hunting to have smaller pieces ;-). Retested on x86-64 Linux. Pushed. On Mon, Aug 29, 2022 at 4:42 PM Toon Moene <toon@moene.org> wrote: > > On 8/29/22 16:36, Aldy Hernandez wrote: > > > On Mon, Aug 29, 2022 at 4:30 PM Toon Moene <toon@moene.org> wrote: > >> > >> On 8/29/22 16:15, Aldy Hernandez wrote: > >> > >>> But even with -ffinite-math-only, is there any benefit to propagating > >>> a known NAN? For example: > >> > >> The original intent (in 2002) for the option -ffinite-math-only was for > >> the optimizers to ignore all the various exceptions to common > >> optimizations because they might not work correctly when presented with > >> a NaN or an Inf. > >> > >> I do not know what the effect for floating point range information would > >> be - offhand. > >> > >> But in the *spirit* of this option would be to ignore that the range > >> [5.0, 5.0] would "also" contain NaN, for instance. > > > > Hmm, this is somewhat similar to what Jakub suggested. Perhaps we > > could categorically set !NAN for !HONOR_NANS at frange construction > > time? > > > > For reference: > > bool > > HONOR_NANS (machine_mode m) > > { > > return MODE_HAS_NANS (m) && !flag_finite_math_only; > > } > > > > Thanks. > > Aldy > > > > Yep, I think that would do it. > > Thanks, > > -- > Toon Moene - e-mail: toon@moene.org - phone: +31 346 214290 > Saturnushof 14, 3738 XG Maartensdijk, The Netherlands >
On Mon, 29 Aug 2022, Jakub Jelinek via Gcc-patches wrote: > On Mon, Aug 29, 2022 at 03:45:33PM +0200, Aldy Hernandez wrote: > > For convenience, singleton_p() returns false for a NAN. IMO, it makes > > the implementation cleaner, but I'm not wed to the idea if someone > > objects. > > If singleton_p() is used to decide whether one can just replace a variable > with singleton range with a constant, then certainly. > If MODE_HAS_SIGNED_ZEROS, zero has 2 representations (-0.0 and 0.0) and Note also for decimal floating point that many real numbers - if they don't require the full precision of the type to represent the number - can be represented with different quantum exponents (so compare equal but can't be used to replace each other).
On 8/29/2022 7:54 AM, Jakub Jelinek via Gcc-patches wrote: > On Mon, Aug 29, 2022 at 03:45:33PM +0200, Aldy Hernandez wrote: >> For convenience, singleton_p() returns false for a NAN. IMO, it makes >> the implementation cleaner, but I'm not wed to the idea if someone >> objects. > If singleton_p() is used to decide whether one can just replace a variable > with singleton range with a constant, then certainly. > If MODE_HAS_SIGNED_ZEROS, zero has 2 representations (-0.0 and 0.0) and > NaNs have lots of different representations (the sign bit is ignored > except for stuff like copysign/signbit, there are qNaNs and sNaNs and > except for the single case how Inf is represented, all other values of the > mantissa mean different representations of NaN). So, unless we track which > exact form of NaN can appear, NaN or any [x, x] range with NaN property > set can't be a singleton. There could be programs that propagate something > important in NaN mantissa and would be upset if frange kills that. > Of course, one needs to take into account that when a FPU creates NaN, it > will create the canonical qNaN. I've always thought of singleton_p as having that purpose -- but in the integer world we never really had to think about multiple representations of the same value. So it's entirely possible we're using singleton_p for multiple purposes. Clearly if the representation has multiple representations for the same value, then we have to be more careful with propagation. So we may need to separate the concept of "this has a value we can propagate" from "this has a constant value, but the value may have multiple represenatations". I don't think it's worth trying to track the various NaN representations, but I do think it's worth tracking +-Inf and +-0.0. jeff
On 8/29/2022 8:15 AM, Aldy Hernandez via Gcc-patches wrote: > On Mon, Aug 29, 2022 at 4:08 PM Toon Moene <toon@moene.org> wrote: >> On 8/29/22 15:54, Jakub Jelinek via Gcc-patches wrote: >> >>> On Mon, Aug 29, 2022 at 03:45:33PM +0200, Aldy Hernandez wrote: >>>> For convenience, singleton_p() returns false for a NAN. IMO, it makes >>>> the implementation cleaner, but I'm not wed to the idea if someone >>>> objects. >>> If singleton_p() is used to decide whether one can just replace a variable >>> with singleton range with a constant, then certainly. >>> If MODE_HAS_SIGNED_ZEROS, zero has 2 representations (-0.0 and 0.0) and >>> NaNs have lots of different representations (the sign bit is ignored >>> except for stuff like copysign/signbit, there are qNaNs and sNaNs and >>> except for the single case how Inf is represented, all other values of the >>> mantissa mean different representations of NaN). So, unless we track which >>> exact form of NaN can appear, NaN or any [x, x] range with NaN property >>> set can't be a singleton. There could be programs that propagate something >>> important in NaN mantissa and would be upset if frange kills that. >>> Of course, one needs to take into account that when a FPU creates NaN, it >>> will create the canonical qNaN. >>> >>> Jakub >>> >> But the NaNs are irrelevant with -ffinite-math-only, as are the various >> Infs (I do not know offhand which MODE_ that is) ... > But even with -ffinite-math-only, is there any benefit to propagating > a known NAN? For example: In theory, yes, but I don't know if it's worth the headache in practice for NaNs, particularly given they can have many representations. Jeff
On 8/29/2022 8:42 AM, Toon Moene wrote: > On 8/29/22 16:36, Aldy Hernandez wrote: > >> On Mon, Aug 29, 2022 at 4:30 PM Toon Moene <toon@moene.org> wrote: >>> >>> On 8/29/22 16:15, Aldy Hernandez wrote: >>> >>>> But even with -ffinite-math-only, is there any benefit to propagating >>>> a known NAN? For example: >>> >>> The original intent (in 2002) for the option -ffinite-math-only was for >>> the optimizers to ignore all the various exceptions to common >>> optimizations because they might not work correctly when presented with >>> a NaN or an Inf. >>> >>> I do not know what the effect for floating point range information >>> would >>> be - offhand. >>> >>> But in the *spirit* of this option would be to ignore that the range >>> [5.0, 5.0] would "also" contain NaN, for instance. >> >> Hmm, this is somewhat similar to what Jakub suggested. Perhaps we >> could categorically set !NAN for !HONOR_NANS at frange construction >> time? >> >> For reference: >> bool >> HONOR_NANS (machine_mode m) >> { >> return MODE_HAS_NANS (m) && !flag_finite_math_only; >> } >> >> Thanks. >> Aldy >> > > Yep, I think that would do it. Agreed. Jeff
On 8/29/2022 8:30 AM, Aldy Hernandez via Gcc-patches wrote: > On Mon, Aug 29, 2022 at 4:27 PM Jakub Jelinek <jakub@redhat.com> wrote: >> On Mon, Aug 29, 2022 at 04:20:16PM +0200, Aldy Hernandez wrote: >>> Sure, I can add the HONOR_NANS, but can we even "see" a NAN in the IL >>> for -ffinite-math-only? >> Sure, you can, e.g. __builtin_nan{,s}{,f,l} etc. would do it. >> It would be UB to use it at runtime in -ffinite-math-only code though. >> Another question is, when making a range VARYING, do you set the NAN >> property or not when !HONOR_NANS && MODE_HAS_NANS? > A range of VARYING sets the NAN property to unknown > (fp_prop::VARYING). If you prefer we can set the property to > fp_prop::NO for !HONOR_NANS && MODE_HAS_NANS. If the format doesn't have NaNs or the user explicitly disables them, then the state should be NO, otherwise YES. Jeff
diff --git a/gcc/value-range-pretty-print.cc b/gcc/value-range-pretty-print.cc index cbf50d3d854..1a026c22f99 100644 --- a/gcc/value-range-pretty-print.cc +++ b/gcc/value-range-pretty-print.cc @@ -122,19 +122,29 @@ vrange_printer::print_irange_bitmasks (const irange &r) const void vrange_printer::visit (const frange &r) const { + tree type = r.type (); + pp_string (pp, "[frange] "); if (r.undefined_p ()) { pp_string (pp, "UNDEFINED"); return; } - dump_generic_node (pp, r.type (), 0, TDF_NONE, false); + dump_generic_node (pp, type, 0, TDF_NONE, false); pp_string (pp, " "); if (r.varying_p ()) { pp_string (pp, "VARYING"); return; } + pp_character (pp, '['); + dump_generic_node (pp, + build_real (type, r.lower_bound ()), 0, TDF_NONE, false); + pp_string (pp, ", "); + dump_generic_node (pp, + build_real (type, r.upper_bound ()), 0, TDF_NONE, false); + pp_string (pp, "] "); + print_frange_prop ("NAN", r.get_nan ()); print_frange_prop ("INF", r.get_inf ()); print_frange_prop ("NINF", r.get_ninf ()); diff --git a/gcc/value-range.cc b/gcc/value-range.cc index d056f7356e1..3d81a884192 100644 --- a/gcc/value-range.cc +++ b/gcc/value-range.cc @@ -287,6 +287,8 @@ frange::set (tree min, tree max, value_range_kind kind) m_kind = kind; m_type = TREE_TYPE (min); m_props.set_varying (); + m_min = *TREE_REAL_CST_PTR (min); + m_max = *TREE_REAL_CST_PTR (max); bool is_min = vrp_val_is_min (min); bool is_max = vrp_val_is_max (max); @@ -332,6 +334,16 @@ frange::set (tree min, tree max, value_range_kind kind) verify_range (); } +// Setter for frange from REAL_VALUE_TYPE endpoints. + +void +frange::set (tree type, + const REAL_VALUE_TYPE &min, const REAL_VALUE_TYPE &max, + value_range_kind kind) +{ + set (build_real (type, min), build_real (type, max), kind); +} + // Normalize range to VARYING or UNDEFINED, or vice versa. Return // TRUE if anything changed. // @@ -343,7 +355,9 @@ frange::set (tree min, tree max, value_range_kind kind) bool frange::normalize_kind () { - if (m_kind == VR_RANGE) + if (m_kind == VR_RANGE + && real_isinf (&m_min, 1) + && real_isinf (&m_max, 0)) { // No FP properties set means varying. if (m_props.varying_p ()) @@ -366,6 +380,8 @@ frange::normalize_kind () if (!m_props.varying_p ()) { m_kind = VR_RANGE; + real_inf (&m_min, 1); + real_inf (&m_max, 0); return true; } } @@ -385,12 +401,22 @@ frange::union_ (const vrange &v) return true; } - bool ret = m_props.union_ (r.m_props); - ret |= normalize_kind (); + bool changed = m_props.union_ (r.m_props); + if (real_less (&r.m_min, &m_min)) + { + m_min = r.m_min; + changed = true; + } + if (real_less (&m_max, &r.m_max)) + { + m_max = r.m_max; + changed = true; + } + changed |= normalize_kind (); if (flag_checking) verify_range (); - return ret; + return changed; } bool @@ -411,12 +437,28 @@ frange::intersect (const vrange &v) return true; } - bool ret = m_props.intersect (r.m_props); - ret |= normalize_kind (); + bool changed = m_props.intersect (r.m_props); + if (real_less (&m_min, &r.m_min)) + { + m_min = r.m_min; + changed = true; + } + if (real_less (&r.m_max, &m_max)) + { + m_max = r.m_max; + changed = true; + } + // If the endpoints are swapped, the ranges are disjoint. + if (real_less (&m_max, &m_min)) + { + set_undefined (); + return true; + } + changed |= normalize_kind (); if (flag_checking) verify_range (); - return ret; + return changed; } frange & @@ -424,6 +466,8 @@ frange::operator= (const frange &src) { m_kind = src.m_kind; m_type = src.m_type; + m_min = src.m_min; + m_max = src.m_max; m_props = src.m_props; if (flag_checking) @@ -442,7 +486,44 @@ frange::operator== (const frange &src) const if (varying_p ()) return types_compatible_p (m_type, src.m_type); - return m_props == src.m_props; + return (real_identical (&m_min, &src.m_min) + && real_identical (&m_max, &src.m_max) + && m_props == src.m_props + && types_compatible_p (m_type, src.m_type)); + } + return false; +} + +// Return TRUE if range contains the TREE_REAL_CST_PTR in CST. + +bool +frange::contains_p (tree cst) const +{ + if (undefined_p ()) + return false; + + if (varying_p ()) + return true; + + gcc_checking_assert (m_kind == VR_RANGE); + + return (real_compare (GE_EXPR, TREE_REAL_CST_PTR (cst), &m_min) + && real_compare (LE_EXPR, TREE_REAL_CST_PTR (cst), &m_max)); +} + +// If range is a singleton, place it in RESULT and return TRUE. If +// RESULT is NULL, just return TRUE. + +bool +frange::singleton_p (tree *result) const +{ + if (m_kind == VR_RANGE + && real_identical (&m_min, &m_max) + && !real_isnan (&m_min)) + { + if (result) + *result = build_real (m_type, m_min); + return true; } return false; } @@ -461,13 +542,32 @@ frange::verify_range () gcc_checking_assert (m_props.undefined_p ()); return; } + gcc_checking_assert (!m_props.undefined_p ()); + if (varying_p ()) { gcc_checking_assert (m_props.varying_p ()); return; } + + // We don't support the inverse of an frange (yet). gcc_checking_assert (m_kind == VR_RANGE); - gcc_checking_assert (!m_props.varying_p () && !m_props.undefined_p ()); + + bool is_nan = real_isnan (&m_min) || real_isnan (&m_max); + if (is_nan) + { + // If either is a NAN, both must be a NAN. + gcc_checking_assert (real_identical (&m_min, &m_max)); + gcc_checking_assert (get_nan ().yes_p ()); + } + else + // Make sure we don't have swapped ranges. + gcc_checking_assert (!real_less (&m_max, &m_min)); + + // If all the properties are clear, we better not span the entire + // domain, because that would make us varying. + if (m_props.varying_p ()) + gcc_checking_assert (!real_isinf (&m_min, 1) || !real_isinf (&m_max, 0)); } // Here we copy between any two irange's. The ranges can be legacy or @@ -3300,6 +3400,146 @@ range_tests_nonzero_bits () ASSERT_TRUE (r0.varying_p ()); } +// Build an frange from string endpoints. + +static inline frange +frange_float (const char *lb, const char *ub, tree type = float_type_node) +{ + REAL_VALUE_TYPE min, max; + real_from_string (&min, lb); + real_from_string (&max, ub); + return frange (type, min, max); +} + +// Set R to maximum representable value for TYPE. + +static inline void +real_max_representable (REAL_VALUE_TYPE *r, tree type) +{ + char buf[128]; + get_max_float (REAL_MODE_FORMAT (TYPE_MODE (type)), + buf, sizeof (buf), false); + real_from_string (r, buf); +} + +// Set R to minimum representable value for TYPE. + +static inline void +real_min_representable (REAL_VALUE_TYPE *r, tree type) +{ + real_max_representable (r, type); + real_value_negate (r); +} + +static void +range_tests_floats () +{ + frange r0, r1; + + + // Equal ranges but with differing NAN bits are not equal. + r1 = frange_float ("10", "12"); + r0 = r1; + ASSERT_EQ (r0, r1); + r0.set_nan (fp_prop::NO); + ASSERT_NE (r0, r1); + r0.set_nan (fp_prop::YES); + ASSERT_NE (r0, r1); + r0.set_nan (fp_prop::VARYING); + ASSERT_EQ (r0, r1); + + // A range of [-INF,+INF] is actually VARYING... + r0 = frange_float ("-Inf", "+Inf"); + ASSERT_TRUE (r0.varying_p ()); + // ...unless it has some special property... + r0.set_nan (fp_prop::NO); + ASSERT_FALSE (r0.varying_p ()); + + // The endpoints of a VARYING are +-INF. + REAL_VALUE_TYPE inf, ninf; + real_inf (&inf, 0); + real_inf (&ninf, 1); + r0.set_varying (float_type_node); + ASSERT_TRUE (real_identical (&r0.lower_bound (), &ninf)); + ASSERT_TRUE (real_identical (&r0.upper_bound (), &inf)); + + // The maximum representable range for a type is still a subset of VARYING. + REAL_VALUE_TYPE q, r; + real_min_representable (&q, float_type_node); + real_max_representable (&r, float_type_node); + r0 = frange (float_type_node, q, r); + // r0 is not a varying, because it does not include -INF/+INF. + ASSERT_FALSE (r0.varying_p ()); + // The upper bound of r0 must be less than +INF. + ASSERT_TRUE (real_less (&r0.upper_bound (), &inf)); + // The lower bound of r0 must be greater than -INF. + ASSERT_TRUE (real_less (&ninf, &r0.lower_bound ())); + + // For most architectures, where float and double are different + // sizes, having the same endpoints does not necessarily mean the + // ranges are equal. + if (!types_compatible_p (float_type_node, double_type_node)) + { + r0 = frange_float ("3.0", "3.0", float_type_node); + r1 = frange_float ("3.0", "3.0", double_type_node); + ASSERT_NE (r0, r1); + } + + // [3,5] U [10,12] = [3,12]. + r0 = frange_float ("3", "5"); + r1 = frange_float ("10", "12"); + r0.union_ (r1); + ASSERT_EQ (r0, frange_float ("3", "12")); + + // [5,10] U [4,8] = [4,10] + r0 = frange_float ("5", "10"); + r1 = frange_float ("4", "8"); + r0.union_ (r1); + ASSERT_EQ (r0, frange_float ("4", "10")); + + // [3,5] U [4,10] = [3,10] + r0 = frange_float ("3", "5"); + r1 = frange_float ("4", "10"); + r0.union_ (r1); + ASSERT_EQ (r0, frange_float ("3", "10")); + + // [4,10] U [5,11] = [4,11] + r0 = frange_float ("4", "10"); + r1 = frange_float ("5", "11"); + r0.union_ (r1); + ASSERT_EQ (r0, frange_float ("4", "11")); + + // [3,12] ^ [10,12] = [10,12]. + r0 = frange_float ("3", "12"); + r1 = frange_float ("10", "12"); + r0.intersect (r1); + ASSERT_EQ (r0, frange_float ("10", "12")); + + // [10,12] ^ [11,11] = [11,11] + r0 = frange_float ("10", "12"); + r1 = frange_float ("11", "11"); + r0.intersect (r1); + ASSERT_EQ (r0, frange_float ("11", "11")); + + // [10,20] ^ [5,15] = [10,15] + r0 = frange_float ("10", "20"); + r1 = frange_float ("5", "15"); + r0.intersect (r1); + ASSERT_EQ (r0, frange_float ("10", "15")); + + // [10,20] ^ [15,25] = [15,20] + r0 = frange_float ("10", "20"); + r1 = frange_float ("15", "25"); + r0.intersect (r1); + ASSERT_EQ (r0, frange_float ("15", "20")); + + // [10,20] ^ [21,25] = [] + r0 = frange_float ("10", "20"); + r1 = frange_float ("21", "25"); + r0.intersect (r1); + ASSERT_TRUE (r0.undefined_p ()); +} + void range_tests () { @@ -3308,6 +3548,7 @@ range_tests () range_tests_int_range_max (); range_tests_strict_enum (); range_tests_nonzero_bits (); + range_tests_floats (); range_tests_misc (); } diff --git a/gcc/value-range.h b/gcc/value-range.h index f0075d0fb1a..7be9e84e13a 100644 --- a/gcc/value-range.h +++ b/gcc/value-range.h @@ -345,21 +345,30 @@ class frange : public vrange public: frange (); frange (const frange &); + frange (tree, tree, value_range_kind = VR_RANGE); + frange (tree type, const REAL_VALUE_TYPE &min, const REAL_VALUE_TYPE &max, + value_range_kind = VR_RANGE); static bool supports_p (const_tree type) { return SCALAR_FLOAT_TYPE_P (type); } virtual tree type () const override; 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); virtual void set_varying (tree type) override; virtual void set_undefined () override; virtual bool union_ (const vrange &) override; virtual bool intersect (const vrange &) override; + virtual bool contains_p (tree) const override; + virtual bool singleton_p (tree *result = NULL) const override; virtual bool supports_type_p (const_tree type) const override; virtual void accept (const vrange_visitor &v) const override; frange& operator= (const frange &); bool operator== (const frange &) const; bool operator!= (const frange &r) const { return !(*this == r); } + const REAL_VALUE_TYPE &lower_bound () const; + const REAL_VALUE_TYPE &upper_bound () const; // Each fp_prop can be accessed with get_PROP() and set_PROP(). FRANGE_PROP_ACCESSOR(nan) @@ -371,8 +380,24 @@ private: frange_props m_props; tree m_type; + REAL_VALUE_TYPE m_min; + REAL_VALUE_TYPE m_max; }; +inline const REAL_VALUE_TYPE & +frange::lower_bound () const +{ + gcc_checking_assert (!undefined_p ()); + return m_min; +} + +inline const REAL_VALUE_TYPE & +frange::upper_bound () const +{ + gcc_checking_assert (!undefined_p ()); + return m_max; +} + // is_a<> and as_a<> implementation for vrange. // Anything we haven't specialized is a hard fail. @@ -1051,8 +1076,8 @@ vrp_val_min (const_tree type) if (frange::supports_p (type)) { REAL_VALUE_TYPE real, real_ninf; - real_inf (&real); - real_ninf = real_value_negate (&real); + real_inf (&real, 0); + real_inf (&real_ninf, 1); return build_real (const_cast <tree> (type), real_ninf); } return NULL_TREE; @@ -1096,6 +1121,26 @@ frange::frange (const frange &src) *this = src; } +// frange constructor from REAL_VALUE_TYPE endpoints. + +inline +frange::frange (tree type, + const REAL_VALUE_TYPE &min, const REAL_VALUE_TYPE &max, + value_range_kind kind) +{ + m_discriminator = VR_FRANGE; + set (type, min, max, kind); +} + +// frange constructor from trees. + +inline +frange::frange (tree min, tree max, value_range_kind kind) +{ + m_discriminator = VR_FRANGE; + set (min, max, kind); +} + inline tree frange::type () const { @@ -1107,6 +1152,8 @@ frange::set_varying (tree type) { m_kind = VR_VARYING; m_type = type; + real_inf (&m_min, 1); + real_inf (&m_max, 0); m_props.set_varying (); } @@ -1116,6 +1163,8 @@ frange::set_undefined () m_kind = VR_UNDEFINED; m_type = NULL; m_props.set_undefined (); + memset (&m_min, 0, sizeof (m_min)); + memset (&m_max, 0, sizeof (m_max)); } #endif // GCC_VALUE_RANGE_H