On Sat, May 25, 2013 at 3:02 PM, David Malcolm <dmalc...@redhat.com> wrote:
> Eliminate all direct references to "cfun" from macros in
> basic-block.h and introduce access methods to control_flow_graph
>
> * basic-block.h (control_flow_graph::get_basic_block_by_idx): New
> accessor methods.
> (control_flow_graph::get_entry_block): New method.
> (control_flow_graph::get_exit_block): New method.
> (control_flow_graph::get_basic_block_info): New methods.
> (control_flow_graph::get_n_basic_blocks): New methods.
> (control_flow_graph::get_n_edges): New methods.
> (control_flow_graph::get_last_basic_block): New methods.
> (control_flow_graph::get_label_to_block_map): New methods.
> (control_flow_graph::get_profile_status): New method.
> (control_flow_graph::set_profile_status): New method.
> (ENTRY_BLOCK_PTR): Eliminate this macro.
> (EXIT_BLOCK_PTR): Likewise.
> (basic_block_info): Likewise.
> (n_basic_blocks): Likewise.
> (n_edges): Likewise.
> (last_basic_block): Likewise.
> (label_to_block_map): Likewise.
> (profile_status): Likewise.
> (BASIC_BLOCK): Likewise.
> (SET_BASIC_BLOCK): Likewise.
> (FOR_EACH_BB_FN): Rewrite in terms of...
> (FOR_EACH_BB_CFG): New macro
> (FOR_EACH_BB): Eliminate this macro
> (FOR_EACH_BB_REVERSE_FN): Rewrite in terms of...
> (FOR_EACH_BB_REVERSE_FN_CFG): New macro
> (FOR_EACH_BB_REVERSE): Eliminate this macro
> (FOR_ALL_BB): Likewise.
> (FOR_ALL_BB_CFG): New macro
I don't like the mix of _CFG / _FN. It's obvious we are talking about
the CFG of a function in 'FOR_EACH_BB' and friends.
> ---
> diff --git a/gcc/basic-block.h b/gcc/basic-block.h
> index eed320c..3949417 100644
> --- a/gcc/basic-block.h
> +++ b/gcc/basic-block.h
> @@ -276,6 +276,57 @@ enum profile_status_d
> fields of this struct are interpreted as the defines for backward
> source compatibility following the definition of this struct. */
> struct GTY(()) control_flow_graph {
> +public:
> + basic_block get_basic_block_by_idx (int idx) const
> + {
> + return (*x_basic_block_info)[idx];
> + }
get_basic_block_by_idx is rather long, any reason to not shorten
it to get_block () or get_bb?
> + void set_basic_block_by_idx (int idx, basic_block bb)
set_block () or set_bb()
> + {
> + (*x_basic_block_info)[idx] = bb;
> + }
> +
> + basic_block get_entry_block () const { return x_entry_block_ptr; }
> +
> + basic_block get_exit_block () const { return x_exit_block_ptr; }
> +
> + vec<basic_block, va_gc> *get_basic_block_info () const
are they really used outside of the iterators?
> + {
> + return x_basic_block_info;
> + }
> + vec<basic_block, va_gc> *&get_basic_block_info ()
> + {
> + return x_basic_block_info;
> + }
> +
> + int get_n_basic_blocks () const { return x_n_basic_blocks; }
> + int& get_n_basic_blocks () { return x_n_basic_blocks; }
> +
> + int get_n_edges () const { return x_n_edges; }
> + int& get_n_edges () { return x_n_edges; }
> +
> + int get_last_basic_block () const { return x_last_basic_block; }
> + int& get_last_basic_block () { return x_last_basic_block; }
As you can't set_ those I'd drop the get_. Any reason to not simply
drop the x_ in the members and allow direct accesses? I fear
an -O0 compiler will get even more un-debuggable that it is now
with all the accessors :/ (undebuggable as in stepping through source)
> + vec<basic_block, va_gc> *get_label_to_block_map () const
> + {
> + return x_label_to_block_map;
> + }
> + vec<basic_block, va_gc> *&get_label_to_block_map ()
> + {
> + return x_label_to_block_map;
> + }
> +
> + enum profile_status_d get_profile_status () const
> + {
> + return x_profile_status;
> + }
> + void set_profile_status (enum profile_status_d status)
> + {
> + x_profile_status = status;
> + }
> +
> +public:
> /* Block pointers for the exit and entry of a function.
> These are always the head and tail of the basic block list. */
> basic_block x_entry_block_ptr;
> @@ -328,32 +379,20 @@ struct GTY(()) control_flow_graph {
> #define SET_BASIC_BLOCK_FOR_FUNCTION(FN,N,BB) \
> ((*basic_block_info_for_function(FN))[(N)] = (BB))
>
> -/* Defines for textual backward source compatibility. */
> -#define ENTRY_BLOCK_PTR (cfun->cfg->x_entry_block_ptr)
> -#define EXIT_BLOCK_PTR (cfun->cfg->x_exit_block_ptr)
> -#define basic_block_info (cfun->cfg->x_basic_block_info)
> -#define n_basic_blocks (cfun->cfg->x_n_basic_blocks)
> -#define n_edges (cfun->cfg->x_n_edges)
> -#define last_basic_block (cfun->cfg->x_last_basic_block)
> -#define label_to_block_map (cfun->cfg->x_label_to_block_map)
> -#define profile_status (cfun->cfg->x_profile_status)
> -
> -#define BASIC_BLOCK(N) ((*basic_block_info)[(N)])
> -#define SET_BASIC_BLOCK(N,BB) ((*basic_block_info)[(N)] = (BB))
> -
Replacements with the macros that already exist and expose cfun would
have been an obvious patch btw, without trying to refactor things to look
like C++ (ugh).
Richard.
> /* For iterating over basic blocks. */
> #define FOR_BB_BETWEEN(BB, FROM, TO, DIR) \
> for (BB = FROM; BB != TO; BB = BB->DIR)
>
> -#define FOR_EACH_BB_FN(BB, FN) \
> - FOR_BB_BETWEEN (BB, (FN)->cfg->x_entry_block_ptr->next_bb,
> (FN)->cfg->x_exit_block_ptr, next_bb)
> +#define FOR_EACH_BB_CFG(BB, CFG) \
> + FOR_BB_BETWEEN (BB, (CFG)->x_entry_block_ptr->next_bb,
> (CFG)->x_exit_block_ptr, next_bb)
>
> -#define FOR_EACH_BB(BB) FOR_EACH_BB_FN (BB, cfun)
> +#define FOR_EACH_BB_FN(BB, FN) FOR_EACH_BB_CFG (BB, (FN)->cfg)
>
> -#define FOR_EACH_BB_REVERSE_FN(BB, FN) \
> - FOR_BB_BETWEEN (BB, (FN)->cfg->x_exit_block_ptr->prev_bb,
> (FN)->cfg->x_entry_block_ptr, prev_bb)
> +#define FOR_EACH_BB_REVERSE_CFG(BB, CFG) \
> + FOR_BB_BETWEEN (BB, (CFG)->x_exit_block_ptr->prev_bb,
> (CFG)->x_entry_block_ptr, prev_bb)
>
> -#define FOR_EACH_BB_REVERSE(BB) FOR_EACH_BB_REVERSE_FN(BB, cfun)
> +#define FOR_EACH_BB_REVERSE_FN(BB, FN) \
> + FOR_EACH_BB_REVERSE_FN_CFG (BB, (FN)->cfg)
>
> /* For iterating over insns in basic block. */
> #define FOR_BB_INSNS(BB, INSN) \
> @@ -380,9 +419,8 @@ struct GTY(()) control_flow_graph {
>
> /* Cycles through _all_ basic blocks, even the fake ones (entry and
> exit block). */
> -
> -#define FOR_ALL_BB(BB) \
> - for (BB = ENTRY_BLOCK_PTR; BB; BB = BB->next_bb)
> +#define FOR_ALL_BB_CFG(BB, CFG) \
> + for (BB = (CFG)->get_entry_block (); BB; BB = BB->next_bb)
>
> #define FOR_ALL_BB_FN(BB, FN) \
> for (BB = ENTRY_BLOCK_PTR_FOR_FUNCTION (FN); BB; BB = BB->next_bb)
> --
> 1.7.11.7
>
