> On Fri, Oct 26, 2012 at 8:05 AM, Jan Hubicka <hubi...@ucw.cz> wrote: > >> Hi, > >> > >> On Tue, Oct 23, 2012 at 3:03 AM, Jan Hubicka <hubi...@ucw.cz> wrote: > >> >> Ping. > >> >> > >> >> > >> >> On Wed, Oct 17, 2012 at 1:48 PM, Easwaran Raman <era...@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 <era...@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
