> Hi,
> this fixes two issues with the new multi-target speculation code which
> reproduce
> on Firefox. I can now build firefox with FDO locally but on Mozilla build
> bots
> it still fails with ICE in speculative_call_info.
>
> One problem is that speuclative code compares call_stmt and lto_stmt_uid in
> a way that may get unwanted effect when these gets out of sync. It does not
> make sense to have both non-zero so I added code clearing it and sanity check
> that it is kept this way.
>
> Other problem is cgraph_edge::make_direct not working well with multiple
> targets. In this case it removed one speuclative target and the indirect call
> leaving other targets in the tree.
>
> This is fixed by iterating across all targets and removing all except the good
> one (if it exists).
>
> I will see if I can reproduce the other issue.
>
> lto-bootstrapped/regtested x86_64-linux. I plan to commit it after bit of
> extra
> testing.
>
> 2020-01-19 Jan Hubicka <hubi...@ucw.cz>
>
> PR lto/93318
> * cgraph.c (cgraph_edge::resolve_speculation): Fix foramting.
> (cgraph_edge::make_direct): Remove all indirect targets.
> (cgraph_edge::redirect_call_stmt_to_callee): Use make_direct..
> (cgraph_node::verify_node): Verify that only one call_stmt or
> lto_stmt_uid is set.
> * cgraphclones.c (cgraph_edge::clone): Set only one call_stmt or
> lto_stmt_uid.
> * lto-cgraph.c (lto_output_edge): Simplify streaming of stmt.
> (lto_output_ref): Simplify streaming of stmt.
> * lto-streamer-in.c (fixup_call_stmt_edges_1): Clear lto_stmt_uid.
Hi,
this is variant of patch I comitted. There was one wrong sanity check
triggering ICE when speculation was resolved to different target then
predicted.
PR lto/93318
* cgraph.c (cgraph_edge::resolve_speculation): Fix foramting.
(cgraph_edge::make_direct): Remove all indirect targets.
(cgraph_edge::redirect_call_stmt_to_callee): Use make_direct..
(cgraph_node::verify_node): Verify that only one call_stmt or
lto_stmt_uid is set.
* cgraphclones.c (cgraph_edge::clone): Set only one call_stmt or
lto_stmt_uid.
* lto-cgraph.c (lto_output_edge): Simplify streaming of stmt.
(lto_output_ref): Simplify streaming of stmt.
* lto-streamer-in.c (fixup_call_stmt_edges_1): Clear lto_stmt_uid.
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 95b523d6be5..c442f334fe2 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -1226,8 +1226,8 @@ cgraph_edge::resolve_speculation (cgraph_edge *edge, tree
callee_decl)
fprintf (dump_file, "Speculative call turned into direct call.\n");
edge = e2;
e2 = tmp;
- /* FIXME: If EDGE is inlined, we should scale up the frequencies and
counts
- in the functions inlined through it. */
+ /* FIXME: If EDGE is inlined, we should scale up the frequencies
+ and counts in the functions inlined through it. */
}
edge->count += e2->count;
if (edge->num_speculative_call_targets_p ())
@@ -1263,11 +1263,52 @@ cgraph_edge::make_direct (cgraph_edge *edge,
cgraph_node *callee)
/* If we are redirecting speculative call, make it non-speculative. */
if (edge->speculative)
{
- edge = resolve_speculation (edge, callee->decl);
+ cgraph_edge *found = NULL;
+ cgraph_edge *direct, *next;
+ ipa_ref *ref;
+
+ edge->speculative_call_info (direct, edge, ref);
- /* On successful speculation just return the pre existing direct edge.
*/
- if (!edge->indirect_unknown_callee)
- return edge;
+ /* Look all speculative targets and remove all but one corresponding
+ to callee (if it exists).
+ If there is only one target we can save one extra call to
+ speculative_call_info. */
+ if (edge->num_speculative_call_targets_p () != 1)
+ for (direct = edge->caller->callees; direct; direct = next)
+ {
+ next = direct->next_callee;
+ if (direct->call_stmt == edge->call_stmt
+ && direct->lto_stmt_uid == edge->lto_stmt_uid)
+ {
+ direct->speculative_call_info (direct, edge, ref);
+
+ /* Compare ref not direct->callee. Direct edge is possibly
+ inlined or redirected. */
+ if (!ref->referred->semantically_equivalent_p (callee))
+ edge = direct->resolve_speculation (direct, NULL);
+ else
+ {
+ gcc_checking_assert (!found);
+ found = direct;
+ }
+ }
+ }
+ else if (!ref->referred->semantically_equivalent_p (callee))
+ edge = direct->resolve_speculation (direct, NULL);
+ else
+ found = direct;
+
+ /* On successful speculation just remove the indirect edge and
+ return the pre existing direct edge.
+ It is important to not remove it and redirect because the direct
+ edge may be inlined or redirected. */
+ if (found)
+ {
+ resolve_speculation (edge, callee->decl);
+ gcc_checking_assert (!found->speculative);
+ return found;
+ }
+ gcc_checking_assert (!edge->speculative);
}
edge->indirect_unknown_callee = 0;
@@ -1328,7 +1369,7 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e)
/* If there already is an direct call (i.e. as a result of inliner's
substitution), forget about speculating. */
if (decl)
- e = resolve_speculation (e, decl);
+ e = make_direct (e, cgraph_node::get (decl));
else
{
/* Expand speculation into GIMPLE code. */
@@ -3116,6 +3157,8 @@ cgraph_node::verify_node (void)
basic_block this_block;
gimple_stmt_iterator gsi;
bool error_found = false;
+ int i;
+ ipa_ref *ref = NULL;
if (seen_error ())
return;
@@ -3201,6 +3244,11 @@ cgraph_node::verify_node (void)
cgraph_debug_gimple_stmt (this_cfun, e->call_stmt);
error_found = true;
}
+ if (e->call_stmt && e->lto_stmt_uid)
+ {
+ error ("edge has both cal_stmt and lto_stmt_uid set");
+ error_found = true;
+ }
}
bool check_comdat = comdat_local_p ();
for (e = callers; e; e = e->next_caller)
@@ -3267,6 +3315,11 @@ cgraph_node::verify_node (void)
fprintf (stderr, "\n");
error_found = true;
}
+ if (e->call_stmt && e->lto_stmt_uid)
+ {
+ error ("edge has both cal_stmt and lto_stmt_uid set");
+ error_found = true;
+ }
}
for (e = indirect_calls; e; e = e->next_callee)
{
@@ -3398,8 +3451,6 @@ cgraph_node::verify_node (void)
if (this_cfun->cfg)
{
hash_set<gimple *> stmts;
- int i;
- ipa_ref *ref = NULL;
/* Reach the trees by walking over the CFG, and note the
enclosing basic-blocks in the call edges. */
@@ -3468,6 +3519,13 @@ cgraph_node::verify_node (void)
/* No CFG available?! */
gcc_unreachable ();
+ for (i = 0; iterate_reference (i, ref); i++)
+ if (ref->stmt && ref->lto_stmt_uid)
+ {
+ error ("reference has both cal_stmt and lto_stmt_uid set");
+ error_found = true;
+ }
+
for (e = callees; e; e = e->next_callee)
{
if (!e->aux && !e->speculative)
diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index e73e0696b91..417488bca1f 100644
--- a/gcc/cgraphclones.c
+++ b/gcc/cgraphclones.c
@@ -132,7 +132,8 @@ cgraph_edge::clone (cgraph_node *n, gcall *call_stmt,
unsigned stmt_uid,
new_edge->inline_failed = inline_failed;
new_edge->indirect_inlining_edge = indirect_inlining_edge;
- new_edge->lto_stmt_uid = stmt_uid;
+ if (!call_stmt)
+ new_edge->lto_stmt_uid = stmt_uid;
new_edge->speculative_id = speculative_id;
/* Clone flags that depend on call_stmt availability manually. */
new_edge->can_throw_external = can_throw_external;
diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
index 621607499e1..b0c7ebf775b 100644
--- a/gcc/lto-cgraph.c
+++ b/gcc/lto-cgraph.c
@@ -257,10 +257,11 @@ lto_output_edge (struct lto_simple_output_block *ob,
struct cgraph_edge *edge,
edge->count.stream_out (ob->main_stream);
bp = bitpack_create (ob->main_stream);
- uid = (!gimple_has_body_p (edge->caller->decl) || edge->caller->thunk.thunk_p
- ? edge->lto_stmt_uid : gimple_uid (edge->call_stmt) + 1);
+ uid = !edge->call_stmt ? edge->lto_stmt_uid
+ : gimple_uid (edge->call_stmt) + 1;
bp_pack_enum (&bp, cgraph_inline_failed_t,
CIF_N_REASONS, edge->inline_failed);
+ gcc_checking_assert (uid || edge->caller->thunk.thunk_p);
bp_pack_var_len_unsigned (&bp, uid);
bp_pack_value (&bp, edge->speculative_id, 16);
bp_pack_value (&bp, edge->indirect_inlining_edge, 1);
@@ -669,7 +670,7 @@ lto_output_ref (struct lto_simple_output_block *ob, struct
ipa_ref *ref,
{
struct bitpack_d bp;
int nref;
- int uid = ref->lto_stmt_uid;
+ int uid = !ref->stmt ? ref->lto_stmt_uid : gimple_uid (ref->stmt) + 1;
struct cgraph_node *node;
bp = bitpack_create (ob->main_stream);
diff --git a/gcc/lto-streamer-in.c b/gcc/lto-streamer-in.c
index 3e64371094e..9566e5ee102 100644
--- a/gcc/lto-streamer-in.c
+++ b/gcc/lto-streamer-in.c
@@ -892,6 +892,7 @@ fixup_call_stmt_edges_1 (struct cgraph_node *node, gimple
**stmts,
fatal_error (input_location,
"Cgraph edge statement index out of range");
cedge->call_stmt = as_a <gcall *> (stmts[cedge->lto_stmt_uid - 1]);
+ cedge->lto_stmt_uid = 0;
if (!cedge->call_stmt)
fatal_error (input_location,
"Cgraph edge statement index not found");
@@ -902,6 +903,7 @@ fixup_call_stmt_edges_1 (struct cgraph_node *node, gimple
**stmts,
fatal_error (input_location,
"Cgraph edge statement index out of range");
cedge->call_stmt = as_a <gcall *> (stmts[cedge->lto_stmt_uid - 1]);
+ cedge->lto_stmt_uid = 0;
if (!cedge->call_stmt)
fatal_error (input_location, "Cgraph edge statement index not found");
}
@@ -912,6 +914,7 @@ fixup_call_stmt_edges_1 (struct cgraph_node *node, gimple
**stmts,
fatal_error (input_location,
"Reference statement index out of range");
ref->stmt = stmts[ref->lto_stmt_uid - 1];
+ ref->lto_stmt_uid = 0;
if (!ref->stmt)
fatal_error (input_location, "Reference statement index not found");
}
