Re: [PATCH] Pass correct type to irange::contains_p() in ipa-cp.cc.

[Aldy Hernandez <aldyh@redhat.com>]

On 4/24/23 10:30, Richard Biener wrote:
> On Mon, Apr 24, 2023 at 9:44 AM Aldy Hernandez via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> There is a call to contains_p() in ipa-cp.cc which passes incompatible
>> types.  This currently works because deep in the call chain, the legacy
>> code uses tree_int_cst_lt which performs the operation with
>> widest_int.  With the upcoming removal of legacy, contains_p() will be
>> stricter.
>>
>> OK pending tests?
>>
>> gcc/ChangeLog:
>>
>>          * ipa-cp.cc (ipa_range_contains_p): New.
>>          (decide_whether_version_node): Use it.
>> ---
>>   gcc/ipa-cp.cc | 16 +++++++++++++++-
>>   1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc
>> index b3e0f62e400..c8013563796 100644
>> --- a/gcc/ipa-cp.cc
>> +++ b/gcc/ipa-cp.cc
>> @@ -6180,6 +6180,19 @@ decide_about_value (struct cgraph_node *node, int index, HOST_WIDE_INT offset,
>>     return true;
>>   }
>>
>> +/* Like irange::contains_p(), but convert VAL to the range of R if
>> +   necessary.  */
>> +
>> +static inline bool
>> +ipa_range_contains_p (const irange &r, tree val)
>> +{
>> +  if (r.undefined_p ())
>> +    return false;
>> +
>> +  val = fold_convert (r.type (), val);
> 
> I think that's wrong, it might truncate 'val'.  I think we'd want
> 
>     if (r.undefined_p () || !int_fits_type_p (val, r.type ()))
>       return false;

This won't work for pointers.  Is there a suitable version that handles 
pointers as well?

> 
> but then I wonder whether contains_p should have an overload
> with widest_int or handle "out of bounds" values itself more gracefully?

Only IPA is currently passing incompatible types to contains_p(), so I'd 
prefer to keep things stricter until there is an actual need for them.

Thanks.
Aldy