From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com [IPv6:2a00:1450:4864:20::42d]) by sourceware.org (Postfix) with ESMTPS id BA746385842D for ; Tue, 7 Sep 2021 12:59:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BA746385842D Received: by mail-wr1-x42d.google.com with SMTP id q11so14311675wrr.9 for ; Tue, 07 Sep 2021 05:59:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:user-agent:in-reply-to :references:message-id:mime-version:content-transfer-encoding; bh=7HBInpWDsNitAVQJczNTJ6boJ0AOzLoloIFqDjhIJQM=; b=fqQkqKD+Gj40Mkne5tizitueJi6q5nFAUJVgbzn/EZH8qXsc0+cWtioU7Q4FP0vGZJ 0KtGxMM09UTW+XtwuzY2DnII8hL0FY6fErwNzq4kCU8sQmGJO5pqtu8gDcNOIz6oxSq/ x38o4tlFjJaQw9uZbPmkkAw+t7iKeEbdzULzxhM5vcR8Hr5CmFHVJUpLh3vRo6fqNTF7 My6pwcFzZX/I6/2+XtZtzvAqrhoJ+ePINK82EMH0Xj1XfEXkr71B67AOk3axwyycz13+ ZNml5rgHWdIucL0l6SvgriEnP5R1AA4PWwHPxz/2nBmteTM7EA03IC5F0ZhlKzzCwchu Eaxg== X-Gm-Message-State: AOAM530v2UpwTZunrxneD0ONjy8axGQW3QxxWHJjyAEWtvK8hKLy7po7 LQcVTlBKy/NXGoXH4+5DeHU= X-Google-Smtp-Source: ABdhPJzqe1Ggl08XpabNPxICqyN8oS+EzCkbrebsFlnOs4656wwiHz7SPnAQXuG+ct+qLqvsP3kIOw== X-Received: by 2002:adf:c54a:: with SMTP id s10mr18808767wrf.125.1631019566802; Tue, 07 Sep 2021 05:59:26 -0700 (PDT) Received: from [127.0.0.1] (dynamic-002-247-253-084.2.247.pool.telefonica.de. [2.247.253.84]) by smtp.gmail.com with ESMTPSA id j17sm10824778wrh.67.2021.09.07.05.59.26 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 07 Sep 2021 05:59:26 -0700 (PDT) Date: Tue, 07 Sep 2021 14:59:24 +0200 From: Richard Biener To: Aldy Hernandez CC: GCC patches , Jeff Law Subject: Re: [PATCH] Abstract PHI and forwarder block checks in jump threader. User-Agent: K-9 Mail for Android In-Reply-To: References: <20210903135832.735852-1-aldyh@redhat.com> Message-ID: <6D546CE0-4156-4C8F-B407-E7DCEA13FBF4@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, 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: Tue, 07 Sep 2021 12:59:29 -0000 On September 7, 2021 12:02:27 PM GMT+02:00, Aldy Hernandez wrote: > > >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=2E >>> >>> Tested on x86-64 Linux=2E >>> >>> OK? >>> >>> gcc/ChangeLog: >>> >>> * tree-ssa-threadedge=2Ec (has_phis_p): New=2E >>> (forwarder_block_p): New=2E >>> (potentially_threadable_block): Call forwarder_block_p=2E >>> (jump_threader::thread_around_empty_blocks): Call has_phis_p= =2E >>> (jump_threader::thread_through_normal_block): Call >>> forwarder_block_p=2E >>> --- >>> gcc/tree-ssa-threadedge=2Ec | 25 +++++++++++++++++++------ >>> 1 file changed, 19 insertions(+), 6 deletions(-) >>> >>> diff --git a/gcc/tree-ssa-threadedge=2Ec b/gcc/tree-ssa-threadedge=2Ec >>> index e57f6d3e39c=2E=2E3db54a199fd 100644 >>> --- a/gcc/tree-ssa-threadedge=2Ec >>> +++ b/gcc/tree-ssa-threadedge=2Ec >>> @@ -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_header= s); >>> } >>> >>> +static inline bool >>> +has_phis_p (basic_block bb) >>> +{ >>> + return !gsi_end_p (gsi_start_phis (bb)); >>=20 >> gimple_seq_empty_p (phi_nodes (bb)) shoud be cheaper=2E Do virtual PHI= s >> count as PHIs for you? > >I don't know=2E The goal was to abstract some common idioms without=20 >changing existing behavior, but if my abstractions confuse other=20 >readers, perhaps I should revert my patch=2E > >FWIW, my initial motivation here was to merge the path profitability=20 >code between the forward and backward threaders=2E It seems the forward= =20 >threader is more permissive than the backward threader, even though the= =20 >latter can thread more paths than it's allowed (per profitable_path_p)=2E > >>=20 >>> +} >>> + >>> +/* Return TRUE for a forwarder block which is defined as having PHIs >>> + but no instructions=2E */ >>> + >>> +static bool >>> +forwarder_block_p (basic_block bb) >>=20 >> There exists a function with exactly the same signature in cfgrtl=2Eh, = likewise >> several similar implementations might exist elsewhere=2E > >Ughh, that's definitely not good=2E > >>=20 >> Your definition is also quite odd, not matching what one would expect >> (the PHI requirement)=2E The tree-cfgcleanup=2Ec variant has >> tree_forwarder_block_p which is explicit about this=2E >>=20 >> Btw, gsi_start_nondebug_bb does not ignore labels=2E > >Would a name like empty_block_with_phis_p be more appropriate? I think so=2E That said, my main concern ist the clash with the same name= d function=2E Richard=2E=20 >Aldy >