From patchwork Wed Sep 18 16:24:47 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Matz X-Patchwork-Id: 275765 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id B74112C0092 for ; Thu, 19 Sep 2013 02:25:00 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:in-reply-to:message-id:references :mime-version:content-type; q=dns; s=default; b=vF04nEUL4Hu+6EvO nEWYhjiYnHe3eCmHyMuPPUcK4qy2EXGPMsbz32bS1b41xnp1mq4C1OTclmyjwwta FocjfBp1DRCqtOvi9/iC8MPaEpfGIXAk+GAJz6K7ZqDc6fa4K1bhpq/AnvtbL7sq 453re+hedtxMNlGoEl4OfH06jYs= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:in-reply-to:message-id:references :mime-version:content-type; s=default; bh=+Srrk9Hf+cYqvT9U3Avp+J C7PkY=; b=o9l0EYrrw5C3rOV7USdB1ffr3fFqqD8mg90RjCcSCUCd4ChvOu0+pA mHEK49xPY2IAASdpXIW9AoDTJ5l6YEoESV+WPJe/zCdXgy82py/UK4Qf2QLUPAZb Rgpd/68zo5PhKbgwxMGYDILA+KHD5w9OkNXxBbFTQPH0gWl4Xj66M= Received: (qmail 23717 invoked by alias); 18 Sep 2013 16:24:53 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 23708 invoked by uid 89); 18 Sep 2013 16:24:52 -0000 Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 18 Sep 2013 16:24:52 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL, BAYES_00, KHOP_THREADED, RDNS_NONE, T_FILL_THIS_FORM_SHORT autolearn=no version=3.3.2 X-HELO: mx2.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id E4B2EA41E0; Wed, 18 Sep 2013 18:24:47 +0200 (CEST) Date: Wed, 18 Sep 2013 18:24:47 +0200 (CEST) From: Michael Matz To: Jeff Law Cc: Trevor Saunders , Richard Biener , GCC Patches Subject: Re: [PATCH] manage dom-walk_data initialization and finalization with constructors and destructors In-Reply-To: <5239126A.6010702@redhat.com> Message-ID: References: <1378264562-30803-1-git-send-email-tsaunders@mozilla.com> <20130904145911.GC17620@tsaunders-iceball.corp.tor1.mozilla.com> <522759C8.5040802@redhat.com> <20130911000350.GA28492@tsaunders-iceball.corp.tor1.mozilla.com> <52389CB1.60504@redhat.com> <5239126A.6010702@redhat.com> User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 X-IsSubscribed: yes 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 conditions_, cases_; - sese region_; + vec 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 equiv_stack_; + vec 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