From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com [IPv6:2a00:1450:4864:20::52c]) by sourceware.org (Postfix) with ESMTPS id 199B93858C27 for ; Mon, 6 Sep 2021 07:19:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 199B93858C27 Received: by mail-ed1-x52c.google.com with SMTP id v5so8208002edc.2 for ; Mon, 06 Sep 2021 00:19:23 -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:cc; bh=6cT5qpEomAQIxBd1H0qU/0bsljWNQquPsFa82OO2g2g=; b=LHv6xG6IPyX12lUJDd9UGl4zOayiovjKBcxo4A2OfKrzXPKC7w0od3ymFk22q8RO4d i6cc34ynTJFnlCY0n90Y2DBNPNZfUGVTKzDOw46lU/FIAXXobMgZ8vYZFHxL8aeMeTgm +0XRBFmUlRW+vsf2sysMEXZhFCrikSChOBCnA2pWysR4fWfUpgtCULWbqqx0VWY4liIt cBS05iZIJPk7WZ/JZrV+xxhTCPaNgW+bm8J4BkyN4sn4FW7kPk4rH4Gr3IPK7QYl23ay +tHtUWNtnoG8umTzluTS+c3qh6kK4bMqnTg746e6Iz9LmnUixTt8ueNNHNWcDNMCGncj LZAg== X-Gm-Message-State: AOAM530DTov7tIwuKiYUXljqHt9QyXjrQgwi3S1A/GMAv1t6XZTkVmTu hpVuxFHq7u27VTKdnphCv5uD9opTD7Dy3Bt7PQo= X-Google-Smtp-Source: ABdhPJwrVhTLlpO/GKzpV6Q+gXxdfD8/GiYiJ2X53c2pK2okT+sCEfwSNPPMB76VU8ml1ejIDpYoBCdm1s0oCbfccMU= X-Received: by 2002:aa7:c9d6:: with SMTP id i22mr12126871edt.307.1630912762051; Mon, 06 Sep 2021 00:19:22 -0700 (PDT) MIME-Version: 1.0 References: <20210903135832.735852-1-aldyh@redhat.com> In-Reply-To: <20210903135832.735852-1-aldyh@redhat.com> From: Richard Biener Date: Mon, 6 Sep 2021 09:19:11 +0200 Message-ID: Subject: Re: [PATCH] Abstract PHI and forwarder block checks in jump threader. To: Aldy Hernandez Cc: GCC patches , Jeff Law Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.7 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 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: Mon, 06 Sep 2021 07:19:24 -0000 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? > +} > + > +/* 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. 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. > +{ > + return gsi_end_p (gsi_start_nondebug_bb (bb)) && has_phis_p (bb); > +} > + > /* Return TRUE if we may be able to thread an incoming edge into > BB to an outgoing edge from BB. Return FALSE otherwise. */ > > @@ -107,9 +122,8 @@ potentially_threadable_block (basic_block bb) > not optimized away because they forward from outside a loop > to the loop header. We want to thread through them as we can > sometimes thread to the loop exit, which is obviously profitable. > - the interesting case here is when the block has PHIs. */ > - if (gsi_end_p (gsi_start_nondebug_bb (bb)) > - && !gsi_end_p (gsi_start_phis (bb))) > + The interesting case here is when the block has PHIs. */ > + if (forwarder_block_p (bb)) > return true; > > /* If BB has a single successor or a single predecessor, then > @@ -854,7 +868,7 @@ jump_threader::thread_around_empty_blocks (vec *path, > /* The key property of these blocks is that they need not be duplicated > when threading. Thus they cannot have visible side effects such > as PHI nodes. */ > - if (!gsi_end_p (gsi_start_phis (bb))) > + if (has_phis_p (bb)) > return false; > > /* Skip over DEBUG statements at the start of the block. */ > @@ -994,8 +1008,7 @@ jump_threader::thread_through_normal_block (vec *path, > { > /* First case. The statement simply doesn't have any instructions, but > does have PHIs. */ > - if (gsi_end_p (gsi_start_nondebug_bb (e->dest)) > - && !gsi_end_p (gsi_start_phis (e->dest))) > + if (forwarder_block_p (e->dest)) > return 0; > > /* Second case. */ > -- > 2.31.1 >