This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] MEM_REF clobber handling fixes/improvements (PR c++/34949, PR c++/50243)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Richard Biener <rguenther at suse dot de>, Jason Merrill <jason at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Thu, 4 Apr 2013 20:15:32 +0200
- Subject: [PATCH] MEM_REF clobber handling fixes/improvements (PR c++/34949, PR c++/50243)
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
Hi!
The vt3.C testcase (from PR34949) ICEd, because sink_clobbers sunk a
MEM_REF[SSA_NAME] clobber from a landing pad which that SSA_NAME definition
dominated to an outer one which wasn't dominated by the definition.
As the ehcleanup nor ehdisp passes keep dominance info current and perform
changes that invalidate it, I can't unfortunately do cheaply a
dominated_by_p check, so the patch just throws away all MEM_REF[SSA_NAME]
clobbers if SSA_NAME isn't a default def which is valid everywhere. As
sink_clobbers is only done on otherwise empty bb's and typically the
clobbers are preceeded by some stores which are to be DSEd if unneeded
and after DSEing aren't really needed anymore, this doesn't seem to hurt
much.
The patch also improves optimize_clobbers, so that it only removes any
clobbers if the bb is actually empty (except for clobbers, resx, maybe debug
stmts or __builtin_stack_restore), that way needed clobbers are kept around
until they are used by DSE.
Also, MEM_REF[SSA_NAME] clobbers aren't useful very late in the optimization
pipeline, but could cause some SSA_NAMEs to be considered unnecessarily live
(especially if they are considered live across EH edges it is undesirable),
such clobbers are mainly useful during DSE1/DSE2, but at expansion are
completely ignored (unlike VAR_DECL clobbers, which are also used for the
stack layout decisions), so the patch removes all those MEM_REF[SSA_NAME]
clobbers shortly after dse2 (in fab pass).
Bootstrapped/regtested on x86_64-linux and i686-linux, on libstdc++ I saw
some code size reduction with the patch.
Ok for trunk?
2013-04-04 Jakub Jelinek <jakub@redhat.com>
PR c++/34949
PR c++/50243
* tree-eh.c (optimize_clobbers): Only remove clobbers if bb doesn't
contain anything but clobbers, at most one __builtin_stack_restore,
optionally debug stmts and final resx, and if it has at least one
incoming EH edge. Don't check for SSA_NAME on LHS of a clobber.
(sink_clobbers): Don't check for SSA_NAME on LHS of a clobber.
Instead of moving clobbers with MEM_REF LHS with SSA_NAME address
which isn't defaut definition, remove them.
(unsplit_eh, cleanup_empty_eh): Use single_{pred,succ}_{p,edge}
instead of EDGE_COUNT comparisons or EDGE_{PRED,SUCC}.
* tree-ssa-ccp.c (execute_fold_all_builtins): Remove clobbers
with MEM_REF LHS with SSA_NAME address.
* g++.dg/opt/vt3.C: New test.
* g++.dg/opt/vt4.C: New test.
--- gcc/tree-eh.c.jj 2013-03-26 10:03:55.000000000 +0100
+++ gcc/tree-eh.c 2013-04-04 13:44:27.718982776 +0200
@@ -3230,14 +3230,48 @@ static void
optimize_clobbers (basic_block bb)
{
gimple_stmt_iterator gsi = gsi_last_bb (bb);
+ bool any_clobbers = false;
+ bool seen_stack_restore = false;
+ edge_iterator ei;
+ edge e;
+
+ /* Only optimize anything if the bb contains at least one clobber,
+ ends with resx (checked by caller), optionally contains some
+ debug stmts or labels, or at most one __builtin_stack_restore
+ call, and has an incoming EH edge. */
for (gsi_prev (&gsi); !gsi_end_p (gsi); gsi_prev (&gsi))
{
gimple stmt = gsi_stmt (gsi);
if (is_gimple_debug (stmt))
continue;
- if (!gimple_clobber_p (stmt)
- || TREE_CODE (gimple_assign_lhs (stmt)) == SSA_NAME)
- return;
+ if (gimple_clobber_p (stmt))
+ {
+ any_clobbers = true;
+ continue;
+ }
+ if (!seen_stack_restore
+ && gimple_call_builtin_p (stmt, BUILT_IN_STACK_RESTORE))
+ {
+ seen_stack_restore = true;
+ continue;
+ }
+ if (gimple_code (stmt) == GIMPLE_LABEL)
+ break;
+ return;
+ }
+ if (!any_clobbers)
+ return;
+ FOR_EACH_EDGE (e, ei, bb->preds)
+ if (e->flags & EDGE_EH)
+ break;
+ if (e == NULL)
+ return;
+ gsi = gsi_last_bb (bb);
+ for (gsi_prev (&gsi); !gsi_end_p (gsi); gsi_prev (&gsi))
+ {
+ gimple stmt = gsi_stmt (gsi);
+ if (!gimple_clobber_p (stmt))
+ continue;
unlink_stmt_vdef (stmt);
gsi_remove (&gsi, true);
release_defs (stmt);
@@ -3278,8 +3312,7 @@ sink_clobbers (basic_block bb)
continue;
if (gimple_code (stmt) == GIMPLE_LABEL)
break;
- if (!gimple_clobber_p (stmt)
- || TREE_CODE (gimple_assign_lhs (stmt)) == SSA_NAME)
+ if (!gimple_clobber_p (stmt))
return 0;
any_clobbers = true;
}
@@ -3292,11 +3325,27 @@ sink_clobbers (basic_block bb)
for (gsi_prev (&gsi); !gsi_end_p (gsi); gsi_prev (&gsi))
{
gimple stmt = gsi_stmt (gsi);
+ tree lhs;
if (is_gimple_debug (stmt))
continue;
if (gimple_code (stmt) == GIMPLE_LABEL)
break;
unlink_stmt_vdef (stmt);
+ lhs = gimple_assign_lhs (stmt);
+ /* Unfortunately we don't have dominance info updated at this
+ point, so checking if
+ dominated_by_p (CDI_DOMINATORS, succbb,
+ gimple_bb (SSA_NAME_DEF_STMT (TREE_OPERAND (lhs, 0)))
+ would be too costly. Thus, avoid sinking any clobbers that
+ refer to non-(D) SSA_NAMEs. */
+ if (TREE_CODE (lhs) == MEM_REF
+ && TREE_CODE (TREE_OPERAND (lhs, 0)) == SSA_NAME
+ && !SSA_NAME_IS_DEFAULT_DEF (TREE_OPERAND (lhs, 0)))
+ {
+ gsi_remove (&gsi, true);
+ release_defs (stmt);
+ continue;
+ }
gsi_remove (&gsi, false);
/* Trigger the operand scanner to cause renaming for virtual
operands for this statement.
@@ -3737,10 +3786,10 @@ unsplit_eh (eh_landing_pad lp)
edge e_in, e_out;
/* Quickly check the edge counts on BB for singularity. */
- if (EDGE_COUNT (bb->preds) != 1 || EDGE_COUNT (bb->succs) != 1)
+ if (!single_pred_p (bb) || !single_succ_p (bb))
return false;
- e_in = EDGE_PRED (bb, 0);
- e_out = EDGE_SUCC (bb, 0);
+ e_in = single_pred_edge (bb);
+ e_out = single_succ_edge (bb);
/* Input edge must be EH and output edge must be normal. */
if ((e_in->flags & EDGE_EH) == 0 || (e_out->flags & EDGE_EH) != 0)
@@ -4142,7 +4191,7 @@ cleanup_empty_eh (eh_landing_pad lp)
e_out = NULL;
break;
case 1:
- e_out = EDGE_SUCC (bb, 0);
+ e_out = single_succ_edge (bb);
break;
default:
return false;
--- gcc/tree-ssa-ccp.c.jj 2013-02-20 18:40:49.000000000 +0100
+++ gcc/tree-ssa-ccp.c 2013-04-04 14:01:08.926335910 +0200
@@ -2396,6 +2396,21 @@ execute_fold_all_builtins (void)
if (gimple_code (stmt) != GIMPLE_CALL)
{
+ /* Remove all *ssaname_N ={v} {CLOBBER}; stmts,
+ after the last GIMPLE DSE they aren't needed and might
+ unnecessarily keep the SSA_NAMEs live. */
+ if (gimple_clobber_p (stmt))
+ {
+ tree lhs = gimple_assign_lhs (stmt);
+ if (TREE_CODE (lhs) == MEM_REF
+ && TREE_CODE (TREE_OPERAND (lhs, 0)) == SSA_NAME)
+ {
+ unlink_stmt_vdef (stmt);
+ gsi_remove (&i, true);
+ release_defs (stmt);
+ continue;
+ }
+ }
gsi_next (&i);
continue;
}
--- gcc/testsuite/g++.dg/opt/vt3.C.jj 2013-04-04 14:33:12.190484331 +0200
+++ gcc/testsuite/g++.dg/opt/vt3.C 2013-04-04 14:32:19.000000000 +0200
@@ -0,0 +1,43 @@
+// PR c++/34949
+// { dg-do compile }
+// { dg-options "-O3" }
+
+struct E {};
+struct A
+{
+ virtual void a (void *) = 0;
+};
+struct B
+{
+ virtual ~B () {};
+ unsigned int b1;
+ E **b2;
+ A *b3;
+};
+struct C : public B
+{
+ ~C ();
+};
+C::~C ()
+{
+ for (unsigned int i = 0; i < b1; i++)
+ b3->a (b2);
+}
+struct D
+{
+ ~D () {}
+ C d;
+};
+struct F { virtual ~F () {}; };
+struct G { void g (); };
+struct H : public F
+{
+ virtual ~H ();
+ D *h1;
+ G *h2;
+};
+H::~H ()
+{
+ h2->g ();
+ delete h1;
+}
--- gcc/testsuite/g++.dg/opt/vt4.C.jj 2013-04-04 14:33:35.684354321 +0200
+++ gcc/testsuite/g++.dg/opt/vt4.C 2013-04-04 14:33:44.245304179 +0200
@@ -0,0 +1,31 @@
+// PR c++/50243
+// { dg-do compile }
+// { dg-options "-O" }
+// { dg-final { scan-assembler-not "_ZTV.A" } }
+
+void foo ();
+
+struct A
+{
+ ~A () { }
+ virtual void a () = 0;
+ virtual void b () = 0;
+ virtual void c () = 0;
+};
+
+struct B : public A
+{
+ ~B () { foo (); }
+ void a () { foo (); }
+ void b () { foo (); }
+ void c () { delete this; }
+};
+
+void
+test ()
+{
+ A *y = new B ();
+ y->a ();
+ y->b ();
+ y->c ();
+}
Jakub