[PUSHED 4/8] Adjust op_with_boolean_value_range_p for irange API.

Aldy Hernandez aldyh@redhat.com
Tue Aug 4 09:45:13 GMT 2020



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



More information about the Gcc-patches mailing list