[patch] fix pr22550
Diego Novillo
dnovillo@redhat.com
Thu Jul 28 15:56:00 GMT 2005
On Tue, Jul 26, 2005 at 11:33:20AM -0600, Jeffrey A Law wrote:
> So the question becomes whether or not removing them in the CFG
> cleanup code rather than in the regular optimizers provides
> us with some benefit -- compile-time speedup, or possibly
> secondary optimization opportunities that would be missed if
> we killed these degenerate PHIs and merged blocks in a
> later optimization pass.
>
I experimented with both approaches and, contrary to what I had
expected, iterating inside CFG cleanup is the winner. Both
patches are attached.
In terms of code generation, there are absolutely no differences.
I got identical results in all the SPEC testers (x86, x86-64 and
ppc64).
However, iterating over CFG cleanup gives us a nice compile time
reduction in DOM which sometimes translates to a minor overall
time reduction:
DOM time Before After % change
--------------------------------------
cc1files 26.64 16.53 -38.0%
DLV 12.75 8.88 -30.4%
MICO 6.24 5.44 -12.8%
TRAMP3D 3.60 2.99 -16.9%
SPEC2000 14.68 10.59 -27.9%
The reason for this reduction is that CFG cleanup now leaves
fewer opportunities for DOM to work with. I've seen some garbage
collection reduction as well. The straightforward
transformations done during CFG cleanup are saving us from the
heavier processing done during DOM.
In fact, looking at the cleanup code, it would not be hard to
teach it to flag an SSA update if it needed to. This would
harden CFG cleanup from messing up the SSA form. There was a PR
for it which Andrew had fixed by preventing CFG cleanup from
propagating constants. Andrew, remember the number? It may be
worth exploring that route.
I plan to go with the iterative CFG cleanup, but I'd like to hear
opinions first since this is a different conclusion to what I had
in mind earlier this week.
-------------- next part --------------
2005-07-26 Diego Novillo <dnovillo@redhat.com>
* tree-cfg.c (tree_can_merge_blocks_p): Return false if B has
PHI nodes.
(tree_merge_blocks): Do not try to remove PHI nodes from B.
testsuite/ChangeLog
* g++.dg/tree-ssa/pr22550.C: New test.
Index: tree-cfg.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-cfg.c,v
retrieving revision 2.211
diff -d -u -p -r2.211 tree-cfg.c
--- tree-cfg.c 12 Jul 2005 13:20:28 -0000 2.211
+++ tree-cfg.c 26 Jul 2005 20:34:03 -0000
@@ -1148,7 +1148,6 @@ tree_can_merge_blocks_p (basic_block a,
{
tree stmt;
block_stmt_iterator bsi;
- tree phi;
if (!single_succ_p (a))
return false;
@@ -1176,19 +1175,11 @@ tree_can_merge_blocks_p (basic_block a,
&& DECL_NONLOCAL (LABEL_EXPR_LABEL (stmt)))
return false;
- /* It must be possible to eliminate all phi nodes in B. If ssa form
- is not up-to-date, we cannot eliminate any phis. */
- phi = phi_nodes (b);
- if (phi)
- {
- if (need_ssa_update_p ())
- return false;
-
- for (; phi; phi = PHI_CHAIN (phi))
- if (!is_gimple_reg (PHI_RESULT (phi))
- && !may_propagate_copy (PHI_RESULT (phi), PHI_ARG_DEF (phi, 0)))
- return false;
- }
+ /* If B has PHI nodes, it cannot be merged. Copy and constant
+ propagation will later remove PHI nodes that are nothing but
+ copies or constants. */
+ if (phi_nodes (b))
+ return false;
/* Do not remove user labels. */
for (bsi = bsi_start (b); !bsi_end_p (bsi); bsi_next (&bsi))
@@ -1285,40 +1276,10 @@ tree_merge_blocks (basic_block a, basic_
{
block_stmt_iterator bsi;
tree_stmt_iterator last;
- tree phi;
if (dump_file)
fprintf (dump_file, "Merging blocks %d and %d\n", a->index, b->index);
- /* Remove the phi nodes. */
- bsi = bsi_last (a);
- for (phi = phi_nodes (b); phi; phi = phi_nodes (b))
- {
- tree def = PHI_RESULT (phi), use = PHI_ARG_DEF (phi, 0);
- tree copy;
-
- if (!may_propagate_copy (def, use)
- /* Propagating pointers might cause the set of vops for statements
- to be changed, and thus require ssa form update. */
- || (is_gimple_reg (def)
- && POINTER_TYPE_P (TREE_TYPE (def))))
- {
- gcc_assert (is_gimple_reg (def));
-
- /* Note that just emitting the copies is fine -- there is no problem
- with ordering of phi nodes. This is because A is the single
- predecessor of B, therefore results of the phi nodes cannot
- appear as arguments of the phi nodes. */
- copy = build2 (MODIFY_EXPR, void_type_node, def, use);
- bsi_insert_after (&bsi, copy, BSI_NEW_STMT);
- SET_PHI_RESULT (phi, NULL_TREE);
- SSA_NAME_DEF_STMT (def) = copy;
- }
- else
- replace_uses_by (def, use);
- remove_phi_node (phi, NULL);
- }
-
/* Ensure that B follows A. */
move_block_after (b, a);
Index: testsuite/g++.dg/tree-ssa/pr22550.C
===================================================================
RCS file: testsuite/g++.dg/tree-ssa/pr22550.C
diff -N testsuite/g++.dg/tree-ssa/pr22550.C
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/g++.dg/tree-ssa/pr22550.C 26 Jul 2005 20:34:04 -0000
@@ -0,0 +1,28 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+class X {
+public:
+ int mfunc1 () {
+ return 1;
+ }
+ int mfunc2 () {
+ return 2;
+ }
+ X (int a, int b) { }
+};
+
+typedef int (X::*memfunc_p_t) ();
+
+memfunc_p_t mf_arr[2] = { &X::mfunc1, &X::mfunc2 };
+
+int
+main ()
+{
+ // Get pntr to the array of pointers to member-funcs
+ memfunc_p_t (*mf_arr_p)[2] = &mf_arr;
+ // Compare indirect against direct access to an array element
+ if ((*mf_arr_p)[0] != mf_arr[0])
+ return 1;
+ return 0;
+}
-------------- next part --------------
2005-07-26 Diego Novillo <dnovillo@redhat.com>
* tree-cfgcleanup.c (cleanup_tree_cfg_1): Extract from ...
(cleanup_tree_cfg): ... here.
Call cleanup_tree_cfg_1 until there are no more cleanups to
do.
testsuite/ChangeLog
* g++.dg/tree-ssa/pr22550.C: New test.
Index: tree-cfgcleanup.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-cfgcleanup.c,v
retrieving revision 2.3
diff -d -u -p -r2.3 tree-cfgcleanup.c
--- tree-cfgcleanup.c 18 Jul 2005 23:20:09 -0000 2.3
+++ tree-cfgcleanup.c 26 Jul 2005 13:08:52 -0000
@@ -489,14 +489,12 @@ cleanup_forwarder_blocks (void)
return changed;
}
-/* Remove unreachable blocks and other miscellaneous clean up work. */
+/* Do one round of CFG cleanup. */
-bool
-cleanup_tree_cfg (void)
+static bool
+cleanup_tree_cfg_1 (void)
{
- bool retval = false;
-
- timevar_push (TV_TREE_CLEANUP_CFG);
+ bool retval;
retval = cleanup_control_flow ();
retval |= delete_unreachable_blocks ();
@@ -516,6 +514,28 @@ cleanup_tree_cfg (void)
end_recording_case_labels ();
}
+ /* Merging the blocks may create new opportunities for folding
+ conditional branches (due to the elimination of single-valued PHI
+ nodes). */
+ retval |= merge_seq_blocks ();
+
+ return retval;
+}
+
+
+/* Remove unreachable blocks and other miscellaneous clean up work. */
+
+bool
+cleanup_tree_cfg (void)
+{
+ bool retval;
+ int i;
+
+ timevar_push (TV_TREE_CLEANUP_CFG);
+
+ for (retval = true, i = 0; i < 3 && retval; i++)
+ retval = cleanup_tree_cfg_1 ();
+
#ifdef ENABLE_CHECKING
if (retval)
{
@@ -526,16 +546,14 @@ cleanup_tree_cfg (void)
}
#endif
- /* Merging the blocks creates no new opportunities for the other
- optimizations, so do it here. */
- retval |= merge_seq_blocks ();
-
compact_blocks ();
#ifdef ENABLE_CHECKING
verify_flow_info ();
#endif
+
timevar_pop (TV_TREE_CLEANUP_CFG);
+
return retval;
}
Index: testsuite/g++.dg/tree-ssa/pr22550.C
===================================================================
RCS file: testsuite/g++.dg/tree-ssa/pr22550.C
diff -N testsuite/g++.dg/tree-ssa/pr22550.C
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/g++.dg/tree-ssa/pr22550.C 26 Jul 2005 13:08:53 -0000
@@ -0,0 +1,28 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+class X {
+public:
+ int mfunc1 () {
+ return 1;
+ }
+ int mfunc2 () {
+ return 2;
+ }
+ X (int a, int b) { }
+};
+
+typedef int (X::*memfunc_p_t) ();
+
+memfunc_p_t mf_arr[2] = { &X::mfunc1, &X::mfunc2 };
+
+int
+main ()
+{
+ // Get pntr to the array of pointers to member-funcs
+ memfunc_p_t (*mf_arr_p)[2] = &mf_arr;
+ // Compare indirect against direct access to an array element
+ if ((*mf_arr_p)[0] != mf_arr[0])
+ return 1;
+ return 0;
+}
More information about the Gcc-patches
mailing list