| Message ID | 20150120200447.GD2232@kam.mff.cuni.cz |
|---|---|
| State | New |
| Headers | show |
On 2015.01.20 at 21:04 +0100, Jan Hubicka wrote: > this patch fixes ICE in ipa_merge_profiles on speculative edges. > > Bootstrapped/regtested x86_64-linux, comitted. Also tested by Markus on > Firefox build. This needs one additional fix. See below. > PR ipa/63576 > * ipa-utils.c (ipa_merge_profiles): Merge speculative edges. > Index: ipa-utils.c > =================================================================== > --- ipa-utils.c (revision 219871) > +++ ipa-utils.c (working copy) > @@ -539,7 +539,7 @@ ipa_merge_profiles (struct cgraph_node * > } > if (match) > { > - struct cgraph_edge *e; > + struct cgraph_edge *e, *e2; > basic_block srcbb, dstbb; > > /* TODO: merge also statement histograms. */ > @@ -562,19 +562,95 @@ ipa_merge_profiles (struct cgraph_node * > pop_cfun (); > for (e = dst->callees; e; e = e->next_callee) > { > - gcc_assert (!e->speculative); > + if (e->speculative) > + continue; > e->count = gimple_bb (e->call_stmt)->count; > e->frequency = compute_call_stmt_bb_frequency > (dst->decl, > gimple_bb (e->call_stmt)); > } > - for (e = dst->indirect_calls; e; e = e->next_callee) > + for (e = dst->indirect_calls, e2 = src->indirect_calls; e; > + e2 = (e2 ? e2->next_callee : NULL), e = e->next_callee) > { > - gcc_assert (!e->speculative); > - e->count = gimple_bb (e->call_stmt)->count; > - e->frequency = compute_call_stmt_bb_frequency > - (dst->decl, > - gimple_bb (e->call_stmt)); > + gcov_type count = gimple_bb (e->call_stmt)->count; > + int freq = compute_call_stmt_bb_frequency > + (dst->decl, > + gimple_bb (e->call_stmt)); > + /* When call is speculative, we need to re-distribute probabilities > + the same way as they was. This is not really correct because > + in the other copy the speculation may differ; but probably it > + is not really worth the effort. */ > + if (e->speculative) > + { > + cgraph_edge *direct, *indirect; > + cgraph_edge *direct2 = NULL, *indirect2 = NULL; > + ipa_ref *ref; > + > + e->speculative_call_info (direct, indirect, ref); > + gcc_assert (e == indirect); > + if (e2 && e2->speculative) > + e2->speculative_call_info (direct2, indirect2, ref); > + if (indirect->count || direct->count) > + { > + /* We should mismatch earlier if there is no matching > + indirect edge. */ > + if (!e2) > + { > + if (dump_file) > + fprintf (dump_file, > + "Mismatch in merging indirect edges\n"); > + } > + else if (!e2->speculative) > + indirect->count += e2->count; > + else if (e2->speculative) > + { > + if (DECL_ASSEMBLER_NAME (direct2->callee->decl) > + != DECL_ASSEMBLER_NAME (direct->callee->decl)) > + { > + if (direct2->count >= direct->count) > + { > + direct->redirect_callee (direct2->callee); > + indirect->count += indirect2->count > + + direct->count; > + direct->count = direct2->count; > + } > + else > + indirect->count += indirect2->count + direct2->count; > + } > + else > + { > + direct->count += direct2->count; > + indirect->count += indirect2->count; > + } > + } > + int prob = RDIV (direct->count * REG_BR_PROB_BASE , > + direct->count + indirect->count); > + direct->frequency = RDIV (freq * prob, REG_BR_PROB_BASE); > + indirect->frequency = RDIV (freq * (REG_BR_PROB_BASE - prob), > + REG_BR_PROB_BASE); > + } > + else > + /* At the moment we should have only profile feedback based > + speculations when merging. */ > + gcc_unreachable (); > + } > + else if (e2->speculative) + else if (e2 && e2->speculative) Otherwise it will crash: lto1: internal compiler error: Segmentation fault 0xa12f6f crash_signal ../../gcc/gcc/toplev.c:381 0x88b190 ipa_merge_profiles(cgraph_node*, cgraph_node*) ../../gcc/gcc/ipa-utils.c:637 0x603722 lto_cgraph_replace_node ../../gcc/gcc/lto/lto-symtab.c:124 0x604cf3 lto_symtab_merge_symbols_1 ../../gcc/gcc/lto/lto-symtab.c:619 0x604cf3 lto_symtab_merge_symbols() ../../gcc/gcc/lto/lto-symtab.c:647 0x5fa52e read_cgraph_and_symbols ../../gcc/gcc/lto/lto.c:3109 0x5fa52e lto_main() ../../gcc/gcc/lto/lto.c:3436 Please submit a full bug report, with preprocessed source if appropriate. > + { > + cgraph_edge *direct, *indirect; > + ipa_ref *ref; > + > + e2->speculative_call_info (direct, indirect, ref); > + e->count = count; > + e->frequency = freq; > + int prob = RDIV (direct->count * REG_BR_PROB_BASE, e->count); > + e->make_speculative (direct->callee, direct->count, > + RDIV (freq * prob, REG_BR_PROB_BASE)); > + } > + else > + { > + e->count = count; > + e->frequency = freq; > + } > } > src->release_body (); > inline_update_overall_summary (dst); >
> On 2015.01.20 at 21:04 +0100, Jan Hubicka wrote: > > this patch fixes ICE in ipa_merge_profiles on speculative edges. > > > > Bootstrapped/regtested x86_64-linux, comitted. Also tested by Markus on > > Firefox build. > > This needs one additional fix. See below. > > Otherwise it will crash: > > lto1: internal compiler error: Segmentation fault > 0xa12f6f crash_signal > ../../gcc/gcc/toplev.c:381 > 0x88b190 ipa_merge_profiles(cgraph_node*, cgraph_node*) > ../../gcc/gcc/ipa-utils.c:637 > 0x603722 lto_cgraph_replace_node > ../../gcc/gcc/lto/lto-symtab.c:124 > 0x604cf3 lto_symtab_merge_symbols_1 > ../../gcc/gcc/lto/lto-symtab.c:619 > 0x604cf3 lto_symtab_merge_symbols() > ../../gcc/gcc/lto/lto-symtab.c:647 > 0x5fa52e read_cgraph_and_symbols > ../../gcc/gcc/lto/lto.c:3109 > 0x5fa52e lto_main() > ../../gcc/gcc/lto/lto.c:3436 > Please submit a full bug report, > with preprocessed source if appropriate. I see, thanks. It means that we do have comdats that diverge in their call statements. I guess we ought to just cancel merging in that case, otherwise the profile can be completely off :( I will need to write some call target compare then. So far the code assumes that if #of BBs match then the bodies match and proceeds with merging. Martin, perhaps we can re-use some of ipa-icf infrastructure here to quickly check that at least CFG shapes and call targets match? I will commit this hack for now; hopefully it is only infrequent side case. Honza
On Tue, Jan 20, 2015 at 2:01 PM, Jan Hubicka <hubicka@ucw.cz> wrote: >> On 2015.01.20 at 21:04 +0100, Jan Hubicka wrote: >> > this patch fixes ICE in ipa_merge_profiles on speculative edges. >> > >> > Bootstrapped/regtested x86_64-linux, comitted. Also tested by Markus on >> > Firefox build. >> >> This needs one additional fix. See below. >> >> Otherwise it will crash: >> >> lto1: internal compiler error: Segmentation fault >> 0xa12f6f crash_signal >> ../../gcc/gcc/toplev.c:381 >> 0x88b190 ipa_merge_profiles(cgraph_node*, cgraph_node*) >> ../../gcc/gcc/ipa-utils.c:637 >> 0x603722 lto_cgraph_replace_node >> ../../gcc/gcc/lto/lto-symtab.c:124 >> 0x604cf3 lto_symtab_merge_symbols_1 >> ../../gcc/gcc/lto/lto-symtab.c:619 >> 0x604cf3 lto_symtab_merge_symbols() >> ../../gcc/gcc/lto/lto-symtab.c:647 >> 0x5fa52e read_cgraph_and_symbols >> ../../gcc/gcc/lto/lto.c:3109 >> 0x5fa52e lto_main() >> ../../gcc/gcc/lto/lto.c:3436 >> Please submit a full bug report, >> with preprocessed source if appropriate. > > I see, thanks. It means that we do have comdats that diverge in their call statements. > I guess we ought to just cancel merging in that case, otherwise the profile can be completely > off :( > I will need to write some call target compare then. So far the code assumes that if #of BBs match > then the bodies match and proceeds with merging. > > Martin, perhaps we can re-use some of ipa-icf infrastructure here to quickly check that at least > CFG shapes and call targets match? > > I will commit this hack for now; hopefully it is only infrequent side case. > > Honza Will it fix https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64710
Index: ipa-utils.c =================================================================== --- ipa-utils.c (revision 219871) +++ ipa-utils.c (working copy) @@ -539,7 +539,7 @@ ipa_merge_profiles (struct cgraph_node * } if (match) { - struct cgraph_edge *e; + struct cgraph_edge *e, *e2; basic_block srcbb, dstbb; /* TODO: merge also statement histograms. */ @@ -562,19 +562,95 @@ ipa_merge_profiles (struct cgraph_node * pop_cfun (); for (e = dst->callees; e; e = e->next_callee) { - gcc_assert (!e->speculative); + if (e->speculative) + continue; e->count = gimple_bb (e->call_stmt)->count; e->frequency = compute_call_stmt_bb_frequency (dst->decl, gimple_bb (e->call_stmt)); } - for (e = dst->indirect_calls; e; e = e->next_callee) + for (e = dst->indirect_calls, e2 = src->indirect_calls; e; + e2 = (e2 ? e2->next_callee : NULL), e = e->next_callee) { - gcc_assert (!e->speculative); - e->count = gimple_bb (e->call_stmt)->count; - e->frequency = compute_call_stmt_bb_frequency - (dst->decl, - gimple_bb (e->call_stmt)); + gcov_type count = gimple_bb (e->call_stmt)->count; + int freq = compute_call_stmt_bb_frequency + (dst->decl, + gimple_bb (e->call_stmt)); + /* When call is speculative, we need to re-distribute probabilities + the same way as they was. This is not really correct because + in the other copy the speculation may differ; but probably it + is not really worth the effort. */ + if (e->speculative) + { + cgraph_edge *direct, *indirect; + cgraph_edge *direct2 = NULL, *indirect2 = NULL; + ipa_ref *ref; + + e->speculative_call_info (direct, indirect, ref); + gcc_assert (e == indirect); + if (e2 && e2->speculative) + e2->speculative_call_info (direct2, indirect2, ref); + if (indirect->count || direct->count) + { + /* We should mismatch earlier if there is no matching + indirect edge. */ + if (!e2) + { + if (dump_file) + fprintf (dump_file, + "Mismatch in merging indirect edges\n"); + } + else if (!e2->speculative) + indirect->count += e2->count; + else if (e2->speculative) + { + if (DECL_ASSEMBLER_NAME (direct2->callee->decl) + != DECL_ASSEMBLER_NAME (direct->callee->decl)) + { + if (direct2->count >= direct->count) + { + direct->redirect_callee (direct2->callee); + indirect->count += indirect2->count + + direct->count; + direct->count = direct2->count; + } + else + indirect->count += indirect2->count + direct2->count; + } + else + { + direct->count += direct2->count; + indirect->count += indirect2->count; + } + } + int prob = RDIV (direct->count * REG_BR_PROB_BASE , + direct->count + indirect->count); + direct->frequency = RDIV (freq * prob, REG_BR_PROB_BASE); + indirect->frequency = RDIV (freq * (REG_BR_PROB_BASE - prob), + REG_BR_PROB_BASE); + } + else + /* At the moment we should have only profile feedback based + speculations when merging. */ + gcc_unreachable (); + } + else if (e2->speculative) + { + cgraph_edge *direct, *indirect; + ipa_ref *ref; + + e2->speculative_call_info (direct, indirect, ref); + e->count = count; + e->frequency = freq; + int prob = RDIV (direct->count * REG_BR_PROB_BASE, e->count); + e->make_speculative (direct->callee, direct->count, + RDIV (freq * prob, REG_BR_PROB_BASE)); + } + else + { + e->count = count; + e->frequency = freq; + } } src->release_body (); inline_update_overall_summary (dst);