* [PATCH] Pass correct type to irange::contains_p() in ipa-cp.cc. @ 2023-04-24 7:43 Aldy Hernandez 2023-04-24 8:30 ` Richard Biener 0 siblings, 1 reply; 8+ messages in thread From: Aldy Hernandez @ 2023-04-24 7:43 UTC (permalink / raw) To: GCC patches; +Cc: Andrew MacLeod, Aldy Hernandez 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); + return r.contains_p (val); +} + /* Decide whether and what specialized clones of NODE should be created. */ static bool @@ -6221,7 +6234,8 @@ decide_whether_version_node (struct cgraph_node *node) supports this only for integers now. */ if (TREE_CODE (val->value) == INTEGER_CST && !plats->m_value_range.bottom_p () - && !plats->m_value_range.m_vr.contains_p (val->value)) + && !ipa_range_contains_p (plats->m_value_range.m_vr, + val->value)) { /* This can happen also if a constant present in the source code falls outside of the range of parameter's type, so we -- 2.40.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Pass correct type to irange::contains_p() in ipa-cp.cc. 2023-04-24 7:43 [PATCH] Pass correct type to irange::contains_p() in ipa-cp.cc Aldy Hernandez @ 2023-04-24 8:30 ` Richard Biener 2023-04-24 11:51 ` Aldy Hernandez 0 siblings, 1 reply; 8+ messages in thread From: Richard Biener @ 2023-04-24 8:30 UTC (permalink / raw) To: Aldy Hernandez; +Cc: GCC patches, Andrew MacLeod 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; but then I wonder whether contains_p should have an overload with widest_int or handle "out of bounds" values itself more gracefully? Thanks, Richard. > + return r.contains_p (val); > +} > + > /* Decide whether and what specialized clones of NODE should be created. */ > > static bool > @@ -6221,7 +6234,8 @@ decide_whether_version_node (struct cgraph_node *node) > supports this only for integers now. */ > if (TREE_CODE (val->value) == INTEGER_CST > && !plats->m_value_range.bottom_p () > - && !plats->m_value_range.m_vr.contains_p (val->value)) > + && !ipa_range_contains_p (plats->m_value_range.m_vr, > + val->value)) > { > /* This can happen also if a constant present in the source > code falls outside of the range of parameter's type, so we > -- > 2.40.0 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Pass correct type to irange::contains_p() in ipa-cp.cc. 2023-04-24 8:30 ` Richard Biener @ 2023-04-24 11:51 ` Aldy Hernandez 2023-04-24 12:10 ` Richard Biener 0 siblings, 1 reply; 8+ messages in thread From: Aldy Hernandez @ 2023-04-24 11:51 UTC (permalink / raw) To: Richard Biener; +Cc: GCC patches, Andrew MacLeod 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Pass correct type to irange::contains_p() in ipa-cp.cc. 2023-04-24 11:51 ` Aldy Hernandez @ 2023-04-24 12:10 ` Richard Biener 2023-04-24 12:32 ` Aldy Hernandez 0 siblings, 1 reply; 8+ messages in thread From: Richard Biener @ 2023-04-24 12:10 UTC (permalink / raw) To: Aldy Hernandez; +Cc: GCC patches, Andrew MacLeod 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? > > > > 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Pass correct type to irange::contains_p() in ipa-cp.cc. 2023-04-24 12:10 ` Richard Biener @ 2023-04-24 12:32 ` Aldy Hernandez 2023-04-24 12:35 ` Richard Biener 0 siblings, 1 reply; 8+ messages in thread From: Aldy Hernandez @ 2023-04-24 12:32 UTC (permalink / raw) To: Richard Biener; +Cc: GCC patches, Andrew MacLeod 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); ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Pass correct type to irange::contains_p() in ipa-cp.cc. 2023-04-24 12:32 ` Aldy Hernandez @ 2023-04-24 12:35 ` Richard Biener 2023-04-24 17:17 ` Aldy Hernandez 0 siblings, 1 reply; 8+ messages in thread From: Richard Biener @ 2023-04-24 12:35 UTC (permalink / raw) To: Aldy Hernandez; +Cc: GCC patches, Andrew MacLeod 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. Richard. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Pass correct type to irange::contains_p() in ipa-cp.cc. 2023-04-24 12:35 ` Richard Biener @ 2023-04-24 17:17 ` Aldy Hernandez 2023-04-25 7:19 ` Richard Biener 0 siblings, 1 reply; 8+ messages in thread From: Aldy Hernandez @ 2023-04-24 17:17 UTC (permalink / raw) To: Richard Biener; +Cc: GCC patches, Andrew MacLeod [-- Attachment #1: Type: text/plain, Size: 3041 bytes --] 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. [-- Attachment #2: 0001-Pass-correct-type-to-irange-contains_p-in-ipa-cp.cc.patch --] [-- Type: text/x-patch, Size: 1921 bytes --] From 947792eefcb8316818d105fcb38b69a76bb93e86 Mon Sep 17 00:00:00 2001 From: Aldy Hernandez <aldyh@redhat.com> Date: Mon, 21 Nov 2022 23:18:43 +0100 Subject: [PATCH] Pass correct type to irange::contains_p() in ipa-cp.cc. 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. gcc/ChangeLog: * ipa-cp.cc (ipa_range_contains_p): New. (decide_whether_version_node): Use it. --- gcc/ipa-cp.cc | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc index b3e0f62e400..65c49558b58 100644 --- a/gcc/ipa-cp.cc +++ b/gcc/ipa-cp.cc @@ -6180,6 +6180,23 @@ 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; + + tree type = r.type (); + if (!wi::fits_to_tree_p (wi::to_wide (val), type)) + return false; + + val = fold_convert (type, val); + return r.contains_p (val); +} + /* Decide whether and what specialized clones of NODE should be created. */ static bool @@ -6221,7 +6238,8 @@ decide_whether_version_node (struct cgraph_node *node) supports this only for integers now. */ if (TREE_CODE (val->value) == INTEGER_CST && !plats->m_value_range.bottom_p () - && !plats->m_value_range.m_vr.contains_p (val->value)) + && !ipa_range_contains_p (plats->m_value_range.m_vr, + val->value)) { /* This can happen also if a constant present in the source code falls outside of the range of parameter's type, so we -- 2.40.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Pass correct type to irange::contains_p() in ipa-cp.cc. 2023-04-24 17:17 ` Aldy Hernandez @ 2023-04-25 7:19 ` Richard Biener 0 siblings, 0 replies; 8+ messages in thread From: Richard Biener @ 2023-04-25 7:19 UTC (permalink / raw) To: Aldy Hernandez; +Cc: GCC patches, Andrew MacLeod On Mon, Apr 24, 2023 at 7:17 PM Aldy Hernandez <aldyh@redhat.com> wrote: > > > > 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. LGTM. > 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-04-25 7:19 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-04-24 7:43 [PATCH] Pass correct type to irange::contains_p() in ipa-cp.cc Aldy Hernandez 2023-04-24 8:30 ` Richard Biener 2023-04-24 11:51 ` Aldy Hernandez 2023-04-24 12:10 ` Richard Biener 2023-04-24 12:32 ` Aldy Hernandez 2023-04-24 12:35 ` Richard Biener 2023-04-24 17:17 ` Aldy Hernandez 2023-04-25 7:19 ` Richard Biener
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).