This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 1/2] C++-ify dominance.c
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Mikhail Maltsev <maltsevm at gmail dot com>
- Cc: gcc-patches <gcc-patches at gnu dot org>, Jeff Law <law at redhat dot com>
- Date: Fri, 14 Aug 2015 09:54:00 +0200
- Subject: Re: [PATCH 1/2] C++-ify dominance.c
- Authentication-results: sourceware.org; auth=none
- References: <55CD3C87 dot 40101 at gmail dot com>
On Fri, Aug 14, 2015 at 2:55 AM, Mikhail Maltsev <maltsevm@gmail.com> 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<basic_block> , 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 <maltsevm@gmail.com>
>
> * (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