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
