* [PATCH] Control all jump threading passes with -fjump-threads.
@ 2021-09-27 15:00 Aldy Hernandez
2021-09-28 1:46 ` Jeff Law
0 siblings, 1 reply; 7+ messages in thread
From: Aldy Hernandez @ 2021-09-27 15:00 UTC (permalink / raw)
To: GCC patches
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(-)
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 ();
--
2.31.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Control all jump threading passes with -fjump-threads.
2021-09-27 15:00 [PATCH] Control all jump threading passes with -fjump-threads Aldy Hernandez
@ 2021-09-28 1:46 ` Jeff Law
2021-09-28 6:17 ` Aldy Hernandez
0 siblings, 1 reply; 7+ messages in thread
From: Jeff Law @ 2021-09-28 1:46 UTC (permalink / raw)
To: Aldy Hernandez, GCC patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Control all jump threading passes with -fjump-threads.
2021-09-28 1:46 ` Jeff Law
@ 2021-09-28 6:17 ` Aldy Hernandez
2021-09-28 6:28 ` Jeff Law
0 siblings, 1 reply; 7+ messages in thread
From: Aldy Hernandez @ 2021-09-28 6:17 UTC (permalink / raw)
To: Jeff Law; +Cc: GCC patches, Andrew MacLeod
[-- Attachment #1: Type: text/plain, Size: 1674 bytes --]
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
[-- Attachment #2: f.patch --]
[-- Type: text/x-patch, Size: 8317 bytes --]
commit 94591b662aa025a39681dce6bfc889cbe3d1d251
Author: Aldy Hernandez <aldyh@redhat.com>
Date: Mon Sep 27 16:41:01 2021 +0200
Control all jump threading passes with -fjump-threads.
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.
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/ChangeLog:
* gcc.dg/auto-init-uninit-1.c: Add -fthread-jumps.
* 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/testsuite/gcc.dg/auto-init-uninit-1.c b/gcc/testsuite/gcc.dg/auto-init-uninit-1.c
index 502db591222..ce8909623ab 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 -ftrivial-auto-var-init=zero" } */
+/* { dg-options "-O -Wuninitialized -fthread-jumps -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 121f0cff274..b8f6e2b57d5 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 -ftrivial-auto-var-init=zero" } */
+/* { dg-options "-O1 -Wuninitialized -fthread-jumps -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 6f1c017a253..37564e55cd1 100644
--- a/gcc/testsuite/gcc.dg/guality/example.c
+++ b/gcc/testsuite/gcc.dg/guality/example.c
@@ -1,4 +1,3 @@
-/* { dg-do run { xfail { ! aarch64*-*-* } } } */
/* { dg-options "-g" } */
/* { dg-xfail-run-if "" aarch64*-*-* "*" { "-O[01g]" } } */
diff --git a/gcc/testsuite/gcc.dg/loop-8.c b/gcc/testsuite/gcc.dg/loop-8.c
index 90ea1c45524..e5218eb4053 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 -fdump-rtl-loop2_invariant" } */
+/* { dg-options "-O1 -fthread-jumps -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 7a97ebb8fe5..7b799104708 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 -fdump-tree-optimized" } */
+ { dg-options "-O1 -fthread-jumps -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 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/testsuite/gcc.dg/uninit-1.c b/gcc/testsuite/gcc.dg/uninit-1.c
index 060ec250ba7..156d34ff732 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" } */
+/* { dg-options "-O -Wuninitialized -fthread-jumps" } */
struct list
{
diff --git a/gcc/testsuite/gcc.dg/uninit-pr44547.c b/gcc/testsuite/gcc.dg/uninit-pr44547.c
index ee1035ad7b8..f1c3b034d14 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" } */
+ { dg-options "-O1 -Wall -fthread-jumps" } */
__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 145af657a76..d0c41b8480d 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")
+#pragma GCC optimize ("Og,thread-jumps")
__attribute__ ((noipa)) int
d_demangle_callback_Og (const char *mangled)
diff --git a/gcc/tree-ssa-threadbackward.c b/gcc/tree-ssa-threadbackward.c
index e6819fe148c..28c7ef8c872 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 0b59cb4845c..a63a9764ff8 100644
--- a/gcc/tree-ssa-threadedge.c
+++ b/gcc/tree-ssa-threadedge.c
@@ -1196,6 +1196,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 ();
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Control all jump threading passes with -fjump-threads.
2021-09-28 6:17 ` Aldy Hernandez
@ 2021-09-28 6:28 ` Jeff Law
2021-09-28 7:41 ` Richard Biener
0 siblings, 1 reply; 7+ messages in thread
From: Jeff Law @ 2021-09-28 6:28 UTC (permalink / raw)
To: Aldy Hernandez; +Cc: GCC patches, Andrew MacLeod
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Control all jump threading passes with -fjump-threads.
2021-09-28 6:28 ` Jeff Law
@ 2021-09-28 7:41 ` Richard Biener
2021-09-28 9:42 ` Aldy Hernandez
0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2021-09-28 7:41 UTC (permalink / raw)
To: Jeff Law; +Cc: Aldy Hernandez, GCC patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Control all jump threading passes with -fjump-threads.
2021-09-28 7:41 ` Richard Biener
@ 2021-09-28 9:42 ` Aldy Hernandez
2021-09-28 10:54 ` Richard Biener
0 siblings, 1 reply; 7+ messages in thread
From: Aldy Hernandez @ 2021-09-28 9:42 UTC (permalink / raw)
To: Richard Biener, Jeff Law; +Cc: GCC patches
[-- Attachment #1: Type: text/plain, Size: 2679 bytes --]
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
[-- Attachment #2: p.patch --]
[-- Type: text/x-patch, Size: 9157 bytes --]
commit e0a5b35c8becda7bef37bc6eca1686ab2e762088
Author: Aldy Hernandez <aldyh@redhat.com>
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<jump_thread_edge *> *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);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Control all jump threading passes with -fjump-threads.
2021-09-28 9:42 ` Aldy Hernandez
@ 2021-09-28 10:54 ` Richard Biener
0 siblings, 0 replies; 7+ messages in thread
From: Richard Biener @ 2021-09-28 10:54 UTC (permalink / raw)
To: Aldy Hernandez; +Cc: Jeff Law, GCC patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-09-28 10:55 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-27 15:00 [PATCH] Control all jump threading passes with -fjump-threads Aldy Hernandez
2021-09-28 1:46 ` Jeff Law
2021-09-28 6:17 ` Aldy Hernandez
2021-09-28 6:28 ` Jeff Law
2021-09-28 7:41 ` Richard Biener
2021-09-28 9:42 ` Aldy Hernandez
2021-09-28 10:54 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).