On 08/14/2015 01:54 AM, Richard Biener wrote:
On Fri, Aug 14, 2015 at 2:55 AM, Mikhail Maltsev <malts...@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?
Or a SEME region. While I don't have an immediate use for dominators regions in the CFG, I believe they'd be useful.


+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').
Presumably we're not at a point where we can push these down as methods in the class? ie, are we providing a class to query and manipulate the dominator tree from outside dominance.c or are we doing it just for internal stuff. I believe the former is more strategic, the latter may have value as well, but I believe it's more limited.


In general I'm biased and refactoring for the sake of refactoring
doesn't go well with me ...
But if we're taking something that's essentially C++ implemented in C and turn it into real C++ with encapsulation, that's a step in the right direction to me.

jeff


Reply via email to