From 1cd151d5b9e1fd46590d5e4c3473a86ea9bbe043 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 10 Feb 2009 18:29:55 -0800 Subject: Simplify dead_stmtexpr_remover * staptree.h (update_visitor::require): Add a clearok parameter for optimizing traversers to signal that they're ready for NULL back. * elaborate.cxx (dead_stmtexpr_remover): Convert to an update_visitor. --- ChangeLog | 3 ++ elaborate.cxx | 91 ++++++++++++++++++++++++----------------------------------- staptree.cxx | 4 +-- staptree.h | 6 ++-- 4 files changed, 45 insertions(+), 59 deletions(-) diff --git a/ChangeLog b/ChangeLog index cf6bf61e..208c8150 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,7 +1,10 @@ 2009-02-10 Josh Stone + * staptree.h (update_visitor::require): Add a clearok parameter for + optimizing traversers to signal that they're ready for NULL back. * elaborate.cxx (dead_assignment_remover): Convert into an update_visitor and remove its now-redundant traversal methods. + * elaborate.cxx (dead_stmtexpr_remover): Convert to an update_visitor. 2009-02-05 Josh Stone diff --git a/elaborate.cxx b/elaborate.cxx index d4b07e8d..bc9c4e7d 100644 --- a/elaborate.cxx +++ b/elaborate.cxx @@ -2187,15 +2187,14 @@ void semantic_pass_opt3 (systemtap_session& s, bool& relaxed_p) // ------------------------------------------------------------------------ -struct dead_stmtexpr_remover: public traversing_visitor +struct dead_stmtexpr_remover: public update_visitor { systemtap_session& session; bool& relaxed_p; - statement** current_stmt; // pointer to current stmt* being iterated set focal_vars; // vars considered subject to side-effects dead_stmtexpr_remover(systemtap_session& s, bool& r): - session(s), relaxed_p(r), current_stmt(0) {} + session(s), relaxed_p(r) {} void visit_block (block *s); void visit_null_statement (null_statement *s); @@ -2214,7 +2213,8 @@ dead_stmtexpr_remover::visit_null_statement (null_statement *s) // easy! if (session.verbose>2) clog << "Eliding side-effect-free null statement " << *s->tok << endl; - *current_stmt = 0; + s = 0; + provide (s); } @@ -2224,13 +2224,11 @@ dead_stmtexpr_remover::visit_block (block *s) vector new_stmts; for (unsigned i=0; istatements.size(); i++ ) { - statement** last_stmt = current_stmt; - current_stmt = & s->statements[i]; - s->statements[i]->visit (this); - if (*current_stmt != 0) + statement* new_stmt = require (s->statements[i], true); + if (new_stmt != 0) { // flatten nested blocks into this one - block *b = dynamic_cast(*current_stmt); + block *b = dynamic_cast(new_stmt); if (b) { if (session.verbose>2) @@ -2240,42 +2238,32 @@ dead_stmtexpr_remover::visit_block (block *s) relaxed_p = false; } else - new_stmts.push_back (*current_stmt); + new_stmts.push_back (new_stmt); } - current_stmt = last_stmt; } if (new_stmts.size() == 0) { if (session.verbose>2) clog << "Eliding side-effect-free empty block " << *s->tok << endl; - *current_stmt = 0; + s = 0; } else if (new_stmts.size() == 1) { if (session.verbose>2) clog << "Eliding side-effect-free singleton block " << *s->tok << endl; - *current_stmt = new_stmts[0]; + provide (new_stmts[0]); + return; } else - { - s->statements = new_stmts; - } + s->statements = new_stmts; + provide (s); } void dead_stmtexpr_remover::visit_if_statement (if_statement *s) { - statement** last_stmt = current_stmt; - current_stmt = & s->thenblock; - s->thenblock->visit (this); - - if (s->elseblock) - { - current_stmt = & s->elseblock; - s->elseblock->visit (this); - // null *current_stmt is OK here. - } - current_stmt = last_stmt; + s->thenblock = require (s->thenblock, true); + s->elseblock = require (s->elseblock, true); if (s->thenblock == 0) { @@ -2290,7 +2278,7 @@ dead_stmtexpr_remover::visit_if_statement (if_statement *s) if (session.verbose>2) clog << "Eliding side-effect-free if statement " << *s->tok << endl; - *current_stmt = 0; // yeah, baby + s = 0; // yeah, baby } else { @@ -2301,7 +2289,8 @@ dead_stmtexpr_remover::visit_if_statement (if_statement *s) expr_statement *es = new expr_statement; es->value = s->condition; es->tok = es->value->tok; - *current_stmt = es; + provide (es); + return; } } else @@ -2320,31 +2309,27 @@ dead_stmtexpr_remover::visit_if_statement (if_statement *s) s->elseblock = 0; } } + provide (s); } void dead_stmtexpr_remover::visit_foreach_loop (foreach_loop *s) { - statement** last_stmt = current_stmt; - current_stmt = & s->block; - s->block->visit (this); - current_stmt = last_stmt; + s->block = require(s->block, true); if (s->block == 0) { if (session.verbose>2) clog << "Eliding side-effect-free foreach statement " << *s->tok << endl; - *current_stmt = 0; // yeah, baby + s = 0; // yeah, baby } + provide (s); } void dead_stmtexpr_remover::visit_for_loop (for_loop *s) { - statement** last_stmt = current_stmt; - current_stmt = & s->block; - s->block->visit (this); - current_stmt = last_stmt; + s->block = require(s->block, true); if (s->block == 0) { @@ -2358,14 +2343,16 @@ dead_stmtexpr_remover::visit_for_loop (for_loop *s) { if (session.verbose>2) clog << "Eliding side-effect-free for statement " << *s->tok << endl; - *current_stmt = 0; // yeah, baby - return; + s = 0; // yeah, baby + } + else + { + // Can't elide this whole statement; put a null in there. + s->block = new null_statement(); + s->block->tok = s->tok; } - - // Can't elide this whole statement; put a null in there. - s->block = new null_statement(); - s->block->tok = s->tok; } + provide (s); } @@ -2375,8 +2362,7 @@ dead_stmtexpr_remover::visit_expr_statement (expr_statement *s) { // Run a varuse query against the operand expression. If it has no // side-effects, replace the entire statement expression by a null - // statement. This replacement is done by overwriting the - // current_stmt pointer. + // statement with the provide() call. // // Unlike many other visitors, we do *not* traverse this outermost // one into the expression subtrees. There is no need - no @@ -2389,8 +2375,7 @@ dead_stmtexpr_remover::visit_expr_statement (expr_statement *s) varuse_collecting_visitor vut; s->value->visit (& vut); - if (vut.side_effect_free_wrt (focal_vars) && - *current_stmt == s) // we're not nested any deeper than expected + if (vut.side_effect_free_wrt (focal_vars)) { /* PR 1119: NB: this message is not a good idea here. It can name some arbitrary RHS expression of an assignment. @@ -2405,10 +2390,10 @@ dead_stmtexpr_remover::visit_expr_statement (expr_statement *s) // NB: this 0 pointer is invalid to leave around for any length of // time, but the parent parse tree objects above handle it. - * current_stmt = 0; - + s = 0; relaxed_p = false; } + provide (s); } @@ -2433,8 +2418,7 @@ void semantic_pass_opt4 (systemtap_session& s, bool& relaxed_p) duv.focal_vars.insert (p->locals.begin(), p->locals.end()); - duv.current_stmt = & p->body; - p->body->visit (& duv); + p->body = duv.require(p->body, true); if (p->body == 0) { if (! s.suppress_warnings) @@ -2459,8 +2443,7 @@ void semantic_pass_opt4 (systemtap_session& s, bool& relaxed_p) duv.focal_vars.insert (s.globals.begin(), s.globals.end()); - duv.current_stmt = & fn->body; - fn->body->visit (& duv); + fn->body = duv.require(fn->body, true); if (fn->body == 0) { if (! s.suppress_warnings) diff --git a/staptree.cxx b/staptree.cxx index 2b963bc3..9ffbaf09 100644 --- a/staptree.cxx +++ b/staptree.cxx @@ -2337,7 +2337,7 @@ update_visitor::visit_hist_op (hist_op* e) } template <> indexable* -update_visitor::require (indexable* src) +update_visitor::require (indexable* src, bool clearok) { indexable *dst = NULL; if (src != NULL) @@ -2351,7 +2351,7 @@ update_visitor::require (indexable* src) dst = require (array_src); else dst = require (hist_src); - assert (dst); + assert(clearok || dst); } return dst; } diff --git a/staptree.h b/staptree.h index 00809ed0..0cd0ee0d 100644 --- a/staptree.h +++ b/staptree.h @@ -815,7 +815,7 @@ struct throwing_visitor: public visitor struct update_visitor: public visitor { - template T require (T src) + template T require (T src, bool clearok=false) { T dst = NULL; if (src != NULL) @@ -824,7 +824,7 @@ struct update_visitor: public visitor assert(!targets.empty()); dst = static_cast(targets.top()); targets.pop(); - assert(dst); + assert(clearok || dst); } return dst; } @@ -874,7 +874,7 @@ private: }; template <> indexable* -update_visitor::require (indexable* src); +update_visitor::require (indexable* src, bool clearok); // A visitor which performs a deep copy of the root node it's applied // to. NB: It does not copy any of the variable or function -- cgit