From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x22f.google.com (mail-lj1-x22f.google.com [IPv6:2a00:1450:4864:20::22f]) by sourceware.org (Postfix) with ESMTPS id EC6753858D1E for ; Tue, 25 Apr 2023 07:19:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EC6753858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lj1-x22f.google.com with SMTP id 38308e7fff4ca-2a8dc00ade2so51979811fa.0 for ; Tue, 25 Apr 2023 00:19:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1682407192; x=1684999192; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=Gm7MppCe/GYBVZZ64eG7LVnt7lb5IRH1I+65T7uIHj8=; b=AX90qtA9GXwqiCgjY4jILDdPHLUhU1sZ/w6X7aqkPT609xRNZ1wYIDOjAx/tpxacll OmPlOy9Wox5t91jqLx6RdGu8EnbaQ8J6Ce407whEXfV1+UH0efaTdnPHVMw42lQltuBj 9KhbP/woAzsHo+MXEE3kv3Q+YJjnWc+wZPrBDWi4a4Lqgxxqlwm+79DOAIaZa+gAb2XP QVHdLgQGylrCpthXByzjI1HQACLH7fVqKvw+Voy1HOPwXXybB4DNEfBq1YkFbG8vIRl5 iVIyv/oCr8J4B16GV6I0U6lB0IiyMG/SGdHWFdQdOtI4y9dHq9Vyzp09BmOmeE4hO0j/ Mhbw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682407192; x=1684999192; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Gm7MppCe/GYBVZZ64eG7LVnt7lb5IRH1I+65T7uIHj8=; b=c0KSaLWkyovyVxHHjF1OlWO+tDbLi8KThOI62jgb94LgBBn7R59z0DQ/C1TSJsU3tU Cp9y4tYjWIXqZWfXeO3jHZvd/fF3pVjvBKL5jh6nIIdLBE4kFyqx3HfIHzHM4XTGLDJA RK0h1UxnaK+Q54YINzVQaAvwOILq9Z3h2KYzuPREBHmx/dLQw8dVW+uPOXNKGR1NKcxx gY0QZJfK/XV/9ahB/9NvJRFgdlQUpwwIBjT0YFToHAf6mYdBUxP22G1amTLV/wycjdR9 fccgYsUiFtqMqA/bDB4Wc2PIsrVMsyBkFcGifZgtEpiVQVWtkRM9/EbpM7CkLkvz8OMF ETcw== X-Gm-Message-State: AAQBX9c1DLaGRFXKH6jdwpF9xmX/oUgBjPXXhO5OU6/fmaF0jf2TvkIH iXxfHxy+7gxs3YLAGfWR6kR+IxkCvt8XG1tdIkk= X-Google-Smtp-Source: AKy350bHO2pD6P8kXPRhNCCLDtxIusccKL0rVJSwQZn638UflTPRMlddoUt/xz5ZFL/nTsrgGsXWKxY4RsQEbmx9bGE= X-Received: by 2002:a2e:361a:0:b0:2a8:c3de:6d10 with SMTP id d26-20020a2e361a000000b002a8c3de6d10mr3766475lja.39.1682407192149; Tue, 25 Apr 2023 00:19:52 -0700 (PDT) MIME-Version: 1.0 References: <20230424074332.141890-1-aldyh@redhat.com> <9db305b9-62fb-1b07-656a-13d029a7f730@redhat.com> <79959ae1-8958-28be-6258-2594d4cbac5b@redhat.com> In-Reply-To: <79959ae1-8958-28be-6258-2594d4cbac5b@redhat.com> From: Richard Biener Date: Tue, 25 Apr 2023 09:19:38 +0200 Message-ID: Subject: Re: [PATCH] Pass correct type to irange::contains_p() in ipa-cp.cc. To: Aldy Hernandez Cc: GCC patches , Andrew MacLeod Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, Apr 24, 2023 at 7:17=E2=80=AFPM Aldy Hernandez w= rote: > > > > On 4/24/23 14:35, Richard Biener wrote: > > On Mon, Apr 24, 2023 at 2:32=E2=80=AFPM Aldy Hernandez wrote: > >> > >> > >> > >> On 4/24/23 14:10, Richard Biener wrote: > >>> On Mon, Apr 24, 2023 at 1:51=E2=80=AFPM Aldy Hernandez wrote: > >>>> > >>>> > >>>> > >>>> On 4/24/23 10:30, Richard Biener wrote: > >>>>> On Mon, Apr 24, 2023 at 9:44=E2=80=AFAM Aldy Hernandez via Gcc-patc= hes > >>>>> wrote: > >>>>>> > >>>>>> There is a call to contains_p() in ipa-cp.cc which passes incompat= ible > >>>>>> types. This currently works because deep in the call chain, the l= egacy > >>>>>> code uses tree_int_cst_lt which performs the operation with > >>>>>> widest_int. With the upcoming removal of legacy, contains_p() wil= l 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 *nod= e, int index, HOST_WIDE_INT offset, > >>>>>> return true; > >>>>>> } > >>>>>> > >>>>>> +/* Like irange::contains_p(), but convert VAL to the range of R i= f > >>>>>> + necessary. */ > >>>>>> + > >>>>>> +static inline bool > >>>>>> +ipa_range_contains_p (const irange &r, tree val) > >>>>>> +{ > >>>>>> + if (r.undefined_p ()) > >>>>>> + return false; > >>>>>> + > >>>>>> + val =3D 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 hand= les > >>>> 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 =3D TYPE_MIN_VALUE (type); > >> type_high_bound =3D 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 =3D 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.