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 [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 9EAA13858415 for ; Tue, 7 Sep 2021 10:02:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9EAA13858415 Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-132-FJxQ3luqPvu3PPxP-NKPrQ-1; Tue, 07 Sep 2021 06:02:31 -0400 X-MC-Unique: FJxQ3luqPvu3PPxP-NKPrQ-1 Received: by mail-wr1-f72.google.com with SMTP id s13-20020a5d69cd000000b00159d49442cbso327384wrw.13 for ; Tue, 07 Sep 2021 03:02:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=47i3Bja54/C4wXsbcGmnYaaKz9QdqdaRetl2ZSwkwl8=; b=HfftcTb72ehiXnpqP+cMj7SQjfAJfra22nmvKp3wcpo/bkPAE2ia1eUGKeHWgQmsF1 jtiDY8HWzZVtaaUsj0Va9snLSBhi9xkfyYb1safM+uTIZAsh5skQz/mNv+8ejOm8iXkT /WwvJuS5BuIVLDisfzCR59zATH3YoG4DSHwG17825w0paQxY5mK47QEhLgEc7/yQ0ybt 8aGhxNCa4IsOTqAOOuazvk8u4U0DRma6mIsX7eDZ5Z7XZ6lIa3kHb9NHbo2tCGeGzS04 CNwbpIjMIHf9GZOULTVq602U8NKUsrCdCLrE3Jtfal2CgW1xtWF+KiTSo0krKpk7cHyR n9Cg== X-Gm-Message-State: AOAM531EV2p427rBKZsFzK2tzQ86s6msG5OHVxnHCytVnbtY3quX1b82 DcRPWWsnDAwIGTMoMfGPZYMfwmToBw79S0hUPiPiUukETanL7jl0YfYL4/zM1+RFpg91o0B0GEX XeBs/j7H8VIE9zsvuUA== X-Received: by 2002:a05:600c:1c95:: with SMTP id k21mr3015898wms.176.1631008949621; Tue, 07 Sep 2021 03:02:29 -0700 (PDT) X-Google-Smtp-Source: ABdhPJybFbx6orZaWnvThMaVuFa9YbmT9TZhBSdRr+H7hbaNmuIDjY7I54LGnWOSGYN8ZIY3ZoY2iQ== X-Received: by 2002:a05:600c:1c95:: with SMTP id k21mr3015858wms.176.1631008949114; Tue, 07 Sep 2021 03:02:29 -0700 (PDT) Received: from abulafia.quesejoda.com ([139.47.33.227]) by smtp.gmail.com with ESMTPSA id n66sm1986743wmn.2.2021.09.07.03.02.28 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 07 Sep 2021 03:02:28 -0700 (PDT) Subject: Re: [PATCH] Abstract PHI and forwarder block checks in jump threader. To: Richard Biener Cc: GCC patches , Jeff Law References: <20210903135832.735852-1-aldyh@redhat.com> From: Aldy Hernandez Message-ID: Date: Tue, 7 Sep 2021 12:02:27 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-13.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, 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: Tue, 07 Sep 2021 10:02:37 -0000 On 9/6/21 9:19 AM, Richard Biener wrote: > On Fri, Sep 3, 2021 at 3:59 PM Aldy Hernandez via Gcc-patches > wrote: >> >> This patch abstracts out a couple common idioms in the forward >> threader that I found useful while navigating the code base. >> >> Tested on x86-64 Linux. >> >> OK? >> >> gcc/ChangeLog: >> >> * tree-ssa-threadedge.c (has_phis_p): New. >> (forwarder_block_p): New. >> (potentially_threadable_block): Call forwarder_block_p. >> (jump_threader::thread_around_empty_blocks): Call has_phis_p. >> (jump_threader::thread_through_normal_block): Call >> forwarder_block_p. >> --- >> gcc/tree-ssa-threadedge.c | 25 +++++++++++++++++++------ >> 1 file changed, 19 insertions(+), 6 deletions(-) >> >> diff --git a/gcc/tree-ssa-threadedge.c b/gcc/tree-ssa-threadedge.c >> index e57f6d3e39c..3db54a199fd 100644 >> --- a/gcc/tree-ssa-threadedge.c >> +++ b/gcc/tree-ssa-threadedge.c >> @@ -95,6 +95,21 @@ jump_threader::thread_through_all_blocks (bool may_peel_loop_headers) >> return m_registry->thread_through_all_blocks (may_peel_loop_headers); >> } >> >> +static inline bool >> +has_phis_p (basic_block bb) >> +{ >> + return !gsi_end_p (gsi_start_phis (bb)); > > gimple_seq_empty_p (phi_nodes (bb)) shoud be cheaper. Do virtual PHIs > count as PHIs for you? I don't know. The goal was to abstract some common idioms without changing existing behavior, but if my abstractions confuse other readers, perhaps I should revert my patch. FWIW, my initial motivation here was to merge the path profitability code between the forward and backward threaders. It seems the forward threader is more permissive than the backward threader, even though the latter can thread more paths than it's allowed (per profitable_path_p). > >> +} >> + >> +/* Return TRUE for a forwarder block which is defined as having PHIs >> + but no instructions. */ >> + >> +static bool >> +forwarder_block_p (basic_block bb) > > There exists a function with exactly the same signature in cfgrtl.h, likewise > several similar implementations might exist elsewhere. Ughh, that's definitely not good. > > Your definition is also quite odd, not matching what one would expect > (the PHI requirement). The tree-cfgcleanup.c variant has > tree_forwarder_block_p which is explicit about this. > > Btw, gsi_start_nondebug_bb does not ignore labels. Would a name like empty_block_with_phis_p be more appropriate? Aldy