Hello,
On Tue, 17 Sep 2013, Jeff Law wrote:
> I put the attached patch through a bootstrap and regression test cycle on
> x86_64-unknown-linux-gnu. As expected no regressions. Installed onto the
> trunk.
>
> Thanks Trevor!
You missed one comment style nit, and some ChangeLog entry nits.
I'm irritated by the member name uglification (e.g. equiv_stack_ with
trailing underscore). I know that's a certain style to mark private
members, but I think it's a bad style (like prefixing variable names with
their type), and before it sets a precedent in GCCs c++ coding style I'd
like this to be changed, like in the below.
I'd also like us to not use member privatization in our classes, but
that's not in the patch, but if we could agree on that it would be nice.
Regstrapped on x86-64-linux, okay?
Ciao,
Michael.
* domwalk.h (dom_walker::dom_direction_): Rename to dom_direction.
* domwalk.c (dom_walker::walk): Adjust.
* graphite-sese-to-poly.c (sese_dom_walker::region_): Rename to
region.
(sese_dom_walker::sese_dom_walker, sese_dom_walker::~sese_dom_walker,
sese_dom_walker::before_dom_children,
sese_dom_walker::after_dom_children): Adjust.
* tree-into-ssa.c (mark_def_dom_walker::kills_): Rename to
kills.
(mark_def_dom_walker::mark_def_dom_walker,
mark_def_dom_walker::before_dom_children): Adjust.
* tree-ssa-dom.c (dom_opt_dom_walker::dummy_cond_): Rename to
dummy_cond.
(dom_opt_dom_walker::thread_across_edge): Adjust.
* tree-ssa-loop-im.c (move_computations_dom_walker::todo_): Rename
to todo.
(move_computations_dom_walker::before_dom,
move_computations): Adjust.
* tree-ssa-phiopt.c (nontrapping_dom_walker::nontrapping_): Rename
to nontrapping.
(nontrapping_dom_walker::before_dom_child): Adjust.
* tree-ssa-uncprop.c (uncprop_dom_walker::equiv_stack_): Rename
to equiv_stack.
(uncprop_dom_walker::uncprop_dom_walker,
uncprop_dom_walker::~uncprop_dom_walker,
uncprop_dom_walker::after_dom_children): Adjust.
Index: domwalk.h
===================================================================
--- domwalk.h (revision 202698)
+++ domwalk.h (working copy)
@@ -21,16 +21,14 @@ along with GCC; see the file COPYING3.
#ifndef GCC_DOM_WALK_H
#define GCC_DOM_WALK_H
-/**
- * This is the main class for the dominator walker. It is expected that
- * consumers will have a custom class inheriting from it, which will over ride
- * at least one of before_dom_children and after_dom_children to implement the
- * custom behavior.
- */
+/* This is the main class for the dominator walker. It is expected that
+ consumers will have a custom class inheriting from it, which will over ride
+ at least one of before_dom_children and after_dom_children to implement the
+ custom behavior. */
class dom_walker
{
public:
- dom_walker (cdi_direction direction) : dom_direction_ (direction) {}
+ dom_walker (cdi_direction direction) : dom_direction (direction) {}
/* Walk the dominator tree. */
void walk (basic_block);
@@ -46,7 +44,7 @@ private:
if it is set to CDI_DOMINATORS, then we walk the dominator tree,
if it is set to CDI_POST_DOMINATORS, then we walk the post
dominator tree. */
- const ENUM_BITFIELD (cdi_direction) dom_direction_ : 2;
+ const ENUM_BITFIELD (cdi_direction) dom_direction : 2;
};
#endif
Index: domwalk.c
===================================================================
--- domwalk.c (revision 202698)
+++ domwalk.c (working copy)
@@ -154,7 +154,7 @@ dom_walker::walk (basic_block bb)
int sp = 0;
int *postorder, postorder_num;
- if (dom_direction_ == CDI_DOMINATORS)
+ if (dom_direction == CDI_DOMINATORS)
{
postorder = XNEWVEC (int, n_basic_blocks);
postorder_num = inverted_post_order_compute (postorder);
@@ -181,10 +181,10 @@ dom_walker::walk (basic_block bb)
worklist[sp++] = NULL;
int saved_sp = sp;
- for (dest = first_dom_son (dom_direction_, bb);
- dest; dest = next_dom_son (dom_direction_, dest))
+ for (dest = first_dom_son (dom_direction, bb);
+ dest; dest = next_dom_son (dom_direction, dest))
worklist[sp++] = dest;
- if (dom_direction_ == CDI_DOMINATORS)
+ if (dom_direction == CDI_DOMINATORS)
switch (sp - saved_sp)
{
case 0:
@@ -210,7 +210,7 @@ dom_walker::walk (basic_block bb)
else
break;
}
- if (dom_direction_ == CDI_DOMINATORS)
+ if (dom_direction == CDI_DOMINATORS)
{
free (bb_postorder);
bb_postorder = NULL;
Index: graphite-sese-to-poly.c
===================================================================
--- graphite-sese-to-poly.c (revision 202698)
+++ graphite-sese-to-poly.c (working copy)
@@ -1226,21 +1226,21 @@ public:
virtual void after_dom_children (basic_block);
private:
- vec<gimple> conditions_, cases_;
- sese region_;
+ vec<gimple> conditions, cases;
+ sese region;
};
sese_dom_walker::sese_dom_walker (cdi_direction direction, sese region)
- : dom_walker (direction), region_ (region)
+ : dom_walker (direction), region (region)
{
- conditions_.create (3);
- cases_.create (3);
+ conditions.create (3);
+ cases.create (3);
}
sese_dom_walker::~sese_dom_walker ()
{
- conditions_.release ();
- cases_.release ();
+ conditions.release ();
+ cases.release ();
}
/* Call-back for dom_walk executed before visiting the dominated
@@ -1252,7 +1252,7 @@ sese_dom_walker::before_dom_children (ba
gimple_bb_p gbb;
gimple stmt;
- if (!bb_in_sese_p (bb, region_))
+ if (!bb_in_sese_p (bb, region))
return;
stmt = single_pred_cond_non_loop_exit (bb);
@@ -1261,20 +1261,20 @@ sese_dom_walker::before_dom_children (ba
{
edge e = single_pred_edge (bb);
- conditions_.safe_push (stmt);
+ conditions.safe_push (stmt);
if (e->flags & EDGE_TRUE_VALUE)
- cases_.safe_push (stmt);
+ cases.safe_push (stmt);
else
- cases_.safe_push (NULL);
+ cases.safe_push (NULL);
}
gbb = gbb_from_bb (bb);
if (gbb)
{
- GBB_CONDITIONS (gbb) = conditions_.copy ();
- GBB_CONDITION_CASES (gbb) = cases_.copy ();
+ GBB_CONDITIONS (gbb) = conditions.copy ();
+ GBB_CONDITION_CASES (gbb) = cases.copy ();
}
}
@@ -1284,13 +1284,13 @@ sese_dom_walker::before_dom_children (ba
void
sese_dom_walker::after_dom_children (basic_block bb)
{
- if (!bb_in_sese_p (bb, region_))
+ if (!bb_in_sese_p (bb, region))
return;
if (single_pred_cond_non_loop_exit (bb))
{
- conditions_.pop ();
- cases_.pop ();
+ conditions.pop ();
+ cases.pop ();
}
}
Index: tree-into-ssa.c
===================================================================
--- tree-into-ssa.c (revision 202698)
+++ tree-into-ssa.c (working copy)
@@ -2075,7 +2075,8 @@ rewrite_update_phi_arguments (basic_bloc
class rewrite_update_dom_walker : public dom_walker
{
public:
- rewrite_update_dom_walker (cdi_direction direction) : dom_walker (direction)
{}
+ rewrite_update_dom_walker (cdi_direction direction) : dom_walker (direction)
+ {}
virtual void before_dom_children (basic_block);
virtual void after_dom_children (basic_block);
@@ -2235,17 +2236,17 @@ private:
/* Notice that this bitmap is indexed using variable UIDs, so it must be
large enough to accommodate all the variables referenced in the
function, not just the ones we are renaming. */
- bitmap kills_;
+ bitmap kills;
};
mark_def_dom_walker::mark_def_dom_walker (cdi_direction direction)
- : dom_walker (direction), kills_ (BITMAP_ALLOC (NULL))
+ : dom_walker (direction), kills (BITMAP_ALLOC (NULL))
{
}
mark_def_dom_walker::~mark_def_dom_walker ()
{
- BITMAP_FREE (kills_);
+ BITMAP_FREE (kills);
}
/* Block processing routine for mark_def_sites. Clear the KILLS bitmap
@@ -2256,9 +2257,9 @@ mark_def_dom_walker::before_dom_children
{
gimple_stmt_iterator gsi;
- bitmap_clear (kills_);
+ bitmap_clear (kills);
for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
- mark_def_sites (bb, gsi_stmt (gsi), kills_);
+ mark_def_sites (bb, gsi_stmt (gsi), kills);
}
/* Initialize internal data needed during renaming. */
Index: tree-ssa-dom.c
===================================================================
--- tree-ssa-dom.c (revision 202698)
+++ tree-ssa-dom.c (working copy)
@@ -774,7 +774,7 @@ class dom_opt_dom_walker : public dom_wa
{
public:
dom_opt_dom_walker (cdi_direction direction)
- : dom_walker (direction), dummy_cond_ (NULL) {}
+ : dom_walker (direction), dummy_cond (NULL) {}
virtual void before_dom_children (basic_block);
virtual void after_dom_children (basic_block);
@@ -782,7 +782,7 @@ public:
private:
void thread_across_edge (edge);
- gimple dummy_cond_;
+ gimple dummy_cond;
};
/* Jump threading, redundancy elimination and const/copy propagation.
@@ -1077,13 +1077,13 @@ simplify_stmt_for_jump_threading (gimple
void
dom_opt_dom_walker::thread_across_edge (edge e)
{
- if (! dummy_cond_)
- dummy_cond_ =
+ if (! dummy_cond)
+ dummy_cond =
gimple_build_cond (NE_EXPR,
integer_zero_node, integer_zero_node,
NULL, NULL);
- ::thread_across_edge (dummy_cond_, e, false,
+ ::thread_across_edge (dummy_cond, e, false,
&const_and_copies_stack,
simplify_stmt_for_jump_threading);
}
Index: tree-ssa-loop-im.c
===================================================================
--- tree-ssa-loop-im.c (revision 202698)
+++ tree-ssa-loop-im.c (working copy)
@@ -1192,11 +1192,11 @@ class move_computations_dom_walker : pub
{
public:
move_computations_dom_walker (cdi_direction direction)
- : dom_walker (direction), todo_ (0) {}
+ : dom_walker (direction), todo (0) {}
virtual void before_dom_children (basic_block);
- unsigned int todo_;
+ unsigned int todo;
};
/* Hoist the statements in basic block BB out of the loops prescribed by
@@ -1268,7 +1268,7 @@ move_computations_dom_walker::before_dom
gimple_phi_result (stmt),
t, arg0, arg1);
SSA_NAME_DEF_STMT (gimple_phi_result (stmt)) = new_stmt;
- todo_ |= TODO_cleanup_cfg;
+ todo |= TODO_cleanup_cfg;
}
gsi_insert_on_edge (loop_preheader_edge (level), new_stmt);
remove_phi_node (&bsi, false);
@@ -1346,7 +1346,7 @@ move_computations (void)
if (need_ssa_update_p (cfun))
rewrite_into_loop_closed_ssa (NULL, TODO_update_ssa);
- return walker.todo_;
+ return walker.todo;
}
/* Checks whether the statement defining variable *INDEX can be hoisted
Index: tree-ssa-phiopt.c
===================================================================
--- tree-ssa-phiopt.c (revision 202698)
+++ tree-ssa-phiopt.c (working copy)
@@ -1379,13 +1379,13 @@ class nontrapping_dom_walker : public do
{
public:
nontrapping_dom_walker (cdi_direction direction, pointer_set_t *ps)
- : dom_walker (direction), nontrapping_ (ps) {}
+ : dom_walker (direction), nontrapping (ps) {}
virtual void before_dom_children (basic_block);
virtual void after_dom_children (basic_block);
private:
- pointer_set_t *nontrapping_;
+ pointer_set_t *nontrapping;
};
/* Called by walk_dominator_tree, when entering the block BB. */
@@ -1416,8 +1416,8 @@ nontrapping_dom_walker::before_dom_child
nt_call_phase++;
else if (gimple_assign_single_p (stmt) && !gimple_has_volatile_ops
(stmt))
{
- add_or_mark_expr (bb, gimple_assign_lhs (stmt), nontrapping_, true);
- add_or_mark_expr (bb, gimple_assign_rhs1 (stmt), nontrapping_, false);
+ add_or_mark_expr (bb, gimple_assign_lhs (stmt), nontrapping, true);
+ add_or_mark_expr (bb, gimple_assign_rhs1 (stmt), nontrapping, false);
}
}
}
Index: tree-ssa-uncprop.c
===================================================================
--- tree-ssa-uncprop.c (revision 202698)
+++ tree-ssa-uncprop.c (working copy)
@@ -360,11 +360,11 @@ public:
uncprop_dom_walker (cdi_direction direction)
: dom_walker (direction)
{
- equiv_stack_.create (2);
+ equiv_stack.create (2);
}
~uncprop_dom_walker ()
{
- equiv_stack_.release ();
+ equiv_stack.release ();
}
virtual void before_dom_children (basic_block);
@@ -376,7 +376,7 @@ private:
leading to this block. If no such edge equivalency exists, then we
record NULL. These equivalences are live until we leave the dominator
subtree rooted at the block where we record the equivalency. */
- vec<tree> equiv_stack_;
+ vec<tree> equiv_stack;
};
/* Main driver for un-cprop. */
@@ -428,7 +428,7 @@ void
uncprop_dom_walker::after_dom_children (basic_block bb ATTRIBUTE_UNUSED)
{
/* Pop the topmost value off the equiv stack. */
- tree value = equiv_stack_.pop ();
+ tree value = equiv_stack.pop ();
/* If that value was non-null, then pop the topmost equivalency off
its equivalency stack. */
@@ -566,13 +566,13 @@ uncprop_dom_walker::before_dom_children
struct edge_equivalency *equiv = (struct edge_equivalency *) e->aux;
record_equiv (equiv->rhs, equiv->lhs);
- equiv_stack_.safe_push (equiv->rhs);
+ equiv_stack.safe_push (equiv->rhs);
recorded = true;
}
}
if (!recorded)
- equiv_stack_.safe_push (NULL_TREE);
+ equiv_stack.safe_push (NULL_TREE);
uncprop_into_successor_phis (bb);
}
Index: ChangeLog
===================================================================
--- ChangeLog (revision 202698)
+++ ChangeLog (working copy)
@@ -91,28 +91,29 @@
(move_computations_stmt): Convert to method
move_computations_dom_walker::before_dom_children.
(move_computations, tree_ssa_lim): Adjust.
- * tree-ssa-phiopt.c (nontrapping_dom_walker): new class
- (nt_init_block): Make method
+ * tree-ssa-phiopt.c (nontrapping_dom_walker): New class.
+ (nt_init_block): Convert to method
notrappping_dom_walker::before_dom_children.
- (nt_fini_block): Make
+ (nt_fini_block): Convert to method
method nontrapping_dom_walker::after_dom_children.
(get_non_trapping): Adjust.
* tree-ssa-pre.c (eliminate_dom_walker): New class.
- (eliminate_bb): Make method eliminate_dom_walker::before_dom_children.
- (eliminate_leave_block): Make method.
+ (eliminate_bb): Convert to method
+ eliminate_dom_walker::before_dom_children.
+ (eliminate_leave_block): Convert to method
eliminate_dom_walker::after_dom_children.
- (eliminate): Adjust
+ (eliminate): Adjust.
* tree-ssa-strlen.c (strlen_dom_walker): New class.
- (strlen_enter_block): Make method
+ (strlen_enter_block): Convert to method
strlen_dom_walker::before_dom_children.
- (strlen_leave_block): Make
+ (strlen_leave_block): Convert to method
method strlen_dom_walker::after_dom_children.
(tree_ssa_strlen): Adjust.
* tree-ssa-uncprop.c (uncprop_dom_walker): New class.
(tree_ssa_uncprop): Adjust.
- (uncprop_leave_block): Make method
+ (uncprop_leave_block): Convert to method
uncprop_dom_walker::after_dom_children.
- (uncprop_leave_block): Make method
+ (uncprop_leave_block): Convert to method
uncprop_dom_walker::before_dom_children.
2013-09-18 Bin Cheng <bin.ch...@arm.com>
