Message ID | 20210927150003.796951-1-aldyh@redhat.com |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id DF46D3858423 for <patchwork@sourceware.org>; Mon, 27 Sep 2021 15:01:20 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DF46D3858423 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1632754880; bh=Lo7qe4rH7LYCV/zPVOtqmQBH/qCwcMZNCtygxfVOySk=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=PizhDocSUKqY76A66sY5xyV7oLWkRBephafliPiTl6IvwmGq1+0MKpuvDE+4eV6Ti huX0zZULoy9gPRzgxVnGp9EMTKU9OjrYV/053BedPde1uGkGBkdhdlSarJHOmwkFmJ uuaxnb1t3Ak0xp0hvtJG6Bw8e/YMLH5WTc5w9atg= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org 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 4F4EB3858405 for <gcc-patches@gcc.gnu.org>; Mon, 27 Sep 2021 15:00:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4F4EB3858405 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-182-O-wevfLaMni47gA5YeZF9A-1; Mon, 27 Sep 2021 11:00:49 -0400 X-MC-Unique: O-wevfLaMni47gA5YeZF9A-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 753E3881290; Mon, 27 Sep 2021 15:00:48 +0000 (UTC) Received: from abulafia.quesejoda.com (unknown [10.39.193.91]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 73DAF60C13; Mon, 27 Sep 2021 15:00:11 +0000 (UTC) Received: from abulafia.quesejoda.com (localhost [127.0.0.1]) by abulafia.quesejoda.com (8.16.1/8.15.2) with ESMTPS id 18RF08kP797037 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Mon, 27 Sep 2021 17:00:09 +0200 Received: (from aldyh@localhost) by abulafia.quesejoda.com (8.16.1/8.16.1/Submit) id 18RF08mR797036; Mon, 27 Sep 2021 17:00:08 +0200 To: GCC patches <gcc-patches@gcc.gnu.org> Subject: [PATCH] Control all jump threading passes with -fjump-threads. Date: Mon, 27 Sep 2021 17:00:03 +0200 Message-Id: <20210927150003.796951-1-aldyh@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" X-Spam-Status: No, score=-13.1 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_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 <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Aldy Hernandez via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Aldy Hernandez <aldyh@redhat.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
Control all jump threading passes with -fjump-threads.
|
|
Commit Message
Aldy Hernandez
Sept. 27, 2021, 3 p.m. UTC
Last year I mentioned that -fthread-jumps was being ignored by the majority of our jump threading passes, and Jeff said he'd be in favor of fixing this. This patch remedies the situation, but it does change existing behavior. Currently -fthread-jumps is only enabled for -O2, -O3, and -Os. This means that even if we restricted all jump threading passes with -fthread-jumps, DOM jump threading would still seep through since it runs at -O1. I propose this patch, but it does mean that DOM jump threading would have to be explicitly enabled with -O1 -fthread-jumps. An alternative would be to also offer a specific -fno-dom-threading, but that seems icky. OK pending tests? gcc/ChangeLog: * tree-ssa-threadbackward.c (pass_thread_jumps::gate): Check flag_thread_jumps. (pass_early_thread_jumps::gate): Same. * tree-ssa-threadedge.c (jump_threader::thread_outgoing_edges): Return if !flag_thread_jumps. * tree-ssa-threadupdate.c (jt_path_registry::register_jump_thread): Assert that flag_thread_jumps is true. --- gcc/testsuite/gcc.dg/tree-ssa/pr18133-2.c | 2 +- gcc/testsuite/gcc.dg/tree-ssa/pr18134.c | 2 +- gcc/tree-ssa-threadbackward.c | 4 ++-- gcc/tree-ssa-threadedge.c | 3 +++ gcc/tree-ssa-threadupdate.c | 2 ++ 5 files changed, 9 insertions(+), 4 deletions(-)
Comments
On 9/27/2021 9:00 AM, Aldy Hernandez wrote: > Last year I mentioned that -fthread-jumps was being ignored by the > majority of our jump threading passes, and Jeff said he'd be in favor > of fixing this. > > This patch remedies the situation, but it does change existing behavior. > Currently -fthread-jumps is only enabled for -O2, -O3, and -Os. This > means that even if we restricted all jump threading passes with > -fthread-jumps, DOM jump threading would still seep through since it > runs at -O1. > > I propose this patch, but it does mean that DOM jump threading would > have to be explicitly enabled with -O1 -fthread-jumps. An > alternative would be to also offer a specific -fno-dom-threading, but > that seems icky. > > OK pending tests? > > gcc/ChangeLog: > > * tree-ssa-threadbackward.c (pass_thread_jumps::gate): Check > flag_thread_jumps. > (pass_early_thread_jumps::gate): Same. > * tree-ssa-threadedge.c (jump_threader::thread_outgoing_edges): > Return if !flag_thread_jumps. > * tree-ssa-threadupdate.c > (jt_path_registry::register_jump_thread): Assert that > flag_thread_jumps is true. OK. Clearly this is going to be even better once we disentangle threading from DOM. jeff
On Tue, Sep 28, 2021 at 3:46 AM Jeff Law <jeffreyalaw@gmail.com> wrote: > > > > On 9/27/2021 9:00 AM, Aldy Hernandez wrote: > > Last year I mentioned that -fthread-jumps was being ignored by the > > majority of our jump threading passes, and Jeff said he'd be in favor > > of fixing this. > > > > This patch remedies the situation, but it does change existing behavior. > > Currently -fthread-jumps is only enabled for -O2, -O3, and -Os. This > > means that even if we restricted all jump threading passes with > > -fthread-jumps, DOM jump threading would still seep through since it > > runs at -O1. > > > > I propose this patch, but it does mean that DOM jump threading would > > have to be explicitly enabled with -O1 -fthread-jumps. An > > alternative would be to also offer a specific -fno-dom-threading, but > > that seems icky. > > > > OK pending tests? > > > > gcc/ChangeLog: > > > > * tree-ssa-threadbackward.c (pass_thread_jumps::gate): Check > > flag_thread_jumps. > > (pass_early_thread_jumps::gate): Same. > > * tree-ssa-threadedge.c (jump_threader::thread_outgoing_edges): > > Return if !flag_thread_jumps. > > * tree-ssa-threadupdate.c > > (jt_path_registry::register_jump_thread): Assert that > > flag_thread_jumps is true. > OK. Clearly this is going to be even better once we disentangle > threading from DOM. Annoyingly, I had to tweak a few more tests, particularly some -Wuninitialized -O1 ones which seem to depend on DOM jump threading to give proper diagnostics. It seems that every change to jump threading needs tweaks to the Wuninitialized code :-(. Attached is the patch I have pushed. Aldy
On 9/28/2021 12:17 AM, Aldy Hernandez wrote: > On Tue, Sep 28, 2021 at 3:46 AM Jeff Law <jeffreyalaw@gmail.com> wrote: >> >> >> On 9/27/2021 9:00 AM, Aldy Hernandez wrote: >>> Last year I mentioned that -fthread-jumps was being ignored by the >>> majority of our jump threading passes, and Jeff said he'd be in favor >>> of fixing this. >>> >>> This patch remedies the situation, but it does change existing behavior. >>> Currently -fthread-jumps is only enabled for -O2, -O3, and -Os. This >>> means that even if we restricted all jump threading passes with >>> -fthread-jumps, DOM jump threading would still seep through since it >>> runs at -O1. >>> >>> I propose this patch, but it does mean that DOM jump threading would >>> have to be explicitly enabled with -O1 -fthread-jumps. An >>> alternative would be to also offer a specific -fno-dom-threading, but >>> that seems icky. >>> >>> OK pending tests? >>> >>> gcc/ChangeLog: >>> >>> * tree-ssa-threadbackward.c (pass_thread_jumps::gate): Check >>> flag_thread_jumps. >>> (pass_early_thread_jumps::gate): Same. >>> * tree-ssa-threadedge.c (jump_threader::thread_outgoing_edges): >>> Return if !flag_thread_jumps. >>> * tree-ssa-threadupdate.c >>> (jt_path_registry::register_jump_thread): Assert that >>> flag_thread_jumps is true. >> OK. Clearly this is going to be even better once we disentangle >> threading from DOM. > Annoyingly, I had to tweak a few more tests, particularly some > -Wuninitialized -O1 ones which seem to depend on DOM jump threading to > give proper diagnostics. It seems that every change to jump threading > needs tweaks to the Wuninitialized code :-(. Well, a lot of jump threading is there to help eliminate false positives from Wuninitialized by eliminating paths through the CFG that we can prove never execute at runtime. SO that's not a huge surprise. jeff
On Tue, Sep 28, 2021 at 8:29 AM Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > > On 9/28/2021 12:17 AM, Aldy Hernandez wrote: > > On Tue, Sep 28, 2021 at 3:46 AM Jeff Law <jeffreyalaw@gmail.com> wrote: > >> > >> > >> On 9/27/2021 9:00 AM, Aldy Hernandez wrote: > >>> Last year I mentioned that -fthread-jumps was being ignored by the > >>> majority of our jump threading passes, and Jeff said he'd be in favor > >>> of fixing this. > >>> > >>> This patch remedies the situation, but it does change existing behavior. > >>> Currently -fthread-jumps is only enabled for -O2, -O3, and -Os. This > >>> means that even if we restricted all jump threading passes with > >>> -fthread-jumps, DOM jump threading would still seep through since it > >>> runs at -O1. > >>> > >>> I propose this patch, but it does mean that DOM jump threading would > >>> have to be explicitly enabled with -O1 -fthread-jumps. An > >>> alternative would be to also offer a specific -fno-dom-threading, but > >>> that seems icky. > >>> > >>> OK pending tests? > >>> > >>> gcc/ChangeLog: > >>> > >>> * tree-ssa-threadbackward.c (pass_thread_jumps::gate): Check > >>> flag_thread_jumps. > >>> (pass_early_thread_jumps::gate): Same. > >>> * tree-ssa-threadedge.c (jump_threader::thread_outgoing_edges): > >>> Return if !flag_thread_jumps. > >>> * tree-ssa-threadupdate.c > >>> (jt_path_registry::register_jump_thread): Assert that > >>> flag_thread_jumps is true. > >> OK. Clearly this is going to be even better once we disentangle > >> threading from DOM. > > Annoyingly, I had to tweak a few more tests, particularly some > > -Wuninitialized -O1 ones which seem to depend on DOM jump threading to > > give proper diagnostics. It seems that every change to jump threading > > needs tweaks to the Wuninitialized code :-(. > Well, a lot of jump threading is there to help eliminate false positives > from Wuninitialized by eliminating paths through the CFG that we can > prove never execute at runtime. SO that's not a huge surprise. I would have suggested to enable -fthread-jumps at -O1 instead and eventually just add && flag_expensive_optimizations to the use in cfgcleanup.c to restrict that to -O2+ Richard. > jeff
On 9/28/21 9:41 AM, Richard Biener wrote: > On Tue, Sep 28, 2021 at 8:29 AM Jeff Law via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> >> >> On 9/28/2021 12:17 AM, Aldy Hernandez wrote: >>> On Tue, Sep 28, 2021 at 3:46 AM Jeff Law <jeffreyalaw@gmail.com> wrote: >>>> >>>> >>>> On 9/27/2021 9:00 AM, Aldy Hernandez wrote: >>>>> Last year I mentioned that -fthread-jumps was being ignored by the >>>>> majority of our jump threading passes, and Jeff said he'd be in favor >>>>> of fixing this. >>>>> >>>>> This patch remedies the situation, but it does change existing behavior. >>>>> Currently -fthread-jumps is only enabled for -O2, -O3, and -Os. This >>>>> means that even if we restricted all jump threading passes with >>>>> -fthread-jumps, DOM jump threading would still seep through since it >>>>> runs at -O1. >>>>> >>>>> I propose this patch, but it does mean that DOM jump threading would >>>>> have to be explicitly enabled with -O1 -fthread-jumps. An >>>>> alternative would be to also offer a specific -fno-dom-threading, but >>>>> that seems icky. >>>>> >>>>> OK pending tests? >>>>> >>>>> gcc/ChangeLog: >>>>> >>>>> * tree-ssa-threadbackward.c (pass_thread_jumps::gate): Check >>>>> flag_thread_jumps. >>>>> (pass_early_thread_jumps::gate): Same. >>>>> * tree-ssa-threadedge.c (jump_threader::thread_outgoing_edges): >>>>> Return if !flag_thread_jumps. >>>>> * tree-ssa-threadupdate.c >>>>> (jt_path_registry::register_jump_thread): Assert that >>>>> flag_thread_jumps is true. >>>> OK. Clearly this is going to be even better once we disentangle >>>> threading from DOM. >>> Annoyingly, I had to tweak a few more tests, particularly some >>> -Wuninitialized -O1 ones which seem to depend on DOM jump threading to >>> give proper diagnostics. It seems that every change to jump threading >>> needs tweaks to the Wuninitialized code :-(. >> Well, a lot of jump threading is there to help eliminate false positives >> from Wuninitialized by eliminating paths through the CFG that we can >> prove never execute at runtime. SO that's not a huge surprise. > > I would have suggested to enable -fthread-jumps at -O1 instead > and eventually just add && flag_expensive_optimizations to the > use in cfgcleanup.c to restrict that to -O2+ Hmmm, that's a much better idea. I was afraid of messing existing behavior, but I suppose adding even more false positives for -O1 -Wuninitialized is worse. BTW, I plugged one more tweak to the registry in remove_jump_threads_including. No need to go add things to the removed edges hash table, if we're not going to thread. OK pending tests? Aldy
On Tue, Sep 28, 2021 at 11:42 AM Aldy Hernandez <aldyh@redhat.com> wrote: > > > > On 9/28/21 9:41 AM, Richard Biener wrote: > > On Tue, Sep 28, 2021 at 8:29 AM Jeff Law via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > >> > >> > >> > >> On 9/28/2021 12:17 AM, Aldy Hernandez wrote: > >>> On Tue, Sep 28, 2021 at 3:46 AM Jeff Law <jeffreyalaw@gmail.com> wrote: > >>>> > >>>> > >>>> On 9/27/2021 9:00 AM, Aldy Hernandez wrote: > >>>>> Last year I mentioned that -fthread-jumps was being ignored by the > >>>>> majority of our jump threading passes, and Jeff said he'd be in favor > >>>>> of fixing this. > >>>>> > >>>>> This patch remedies the situation, but it does change existing behavior. > >>>>> Currently -fthread-jumps is only enabled for -O2, -O3, and -Os. This > >>>>> means that even if we restricted all jump threading passes with > >>>>> -fthread-jumps, DOM jump threading would still seep through since it > >>>>> runs at -O1. > >>>>> > >>>>> I propose this patch, but it does mean that DOM jump threading would > >>>>> have to be explicitly enabled with -O1 -fthread-jumps. An > >>>>> alternative would be to also offer a specific -fno-dom-threading, but > >>>>> that seems icky. > >>>>> > >>>>> OK pending tests? > >>>>> > >>>>> gcc/ChangeLog: > >>>>> > >>>>> * tree-ssa-threadbackward.c (pass_thread_jumps::gate): Check > >>>>> flag_thread_jumps. > >>>>> (pass_early_thread_jumps::gate): Same. > >>>>> * tree-ssa-threadedge.c (jump_threader::thread_outgoing_edges): > >>>>> Return if !flag_thread_jumps. > >>>>> * tree-ssa-threadupdate.c > >>>>> (jt_path_registry::register_jump_thread): Assert that > >>>>> flag_thread_jumps is true. > >>>> OK. Clearly this is going to be even better once we disentangle > >>>> threading from DOM. > >>> Annoyingly, I had to tweak a few more tests, particularly some > >>> -Wuninitialized -O1 ones which seem to depend on DOM jump threading to > >>> give proper diagnostics. It seems that every change to jump threading > >>> needs tweaks to the Wuninitialized code :-(. > >> Well, a lot of jump threading is there to help eliminate false positives > >> from Wuninitialized by eliminating paths through the CFG that we can > >> prove never execute at runtime. SO that's not a huge surprise. > > > > I would have suggested to enable -fthread-jumps at -O1 instead > > and eventually just add && flag_expensive_optimizations to the > > use in cfgcleanup.c to restrict that to -O2+ > > Hmmm, that's a much better idea. I was afraid of messing existing > behavior, but I suppose adding even more false positives for -O1 > -Wuninitialized is worse. > > BTW, I plugged one more tweak to the registry in > remove_jump_threads_including. No need to go add things to the removed > edges hash table, if we're not going to thread. > > OK pending tests? OK. Richard. > Aldy
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr18133-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr18133-2.c index 8717640e327..1b409852189 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/pr18133-2.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr18133-2.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O1 -fdump-tree-optimized-blocks" } */ +/* { dg-options "-O1 -fthread-jumps -fdump-tree-optimized-blocks" } */ int c, d; diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr18134.c b/gcc/testsuite/gcc.dg/tree-ssa/pr18134.c index cd40ab2c162..d7f5d241eb9 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/pr18134.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr18134.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O1 -fdump-tree-optimized" } */ +/* { dg-options "-O1 -fthread-jumps -fdump-tree-optimized" } */ int foo (int a) { diff --git a/gcc/tree-ssa-threadbackward.c b/gcc/tree-ssa-threadbackward.c index 95542079faf..8940728cbf2 100644 --- a/gcc/tree-ssa-threadbackward.c +++ b/gcc/tree-ssa-threadbackward.c @@ -943,7 +943,7 @@ public: bool pass_thread_jumps::gate (function *fun ATTRIBUTE_UNUSED) { - return flag_expensive_optimizations; + return flag_thread_jumps && flag_expensive_optimizations; } // Try to thread blocks in FUN. Return TRUE if any jump thread paths were @@ -1013,7 +1013,7 @@ public: bool pass_early_thread_jumps::gate (function *fun ATTRIBUTE_UNUSED) { - return true; + return flag_thread_jumps; } unsigned int diff --git a/gcc/tree-ssa-threadedge.c b/gcc/tree-ssa-threadedge.c index ae77e5eb396..e6f0ff0b54b 100644 --- a/gcc/tree-ssa-threadedge.c +++ b/gcc/tree-ssa-threadedge.c @@ -1195,6 +1195,9 @@ jump_threader::thread_outgoing_edges (basic_block bb) int flags = (EDGE_IGNORE | EDGE_COMPLEX | EDGE_ABNORMAL); gimple *last; + if (!flag_thread_jumps) + return; + /* If we have an outgoing edge to a block with multiple incoming and outgoing edges, then we may be able to thread the edge, i.e., we may be able to statically determine which of the outgoing edges diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c index 2b9b8f81274..cf96c903668 100644 --- a/gcc/tree-ssa-threadupdate.c +++ b/gcc/tree-ssa-threadupdate.c @@ -2822,6 +2822,8 @@ jt_path_registry::cancel_invalid_paths (vec<jump_thread_edge *> &path) bool jt_path_registry::register_jump_thread (vec<jump_thread_edge *> *path) { + gcc_checking_assert (flag_thread_jumps); + if (!dbg_cnt (registered_jump_thread)) { path->release ();