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