Propagate profile counts during switch expansion

Hi,

+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.

With the these changes, the patch is OK

Thanks,
Honza

On Sun, Oct 14, 2012 at 8:09 AM, Jan Hubicka <hubicka@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

> On Sun, Oct 14, 2012 at 8:09 AM, Jan Hubicka <hubicka@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.

In that case profile_status is PROFILE_GUESSED.
> 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);
> +    }
> +

 OK.

Honza