Hi!
The split_critical_edges() function has multiple uses and it seems, a
portion of its code was added to work only when called from tree-ssa-pre
but right now it is executed regardless of the caller.
The below patch survives bootstrap and regression testing on
x86_64-pc-linux-gnu. Does it make sense?
>From d6d843653b82f277e780f19f2d2b3cc3125db8b5 Mon Sep 17 00:00:00 2001
From: Vladislav Ivanishin <v...@ispras.ru>
Date: Wed, 8 May 2019 20:29:34 +0300
Subject: [PATCH] Don't introduce useless edge splits unless in pre
gcc/Changelog:
* tree-cfg.h (split_critical_edges): Add in_pre_p parameter with default
value false to declaration.
* tree-cfg.c (split_critical_edges): Don't split non-critical edges if
in_pre_p is false. Fix whitespace.
* tree-ssa-pre.c (pass_pre::execute): Pass in_pre_p = true to
split_critical_edges.
---
gcc/tree-cfg.c | 14 ++++++++------
gcc/tree-cfg.h | 2 +-
gcc/tree-ssa-pre.c | 2 +-
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 587408150fb..11683e63cd1 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -8870,10 +8870,11 @@ struct cfg_hooks gimple_cfg_hooks = {
};
-/* Split all critical edges. */
+/* Split all critical edges. Split some extra edges to help PRE if IN_PRE_P is
+ true. */
unsigned int
-split_critical_edges (void)
+split_critical_edges (bool in_pre_p /* = false */)
{
basic_block bb;
edge e;
@@ -8896,11 +8897,12 @@ split_critical_edges (void)
end by control flow statements, such as RESX.
Go ahead and split them too. This matches the logic in
gimple_find_edge_insert_loc. */
- else if ((!single_pred_p (e->dest)
- || !gimple_seq_empty_p (phi_nodes (e->dest))
- || e->dest == EXIT_BLOCK_PTR_FOR_FN (cfun))
+ else if (in_pre_p
+ && (!single_pred_p (e->dest)
+ || !gimple_seq_empty_p (phi_nodes (e->dest))
+ || e->dest == EXIT_BLOCK_PTR_FOR_FN (cfun))
&& e->src != ENTRY_BLOCK_PTR_FOR_FN (cfun)
- && !(e->flags & EDGE_ABNORMAL))
+ && !(e->flags & EDGE_ABNORMAL))
{
gimple_stmt_iterator gsi;
diff --git a/gcc/tree-cfg.h b/gcc/tree-cfg.h
index 212f5ff5919..3fbf983b36a 100644
--- a/gcc/tree-cfg.h
+++ b/gcc/tree-cfg.h
@@ -105,7 +105,7 @@ extern void extract_true_false_edges_from_block (basic_block, edge *, edge *);
extern tree find_case_label_for_value (const gswitch *switch_stmt, tree val);
extern edge find_taken_edge_switch_expr (const gswitch *switch_stmt, tree val);
extern unsigned int execute_fixup_cfg (void);
-extern unsigned int split_critical_edges (void);
+extern unsigned int split_critical_edges (bool in_pre_p = false);
extern basic_block insert_cond_bb (basic_block, gimple *, gimple *,
profile_probability);
extern bool gimple_find_sub_bbs (gimple_seq, gimple_stmt_iterator *);
diff --git a/gcc/tree-ssa-pre.c b/gcc/tree-ssa-pre.c
index 09335faa6a9..2c3645b5301 100644
--- a/gcc/tree-ssa-pre.c
+++ b/gcc/tree-ssa-pre.c
@@ -4195,7 +4195,7 @@ pass_pre::execute (function *fun)
/* This has to happen before VN runs because
loop_optimizer_init may create new phis, etc. */
loop_optimizer_init (LOOPS_NORMAL);
- split_critical_edges ();
+ split_critical_edges (/*in_pre_p=*/true);
scev_initialize ();
calculate_dominance_info (CDI_DOMINATORS);
--
2.21.0
If it does, then it uncovers what I think might be a latent bug in the
late uninit pass.
Without the patch, the crited pass inserts an empty basic block that
almost magically prevents the uninit pass from reporting a false
positive.
$ gcc -c -fdump-tree-all -fgimple -O -Wuninitialized gimple-fp-hfs.c
int __GIMPLE (ssa,startwith("uninit1"))
probe_hfsplus ()
{
int D_1913;
unsigned int ext_block_start;
int ext;
unsigned int _1;
int _14;
__BB(2):
ext_7 = 0;
goto __BB4;
__BB(3):
ext_block_start_11 = 1u;
_1 = 0u;
ext_13 = ext_4 + 1;
goto __BB4;
__BB(4,loop_header(1)):
ext_block_start_2 = __PHI (__BB2: ext_block_start_8(D), __BB3:
ext_block_start_11);
ext_4 = __PHI (__BB2: 0, __BB3: ext_13);
if (ext_4 <= 7)
goto __BB3;
else
goto __BB5;
__BB(5):
ext_block_start_3 = __PHI (__BB4: ext_block_start_2);
_14 = (int) ext_block_start_3;
return _14;
}
(no warning)
--- gimple-fp-hfs.c.170t.veclower21
+++ gimple-fp-hfs.c.194t.crited1
@@ -24,10 +24,12 @@
if (ext_4 <= 7)
goto <bb 3>; [INV]
else
- goto <bb 5>; [INV]
+ goto <bb 6>; [INV]
+
+ <bb 6> :
<bb 5> :
- # ext_block_start_3 = PHI <ext_block_start_2(4)>
+ # ext_block_start_3 = PHI <ext_block_start_2(6)>
_14 = (int) ext_block_start_3;
return _14;
And with the patch, the useless bb 6 is not inserted and the following
is produced:
gimple-fp-hfs.c: In function 'probe_hfsplus':
gimple-fp-hfs.c:5:16: warning: 'ext_block_start' may be used uninitialized in
this function [-Wmaybe-uninitialized]
5 | unsigned int ext_block_start;
| ^~~~~~~~~~~~~~~
If this patch is OK, I'll try to fix the uninit pass next.
--
Vlad
