On Sun, Oct 14, 2012 at 8:09 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
> Hi,
>
> Index: optabs.c
> ===================================================================
> --- optabs.c (revision 191879)
> +++ optabs.c (working copy)
> @@ -4249,7 +4249,7 @@ prepare_operand (enum insn_code icode, rtx x, int
> we can do the branch. */
>
> static void
> -emit_cmp_and_jump_insn_1 (rtx test, enum machine_mode mode, rtx label)
> +emit_cmp_and_jump_insn_1 (rtx test, enum machine_mode mode, rtx label, int
> prob)
> {
> enum machine_mode optab_mode;
> enum mode_class mclass;
> @@ -4261,7 +4261,16 @@ static void
>
> gcc_assert (icode != CODE_FOR_nothing);
> gcc_assert (insn_operand_matches (icode, 0, test));
> - emit_jump_insn (GEN_FCN (icode) (test, XEXP (test, 0), XEXP (test, 1),
> label));
> + rtx insn = emit_insn (
> + GEN_FCN (icode) (test, XEXP (test, 0), XEXP (test, 1), label));
>
> I think we did not change to style of mixing declaration and code yet. So
> please put declaration ahead.
Ok.
>
> I think you want to keep emit_jump_insn. Also do nothing when profile_status
> == PROFILE_ABSENT.
Why should this be dependent on profile_status? The PROB passed could
also come from static prediction right.
> Index: cfgbuild.c
> ===================================================================
> --- cfgbuild.c (revision 191879)
> +++ cfgbuild.c (working copy)
> @@ -559,8 +559,11 @@ compute_outgoing_frequencies (basic_block b)
> f->count = b->count - e->count;
> return;
> }
> + else
> + {
> + guess_outgoing_edge_probabilities (b);
> + }
>
> Add comment here that we rely on multiway BBs having sane probabilities
> already.
> You still want to do guessing when the edges out are EH. Those also can be
> many.
I think this should work:
- if (single_succ_p (b))
+ else if (single_succ_p (b))
{
e = single_succ_edge (b);
e->probability = REG_BR_PROB_BASE;
e->count = b->count;
return;
}
- guess_outgoing_edge_probabilities (b);
+ else
+ {
+ /* We rely on BBs with more than two successors to have sane
probabilities
+ and do not guess them here. For BBs terminated by switch statements
+ expanded to jump-table jump, we have done the right thing during
+ expansion. For EH edges, we still guess the probabilities here. */
+ bool complex_edge = false;
+ FOR_EACH_EDGE (e, ei, b->succs)
+ if (e->flags & EDGE_COMPLEX)
+ {
+ complex_edge = true;
+ break;
+ }
+ if (complex_edge)
+ guess_outgoing_edge_probabilities (b);
+ }
+
> Index: expr.h
> ===================================================================
> --- expr.h (revision 191879)
> +++ expr.h (working copy)
> @@ -190,7 +190,7 @@ extern int have_sub2_insn (rtx, rtx);
> /* Emit a pair of rtl insns to compare two rtx's and to jump
> to a label if the comparison is true. */
> extern void emit_cmp_and_jump_insns (rtx, rtx, enum rtx_code, rtx,
> - enum machine_mode, int, rtx);
> + enum machine_mode, int, rtx, int
> prob=-1);
>
> Hmm, probably first appreance of this C++ construct. I suppose it is OK.
http://gcc.gnu.org/codingconventions.html#Default says it is ok for POD values.
>
> +static inline void
> +reset_out_edges_aux (basic_block bb)
> +{
> + edge e;
> + edge_iterator ei;
> + FOR_EACH_EDGE(e, ei, bb->succs)
> + e->aux = (void *)0;
> +}
> +static inline void
> +compute_cases_per_edge (gimple stmt)
> +{
> + basic_block bb = gimple_bb (stmt);
> + reset_out_edges_aux (bb);
> + int ncases = gimple_switch_num_labels (stmt);
> + for (int i = ncases - 1; i >= 1; --i)
> + {
> + tree elt = gimple_switch_label (stmt, i);
> + tree lab = CASE_LABEL (elt);
> + basic_block case_bb = label_to_block_fn (cfun, lab);
> + edge case_edge = find_edge (bb, case_bb);
> + case_edge->aux = (void *)((long)(case_edge->aux) + 1);
> + }
> +}
>
> Comments and newlines per coding standard.
Ok.
> With the these changes, the patch is OK
Thanks,
Easwaran
>
> Thanks,
> Honza
