Message ID | 20130404181532.GU4201@tucnak.redhat.com |
---|---|
State | New |
Headers | show |
On Thu, 4 Apr 2013, Jakub Jelinek wrote: > 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? Ok. Thanks, Richard. > 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 > >
--- 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 (); +}