On Tue, Aug 12, 2014 at 10:46:46AM -0700, Steve Ellcey wrote:
> --- /dev/null
> +++ b/gcc/tree-switch-shortcut.c
> +/* This file implements an optimization where, when a variable is set
> + to a constant value and there is a path that leads from this definition
> + to a switch statement that uses that variable as its controlling
> expression
> + we duplicate the blocks on this path and change the switch goto to a
> + direct goto to the label of the switch block that control would goto based
> + on the value of the variable. */
Would be nice to have some short C example here in the comment.
> +static int
> +find_path_1(basic_block start_bb, basic_block end_bb, hash_set<basic_block>
> *visited_bbs)
> +{
> + edge_iterator ei;
> + edge e;
> +
> + if (start_bb == end_bb) return 1;
Sorry for pedantry, but you don't have space before ( in many places,
the find_path_1 line is too long.
> +
> + if (!visited_bbs->add(start_bb))
> + {
> + FOR_EACH_EDGE (e, ei, start_bb->succs)
> + if (find_path_1 (e->dest, end_bb, visited_bbs))
> + return 1;
> + }
> + return 0;
return 0 not below if.
> +static int
> +find_path(basic_block start_bb, basic_block end_bb)
Again missing space.
> + if (!visited_bbs.add(start_bb))
Likewise.
> +static basic_block **bbs_list_array;
> +static int *val_array;
> +static int *bbs_list_size;
> +static int max_path_count;
> +static int max_insn_count;
> +static int n_bbs_list;
For a simple pass like this, is it really necessary to add new global
variables?
> + FOR_EACH_EDGE (e, ei, bbx->preds)
> + {
> + if (find_path (var_bb, e->src))
> + {
> + bbs_list[n] = e->src;
> + n = n + 1;
> + e_count = e_count + 1;
> + }
> + }
Weird indentation.
> + if ((gimple_code (def_stmt) == GIMPLE_PHI)
Extraneous parens around the equality check.
> + else if (arg && (TREE_CODE (arg) == SSA_NAME))
Likewise.
> + {
> + bbs_list[n] = gimple_phi_arg_edge (def_stmt, i)->src;
> + process_switch (arg, switch_stmt, visited_phis, bbs_list,
> n+1);
Weird indentation.
Jakub
