Message ID | 4C3F2E42.4090605@redhat.com |
---|---|
State | New |
Headers | show |
On 15 Jul 2010, at 16:50, Richard Henderson wrote: > On 07/15/2010 12:59 AM, IainS wrote: >> unfortunately, although it applies cleanly, it doesn't bootstrap >> for me >> (against either 162180 or 162193). >> (fails in building libgomp/parallel.c with 'caller edge frequency >> 1000 >> does not match BB frequency of 61')... >> ... I will look a little more - but this is uncharted territory for >> me ;) > > Incremental patch follows. I applied emutls-7 + the increment to 162222. It is clean on cris-elf "tls.exp=*" (emutls target with alias, but no libgomp) also on i686-apple-darwin9 (emutls target without alias but with libgomp) These were done with all the test-suite additions mentioned below. ---- I believe this clears (at least): http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44132 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44137 (together with the ObjC LTO patch already applied) http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44276 ----- Note1: an updated version of this could be applied: http://gcc.gnu.org/ml/gcc-patches/2010-05/msg01738.html In fact, it could be applied anyway, the current emutls implementation works with these tests. updated patch for that attached - the changelog should still stand (+/- minor adjustments). ---- Note2: The cse test I sent you only works on darwin (m32) when the branch island patch is also applied. (although, of course, it should be valid for other targets and darwin at m64). --- Note 3: I've got a set of tls tests for ObjC++, will post those separately (I want to trim them a bit). ---- Note 4: I'll also un-xfail the ObjC tls tests when this goes in. ===== Thanks very much for working on this.... Iain ==== extended tls test-suite vs 162229.
On Thu, Jul 15, 2010 at 08:50:26AM -0700, Richard Henderson wrote: > On 07/15/2010 12:59 AM, IainS wrote: > > unfortunately, although it applies cleanly, it doesn't bootstrap for me > > (against either 162180 or 162193). > > (fails in building libgomp/parallel.c with 'caller edge frequency 1000 > > does not match BB frequency of 61')... > > ... I will look a little more - but this is uncharted territory for me ;) > > Incremental patch follows. > > > r~ Richard, Do you plan to repost the emutls_7 patch combined with this patch and corrected for bit rot? Also, is there anything left to be done with the emutls lto patch ? If not, perhaps you can just commit the current version. Thanks in advance. Jack > diff --git a/gcc/tree-emutls.c b/gcc/tree-emutls.c > index f7eb4cd..275e16b 100644 > --- a/gcc/tree-emutls.c > +++ b/gcc/tree-emutls.c > @@ -580,6 +580,54 @@ lower_emutls_phi_arg (gimple phi, unsigned int i, struct lower_emutls_data *d) > } > } > > +/* Mirror a portion of the logic from gimple_find_edge_insert_loc. > + Determine if statements inserted on E will be insertted into the > + predecessor block. We *must* match gimple_find_edge_insert_loc > + in order to pass verify_cgraph. > + > + If the destination has one predecessor and no PHI nodes, insert > + there... Except that we *know* it has PHI nodes or else there's > + nothing for us to do. Skip this step. > + > + If the source has one successor, the edge is not abnormal and the > + last statement does not end a basic block, insert there. */ > + > +static bool > +will_insert_in_edge_src (edge e) > +{ > + basic_block src = e->src; > + > + if ((e->flags & EDGE_ABNORMAL) == 0 > + && src != ENTRY_BLOCK_PTR > + && single_succ_p (src)) > + { > + gimple_stmt_iterator gsi; > + gimple last; > + > + /* If the block is empty, of course we use it. */ > + gsi = gsi_last_bb (src); > + if (gsi_end_p (gsi)) > + return true; > + > + /* If the last stmt does not end the block, we insert after. */ > + last = gsi_stmt (gsi); > + if (!stmt_ends_bb_p (last)) > + return true; > + > + /* If the last stmt is a trivial control, we insert before. */ > + switch (gimple_code (last)) > + { > + case GIMPLE_RETURN: > + case GIMPLE_RESX: > + return true; > + default: > + break; > + } > + } > + > + return false; > +} > + > /* Lower the entire function NODE. */ > > static void > @@ -605,34 +653,41 @@ lower_emutls_function_body (struct cgraph_node *node) > gimple_stmt_iterator gsi; > unsigned int i, nedge; > > - /* ??? This is the frequency of the new block that might be created by > - split_edge. One might have thought EDGE_FREQUENCY. However, this > - *must* match split_edge, or we'll fail verify_cgraph. */ > - d.bb_freq = CGRAPH_FREQ_BASE; > - > /* Lower each of the PHI nodes of the block, as we may have > propagated &tlsvar into a PHI argument. These loops are > arranged so that we process each edge at once, and each > PHI argument for that edge. */ > - > - nedge = EDGE_COUNT (d.bb->preds); > - for (i = 0; i < nedge; ++i) > + if (!gimple_seq_empty_p (phi_nodes (d.bb))) > { > - edge e = EDGE_PRED (d.bb, i); > - > - /* We can re-use any SSA_NAME created on this edge. */ > - memset (VEC_address (tree, d.accessed), 0, n_tls * sizeof(tree)); > - d.seq = NULL; > - > - for (gsi = gsi_start_phis (d.bb); !gsi_end_p (gsi); gsi_next (&gsi)) > - lower_emutls_phi_arg (gsi_stmt (gsi), i, &d); > - > - /* Insert all statements generated by all phi nodes for this > - particular edge all at once. */ > - if (d.seq) > + nedge = EDGE_COUNT (d.bb->preds); > + for (i = 0; i < nedge; ++i) > { > - gsi_insert_seq_on_edge (e, d.seq); > - any_edge_inserts = true; > + edge e = EDGE_PRED (d.bb, i); > + > + /* Choose the correct frequency for stmts to be > + insertted on this edge. */ > + if (will_insert_in_edge_src (e)) > + d.bb_freq = (compute_call_stmt_bb_frequency > + (current_function_decl, e->src)); > + else > + d.bb_freq = EDGE_FREQUENCY (e); > + > + /* We can re-use any SSA_NAME created on this edge. */ > + memset (VEC_address (tree, d.accessed), 0, n_tls * sizeof(tree)); > + d.seq = NULL; > + > + for (gsi = gsi_start_phis (d.bb); > + !gsi_end_p (gsi); > + gsi_next (&gsi)) > + lower_emutls_phi_arg (gsi_stmt (gsi), i, &d); > + > + /* Insert all statements generated by all phi nodes for this > + particular edge all at once. */ > + if (d.seq) > + { > + gsi_insert_seq_on_edge (e, d.seq); > + any_edge_inserts = true; > + } > } > } >
diff --git a/gcc/tree-emutls.c b/gcc/tree-emutls.c index f7eb4cd..275e16b 100644 --- a/gcc/tree-emutls.c +++ b/gcc/tree-emutls.c @@ -580,6 +580,54 @@ lower_emutls_phi_arg (gimple phi, unsigned int i, struct lower_emutls_data *d) } } +/* Mirror a portion of the logic from gimple_find_edge_insert_loc. + Determine if statements inserted on E will be insertted into the + predecessor block. We *must* match gimple_find_edge_insert_loc + in order to pass verify_cgraph. + + If the destination has one predecessor and no PHI nodes, insert + there... Except that we *know* it has PHI nodes or else there's + nothing for us to do. Skip this step. + + If the source has one successor, the edge is not abnormal and the + last statement does not end a basic block, insert there. */ + +static bool +will_insert_in_edge_src (edge e) +{ + basic_block src = e->src; + + if ((e->flags & EDGE_ABNORMAL) == 0 + && src != ENTRY_BLOCK_PTR + && single_succ_p (src)) + { + gimple_stmt_iterator gsi; + gimple last; + + /* If the block is empty, of course we use it. */ + gsi = gsi_last_bb (src); + if (gsi_end_p (gsi)) + return true; + + /* If the last stmt does not end the block, we insert after. */ + last = gsi_stmt (gsi); + if (!stmt_ends_bb_p (last)) + return true; + + /* If the last stmt is a trivial control, we insert before. */ + switch (gimple_code (last)) + { + case GIMPLE_RETURN: + case GIMPLE_RESX: + return true; + default: + break; + } + } + + return false; +} + /* Lower the entire function NODE. */ static void @@ -605,34 +653,41 @@ lower_emutls_function_body (struct cgraph_node *node) gimple_stmt_iterator gsi; unsigned int i, nedge; - /* ??? This is the frequency of the new block that might be created by - split_edge. One might have thought EDGE_FREQUENCY. However, this - *must* match split_edge, or we'll fail verify_cgraph. */ - d.bb_freq = CGRAPH_FREQ_BASE; - /* Lower each of the PHI nodes of the block, as we may have propagated &tlsvar into a PHI argument. These loops are arranged so that we process each edge at once, and each PHI argument for that edge. */ - - nedge = EDGE_COUNT (d.bb->preds); - for (i = 0; i < nedge; ++i) + if (!gimple_seq_empty_p (phi_nodes (d.bb))) { - edge e = EDGE_PRED (d.bb, i); - - /* We can re-use any SSA_NAME created on this edge. */ - memset (VEC_address (tree, d.accessed), 0, n_tls * sizeof(tree)); - d.seq = NULL; - - for (gsi = gsi_start_phis (d.bb); !gsi_end_p (gsi); gsi_next (&gsi)) - lower_emutls_phi_arg (gsi_stmt (gsi), i, &d); - - /* Insert all statements generated by all phi nodes for this - particular edge all at once. */ - if (d.seq) + nedge = EDGE_COUNT (d.bb->preds); + for (i = 0; i < nedge; ++i) { - gsi_insert_seq_on_edge (e, d.seq); - any_edge_inserts = true; + edge e = EDGE_PRED (d.bb, i); + + /* Choose the correct frequency for stmts to be + insertted on this edge. */ + if (will_insert_in_edge_src (e)) + d.bb_freq = (compute_call_stmt_bb_frequency + (current_function_decl, e->src)); + else + d.bb_freq = EDGE_FREQUENCY (e); + + /* We can re-use any SSA_NAME created on this edge. */ + memset (VEC_address (tree, d.accessed), 0, n_tls * sizeof(tree)); + d.seq = NULL; + + for (gsi = gsi_start_phis (d.bb); + !gsi_end_p (gsi); + gsi_next (&gsi)) + lower_emutls_phi_arg (gsi_stmt (gsi), i, &d); + + /* Insert all statements generated by all phi nodes for this + particular edge all at once. */ + if (d.seq) + { + gsi_insert_seq_on_edge (e, d.seq); + any_edge_inserts = true; + } } }