Re: Fix bugs introduced by switch-case profile propagation

[Jan Hubicka <hubicka at ucw dot cz>]

> On Fri, Oct 26, 2012 at 8:05 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> >> Hi,
> >>
> >> On Tue, Oct 23, 2012 at 3:03 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> >> >> Ping.
> >> >>
> >> >>
> >> >> On Wed, Oct 17, 2012 at 1:48 PM, Easwaran Raman <eraman@google.com> wrote:
> >> >> > Hi,
> >> >> >  This patch fixes bugs introduced by my previous patch to propagate
> >> >> > profiles during switch expansion. Bootstrap and profiledbootstrap
> >> >> > successful on x86_64. Confirmed that it fixes the crashes reported in
> >> >> > PR middle-end/54957. OK for trunk?
> >> >> >
> >> >> > - Easwaran
> >> >> >
> >> >> > 2012-10-17   Easwaran Raman  <eraman@google.com>
> >> >> >
> >> >> >         PR target/54938
> >> >> >         PR middle-end/54957
> >> >> >         * optabs.c (emit_cmp_and_jump_insn_1): Add REG_BR_PROB note
> >> >> >         only if it doesn't already exist.
> >> >> >         * except.c (sjlj_emit_function_enter): Remove unused variable.
> >> >> >         * stmt.c (get_outgoing_edge_probs): Return 0 if BB is NULL.
> >> >
> >> > Seems fine, but under what conditions you get NULL here?
> >> Wasn't sure if this is an OK for the patch or if I need to address
> >> anything else.
> >
> > Actually I think you should make the except.c to setcurrent_bb when expanding
> > the switch instead.
> 
> In the current code, in sjlj_emit_dispatch_table (except.c), a
> sequence of instructions including the dispatch table jump are first
> generated and emitted to a BB before the first of the jump table
> targets. I think you are asking me to first emit the instructions
> before the indirect jump into a new bb so that BLOCK_FOR_INSN
> (before_case) in stmt.c is non NULL. This would need some refactoring
> inside except.c, but more importantly this BB won't have the correct
> control flow right? So, while I can avoid the check for BB being NULL
> in get_outgoing_edge_probs, I still need to guard for default_edge
> being NULL and default_prob will still be 0. Would it be ok if I
> remove the check for NULL inside get_outgoing_edge_probs and instead
> check it when I initialize BASE?

Hmm,
in general we should set insn_bb for all emit code since some code size/speed
tradeoffs are fixed at expansion time and EH code is a good example where it
would make difference (because it is almost always cold).  But I see it
will be more work, since except.c is somewhat dodgy about how it creates the
control flow.

I guess it is OK to go with the original version of the patch and hopefully
we can clean this up incrementally.

thanks,
Honza
> 
> Thanks,
> Easwaran
> 
> > OK with this change.
> >
> > Honza