| Message ID | 20170718072624.GA52973@kam.mff.cuni.cz |
|---|---|
| State | New |
| Headers | show |
On Tue, 18 Jul 2017, Jan Hubicka wrote: > Hi, > this patch fixes wrong code issue with BB partitioning where sometimes EH > is not delivered. This is very old issue that affect all release branches > with -fprofile-use, but recently surfaced as libstdc++ testsuite regression > because we now partition functions based on static profile prediction. > > The problem is that EH tables are stored as offsets from start of functions. > In the record however value 0 is special and means that there is no landing > pad for a given region. Normally this is safe because landing pads never > appear as very first label in function. This is however no longer true with > partitining where cold partition is actually quite likely to start by landing > pad. > > The change in except.c adds sanity check that no EH landing pads are very first > in the insn stream. The change in bb-reorder makes reorder to chose > non-landing-pad BB as first trace for the cold partition. Such BB always exists > because landing pads must be in same partition as the instruction throwing them > and we never make BB both landing pad and reachable by normal control folow. > However I am not thrilled by the fix as it is bit fragile in case some > optimization happends after bb partitioning and code is moved away. Also the > logic can be confused by asm statement which may result in no code (again > however the BB reachable from outside world should contain something that > produce EH that is a real instruction). > > Ideas for better fix would be welcome then. If the assert I added triggers > for valid reasons, we may just end up adding a NOP in the rare case we do > not suceed arranging cold partition to not start with landing pad. Yeah, I'd rather pad the function start with a nop if it starts with a landing pad. How difficult would it be to arrange for this? I suppose we'd need to touch each and every target to accomplish this? Or end up using gen_nop in generic code? Richard. > Bootstrapped/regtested x86_64-linux, looks sane? > > Honza > > PR middle-end/81331 > * except.c (first_in_partition): New function. > (dw2_output_call_site_table): Sanity check that landing pads are not > very first in the partition. > * bb-reorder.c (ok_to_be_first): New function. > (connect_traces): Avoid traces that are !ok_to_be_first to start > partitions. > Index: except.c > =================================================================== > --- except.c (revision 250226) > +++ except.c (working copy) > @@ -2724,6 +2724,23 @@ sjlj_size_of_call_site_table (void) > return size; > } > > +/* Return true if L will appear as very first in its partition. */ > + > +bool > +first_in_partition (rtx_insn *l) > +{ > + while (l != NULL_RTX) > + { > + if (active_insn_p (l)) > + return false; > + else if (GET_CODE (l) == NOTE > + && NOTE_KIND (l) == NOTE_INSN_SWITCH_TEXT_SECTIONS) > + return true; > + l = PREV_INSN (l); > + } > + return true; > +} > + > static void > dw2_output_call_site_table (int cs_format, int section) > { > @@ -2749,8 +2766,14 @@ dw2_output_call_site_table (int cs_forma > ASM_GENERATE_INTERNAL_LABEL (reg_end_lab, "LEHE", call_site_base + i); > > if (cs->landing_pad) > - ASM_GENERATE_INTERNAL_LABEL (landing_pad_lab, "L", > - CODE_LABEL_NUMBER (cs->landing_pad)); > + { > + ASM_GENERATE_INTERNAL_LABEL (landing_pad_lab, "L", > + CODE_LABEL_NUMBER (cs->landing_pad)); > + /* Be sure that the offset will not be 0 as that would make EH > + delivery code to think that there is no landing pad. */ > + gcc_checking_assert (!first_in_partition > + (as_a <rtx_insn *> (cs->landing_pad))); > + } > > /* ??? Perhaps use insn length scaling if the assembler supports > generic arithmetic. */ > Index: bb-reorder.c > =================================================================== > --- bb-reorder.c (revision 250226) > +++ bb-reorder.c (working copy) > @@ -1066,6 +1066,21 @@ connect_better_edge_p (const_edge e, boo > return is_better_edge; > } > > +/* If we place EH landing pad as very first BB in the partition, its offset > + from start of function is 0 which is special cased by the eh table to mean > + no landing pad. For this reason such BBs can not appear as very first in > + the partition. */ > +static bool > +ok_to_be_first (struct trace *t) > +{ > + edge e; > + edge_iterator ei; > + FOR_EACH_EDGE (e, ei, t->first->preds) > + if (e->flags & EDGE_EH) > + return false; > + return true; > +} > + > /* Connect traces in array TRACES, N_TRACES is the count of traces. */ > > static void > @@ -1080,6 +1095,7 @@ connect_traces (int n_traces, struct tra > int freq_threshold; > gcov_type count_threshold; > bool for_size = optimize_function_for_size_p (cfun); > + bool first_in_partition; > > freq_threshold = max_entry_frequency * DUPLICATION_THRESHOLD / 1000; > if (max_entry_count.to_gcov_type () < INT_MAX / 1000) > @@ -1092,6 +1108,7 @@ connect_traces (int n_traces, struct tra > current_pass = 1; > current_partition = BB_PARTITION (traces[0].first); > two_passes = false; > + first_in_partition = true; > > if (crtl->has_bb_partition) > for (i = 0; i < n_traces && !two_passes; i++) > @@ -1116,6 +1133,7 @@ connect_traces (int n_traces, struct tra > current_partition = BB_COLD_PARTITION; > else > current_partition = BB_HOT_PARTITION; > + first_in_partition = true; > } > > if (connected[t]) > @@ -1125,6 +1143,9 @@ connect_traces (int n_traces, struct tra > && BB_PARTITION (traces[t].first) != current_partition) > continue; > > + if (first_in_partition && !ok_to_be_first (&traces[t])) > + continue; > + > connected[t] = true; > > /* Find the predecessor traces. */ > @@ -1143,7 +1164,9 @@ connect_traces (int n_traces, struct tra > && bbd[si].end_of_trace >= 0 > && !connected[bbd[si].end_of_trace] > && (BB_PARTITION (e->src) == current_partition) > - && connect_better_edge_p (e, true, best_len, best, traces)) > + && connect_better_edge_p (e, true, best_len, best, traces) > + && (!first_in_partition > + || ok_to_be_first (&traces[bbd[si].end_of_trace]))) > { > best = e; > best_len = traces[bbd[si].end_of_trace].length; > @@ -1151,6 +1174,8 @@ connect_traces (int n_traces, struct tra > } > if (best) > { > + gcc_checking_assert (BB_PARTITION (best->src) > + == BB_PARTITION (best->dest)); > best->src->aux = best->dest; > t2 = bbd[best->src->index].end_of_trace; > connected[t2] = true; > @@ -1166,8 +1191,13 @@ connect_traces (int n_traces, struct tra > } > > if (last_trace >= 0) > - traces[last_trace].last->aux = traces[t2].first; > + { > + gcc_checking_assert (!first_in_partition > + || ok_to_be_first (&traces[t2])); > + traces[last_trace].last->aux = traces[t2].first; > + } > last_trace = t; > + first_in_partition = false; > > /* Find the successor traces. */ > while (1) > @@ -1226,6 +1256,8 @@ connect_traces (int n_traces, struct tra > fprintf (dump_file, "Connection: %d %d\n", > best->src->index, best->dest->index); > > + gcc_checking_assert (BB_PARTITION (best->dest) > + == BB_PARTITION (best->src)); > t = bbd[best->dest->index].start_of_trace; > traces[last_trace].last->aux = traces[t].first; > connected[t] = true; > @@ -1238,6 +1270,8 @@ connect_traces (int n_traces, struct tra > fprintf (dump_file, "Connection: %d %d\n", > best->src->index, best->dest->index); > } > + gcc_checking_assert (BB_PARTITION (best->dest) > + == BB_PARTITION (best->src)); > t = bbd[best->dest->index].start_of_trace; > traces[last_trace].last->aux = traces[t].first; > connected[t] = true; > >
> On Tue, 18 Jul 2017, Jan Hubicka wrote: > > > Hi, > > this patch fixes wrong code issue with BB partitioning where sometimes EH > > is not delivered. This is very old issue that affect all release branches > > with -fprofile-use, but recently surfaced as libstdc++ testsuite regression > > because we now partition functions based on static profile prediction. > > > > The problem is that EH tables are stored as offsets from start of functions. > > In the record however value 0 is special and means that there is no landing > > pad for a given region. Normally this is safe because landing pads never > > appear as very first label in function. This is however no longer true with > > partitining where cold partition is actually quite likely to start by landing > > pad. > > > > The change in except.c adds sanity check that no EH landing pads are very first > > in the insn stream. The change in bb-reorder makes reorder to chose > > non-landing-pad BB as first trace for the cold partition. Such BB always exists > > because landing pads must be in same partition as the instruction throwing them > > and we never make BB both landing pad and reachable by normal control folow. > > However I am not thrilled by the fix as it is bit fragile in case some > > optimization happends after bb partitioning and code is moved away. Also the > > logic can be confused by asm statement which may result in no code (again > > however the BB reachable from outside world should contain something that > > produce EH that is a real instruction). > > > > Ideas for better fix would be welcome then. If the assert I added triggers > > for valid reasons, we may just end up adding a NOP in the rare case we do > > not suceed arranging cold partition to not start with landing pad. > > Yeah, I'd rather pad the function start with a nop if it starts with a > landing pad. How difficult would it be to arrange for this? I suppose > we'd need to touch each and every target to accomplish this? Or end up > using gen_nop in generic code? I think we could just output from generic code - I think it can be done by final_scan_insn. I don't know however if we have a way to tell if the section starts with a landing pad? Honza > > Richard. > > > Bootstrapped/regtested x86_64-linux, looks sane? > > > > Honza > > > > PR middle-end/81331 > > * except.c (first_in_partition): New function. > > (dw2_output_call_site_table): Sanity check that landing pads are not > > very first in the partition. > > * bb-reorder.c (ok_to_be_first): New function. > > (connect_traces): Avoid traces that are !ok_to_be_first to start > > partitions. > > Index: except.c > > =================================================================== > > --- except.c (revision 250226) > > +++ except.c (working copy) > > @@ -2724,6 +2724,23 @@ sjlj_size_of_call_site_table (void) > > return size; > > } > > > > +/* Return true if L will appear as very first in its partition. */ > > + > > +bool > > +first_in_partition (rtx_insn *l) > > +{ > > + while (l != NULL_RTX) > > + { > > + if (active_insn_p (l)) > > + return false; > > + else if (GET_CODE (l) == NOTE > > + && NOTE_KIND (l) == NOTE_INSN_SWITCH_TEXT_SECTIONS) > > + return true; > > + l = PREV_INSN (l); > > + } > > + return true; > > +} > > + > > static void > > dw2_output_call_site_table (int cs_format, int section) > > { > > @@ -2749,8 +2766,14 @@ dw2_output_call_site_table (int cs_forma > > ASM_GENERATE_INTERNAL_LABEL (reg_end_lab, "LEHE", call_site_base + i); > > > > if (cs->landing_pad) > > - ASM_GENERATE_INTERNAL_LABEL (landing_pad_lab, "L", > > - CODE_LABEL_NUMBER (cs->landing_pad)); > > + { > > + ASM_GENERATE_INTERNAL_LABEL (landing_pad_lab, "L", > > + CODE_LABEL_NUMBER (cs->landing_pad)); > > + /* Be sure that the offset will not be 0 as that would make EH > > + delivery code to think that there is no landing pad. */ > > + gcc_checking_assert (!first_in_partition > > + (as_a <rtx_insn *> (cs->landing_pad))); > > + } > > > > /* ??? Perhaps use insn length scaling if the assembler supports > > generic arithmetic. */ > > Index: bb-reorder.c > > =================================================================== > > --- bb-reorder.c (revision 250226) > > +++ bb-reorder.c (working copy) > > @@ -1066,6 +1066,21 @@ connect_better_edge_p (const_edge e, boo > > return is_better_edge; > > } > > > > +/* If we place EH landing pad as very first BB in the partition, its offset > > + from start of function is 0 which is special cased by the eh table to mean > > + no landing pad. For this reason such BBs can not appear as very first in > > + the partition. */ > > +static bool > > +ok_to_be_first (struct trace *t) > > +{ > > + edge e; > > + edge_iterator ei; > > + FOR_EACH_EDGE (e, ei, t->first->preds) > > + if (e->flags & EDGE_EH) > > + return false; > > + return true; > > +} > > + > > /* Connect traces in array TRACES, N_TRACES is the count of traces. */ > > > > static void > > @@ -1080,6 +1095,7 @@ connect_traces (int n_traces, struct tra > > int freq_threshold; > > gcov_type count_threshold; > > bool for_size = optimize_function_for_size_p (cfun); > > + bool first_in_partition; > > > > freq_threshold = max_entry_frequency * DUPLICATION_THRESHOLD / 1000; > > if (max_entry_count.to_gcov_type () < INT_MAX / 1000) > > @@ -1092,6 +1108,7 @@ connect_traces (int n_traces, struct tra > > current_pass = 1; > > current_partition = BB_PARTITION (traces[0].first); > > two_passes = false; > > + first_in_partition = true; > > > > if (crtl->has_bb_partition) > > for (i = 0; i < n_traces && !two_passes; i++) > > @@ -1116,6 +1133,7 @@ connect_traces (int n_traces, struct tra > > current_partition = BB_COLD_PARTITION; > > else > > current_partition = BB_HOT_PARTITION; > > + first_in_partition = true; > > } > > > > if (connected[t]) > > @@ -1125,6 +1143,9 @@ connect_traces (int n_traces, struct tra > > && BB_PARTITION (traces[t].first) != current_partition) > > continue; > > > > + if (first_in_partition && !ok_to_be_first (&traces[t])) > > + continue; > > + > > connected[t] = true; > > > > /* Find the predecessor traces. */ > > @@ -1143,7 +1164,9 @@ connect_traces (int n_traces, struct tra > > && bbd[si].end_of_trace >= 0 > > && !connected[bbd[si].end_of_trace] > > && (BB_PARTITION (e->src) == current_partition) > > - && connect_better_edge_p (e, true, best_len, best, traces)) > > + && connect_better_edge_p (e, true, best_len, best, traces) > > + && (!first_in_partition > > + || ok_to_be_first (&traces[bbd[si].end_of_trace]))) > > { > > best = e; > > best_len = traces[bbd[si].end_of_trace].length; > > @@ -1151,6 +1174,8 @@ connect_traces (int n_traces, struct tra > > } > > if (best) > > { > > + gcc_checking_assert (BB_PARTITION (best->src) > > + == BB_PARTITION (best->dest)); > > best->src->aux = best->dest; > > t2 = bbd[best->src->index].end_of_trace; > > connected[t2] = true; > > @@ -1166,8 +1191,13 @@ connect_traces (int n_traces, struct tra > > } > > > > if (last_trace >= 0) > > - traces[last_trace].last->aux = traces[t2].first; > > + { > > + gcc_checking_assert (!first_in_partition > > + || ok_to_be_first (&traces[t2])); > > + traces[last_trace].last->aux = traces[t2].first; > > + } > > last_trace = t; > > + first_in_partition = false; > > > > /* Find the successor traces. */ > > while (1) > > @@ -1226,6 +1256,8 @@ connect_traces (int n_traces, struct tra > > fprintf (dump_file, "Connection: %d %d\n", > > best->src->index, best->dest->index); > > > > + gcc_checking_assert (BB_PARTITION (best->dest) > > + == BB_PARTITION (best->src)); > > t = bbd[best->dest->index].start_of_trace; > > traces[last_trace].last->aux = traces[t].first; > > connected[t] = true; > > @@ -1238,6 +1270,8 @@ connect_traces (int n_traces, struct tra > > fprintf (dump_file, "Connection: %d %d\n", > > best->src->index, best->dest->index); > > } > > + gcc_checking_assert (BB_PARTITION (best->dest) > > + == BB_PARTITION (best->src)); > > t = bbd[best->dest->index].start_of_trace; > > traces[last_trace].last->aux = traces[t].first; > > connected[t] = true; > > > > > > -- > Richard Biener <rguenther@suse.de> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
On Tue, 18 Jul 2017, Jan Hubicka wrote: > > On Tue, 18 Jul 2017, Jan Hubicka wrote: > > > > > Hi, > > > this patch fixes wrong code issue with BB partitioning where sometimes EH > > > is not delivered. This is very old issue that affect all release branches > > > with -fprofile-use, but recently surfaced as libstdc++ testsuite regression > > > because we now partition functions based on static profile prediction. > > > > > > The problem is that EH tables are stored as offsets from start of functions. > > > In the record however value 0 is special and means that there is no landing > > > pad for a given region. Normally this is safe because landing pads never > > > appear as very first label in function. This is however no longer true with > > > partitining where cold partition is actually quite likely to start by landing > > > pad. > > > > > > The change in except.c adds sanity check that no EH landing pads are very first > > > in the insn stream. The change in bb-reorder makes reorder to chose > > > non-landing-pad BB as first trace for the cold partition. Such BB always exists > > > because landing pads must be in same partition as the instruction throwing them > > > and we never make BB both landing pad and reachable by normal control folow. > > > However I am not thrilled by the fix as it is bit fragile in case some > > > optimization happends after bb partitioning and code is moved away. Also the > > > logic can be confused by asm statement which may result in no code (again > > > however the BB reachable from outside world should contain something that > > > produce EH that is a real instruction). > > > > > > Ideas for better fix would be welcome then. If the assert I added triggers > > > for valid reasons, we may just end up adding a NOP in the rare case we do > > > not suceed arranging cold partition to not start with landing pad. > > > > Yeah, I'd rather pad the function start with a nop if it starts with a > > landing pad. How difficult would it be to arrange for this? I suppose > > we'd need to touch each and every target to accomplish this? Or end up > > using gen_nop in generic code? > > I think we could just output from generic code - I think it can be done by > final_scan_insn. I don't know however if we have a way to tell if the section > starts with a landing pad? Not sure either -- some insn note / bb note? Some flag on the label? At least the latter should be easy to add if it's not there already. Richard. > Honza > > > > Richard. > > > > > Bootstrapped/regtested x86_64-linux, looks sane? > > > > > > Honza > > > > > > PR middle-end/81331 > > > * except.c (first_in_partition): New function. > > > (dw2_output_call_site_table): Sanity check that landing pads are not > > > very first in the partition. > > > * bb-reorder.c (ok_to_be_first): New function. > > > (connect_traces): Avoid traces that are !ok_to_be_first to start > > > partitions. > > > Index: except.c > > > =================================================================== > > > --- except.c (revision 250226) > > > +++ except.c (working copy) > > > @@ -2724,6 +2724,23 @@ sjlj_size_of_call_site_table (void) > > > return size; > > > } > > > > > > +/* Return true if L will appear as very first in its partition. */ > > > + > > > +bool > > > +first_in_partition (rtx_insn *l) > > > +{ > > > + while (l != NULL_RTX) > > > + { > > > + if (active_insn_p (l)) > > > + return false; > > > + else if (GET_CODE (l) == NOTE > > > + && NOTE_KIND (l) == NOTE_INSN_SWITCH_TEXT_SECTIONS) > > > + return true; > > > + l = PREV_INSN (l); > > > + } > > > + return true; > > > +} > > > + > > > static void > > > dw2_output_call_site_table (int cs_format, int section) > > > { > > > @@ -2749,8 +2766,14 @@ dw2_output_call_site_table (int cs_forma > > > ASM_GENERATE_INTERNAL_LABEL (reg_end_lab, "LEHE", call_site_base + i); > > > > > > if (cs->landing_pad) > > > - ASM_GENERATE_INTERNAL_LABEL (landing_pad_lab, "L", > > > - CODE_LABEL_NUMBER (cs->landing_pad)); > > > + { > > > + ASM_GENERATE_INTERNAL_LABEL (landing_pad_lab, "L", > > > + CODE_LABEL_NUMBER (cs->landing_pad)); > > > + /* Be sure that the offset will not be 0 as that would make EH > > > + delivery code to think that there is no landing pad. */ > > > + gcc_checking_assert (!first_in_partition > > > + (as_a <rtx_insn *> (cs->landing_pad))); > > > + } > > > > > > /* ??? Perhaps use insn length scaling if the assembler supports > > > generic arithmetic. */ > > > Index: bb-reorder.c > > > =================================================================== > > > --- bb-reorder.c (revision 250226) > > > +++ bb-reorder.c (working copy) > > > @@ -1066,6 +1066,21 @@ connect_better_edge_p (const_edge e, boo > > > return is_better_edge; > > > } > > > > > > +/* If we place EH landing pad as very first BB in the partition, its offset > > > + from start of function is 0 which is special cased by the eh table to mean > > > + no landing pad. For this reason such BBs can not appear as very first in > > > + the partition. */ > > > +static bool > > > +ok_to_be_first (struct trace *t) > > > +{ > > > + edge e; > > > + edge_iterator ei; > > > + FOR_EACH_EDGE (e, ei, t->first->preds) > > > + if (e->flags & EDGE_EH) > > > + return false; > > > + return true; > > > +} > > > + > > > /* Connect traces in array TRACES, N_TRACES is the count of traces. */ > > > > > > static void > > > @@ -1080,6 +1095,7 @@ connect_traces (int n_traces, struct tra > > > int freq_threshold; > > > gcov_type count_threshold; > > > bool for_size = optimize_function_for_size_p (cfun); > > > + bool first_in_partition; > > > > > > freq_threshold = max_entry_frequency * DUPLICATION_THRESHOLD / 1000; > > > if (max_entry_count.to_gcov_type () < INT_MAX / 1000) > > > @@ -1092,6 +1108,7 @@ connect_traces (int n_traces, struct tra > > > current_pass = 1; > > > current_partition = BB_PARTITION (traces[0].first); > > > two_passes = false; > > > + first_in_partition = true; > > > > > > if (crtl->has_bb_partition) > > > for (i = 0; i < n_traces && !two_passes; i++) > > > @@ -1116,6 +1133,7 @@ connect_traces (int n_traces, struct tra > > > current_partition = BB_COLD_PARTITION; > > > else > > > current_partition = BB_HOT_PARTITION; > > > + first_in_partition = true; > > > } > > > > > > if (connected[t]) > > > @@ -1125,6 +1143,9 @@ connect_traces (int n_traces, struct tra > > > && BB_PARTITION (traces[t].first) != current_partition) > > > continue; > > > > > > + if (first_in_partition && !ok_to_be_first (&traces[t])) > > > + continue; > > > + > > > connected[t] = true; > > > > > > /* Find the predecessor traces. */ > > > @@ -1143,7 +1164,9 @@ connect_traces (int n_traces, struct tra > > > && bbd[si].end_of_trace >= 0 > > > && !connected[bbd[si].end_of_trace] > > > && (BB_PARTITION (e->src) == current_partition) > > > - && connect_better_edge_p (e, true, best_len, best, traces)) > > > + && connect_better_edge_p (e, true, best_len, best, traces) > > > + && (!first_in_partition > > > + || ok_to_be_first (&traces[bbd[si].end_of_trace]))) > > > { > > > best = e; > > > best_len = traces[bbd[si].end_of_trace].length; > > > @@ -1151,6 +1174,8 @@ connect_traces (int n_traces, struct tra > > > } > > > if (best) > > > { > > > + gcc_checking_assert (BB_PARTITION (best->src) > > > + == BB_PARTITION (best->dest)); > > > best->src->aux = best->dest; > > > t2 = bbd[best->src->index].end_of_trace; > > > connected[t2] = true; > > > @@ -1166,8 +1191,13 @@ connect_traces (int n_traces, struct tra > > > } > > > > > > if (last_trace >= 0) > > > - traces[last_trace].last->aux = traces[t2].first; > > > + { > > > + gcc_checking_assert (!first_in_partition > > > + || ok_to_be_first (&traces[t2])); > > > + traces[last_trace].last->aux = traces[t2].first; > > > + } > > > last_trace = t; > > > + first_in_partition = false; > > > > > > /* Find the successor traces. */ > > > while (1) > > > @@ -1226,6 +1256,8 @@ connect_traces (int n_traces, struct tra > > > fprintf (dump_file, "Connection: %d %d\n", > > > best->src->index, best->dest->index); > > > > > > + gcc_checking_assert (BB_PARTITION (best->dest) > > > + == BB_PARTITION (best->src)); > > > t = bbd[best->dest->index].start_of_trace; > > > traces[last_trace].last->aux = traces[t].first; > > > connected[t] = true; > > > @@ -1238,6 +1270,8 @@ connect_traces (int n_traces, struct tra > > > fprintf (dump_file, "Connection: %d %d\n", > > > best->src->index, best->dest->index); > > > } > > > + gcc_checking_assert (BB_PARTITION (best->dest) > > > + == BB_PARTITION (best->src)); > > > t = bbd[best->dest->index].start_of_trace; > > > traces[last_trace].last->aux = traces[t].first; > > > connected[t] = true; > > > > > > > > > > -- > > Richard Biener <rguenther@suse.de> > > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg) > >
Index: except.c =================================================================== --- except.c (revision 250226) +++ except.c (working copy) @@ -2724,6 +2724,23 @@ sjlj_size_of_call_site_table (void) return size; } +/* Return true if L will appear as very first in its partition. */ + +bool +first_in_partition (rtx_insn *l) +{ + while (l != NULL_RTX) + { + if (active_insn_p (l)) + return false; + else if (GET_CODE (l) == NOTE + && NOTE_KIND (l) == NOTE_INSN_SWITCH_TEXT_SECTIONS) + return true; + l = PREV_INSN (l); + } + return true; +} + static void dw2_output_call_site_table (int cs_format, int section) { @@ -2749,8 +2766,14 @@ dw2_output_call_site_table (int cs_forma ASM_GENERATE_INTERNAL_LABEL (reg_end_lab, "LEHE", call_site_base + i); if (cs->landing_pad) - ASM_GENERATE_INTERNAL_LABEL (landing_pad_lab, "L", - CODE_LABEL_NUMBER (cs->landing_pad)); + { + ASM_GENERATE_INTERNAL_LABEL (landing_pad_lab, "L", + CODE_LABEL_NUMBER (cs->landing_pad)); + /* Be sure that the offset will not be 0 as that would make EH + delivery code to think that there is no landing pad. */ + gcc_checking_assert (!first_in_partition + (as_a <rtx_insn *> (cs->landing_pad))); + } /* ??? Perhaps use insn length scaling if the assembler supports generic arithmetic. */ Index: bb-reorder.c =================================================================== --- bb-reorder.c (revision 250226) +++ bb-reorder.c (working copy) @@ -1066,6 +1066,21 @@ connect_better_edge_p (const_edge e, boo return is_better_edge; } +/* If we place EH landing pad as very first BB in the partition, its offset + from start of function is 0 which is special cased by the eh table to mean + no landing pad. For this reason such BBs can not appear as very first in + the partition. */ +static bool +ok_to_be_first (struct trace *t) +{ + edge e; + edge_iterator ei; + FOR_EACH_EDGE (e, ei, t->first->preds) + if (e->flags & EDGE_EH) + return false; + return true; +} + /* Connect traces in array TRACES, N_TRACES is the count of traces. */ static void @@ -1080,6 +1095,7 @@ connect_traces (int n_traces, struct tra int freq_threshold; gcov_type count_threshold; bool for_size = optimize_function_for_size_p (cfun); + bool first_in_partition; freq_threshold = max_entry_frequency * DUPLICATION_THRESHOLD / 1000; if (max_entry_count.to_gcov_type () < INT_MAX / 1000) @@ -1092,6 +1108,7 @@ connect_traces (int n_traces, struct tra current_pass = 1; current_partition = BB_PARTITION (traces[0].first); two_passes = false; + first_in_partition = true; if (crtl->has_bb_partition) for (i = 0; i < n_traces && !two_passes; i++) @@ -1116,6 +1133,7 @@ connect_traces (int n_traces, struct tra current_partition = BB_COLD_PARTITION; else current_partition = BB_HOT_PARTITION; + first_in_partition = true; } if (connected[t]) @@ -1125,6 +1143,9 @@ connect_traces (int n_traces, struct tra && BB_PARTITION (traces[t].first) != current_partition) continue; + if (first_in_partition && !ok_to_be_first (&traces[t])) + continue; + connected[t] = true; /* Find the predecessor traces. */ @@ -1143,7 +1164,9 @@ connect_traces (int n_traces, struct tra && bbd[si].end_of_trace >= 0 && !connected[bbd[si].end_of_trace] && (BB_PARTITION (e->src) == current_partition) - && connect_better_edge_p (e, true, best_len, best, traces)) + && connect_better_edge_p (e, true, best_len, best, traces) + && (!first_in_partition + || ok_to_be_first (&traces[bbd[si].end_of_trace]))) { best = e; best_len = traces[bbd[si].end_of_trace].length; @@ -1151,6 +1174,8 @@ connect_traces (int n_traces, struct tra } if (best) { + gcc_checking_assert (BB_PARTITION (best->src) + == BB_PARTITION (best->dest)); best->src->aux = best->dest; t2 = bbd[best->src->index].end_of_trace; connected[t2] = true; @@ -1166,8 +1191,13 @@ connect_traces (int n_traces, struct tra } if (last_trace >= 0) - traces[last_trace].last->aux = traces[t2].first; + { + gcc_checking_assert (!first_in_partition + || ok_to_be_first (&traces[t2])); + traces[last_trace].last->aux = traces[t2].first; + } last_trace = t; + first_in_partition = false; /* Find the successor traces. */ while (1) @@ -1226,6 +1256,8 @@ connect_traces (int n_traces, struct tra fprintf (dump_file, "Connection: %d %d\n", best->src->index, best->dest->index); + gcc_checking_assert (BB_PARTITION (best->dest) + == BB_PARTITION (best->src)); t = bbd[best->dest->index].start_of_trace; traces[last_trace].last->aux = traces[t].first; connected[t] = true; @@ -1238,6 +1270,8 @@ connect_traces (int n_traces, struct tra fprintf (dump_file, "Connection: %d %d\n", best->src->index, best->dest->index); } + gcc_checking_assert (BB_PARTITION (best->dest) + == BB_PARTITION (best->src)); t = bbd[best->dest->index].start_of_trace; traces[last_trace].last->aux = traces[t].first; connected[t] = true;