From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id A5A573858402 for ; Wed, 21 Jul 2021 15:29:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A5A573858402 Received: from mail-lf1-f71.google.com (mail-lf1-f71.google.com [209.85.167.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-333-HilM9QBYO3uLbR789St-kg-1; Wed, 21 Jul 2021 11:29:17 -0400 X-MC-Unique: HilM9QBYO3uLbR789St-kg-1 Received: by mail-lf1-f71.google.com with SMTP id i9-20020ac252290000b029036be88ae752so529080lfl.4 for ; Wed, 21 Jul 2021 08:29:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=74l4zLU9Tvv1ka3WFFIdwiVxpZrueUUTad8mcR7ySgE=; b=In5CMceebFW+5sv36Ot7wswEfZ2RZhASaagqhPmibh3te0LywwvyScxUlYTMLOPQyf iq49naDbq/r0jY07i0RyH6N38e/stdQ+rs8ymPCHVqrRPhipfHALWK7E5ldMaQ96QQ9F KQI6cCleqSYHTij1oe2gUqv5hPllCyMttHzialt5ONgiW1U7QRFsul8Qd5uHzfwfi+gj tuMbNwvfRNmTnQVPXK+VGxItRAWJSeh19b8kVDTO9xY+rxuXCqA6AJTN4JubS4yQAW3Z X/wimPfD4dZyBpU23lsKe5nel4bW7OXqUt5uyWbvc3RkaIPpSwmd78rHQtpYqLo2CG5j D5Wg== X-Gm-Message-State: AOAM530V27AP/pWRgpcP212DY7CgObGPyWvhUtUTrto974bbs2oJh7Tr wQZtSShl9l5CRs/PvbRS+ZHP3lBsFsj5G2ZF5yYs3V4O02De7pWhh+mgO8yGw3amrlcGVdCggZo bYBsARahOyneFH9Bnn68jIRJmZ0pBTwDEIw== X-Received: by 2002:a19:fc03:: with SMTP id a3mr26555282lfi.327.1626881355897; Wed, 21 Jul 2021 08:29:15 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxkrCGlK3xD7+5QbZOv5FCMEWZYh+me3ylahY5g5LaaVtDc+Hc9O9OMzg+gVK+DcTCp1I1Qc7Z1PpxSEVI2SSM= X-Received: by 2002:a19:fc03:: with SMTP id a3mr26555268lfi.327.1626881355605; Wed, 21 Jul 2021 08:29:15 -0700 (PDT) MIME-Version: 1.0 References: <20210715122054.2002001-1-aldyh@redhat.com> In-Reply-To: <20210715122054.2002001-1-aldyh@redhat.com> From: Aldy Hernandez Date: Wed, 21 Jul 2021 17:29:04 +0200 Message-ID: Subject: Re: [PUSHED] Abstract out non_null adjustments in ranger. To: GCC patches , Andrew MacLeod X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/mixed; boundary="0000000000004b2b6205c7a3d502" X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Jul 2021 15:29:23 -0000 --0000000000004b2b6205c7a3d502 Content-Type: text/plain; charset="UTF-8" As I mentioned when I pushed the patch in this thread, I have run into cases where a pointer has a non-varying range, but it includes 0. The varying check causes no further refinements to be done. The previous cases I had seen were in follow-up threader work, so I was delaying pushing this until then. But I'm now going through VRP cases that evrp is missing, and this is one of the main culprits. I will push this once tests are done. If for some reason, varying restrictions are needed, we can always adjust the callers to adjust_range. Aldy On Thu, Jul 15, 2021 at 2:21 PM Aldy Hernandez wrote: > > There are 4 exact copies of the non-null range adjusting code in the > ranger. This patch abstracts the functionality into a separate method. > > As a follow-up I would like to remove the varying_p check, since I have > seen incoming ranges such as [0, 0xff....ef] which are not varying, but > are not-null. Removing the varying restriction catches those. > > Tested on x86-64 Linux. > > Pushed to trunk. > > p.s. Andrew, what are your thoughts on removing the varying_p() check as > a follow-up? > > gcc/ChangeLog: > > * gimple-range-cache.cc (non_null_ref::adjust_range): New. > (ranger_cache::range_of_def): Call adjust_range. > (ranger_cache::entry_range): Same. > * gimple-range-cache.h (non_null_ref::adjust_range): New. > * gimple-range.cc (gimple_ranger::range_of_expr): Call > adjust_range. > (gimple_ranger::range_on_entry): Same. > --- > gcc/gimple-range-cache.cc | 35 ++++++++++++++++++++++++++--------- > gcc/gimple-range-cache.h | 2 ++ > gcc/gimple-range.cc | 8 ++------ > 3 files changed, 30 insertions(+), 15 deletions(-) > > diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc > index 98ecdbbd68e..23597ade802 100644 > --- a/gcc/gimple-range-cache.cc > +++ b/gcc/gimple-range-cache.cc > @@ -81,6 +81,29 @@ non_null_ref::non_null_deref_p (tree name, basic_block bb, bool search_dom) > return false; > } > > +// If NAME has a non-null dereference in block BB, adjust R with the > +// non-zero information from non_null_deref_p, and return TRUE. If > +// SEARCH_DOM is true, non_null_deref_p should search the dominator tree. > + > +bool > +non_null_ref::adjust_range (irange &r, tree name, basic_block bb, > + bool search_dom) > +{ > + // Check if pointers have any non-null dereferences. Non-call > + // exceptions mean we could throw in the middle of the block, so just > + // punt for now on those. > + if (!cfun->can_throw_non_call_exceptions > + && r.varying_p () > + && non_null_deref_p (name, bb, search_dom)) > + { > + int_range<2> nz; > + nz.set_nonzero (TREE_TYPE (name)); > + r.intersect (nz); > + return true; > + } > + return false; > +} > + > // Allocate an populate the bitmap for NAME. An ON bit for a block > // index indicates there is a non-null reference in that block. In > // order to populate the bitmap, a quick run of all the immediate uses > @@ -857,9 +880,8 @@ ranger_cache::range_of_def (irange &r, tree name, basic_block bb) > r = gimple_range_global (name); > } > > - if (bb && r.varying_p () && m_non_null.non_null_deref_p (name, bb, false) && > - !cfun->can_throw_non_call_exceptions) > - r = range_nonzero (TREE_TYPE (name)); > + if (bb) > + m_non_null.adjust_range (r, name, bb, false); > } > > // Get the range of NAME as it occurs on entry to block BB. > @@ -878,12 +900,7 @@ ranger_cache::entry_range (irange &r, tree name, basic_block bb) > if (!m_on_entry.get_bb_range (r, name, bb)) > range_of_def (r, name); > > - // Check if pointers have any non-null dereferences. Non-call > - // exceptions mean we could throw in the middle of the block, so just > - // punt for now on those. > - if (r.varying_p () && m_non_null.non_null_deref_p (name, bb, false) && > - !cfun->can_throw_non_call_exceptions) > - r = range_nonzero (TREE_TYPE (name)); > + m_non_null.adjust_range (r, name, bb, false); > } > > // Get the range of NAME as it occurs on exit from block BB. > diff --git a/gcc/gimple-range-cache.h b/gcc/gimple-range-cache.h > index ecf63dc01b3..f842e9c092a 100644 > --- a/gcc/gimple-range-cache.h > +++ b/gcc/gimple-range-cache.h > @@ -34,6 +34,8 @@ public: > non_null_ref (); > ~non_null_ref (); > bool non_null_deref_p (tree name, basic_block bb, bool search_dom = true); > + bool adjust_range (irange &r, tree name, basic_block bb, > + bool search_dom = true); > private: > vec m_nn; > void process_name (tree name); > diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc > index 1851339c528..b210787d0b7 100644 > --- a/gcc/gimple-range.cc > +++ b/gcc/gimple-range.cc > @@ -69,9 +69,7 @@ gimple_ranger::range_of_expr (irange &r, tree expr, gimple *stmt) > if (def_stmt && gimple_bb (def_stmt) == bb) > { > range_of_stmt (r, def_stmt, expr); > - if (!cfun->can_throw_non_call_exceptions && r.varying_p () && > - m_cache.m_non_null.non_null_deref_p (expr, bb)) > - r = range_nonzero (TREE_TYPE (expr)); > + m_cache.m_non_null.adjust_range (r, expr, bb, true); > } > else > // Otherwise OP comes from outside this block, use range on entry. > @@ -95,9 +93,7 @@ gimple_ranger::range_on_entry (irange &r, basic_block bb, tree name) > if (m_cache.block_range (entry_range, bb, name)) > r.intersect (entry_range); > > - if (!cfun->can_throw_non_call_exceptions && r.varying_p () && > - m_cache.m_non_null.non_null_deref_p (name, bb)) > - r = range_nonzero (TREE_TYPE (name)); > + m_cache.m_non_null.adjust_range (r, name, bb, true); > } > > // Calculate the range for NAME at the end of block BB and return it in R. > -- > 2.31.1 > --0000000000004b2b6205c7a3d502 Content-Type: text/x-patch; charset="US-ASCII"; name="0001-Allow-non-null-adjustments-for-pointers-even-when-th.patch" Content-Disposition: attachment; filename="0001-Allow-non-null-adjustments-for-pointers-even-when-th.patch" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: f_krdmy99u0 RnJvbSBmYzdlMGU3NzU1YWY0OGYxNWYzZWFlOGNmYjg3ODJlZDA3Mjc0ZTkxIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBBbGR5IEhlcm5hbmRleiA8YWxkeWhAcmVkaGF0LmNvbT4KRGF0 ZTogV2VkLCAyMSBKdWwgMjAyMSAxNzoxNDo0MyArMDIwMApTdWJqZWN0OiBbUEFUQ0hdIEFsbG93 IG5vbi1udWxsIGFkanVzdG1lbnRzIGZvciBwb2ludGVycyBldmVuIHdoZW4gdGhlcmUgaXMgYQog a25vd24gcmFuZ2UuCgpJIGhhdmUgcnVuIGludG8geWV0IGFub3RoZXIgY2FzZSB3aGVyZSB0aGVy ZSBpcyBhIGtub3duIHJhbmdlIGZvciBhCnBvaW50ZXIsIGJ1dCBpdCBkb2VzIG5vdCBleGNsdWRl IG5vbi1udWxsLCBzbyBubyBhZGp1c3RtZW50cyBhcmUKcGVyZm9ybWVkLgoKZ2NjL0NoYW5nZUxv ZzoKCgkqIGdpbXBsZS1yYW5nZS1jYWNoZS5jYyAobm9uX251bGxfcmVmOjphZGp1c3RfcmFuZ2Up OiBEbyBub3QKCWNoZWNrIGZvciB2YXJ5aW5nLgotLS0KIGdjYy9naW1wbGUtcmFuZ2UtY2FjaGUu Y2MgfCAxIC0KIDEgZmlsZSBjaGFuZ2VkLCAxIGRlbGV0aW9uKC0pCgpkaWZmIC0tZ2l0IGEvZ2Nj L2dpbXBsZS1yYW5nZS1jYWNoZS5jYyBiL2djYy9naW1wbGUtcmFuZ2UtY2FjaGUuY2MKaW5kZXgg MjM1OTdhZGU4MDIuLjk1NTAwZWQ2ZGMxIDEwMDY0NAotLS0gYS9nY2MvZ2ltcGxlLXJhbmdlLWNh Y2hlLmNjCisrKyBiL2djYy9naW1wbGUtcmFuZ2UtY2FjaGUuY2MKQEAgLTkzLDcgKzkzLDYgQEAg bm9uX251bGxfcmVmOjphZGp1c3RfcmFuZ2UgKGlyYW5nZSAmciwgdHJlZSBuYW1lLCBiYXNpY19i bG9jayBiYiwKICAgLy8gZXhjZXB0aW9ucyBtZWFuIHdlIGNvdWxkIHRocm93IGluIHRoZSBtaWRk bGUgb2YgdGhlIGJsb2NrLCBzbyBqdXN0CiAgIC8vIHB1bnQgZm9yIG5vdyBvbiB0aG9zZS4KICAg aWYgKCFjZnVuLT5jYW5fdGhyb3dfbm9uX2NhbGxfZXhjZXB0aW9ucwotICAgICAgJiYgci52YXJ5 aW5nX3AgKCkKICAgICAgICYmIG5vbl9udWxsX2RlcmVmX3AgKG5hbWUsIGJiLCBzZWFyY2hfZG9t KSkKICAgICB7CiAgICAgICBpbnRfcmFuZ2U8Mj4gbno7Ci0tIAoyLjMxLjEKCg== --0000000000004b2b6205c7a3d502--