From bf6fc129c0568da74270474a1cf0737120796893 Mon Sep 17 00:00:00 2001 From: Jan Hubicka Date: Sun, 19 Jan 2020 16:41:11 +0100 Subject: [PATCH] Implement speculative call verifier this patch implements verifier and fixes one bug where speculative calls produced by ipa-devirt ended up having num_speculative_call_targets = 0 instead of 1. * cgraph.c (cgraph_edge::make_speculative): Increase number of speculative targets. (verify_speculative_call): New function (cgraph_node::verify_node): Use it. * ipa-profile.c (ipa_profile): Fix formating; do not set number of speculations. --- gcc/ChangeLog | 10 +++ gcc/cgraph.c | 151 ++++++++++++++++++++++++++++++++++++++-- gcc/ipa-profile.c | 174 +++++++++++++++++++++++----------------------- 3 files changed, 241 insertions(+), 94 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 0e24b4a6472..a50847992f9 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,13 @@ +2020-01-18 Jan Hubicka + + PR lto/93318 + * cgraph.c (cgraph_edge::make_speculative): Increase number of + speculative targets. + (verify_speculative_call): New function + (cgraph_node::verify_node): Use it. + * ipa-profile.c (ipa_profile): Fix formating; do not set number of + speculations. + 2020-01-18 Jan Hubicka PR lto/93318 diff --git a/gcc/cgraph.c b/gcc/cgraph.c index c442f334fe2..187f6ed30ba 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -1076,6 +1076,7 @@ cgraph_edge::make_speculative (cgraph_node *n2, profile_count direct_count, e2->speculative_id = speculative_id; e2->target_prob = target_prob; e2->in_polymorphic_cdtor = in_polymorphic_cdtor; + indirect_info->num_speculative_call_targets++; count -= e2->count; symtab->call_edge_duplication_hooks (this, e2); ref = n->create_reference (n2, IPA_REF_ADDR, call_stmt); @@ -3148,6 +3149,128 @@ cgraph_edge::verify_corresponds_to_fndecl (tree decl) # pragma GCC diagnostic ignored "-Wformat-diag" #endif +/* Verify consistency of speculative call in NODE corresponding to STMT + and LTO_STMT_UID. If INDIRECT is set, assume that it is the indirect + edge of call sequence. Return true if error is found. + + This function is called to every component of indirect call (direct edges, + indirect edge and refs). To save duplicated work, do full testing only + in that case. */ +static bool +verify_speculative_call (struct cgraph_node *node, gimple *stmt, + unsigned int lto_stmt_uid, + struct cgraph_edge *indirect) +{ + if (indirect == NULL) + { + for (indirect = node->indirect_calls; indirect; + indirect = indirect->next_callee) + if (indirect->call_stmt == stmt + && indirect->lto_stmt_uid == lto_stmt_uid) + break; + if (!indirect) + { + error ("missing indirect call in speculative call sequence"); + return true; + } + if (!indirect->speculative) + { + error ("indirect call in speculative call sequence has no " + "speculative flag"); + return true; + } + return false; + } + + /* Maximal number of targets. We probably will never want to have more than + this. */ + const unsigned int num = 256; + cgraph_edge *direct_calls[num]; + ipa_ref *refs[num]; + + for (unsigned int i = 0; i < num; i++) + { + direct_calls[i] = NULL; + refs[i] = NULL; + } + + for (cgraph_edge *direct = node->callees; direct; + direct = direct->next_callee) + if (direct->call_stmt == stmt && direct->lto_stmt_uid == lto_stmt_uid) + { + if (!direct->speculative) + { + error ("direct call to %s in speculative call sequence has no " + "speculative flag", direct->callee->dump_name ()); + return true; + } + if (direct->speculative_id >= num) + { + error ("direct call to %s in speculative call sequence has " + "speculative_uid %i out of range", + direct->callee->dump_name (), direct->speculative_id); + return true; + } + if (direct_calls[direct->speculative_id]) + { + error ("duplicate direct call to %s in speculative call sequence " + "with speculative_uid %i", + direct->callee->dump_name (), direct->speculative_id); + return true; + } + direct_calls[direct->speculative_id] = direct; + } + + ipa_ref *ref; + for (int i = 0; node->iterate_reference (i, ref); i++) + if (ref->speculative + && ref->stmt == stmt && ref->lto_stmt_uid == lto_stmt_uid) + { + if (ref->speculative_id >= num) + { + error ("direct call to %s in speculative call sequence has " + "speculative_uid %i out of range", + ref->referred->dump_name (), ref->speculative_id); + return true; + } + if (refs[ref->speculative_id]) + { + error ("duplicate reference %s in speculative call sequence " + "with speculative_uid %i", + ref->referred->dump_name (), ref->speculative_id); + return true; + } + refs[ref->speculative_id] = ref; + } + + int num_targets = 0; + for (unsigned int i = 0 ; i < num ; i++) + { + if (refs[i] && !direct_calls[i]) + { + error ("missing direct call for speculation %i", i); + return true; + } + if (!refs[i] && direct_calls[i]) + { + error ("missing ref for speculation %i", i); + return true; + } + if (refs[i] != NULL) + num_targets++; + } + + if (num_targets != indirect->num_speculative_call_targets_p ()) + { + error ("number of speculative targets %i mismatched with " + "num_speculative_targets %i", + num_targets, + indirect->num_speculative_call_targets_p ()); + return true; + } + return false; +} + /* Verify cgraph nodes of given cgraph node. */ DEBUG_FUNCTION void cgraph_node::verify_node (void) @@ -3320,6 +3443,10 @@ cgraph_node::verify_node (void) error ("edge has both cal_stmt and lto_stmt_uid set"); error_found = true; } + if (e->speculative + && verify_speculative_call (e->caller, e->call_stmt, e->lto_stmt_uid, + NULL)) + error_found = true; } for (e = indirect_calls; e; e = e->next_callee) { @@ -3342,7 +3469,24 @@ cgraph_node::verify_node (void) fprintf (stderr, "\n"); error_found = true; } + if (e->speculative + && verify_speculative_call (e->caller, e->call_stmt, e->lto_stmt_uid, + e)) + error_found = true; } + 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; + } + if (ref->speculative + && verify_speculative_call (this, ref->stmt, + ref->lto_stmt_uid, NULL)) + error_found = true; + } + if (!callers && inlined_to) { error ("inlined_to pointer is set but no predecessors found"); @@ -3519,13 +3663,6 @@ 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/ipa-profile.c b/gcc/ipa-profile.c index 03272f20987..670d9e2fb73 100644 --- a/gcc/ipa-profile.c +++ b/gcc/ipa-profile.c @@ -864,104 +864,104 @@ ipa_profile (void) } unsigned speculative_id = 0; - bool speculative_found = false; for (unsigned i = 0; i < spec_count; i++) - { - speculative_call_target item - = csum->speculative_call_targets[i]; - n2 = find_func_by_profile_id (item.target_id); - if (n2) { - if (dump_file) - { - fprintf (dump_file, "Indirect call -> direct call from" - " other module %s => %s, prob %3.2f\n", - n->dump_name (), - n2->dump_name (), - item.target_probability - / (float) REG_BR_PROB_BASE); - } - if (item.target_probability < REG_BR_PROB_BASE / 2) - { - nuseless++; - if (dump_file) - fprintf (dump_file, - "Not speculating: probability is too low.\n"); - } - else if (!e->maybe_hot_p ()) - { - nuseless++; - if (dump_file) - fprintf (dump_file, - "Not speculating: call is cold.\n"); - } - else if (n2->get_availability () <= AVAIL_INTERPOSABLE - && n2->can_be_discarded_p ()) - { - nuseless++; - if (dump_file) - fprintf (dump_file, - "Not speculating: target is overwritable " - "and can be discarded.\n"); - } - else if (!check_argument_count (n2, e)) + speculative_call_target item + = csum->speculative_call_targets[i]; + n2 = find_func_by_profile_id (item.target_id); + if (n2) { - nmismatch++; if (dump_file) - fprintf (dump_file, - "Not speculating: " - "parameter count mismatch\n"); + { + fprintf (dump_file, + "Indirect call -> direct call from" + " other module %s => %s, prob %3.2f\n", + n->dump_name (), + n2->dump_name (), + item.target_probability + / (float) REG_BR_PROB_BASE); + } + if (item.target_probability < REG_BR_PROB_BASE / 2) + { + nuseless++; + if (dump_file) + fprintf (dump_file, + "Not speculating: " + "probability is too low.\n"); + } + else if (!e->maybe_hot_p ()) + { + nuseless++; + if (dump_file) + fprintf (dump_file, + "Not speculating: call is cold.\n"); + } + else if (n2->get_availability () <= AVAIL_INTERPOSABLE + && n2->can_be_discarded_p ()) + { + nuseless++; + if (dump_file) + fprintf (dump_file, + "Not speculating: target is overwritable " + "and can be discarded.\n"); + } + else if (!check_argument_count (n2, e)) + { + nmismatch++; + if (dump_file) + fprintf (dump_file, + "Not speculating: " + "parameter count mismatch\n"); + } + else if (e->indirect_info->polymorphic + && !opt_for_fn (n->decl, flag_devirtualize) + && !possible_polymorphic_call_target_p (e, n2)) + { + nimpossible++; + if (dump_file) + fprintf (dump_file, + "Not speculating: " + "function is not in the polymorphic " + "call target list\n"); + } + else + { + /* Target may be overwritable, but profile says that + control flow goes to this particular implementation + of N2. Speculate on the local alias to allow + inlining. */ + if (!n2->can_be_discarded_p ()) + { + cgraph_node *alias; + alias = dyn_cast + (n2->noninterposable_alias ()); + if (alias) + n2 = alias; + } + nconverted++; + e->make_speculative (n2, + e->count.apply_probability ( + item.target_probability), + speculative_id, + item.target_probability); + update = true; + speculative_id++; + } } - else if (e->indirect_info->polymorphic - && !opt_for_fn (n->decl, flag_devirtualize) - && !possible_polymorphic_call_target_p (e, n2)) + else { - nimpossible++; if (dump_file) fprintf (dump_file, - "Not speculating: " - "function is not in the polymorphic " - "call target list\n"); - } - else - { - /* Target may be overwritable, but profile says that - control flow goes to this particular implementation - of N2. Speculate on the local alias to allow inlining. - */ - if (!n2->can_be_discarded_p ()) - { - cgraph_node *alias; - alias = dyn_cast (n2->noninterposable_alias ()); - if (alias) - n2 = alias; - } - nconverted++; - e->make_speculative (n2, - e->count.apply_probability ( - item.target_probability), - speculative_id, - item.target_probability); - update = true; - speculative_id++; - speculative_found = true; + "Function with profile-id %i not found.\n", + item.target_id); + nunknown++; } } - else - { - if (dump_file) - fprintf (dump_file, "Function with profile-id %i not found.\n", - item.target_id); - nunknown++; - } - } - if (speculative_found) - e->indirect_info->num_speculative_call_targets = speculative_id; } - } - if (update) - ipa_update_overall_fn_summary (n); - } + } + if (update) + ipa_update_overall_fn_summary (n); + } if (node_map_initialized) del_node_map (); if (dump_file && nindirect) -- 2.30.2