From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 71507 invoked by alias); 14 Aug 2015 07:54:08 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 71492 invoked by uid 89); 14 Aug 2015 07:54:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: fencepost.gnu.org Received: from fencepost.gnu.org (HELO fencepost.gnu.org) (208.118.235.10) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 14 Aug 2015 07:54:06 +0000 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53522) by fencepost.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1ZQ9oR-0007Rl-FO for gcc-patches@gnu.org; Fri, 14 Aug 2015 03:54:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZQ9oP-0003Tf-VD for gcc-patches@gnu.org; Fri, 14 Aug 2015 03:54:03 -0400 Received: from mail-ig0-x22a.google.com ([2607:f8b0:4001:c05::22a]:36179) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZQ9oP-0003TT-Q7 for gcc-patches@gnu.org; Fri, 14 Aug 2015 03:54:01 -0400 Received: by igxp17 with SMTP id p17so7135461igx.1 for ; Fri, 14 Aug 2015 00:54:01 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.50.176.138 with SMTP id ci10mr1538516igc.32.1439538840869; Fri, 14 Aug 2015 00:54:00 -0700 (PDT) Received: by 10.107.32.140 with HTTP; Fri, 14 Aug 2015 00:54:00 -0700 (PDT) In-Reply-To: <55CD3C87.40101@gmail.com> References: <55CD3C87.40101@gmail.com> Date: Fri, 14 Aug 2015 07:54:00 -0000 Message-ID: Subject: Re: [PATCH 1/2] C++-ify dominance.c From: Richard Biener To: Mikhail Maltsev Cc: gcc-patches , Jeff Law Content-Type: text/plain; charset=UTF-8 X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2607:f8b0:4001:c05::22a X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg00758.txt.bz2 On Fri, Aug 14, 2015 at 2:55 AM, Mikhail Maltsev wrote: > Hi all. > > These two patches are refactoring of dominator-related code. > > The comment in dominance.c says: "We work in a poor-mans object oriented > fashion, and carry an instance of this structure through all our 'methods'". So, > the first patch converts the mentioned structure (dom_info) into a class with > proper encapsulation. It also adds a new member - m_fn (the function currently > being compiled) to this structure and replaces some uses of cfun with m_fn. It > also contains some fixes, related to current coding standards: move variable > declarations to place of first use, replace elaborated type specifiers (i.e. > "struct/enum foo") by simple ones (i.e., just "foo") in function prototypes. Putting in m_fn looks backwards to me - it looks like we only need to remember the entry and exit BBs and the number of blocks. In fact initializing dom_info from that would allow it to work on SESE regions as well? +unsigned bb_dom_dfs_in (cdi_direction, basic_block); +unsigned bb_dom_dfs_out (cdi_direction, basic_block); +extern void verify_dominators (cdi_direction); +basic_block recompute_dominator (cdi_direction, basic_block); +extern void iterate_fix_dominators (cdi_direction, vec , bool); +extern void add_to_dominance_info (cdi_direction, basic_block); if you are here please fix the 'extern' vs. w/o 'extern' inconsistencies as well (we prefer 'extern'). In general I'm biased and refactoring for the sake of refactoring doesn't go well with me ... At least the above is a constructive comment, leaving the rest to Jeff. Thanks, Richard. > > Bootstrapped and regtested on x86_64-linux. Tested build of config-list.mk. > > gcc/ChangeLog: > > 2015-08-14 Mikhail Maltsev > > * (ENABLE_CHECKING): Define as 0 by default. > dominance.c (new_zero_array): Define. > (dom_info): Define as class instead of struct. > (dom_info::dom_info, ~dom_info): Define. Use new/delete for memory > allocations/deallocations. Pass function as parameter (instead of > using cfun). > (dom_info::get_idom): Define accessor method. > (dom_info::calc_dfs_tree_nonrec, calc_dfs_tree, compress, eval, > link_roots, calc_idoms): Redefine as class members. Use m_fn instead > of cfun. > (init_dom_info, free_dom_info): Remove (use dom_info ctor/dtor). > (dom_convert_dir_to_idx): Fix prototype. > (assign_dfs_numbers): Move variable declarations to their first uses. > (calculate_dominance_info): Remove conditional compilation, move > variables. > (free_dominance_info, get_immediate_dominator, set_immediate_dominator, > get_dominated_b, get_dominated_by_region, get_dominated_to_depth, > redirect_immediate_dominators, nearest_common_dominator_for_set, > dominated_by_p, bb_dom_dfs_in, bb_dom_dfs_out, verify_dominators, > determine_dominators_for_sons, iterate_fix_dominators, first_dom_son, > next_dom_son, debug_dominance_info, debug_dominance_tree_1): Adjust to > use class dom_info. Move variable declarations to the place of first > use. Fix prototypes (remove struct/enum). > * dominance.h: Fix prototypes (remove struct/enum). > > -- > Regards, > Mikhail Maltsev