On 4/24/23 14:35, Richard Biener wrote:
> On Mon, Apr 24, 2023 at 2:32 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>>
>>
>>
>> On 4/24/23 14:10, Richard Biener wrote:
>>> On Mon, Apr 24, 2023 at 1:51 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>>>>
>>>>
>>>>
>>>> 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?
>>>
>>> Where does it not work?  And when do you get pointer values/types
>>> where they mismatch sufficiently (how?) to make ranger unhappy?
>>>
>>
>> It's not ranger that's unhappy, this is just irange::contains_p().
>>
>> IPA works on both integers and pointers, and pointers don't have
>> TYPE_MIN/MAX_VALUE defined, so we ICE in int_fits_type_p:
>>
>>     type_low_bound = TYPE_MIN_VALUE (type);
>>     type_high_bound = TYPE_MAX_VALUE (type);
> 
> Ah.  The code handles NULL TYPE_MIN/MAX_VALUE just fine so
> I wonder if it works to change the above to
> 
>     type_low_bound = INTEGRAL_TYPE_P (type) ? TYPE_MIN_VALUE (type) : NULL_TREE;
> ...
> 
> alternatively you could use wi::fits_to_tree_p (wi::to_wide (val),
> type) from the IPA code.

That works.

How's this?  Tested on x86-64 Linux.

p.s. Once we convert the API to wide_int's (upcoming patches), I can 
move this logic into irange::contains_p (tree) virtual instead of 
ipa_range_contains_p.  We'll also have an irange::contains_p (const 
wide_int &) which we can upgrade to a widest_int overload if need be.