| Message ID | 20200804063441.517123-5-aldyh@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | irange API adjustments for various files | expand |
On Tue, Aug 4, 2020 at 8:39 AM Aldy Hernandez via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > It seems to me that we should also check for [0,0] and [1,1] in the > range, but I am leaving things as is to avoid functional changes. > > gcc/ChangeLog: > > * vr-values.c (simplify_using_ranges::op_with_boolean_value_range_p): Adjust > for irange API. > --- > gcc/vr-values.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/gcc/vr-values.c b/gcc/vr-values.c > index 609375c072e..1190fa96453 100644 > --- a/gcc/vr-values.c > +++ b/gcc/vr-values.c > @@ -448,10 +448,11 @@ simplify_using_ranges::op_with_boolean_value_range_p (tree op) > if (TREE_CODE (op) != SSA_NAME) > return false; > > + /* ?? Errr, this should probably check for [0,0] and [1,1] as well > + as [0,1]. */ > const value_range *vr = get_value_range (op); > - return (vr->kind () == VR_RANGE > - && integer_zerop (vr->min ()) > - && integer_onep (vr->max ())); > + return *vr == value_range (build_zero_cst (TREE_TYPE (op)), > + build_one_cst (TREE_TYPE (op))); This now builds trees in a predicate which is highly dubious. Isn't there a better (cheaper) primitive to build a [0, 1] range to compare against? I guess from what I read it will also allocate memory for the range entry? Richard. > } > > /* Extract value range information for VAR when (OP COND_CODE LIMIT) is > -- > 2.26.2 >
On 8/4/20 8:55 AM, Richard Biener wrote: > On Tue, Aug 4, 2020 at 8:39 AM Aldy Hernandez via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> It seems to me that we should also check for [0,0] and [1,1] in the >> range, but I am leaving things as is to avoid functional changes. >> >> gcc/ChangeLog: >> >> * vr-values.c (simplify_using_ranges::op_with_boolean_value_range_p): Adjust >> for irange API. >> --- >> gcc/vr-values.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/gcc/vr-values.c b/gcc/vr-values.c >> index 609375c072e..1190fa96453 100644 >> --- a/gcc/vr-values.c >> +++ b/gcc/vr-values.c >> @@ -448,10 +448,11 @@ simplify_using_ranges::op_with_boolean_value_range_p (tree op) >> if (TREE_CODE (op) != SSA_NAME) >> return false; >> >> + /* ?? Errr, this should probably check for [0,0] and [1,1] as well >> + as [0,1]. */ >> const value_range *vr = get_value_range (op); >> - return (vr->kind () == VR_RANGE >> - && integer_zerop (vr->min ()) >> - && integer_onep (vr->max ())); >> + return *vr == value_range (build_zero_cst (TREE_TYPE (op)), >> + build_one_cst (TREE_TYPE (op))); > > This now builds trees in a predicate which is highly dubious. Isn't > there a better (cheaper) primitive to build a [0, 1] range to compare against? > I guess from what I read it will also allocate memory for the range entry? Both 0 and 1 are cached for all integer types. We are also caching 1 and MAX for pointers per the irange patchset, so this shouldn't be a performance issue. Also, we do the same thing for irange::nonzero_p(), which is used a lot more often (and probably on critical paths), and per our benchmarks we didn't find it to take any significant time: tree zero = build_zero_cst (type ()); return *this == int_range<1> (zero, zero, VR_ANTI_RANGE); However, I suppose we could implement it with something like: return vr->num_pairs() == 1 && vr->lower_bound () == 0 && vr->upper_bound () == 1 but that's hardly any faster, especially since num_pairs() is non-trivial for the value_range legacy code. I just don't think it's worth it. Aldy
diff --git a/gcc/vr-values.c b/gcc/vr-values.c index 609375c072e..1190fa96453 100644 --- a/gcc/vr-values.c +++ b/gcc/vr-values.c @@ -448,10 +448,11 @@ simplify_using_ranges::op_with_boolean_value_range_p (tree op) if (TREE_CODE (op) != SSA_NAME) return false; + /* ?? Errr, this should probably check for [0,0] and [1,1] as well + as [0,1]. */ const value_range *vr = get_value_range (op); - return (vr->kind () == VR_RANGE - && integer_zerop (vr->min ()) - && integer_onep (vr->max ())); + return *vr == value_range (build_zero_cst (TREE_TYPE (op)), + build_one_cst (TREE_TYPE (op))); } /* Extract value range information for VAR when (OP COND_CODE LIMIT) is