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 6A4023858437 for ; Tue, 28 Sep 2021 09:42:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6A4023858437 Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-459-I0WgQrDsNo21q21b8J1phw-1; Tue, 28 Sep 2021 05:42:30 -0400 X-MC-Unique: I0WgQrDsNo21q21b8J1phw-1 Received: by mail-wr1-f71.google.com with SMTP id m1-20020a056000180100b0015e1ec30ac3so14896915wrh.8 for ; Tue, 28 Sep 2021 02:42:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=OzT0/209fYHT0o5MJ+rx8lhBILC65Uj9eMr6BmqIdHE=; b=x3gBz+QNaqNdvOcBeeIePvWP1pHrt/ClsGfQ//a+wKeP2mksKWnQokU4OHRYBWGwbk H1mL+FIB9zSKeBDGBA+493z+5W9jtWKFsYJ2Zmv9HJEnbM717WilkKhTWqoE52TulM8k BONg6sm38t9mJd4clNNO9HUnJrfoLJJG76Spxcrw5H2tZp7HSBmu88zxF22Wr83be2H7 n9m+t8aGi35EspIHSuQ0p9tE4XAU1UdsPHzEXnMTdQrnE+dfrHDVOJI4k3lkMc+zUqYe QFUMFmXnqRSvF4Zjv9K+hYh5lDffnJ6dV4QjaXfle1JO39dG/y6lc/LxUG+I/YqgwT1n lbVg== X-Gm-Message-State: AOAM533TVNFw5zLXSCvns3IQwmJSu9SPjumkTD/h75vXg24e2X6RWPDv kgIixz57PQ2o0uG0xV4Ms73tE5Hxk4+hbXtIYz122TG63I13+4xk0Cav4pskNTDVFT8TaXXpbxF UO7RJQL4QfSuG87HmfIcczLjX4YnUhcyxXOJvQGoe9vg68rt/7+4sUh6ImGsGVvaRMQ== X-Received: by 2002:a5d:4561:: with SMTP id a1mr5389578wrc.167.1632822149189; Tue, 28 Sep 2021 02:42:29 -0700 (PDT) X-Google-Smtp-Source: ABdhPJydd8d+LlDyOI3ZHmgkfDaGmh7LjWsvI0S6QCQHdn63sS1HDvhlqeLXT2I2vjVHVHFDPviLSg== X-Received: by 2002:a5d:4561:: with SMTP id a1mr5389544wrc.167.1632822148786; Tue, 28 Sep 2021 02:42:28 -0700 (PDT) Received: from abulafia.quesejoda.com ([139.47.33.227]) by smtp.gmail.com with ESMTPSA id f8sm18998427wrx.15.2021.09.28.02.42.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 28 Sep 2021 02:42:28 -0700 (PDT) Subject: Re: [PATCH] Control all jump threading passes with -fjump-threads. To: Richard Biener , Jeff Law Cc: GCC patches References: <20210927150003.796951-1-aldyh@redhat.com> <7ea0e648-1456-8c9f-1d26-83cd149fc2d3@gmail.com> <0c1d857c-6d17-6f91-6aa7-4af1a8391ec8@gmail.com> From: Aldy Hernandez Message-ID: Date: Tue, 28 Sep 2021 11:42: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: multipart/mixed; boundary="------------9F3969BC515C593FD62FE36C" Content-Language: en-US X-Spam-Status: No, score=-13.6 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, 28 Sep 2021 09:42:36 -0000 This is a multi-part message in MIME format. --------------9F3969BC515C593FD62FE36C Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 9/28/21 9:41 AM, Richard Biener wrote: > On Tue, Sep 28, 2021 at 8:29 AM Jeff Law via Gcc-patches > wrote: >> >> >> >> On 9/28/2021 12:17 AM, Aldy Hernandez wrote: >>> On Tue, Sep 28, 2021 at 3:46 AM Jeff Law 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 --------------9F3969BC515C593FD62FE36C Content-Type: text/x-patch; charset=UTF-8; name="p.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="p.patch" commit e0a5b35c8becda7bef37bc6eca1686ab2e762088 Author: Aldy Hernandez Date: Tue Sep 28 11:33:11 2021 +0200 Enable jump threading at -O1. My previous patch gating all jump threading by -fthread-jumps had the side effect of turning off DOM jump threading at -O1. This causes numerous -Wuninitialized false positives. This patch turns on jump threading at -O1 to minimize the disruption. gcc/ChangeLog: * cfgcleanup.c (pass_jump::execute): Check flag_expensive_optimizations. (pass_jump_after_combine::gate): Same. * doc/invoke.texi (-fthread-jumps): Enable for -O1. * opts.c (default_options_table): Enable -fthread-jumps at -O1. * tree-ssa-threadupdate.c (fwd_jt_path_registry::remove_jump_threads_including): Bail unless flag_thread_jumps. gcc/testsuite/ChangeLog: * gcc.dg/auto-init-uninit-1.c: Adjust. * gcc.dg/auto-init-uninit-15.c: Same. * gcc.dg/guality/example.c: Same. * gcc.dg/loop-8.c: Same. * gcc.dg/strlenopt-40.c: Same. * gcc.dg/tree-ssa/pr18133-2.c: Same. * gcc.dg/tree-ssa/pr18134.c: Same. * gcc.dg/uninit-1.c: Same. * gcc.dg/uninit-pr44547.c: Same. * gcc.dg/uninit-pr59970.c: Same. diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c index 7b1e1ba6e80..82fc505ff50 100644 --- a/gcc/cfgcleanup.c +++ b/gcc/cfgcleanup.c @@ -3239,7 +3239,8 @@ pass_jump::execute (function *) if (dump_file) dump_flow_info (dump_file, dump_flags); cleanup_cfg ((optimize ? CLEANUP_EXPENSIVE : 0) - | (flag_thread_jumps ? CLEANUP_THREADING : 0)); + | (flag_thread_jumps && flag_expensive_optimizations + ? CLEANUP_THREADING : 0)); return 0; } @@ -3274,7 +3275,10 @@ public: {} /* opt_pass methods: */ - virtual bool gate (function *) { return flag_thread_jumps; } + virtual bool gate (function *) + { + return flag_thread_jumps && flag_expensive_optimizations; + } virtual unsigned int execute (function *); }; // class pass_jump_after_combine diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index ba98eab68a5..6d9a107acd0 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -10781,7 +10781,7 @@ so, the first branch is redirected to either the destination of the second branch or a point immediately following it, depending on whether the condition is known to be true or false. -Enabled at levels @option{-O2}, @option{-O3}, @option{-Os}. +Enabled at levels @option{-O1}, @option{-O2}, @option{-O3}, @option{-Os}. @item -fsplit-wide-types @opindex fsplit-wide-types diff --git a/gcc/opts.c b/gcc/opts.c index 38b42db2a4f..6503980cd33 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -569,6 +569,7 @@ static const struct default_options default_options_table[] = { OPT_LEVELS_1_PLUS, OPT_freorder_blocks, NULL, 1 }, { OPT_LEVELS_1_PLUS, OPT_fshrink_wrap, NULL, 1 }, { OPT_LEVELS_1_PLUS, OPT_fsplit_wide_types, NULL, 1 }, + { OPT_LEVELS_1_PLUS, OPT_fthread_jumps, NULL, 1 }, { OPT_LEVELS_1_PLUS, OPT_ftree_builtin_call_dce, NULL, 1 }, { OPT_LEVELS_1_PLUS, OPT_ftree_ccp, NULL, 1 }, { OPT_LEVELS_1_PLUS, OPT_ftree_ch, NULL, 1 }, @@ -629,7 +630,6 @@ static const struct default_options default_options_table[] = #endif { OPT_LEVELS_2_PLUS, OPT_fstrict_aliasing, NULL, 1 }, { OPT_LEVELS_2_PLUS, OPT_fstore_merging, NULL, 1 }, - { OPT_LEVELS_2_PLUS, OPT_fthread_jumps, NULL, 1 }, { OPT_LEVELS_2_PLUS, OPT_ftree_pre, NULL, 1 }, { OPT_LEVELS_2_PLUS, OPT_ftree_switch_conversion, NULL, 1 }, { OPT_LEVELS_2_PLUS, OPT_ftree_tail_merge, NULL, 1 }, diff --git a/gcc/testsuite/gcc.dg/auto-init-uninit-1.c b/gcc/testsuite/gcc.dg/auto-init-uninit-1.c index ce8909623ab..502db591222 100644 --- a/gcc/testsuite/gcc.dg/auto-init-uninit-1.c +++ b/gcc/testsuite/gcc.dg/auto-init-uninit-1.c @@ -1,5 +1,5 @@ /* Spurious uninitialized variable warnings, case 1. Taken from cppfiles.c (merge_include_chains) */ /* { dg-do compile } */ -/* { dg-options "-O -Wuninitialized -fthread-jumps -ftrivial-auto-var-init=zero" } */ +/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=zero" } */ #include "uninit-1.c" diff --git a/gcc/testsuite/gcc.dg/auto-init-uninit-15.c b/gcc/testsuite/gcc.dg/auto-init-uninit-15.c index b8f6e2b57d5..121f0cff274 100644 --- a/gcc/testsuite/gcc.dg/auto-init-uninit-15.c +++ b/gcc/testsuite/gcc.dg/auto-init-uninit-15.c @@ -5,7 +5,7 @@ But it is of course ok if we warn in bar about uninitialized use of j. (not xfailed alternative) */ /* { dg-do compile } */ -/* { dg-options "-O1 -Wuninitialized -fthread-jumps -ftrivial-auto-var-init=zero" } */ +/* { dg-options "-O1 -Wuninitialized -ftrivial-auto-var-init=zero" } */ inline int foo (int i) diff --git a/gcc/testsuite/gcc.dg/guality/example.c b/gcc/testsuite/gcc.dg/guality/example.c index 37564e55cd1..32014e2b4c0 100644 --- a/gcc/testsuite/gcc.dg/guality/example.c +++ b/gcc/testsuite/gcc.dg/guality/example.c @@ -1,4 +1,5 @@ /* { dg-options "-g" } */ +/* { dg-do run { xfail { ! aarch64*-*-* } } } */ /* { dg-xfail-run-if "" aarch64*-*-* "*" { "-O[01g]" } } */ #define GUALITY_DONT_FORCE_LIVE_AFTER -1 diff --git a/gcc/testsuite/gcc.dg/loop-8.c b/gcc/testsuite/gcc.dg/loop-8.c index e5218eb4053..90ea1c45524 100644 --- a/gcc/testsuite/gcc.dg/loop-8.c +++ b/gcc/testsuite/gcc.dg/loop-8.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O1 -fthread-jumps -fdump-rtl-loop2_invariant" } */ +/* { dg-options "-O1 -fdump-rtl-loop2_invariant" } */ /* { dg-skip-if "unexpected IV" { "hppa*-*-* mips*-*-* visium-*-* powerpc*-*-* riscv*-*-* mmix-*-* vax-*-*" } } */ /* Load immediate on condition is available from z13 on and prevents moving the load out of the loop, so always run this test with -march=zEC12 that diff --git a/gcc/testsuite/gcc.dg/strlenopt-40.c b/gcc/testsuite/gcc.dg/strlenopt-40.c index 7b799104708..7a97ebb8fe5 100644 --- a/gcc/testsuite/gcc.dg/strlenopt-40.c +++ b/gcc/testsuite/gcc.dg/strlenopt-40.c @@ -1,7 +1,7 @@ /* PR tree-optimization/83671 - fix for false positive reported by -Wstringop-overflow does not work with inlining { dg-do compile } - { dg-options "-O1 -fthread-jumps -fdump-tree-optimized" } */ + { dg-options "-O1 -fdump-tree-optimized" } */ #include "strlenopt.h" diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr18133-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr18133-2.c index 1b409852189..8717640e327 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 -fthread-jumps -fdump-tree-optimized-blocks" } */ +/* { dg-options "-O1 -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 d7f5d241eb9..cd40ab2c162 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 -fthread-jumps -fdump-tree-optimized" } */ +/* { dg-options "-O1 -fdump-tree-optimized" } */ int foo (int a) { diff --git a/gcc/testsuite/gcc.dg/uninit-1.c b/gcc/testsuite/gcc.dg/uninit-1.c index 156d34ff732..060ec250ba7 100644 --- a/gcc/testsuite/gcc.dg/uninit-1.c +++ b/gcc/testsuite/gcc.dg/uninit-1.c @@ -1,7 +1,7 @@ /* Spurious uninitialized variable warnings, case 1. Taken from cppfiles.c (merge_include_chains) */ /* { dg-do compile } */ -/* { dg-options "-O -Wuninitialized -fthread-jumps" } */ +/* { dg-options "-O -Wuninitialized" } */ struct list { diff --git a/gcc/testsuite/gcc.dg/uninit-pr44547.c b/gcc/testsuite/gcc.dg/uninit-pr44547.c index f1c3b034d14..ee1035ad7b8 100644 --- a/gcc/testsuite/gcc.dg/uninit-pr44547.c +++ b/gcc/testsuite/gcc.dg/uninit-pr44547.c @@ -1,7 +1,7 @@ /* PR tree-optimization/44547 - -Wuninitialized reports false warning in nested switch statements { dg-do compile } - { dg-options "-O1 -Wall -fthread-jumps" } */ + { dg-options "-O1 -Wall" } */ __attribute__ ((noipa)) int test_O1 (int argc) { diff --git a/gcc/testsuite/gcc.dg/uninit-pr59970.c b/gcc/testsuite/gcc.dg/uninit-pr59970.c index d0c41b8480d..145af657a76 100644 --- a/gcc/testsuite/gcc.dg/uninit-pr59970.c +++ b/gcc/testsuite/gcc.dg/uninit-pr59970.c @@ -41,7 +41,7 @@ d_demangle_callback_O1 (const char *mangled) #pragma GCC pop_options -#pragma GCC optimize ("Og,thread-jumps") +#pragma GCC optimize ("Og") __attribute__ ((noipa)) int d_demangle_callback_Og (const char *mangled) diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c index 905dea2e6ca..dcabfdb30d2 100644 --- a/gcc/tree-ssa-threadupdate.c +++ b/gcc/tree-ssa-threadupdate.c @@ -2575,7 +2575,7 @@ valid_jump_thread_path (vec *path) void fwd_jt_path_registry::remove_jump_threads_including (edge_def *e) { - if (!m_paths.exists ()) + if (!m_paths.exists () || !flag_thread_jumps) return; edge *slot = m_removed_edges->find_slot (e, INSERT); --------------9F3969BC515C593FD62FE36C--