Re: [PATCH 1/2] C++-ify dominance.c

[Richard Biener <richard dot guenther 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