| Message ID | 9f5c65f3-7bc6-7b60-0bb3-483ad56215ce@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Ping*2: [PATCH v5] Missed function specialization + partial devirtualization | expand |
> PR ipa/69678 > * cgraph.c (symbol_table::create_edge): Init speculative_id. > (cgraph_edge::make_speculative): Add param for setting speculative_id. > (cgraph_edge::speculative_call_info): Find reference by > speculative_id for multiple indirect targets. > (cgraph_edge::resolve_speculation): Decrease the speculations > for indirect edge, drop it's speculative if not direct target > left. > (cgraph_edge::redirect_call_stmt_to_callee): Likewise. > (cgraph_node::verify_node): Don't report error if speculative > edge not include statement. > (cgraph_edge::has_multiple_indirect_call_p): New function. > (cgraph_edge::has_indirect_call_p): New function. > * cgraph.h (struct indirect_target_info): New struct. > (indirect_call_targets): New vector variable. > (make_speculative): Add param for setting speculative_id. > (cgraph_edge::has_multiple_indirect_call_p): New declare. > (cgraph_edge::has_indirect_call_p): New declare. > (speculative_id): New variable. > * cgraphclones.c (cgraph_node::create_clone): Clone speculative_id. > * cgraphunit.c: Fix comments typo. > * ipa-comdats.c: Fix comments typo. > * ipa-inline.c (inline_small_functions): Fix iterator update. > * ipa-profile.c (ipa_profile_generate_summary): Add indirect > multiple targets logic. > (ipa_profile): Likewise. > * ipa-ref.h (speculative_id): New variable. > * ipa.c (process_references): Fix typo. > * lto-cgraph.c (lto_output_edge): Add indirect multiple targets > logic. Stream out speculative_id. > (input_edge): Likewise. > * predict.c (dump_prediction): Revome edges count assert to be > precise. > * symtab.c (symtab_node::create_reference): Init speculative_id. > (symtab_node::clone_references): Clone speculative_id. > (symtab_node::clone_referring): Clone speculative_id. > (symtab_node::clone_reference): Clone speculative_id. > (symtab_node::clear_stmts_in_references): Clear speculative_id. > * tree-inline.c (copy_bb): Duplicate all the speculative edges > if indirect call contains multiple speculative targets. > * tree-profile.c (gimple_gen_ic_profiler): Use the new variable > __gcov_indirect_call.counters and __gcov_indirect_call.callee. > (gimple_gen_ic_func_profiler): Likewise. > (pass_ipa_tree_profile::gate): Fix comment typos. > * value-prof.h (check_ic_target): Remove. > * value-prof.c (gimple_value_profile_transformations): > Use void function gimple_ic_transform. > * value-prof.c (gimple_ic_transform): Handle topn case. > Fix comment typos. Change it to a void function. > > gcc/testsuite/ChangeLog > > 2019-11-14 Xiong Hu Luo <luoxhu@linux.ibm.com> > > PR ipa/69678 > * gcc.dg/tree-prof/indir-call-prof-topn.c: New testcase. > * gcc.dg/tree-prof/crossmodule-indir-call-topn-1.c: New testcase. > * gcc.dg/tree-prof/crossmodule-indir-call-topn-1a.c: New testcase. > * gcc.dg/tree-prof/crossmodule-indir-call-topn-2.c: New testcase. > * lib/scandump.exp: Dump executable file name. > * lib/scanwpaipa.exp: New scan-pgo-wap-ipa-dump. > @@ -1089,6 +1093,38 @@ cgraph_edge::make_speculative (cgraph_node *n2, profile_count direct_count) > call) and if one of them exists, all of them must exist. > > Given speculative call edge, return all three components. > + > + For some indirect edge, it may maps to multiple direct edges, i.e. 1:N. > + check the speculative_id to return all the three components for specified > + direct edge or indirect edge. > + If input is indirect, caller of this function will get the direct edge one by > + one, get_edge will just return one of the direct edge mapped to the indirect > + edge, the returned direct edge will be resolved or redirected by the caller, > + then number of indirect calls (speculations) is deceased in each access. > + If input is direct, this function will get the indirect edge and reference > + with matched speculative_id, the returned edge will also be resolved or > + redirected, decrease the speculations accordingly. > + Speculations of indirect edge will be dropped only if all direct edges > + be handled. > + > + e.g. for indirect edge E statement "call call_dest": > + > + Redirect N3 after redirected N2: > + > + if (call_dest == N2) > + n2 (); > + else if (call_dest == N3) > + n3 (); > + else > + call call_dest > + > + Resolve N3 and only redirect N2: > + > + if (call_dest == N2) > + n2 (); > + else > + call call_dest > + I find this comment hard to read. Reader probably does not know what speculative edges are and we only want to describe speculative_call_info function not also the way we resolve calls. So what about something like this: Speculative calls represent a transformation of indirect calls which may be later inserted into gimple in the following form: if (call_dest == target1) target1 (); else if (call_dest == target2) target (); else call_dest (); This is a win in case target1 and target2 are common values for call_dest as determined by ipa-devirt or indirect call profiling. In particular this may enable inlining and other optimizations. Speculative call consists of the following main components: 1) One or more direct call 2) One or more IPA_REF_ADDR references (representing the fact that code above takes address of target1 and target2) 3) The fallback indirect call Direct calls and corresponidng references are linked by speculative_id. speculative_call_info returns tripple (direct_call, IPA_REF_ADDR reference, indirect call) when called on one edge participating in the speculative call. If called on direct call its corresponding IPA_REF_ADDR is returned. If caled on indirect call it will return one of direct edges and its matching IPA_REF_ADDR. All direct calls corresponding to a given speculative call can be enumerated by indirect_edge->indirect_info->indirect_call_targets > */ > > void > @@ -1128,7 +1164,7 @@ cgraph_edge::speculative_call_info (cgraph_edge *&direct, > > reference = NULL; > for (i = 0; e->caller->iterate_reference (i, ref); i++) > - if (ref->speculative > + if (ref->speculative && ref->speculative_id == e->speculative_id > && ((ref->stmt && ref->stmt == e->call_stmt) > || (!ref->stmt && ref->lto_stmt_uid == e->lto_stmt_uid))) > { > @@ -1189,7 +1225,21 @@ cgraph_edge::resolve_speculation (tree callee_decl) Rewrite comment here to make clear what resolve_speculation does for multiple targets. > in the functions inlined through it. */ > } > edge->count += e2->count; > - edge->speculative = false; > + /* edge is indirect, e2 is direct. If edge contains multiple speculations, > + remove one of speculations for this indirect edge, then if edge still > + contains direct target, keep the speculation, next direct target > + will continue use it. Give up speculation completely if no direct > + target is left for this indirect edge. */ > + if (edge->has_indirect_call_p ()) > + { > + /* As the direct targets are sorted by decrease, delete the first target > + when it is resolved. */ > + edge->indirect_info->indirect_call_targets->ordered_remove (0); > + if (edge->indirect_info->indirect_call_targets->is_empty ()) > + edge->speculative = false; > + } > + else > + edge->speculative = false; > e2->speculative = false; > ref->remove_reference (); > if (e2->indirect_unknown_callee || e2->inline_failed) > @@ -1297,7 +1347,21 @@ cgraph_edge::redirect_call_stmt_to_callee (void) > e->caller->set_call_stmt_including_clones (e->call_stmt, new_stmt, > false); > e->count = gimple_bb (e->call_stmt)->count; > - e2->speculative = false; > + /* edge is direct, e2 is indirect here. If e2 contains multiple > + speculations, remove one of speculations for this indirect edge, > + then if e2 still contains direct target, keep the speculation, > + next direct target will continue use it. Give up speculation > + completely if no direct target is left for this indirect e2. */ > + if (e2->has_indirect_call_p ()) > + { > + /* As the direct targets are sorted by decrease, delete the first > + target when it is redirected. */ > + e2->indirect_info->indirect_call_targets->ordered_remove (0); > + if (e2->indirect_info->indirect_call_targets->is_empty ()) > + e2->speculative = false; > + } > + else > + e2->speculative = false; I think this also needs better explanation (and especially in the comment in the front of function) > @@ -3696,6 +3760,22 @@ cgraph_edge::possibly_call_in_translation_unit_p (void) > return node->get_availability () >= AVAIL_INTERPOSABLE; > } > > +/* Return true if this edge has multiple indirect call targets. */ > +bool > +cgraph_edge::has_multiple_indirect_call_p (void) Probaby better as multiple_speculative_call_targets_p It seems to me that here you use indirect_call_targets array which does not make much sense once the indirect edge is turned into speculative call. This vector should be freed after that and as I write late it should not be in the callgraph itself but rather in a ipa-profile summary. (I know that common_target_probability and common_target_id is there right now but it is code predating summaries and it does not belong there anymore) I would simply add "num_speculative_call_targets" into cgraph_indirect_call_info datastructure. > +{ > + return (indirect_info && indirect_info->indirect_call_targets > + && indirect_info->indirect_call_targets->length () > 1); Maybe it would make sense to call this speculative_call_targets? Since they are after all direct calls... > +} > + > +/* Return true if this edge has at least one indirect call target. */ > +bool > +cgraph_edge::has_indirect_call_p (void) > +{ > + return (indirect_info && indirect_info->indirect_call_targets > + && !indirect_info->indirect_call_targets->is_empty ()); How this is different from speculative flag on an indirect call? > +} > + > /* A stashed copy of "symtab" for use by selftest::symbol_table_test. > This needs to be a global so that it can be a GC root, and thus > prevent the stashed copy from being garbage-collected if the GC runs > diff --git a/gcc/cgraph.h b/gcc/cgraph.h > index a4f14743f00..1299702788d 100644 > --- a/gcc/cgraph.h > +++ b/gcc/cgraph.h > @@ -1636,6 +1636,21 @@ private: > void make_speculative (tree otr_type = NULL); > }; > > +/* Structure containing indirect target information from profile. */ > + > +struct GTY (()) indirect_target_info Probably better as speculative_call_target? > +{ > + indirect_target_info (unsigned int id, int prob) > + : common_target_id (id), common_target_probability (prob) > + { > + } > + > + /* Profile_id of common target obtained from profile. */ > + unsigned int common_target_id; > + /* Probability that call will land in function with COMMON_TARGET_ID. */ > + int common_target_probability; I think this would be more like frequent then common, but perhaps target_id and target_probablity is better. > +}; > + > /* Structure containing additional information about an indirect call. */ > > class GTY(()) cgraph_indirect_call_info > @@ -1654,10 +1669,9 @@ public: > int param_index; > /* ECF flags determined from the caller. */ > int ecf_flags; > - /* Profile_id of common target obtained from profile. */ > - int common_target_id; > - /* Probability that call will land in function with COMMON_TARGET_ID. */ > - int common_target_probability; > + > + /* An indirect call may contain one or multiple call targets. */ > + vec<indirect_target_info, va_gc> *indirect_call_targets; And speculative_call_targets? Please put these into cgraph_edge_summary template. This is a short lived annotation which is private to ipa-profile and does not need to be in the core datastructures. So you can then also delcare it within ipa-profile.c as no one lese cares about it. Also stream in ipa_profile_write_summary rather than lto-cgraph. > + /* speculative id is used by multiple indirect targets when the function is ... is used to link direct calls with their corresponding IPA_REF_ADDR references when representing speculative cals. */ Even if there is only one speculation the values must match, right? :) > + speculated. */ > + unsigned int speculative_id; The bitfields will pack well with the speculative_id if we make it 16bit rather than 32bit. I do not think we will ever want to support more than 64K speculative targets. > diff --git a/gcc/ipa-comdats.c b/gcc/ipa-comdats.c > index b496497ff66..0672d6d6575 100644 > --- a/gcc/ipa-comdats.c > +++ b/gcc/ipa-comdats.c > @@ -18,7 +18,7 @@ along with GCC; see the file COPYING3. If not see > <http://www.gnu.org/licenses/>. */ > > /* This is very simple pass that looks for static symbols that are used > - exlusively by symbol within one comdat group. In this case it makes > + exclusively by symbol within one comdat group. In this case it makes This can go in separately as obovius. > sense to bring the symbol itself into the group to avoid dead code > that would arrise when the comdat group from current unit is replaced > by a different copy. Consider for example: > diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c > index 78ec0ec685f..8fc67ef88a5 100644 > --- a/gcc/ipa-inline.c > +++ b/gcc/ipa-inline.c > @@ -1945,12 +1945,15 @@ inline_small_functions (void) > } > if (has_speculative) > for (edge = node->callees; edge; edge = next) > - if (edge->speculative && !speculation_useful_p (edge, > - edge->aux != NULL)) > - { > - edge->resolve_speculation (); > - update = true; > - } > + { > + next = edge->next_callee; > + if (edge->speculative > + && !speculation_useful_p (edge, edge->aux != NULL)) > + { > + edge->resolve_speculation (); > + update = true; > + } > + } This looks like pasto. So next is initialized only in the loop before and it happens that it never has speculative flag on it? If so, this is also independent fix, so please send it as separate patch. > @@ -563,7 +573,7 @@ ipa_profile (void) > histogram.release (); > histogram_pool.release (); > > - /* Produce speculative calls: we saved common traget from porfiling into > + /* Produce speculative calls: we saved common target from profiling into Also just commit this as obvious. > diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c > index bdc332dcc23..6c8ebffa11d 100644 > --- a/gcc/tree-inline.c > +++ b/gcc/tree-inline.c > @@ -2194,6 +2194,25 @@ copy_bb (copy_body_data *id, basic_block bb, > > gcc_assert (!edge->indirect_unknown_callee); > old_edge->speculative_call_info (direct, indirect, ref); > + while (old_edge->next_callee > + && old_edge->next_callee->speculative > + && indirect->has_multiple_indirect_call_p ()) > + { > + /* Some speculative calls may contain more than > + one direct target, loop iterate it to clone all I gues better as "Iterate through all direct calls associated to the speculative call and clone all..." > + related direct edges before cloning the related > + indirect edge. */ > + id->dst_node->clone_reference (ref, stmt); > + > + edge = old_edge->next_callee; > + edge = edge->clone (id->dst_node, call_stmt, > + gimple_uid (stmt), num, den, > + true); > + old_edge = old_edge->next_callee; > + gcc_assert (!edge->indirect_unknown_callee); > + old_edge->speculative_call_info (direct, indirect, > + ref); Why do you call this at the end of loop? And how do you know that al the edges are associated to the given speculative call? > + } > > profile_count indir_cnt = indirect->count; > indirect = indirect->clone (id->dst_node, call_stmt, > diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c > index 6a4e62f5bae..b4435b9b2a8 100644 > --- a/gcc/tree-profile.c > +++ b/gcc/tree-profile.c > @@ -73,8 +73,8 @@ static GTY(()) tree ic_tuple_callee_field; > /* Do initialization work for the edge profiler. */ > > /* Add code: > - __thread gcov* __gcov_indirect_call_counters; // pointer to actual counter > - __thread void* __gcov_indirect_call_callee; // actual callee address > + __thread gcov* __gcov_indirect_call.counters; // pointer to actual counter > + __thread void* __gcov_indirect_call.callee; // actual callee address > __thread int __gcov_function_counter; // time profiler function counter > */ > static void > @@ -381,7 +381,7 @@ gimple_gen_ic_profiler (histogram_value value, unsigned tag) > f_1 = foo; > __gcov_indirect_call.counters = &__gcov4.main[0]; > PROF_9 = f_1; > - __gcov_indirect_call_callee = PROF_9; > + __gcov_indirect_call.callee = PROF_9; > _4 = f_1 (); > */ > > @@ -444,11 +444,11 @@ gimple_gen_ic_func_profiler (void) > > /* Insert code: > > - if (__gcov_indirect_call_callee != NULL) > + if (__gcov_indirect_call.callee != NULL) > __gcov_indirect_call_profiler_v3 (profile_id, ¤t_function_decl); > > The function __gcov_indirect_call_profiler_v3 is responsible for > - resetting __gcov_indirect_call_callee to NULL. */ > + resetting __gcov_indirect_call.callee to NULL. */ > > gimple_stmt_iterator gsi = gsi_start_bb (cond_bb); > void0 = build_int_cst (ptr_type_node, 0); > @@ -890,7 +890,7 @@ pass_ipa_tree_profile::gate (function *) > { > /* When profile instrumentation, use or test coverage shall be performed. > But for AutoFDO, this there is no instrumentation, thus this pass is > - diabled. */ > + disabled. */ > return (!in_lto_p && !flag_auto_profile > && (flag_branch_probabilities || flag_test_coverage > || profile_arc_flag)); Also commit this independently? Patch looks really nice overall. It will be easy to teach ipa-devirt to use multiple targets too. I apologize for very late response and thanks for all the pings (I will try to get more timely) Honza > diff --git a/gcc/value-prof.c b/gcc/value-prof.c > index cc3542f0295..f64f515c1ee 100644 > --- a/gcc/value-prof.c > +++ b/gcc/value-prof.c > @@ -106,7 +106,7 @@ static bool gimple_divmod_fixed_value_transform (gimple_stmt_iterator *); > static bool gimple_mod_pow2_value_transform (gimple_stmt_iterator *); > static bool gimple_mod_subtract_transform (gimple_stmt_iterator *); > static bool gimple_stringops_transform (gimple_stmt_iterator *); > -static bool gimple_ic_transform (gimple_stmt_iterator *); > +static void gimple_ic_transform (gimple_stmt_iterator *); > > /* Allocate histogram value. */ > > @@ -616,8 +616,7 @@ gimple_value_profile_transformations (void) > if (gimple_mod_subtract_transform (&gsi) > || gimple_divmod_fixed_value_transform (&gsi) > || gimple_mod_pow2_value_transform (&gsi) > - || gimple_stringops_transform (&gsi) > - || gimple_ic_transform (&gsi)) > + || gimple_stringops_transform (&gsi)) > { > stmt = gsi_stmt (gsi); > changed = true; > @@ -628,6 +627,9 @@ gimple_value_profile_transformations (void) > gsi = gsi_for_stmt (stmt); > } > } > + > + /* The function never thansforms a GIMPLE statement. */ > + gimple_ic_transform (&gsi); > } > } > > @@ -1386,13 +1388,12 @@ gimple_ic (gcall *icall_stmt, struct cgraph_node *direct_call, > return dcall_stmt; > } > > -/* > - For every checked indirect/virtual call determine if most common pid of > - function/class method has probability more than 50%. If yes modify code of > - this call to: > - */ > +/* There maybe multiple indirect targets in histogram. Check every > + indirect/virtual call if callee function exists, if not exist, leave it to > + LTO stage for later process. Modify code of this indirect call to an if-else > + structure in ipa-profile finally. */ > > -static bool > +static void > gimple_ic_transform (gimple_stmt_iterator *gsi) > { > gcall *stmt; > @@ -1402,52 +1403,58 @@ gimple_ic_transform (gimple_stmt_iterator *gsi) > > stmt = dyn_cast <gcall *> (gsi_stmt (*gsi)); > if (!stmt) > - return false; > + return; > > if (gimple_call_fndecl (stmt) != NULL_TREE) > - return false; > + return; > > if (gimple_call_internal_p (stmt)) > - return false; > + return; > > histogram = gimple_histogram_value_of_type (cfun, stmt, HIST_TYPE_INDIR_CALL); > if (!histogram) > - return false; > + return; > > - if (!get_nth_most_common_value (NULL, "indirect call", histogram, &val, > - &count, &all)) > - return false; > + count = 0; > + all = histogram->hvalue.counters[0]; > > - if (4 * count <= 3 * all) > - return false; > + for (unsigned j = 0; j < GCOV_TOPN_VALUES; j++) > + { > + if (!get_nth_most_common_value (NULL, "indirect call", histogram, &val, > + &count, &all, j)) > + return; > > - direct_call = find_func_by_profile_id ((int)val); > + /* Minimum probability. should be higher than 25%. */ > + if (4 * count <= all) > + return; > > - if (direct_call == NULL) > - { > - if (val) > + direct_call = find_func_by_profile_id ((int) val); > + > + if (direct_call == NULL) > { > - if (dump_enabled_p ()) > - dump_printf_loc (MSG_MISSED_OPTIMIZATION, stmt, > - "Indirect call -> direct call from other " > - "module %T=> %i (will resolve only with LTO)\n", > - gimple_call_fn (stmt), (int)val); > + if (val) > + { > + if (dump_enabled_p ()) > + dump_printf_loc ( > + MSG_MISSED_OPTIMIZATION, stmt, > + "Indirect call -> direct call from other " > + "module %T=> %i (will resolve only with LTO)\n", > + gimple_call_fn (stmt), (int) val); > + } > + return; > } > - return false; > - } > > - if (dump_enabled_p ()) > - { > - dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, stmt, > - "Indirect call -> direct call " > - "%T => %T transformation on insn postponed\n", > - gimple_call_fn (stmt), direct_call->decl); > - dump_printf_loc (MSG_NOTE, stmt, > - "hist->count %" PRId64 > - " hist->all %" PRId64"\n", count, all); > + if (dump_enabled_p ()) > + { > + dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, stmt, > + "Indirect call -> direct call " > + "%T => %T transformation on insn postponed\n", > + gimple_call_fn (stmt), direct_call->decl); > + dump_printf_loc (MSG_NOTE, stmt, > + "hist->count %" PRId64 " hist->all %" PRId64 "\n", > + count, all); > + } > } > - > - return true; > } > > /* Return true if the stringop CALL shall be profiled. SIZE_ARG be > diff --git a/gcc/value-prof.h b/gcc/value-prof.h > index 77c06f60096..b3eeb57d37d 100644 > --- a/gcc/value-prof.h > +++ b/gcc/value-prof.h > @@ -89,7 +89,6 @@ void verify_histograms (void); > void free_histograms (function *); > void stringop_block_profile (gimple *, unsigned int *, HOST_WIDE_INT *); > gcall *gimple_ic (gcall *, struct cgraph_node *, profile_probability); > -bool check_ic_target (gcall *, struct cgraph_node *); > bool get_nth_most_common_value (gimple *stmt, const char *counter_type, > histogram_value hist, gcov_type *value, > gcov_type *count, gcov_type *all, > -- > 2.21.0.777.g83232e3864 > >
Hi Thanks, On 2019/11/14 17:04, Jan Hubicka wrote: >> PR ipa/69678 >> * cgraph.c (symbol_table::create_edge): Init speculative_id. >> (cgraph_edge::make_speculative): Add param for setting speculative_id. >> (cgraph_edge::speculative_call_info): Find reference by >> speculative_id for multiple indirect targets. >> (cgraph_edge::resolve_speculation): Decrease the speculations >> for indirect edge, drop it's speculative if not direct target >> left. >> (cgraph_edge::redirect_call_stmt_to_callee): Likewise. >> (cgraph_node::verify_node): Don't report error if speculative >> edge not include statement. >> (cgraph_edge::has_multiple_indirect_call_p): New function. >> (cgraph_edge::has_indirect_call_p): New function. >> * cgraph.h (struct indirect_target_info): New struct. >> (indirect_call_targets): New vector variable. >> (make_speculative): Add param for setting speculative_id. >> (cgraph_edge::has_multiple_indirect_call_p): New declare. >> (cgraph_edge::has_indirect_call_p): New declare. >> (speculative_id): New variable. >> * cgraphclones.c (cgraph_node::create_clone): Clone speculative_id. >> * cgraphunit.c: Fix comments typo. >> * ipa-comdats.c: Fix comments typo. >> * ipa-inline.c (inline_small_functions): Fix iterator update. >> * ipa-profile.c (ipa_profile_generate_summary): Add indirect >> multiple targets logic. >> (ipa_profile): Likewise. >> * ipa-ref.h (speculative_id): New variable. >> * ipa.c (process_references): Fix typo. >> * lto-cgraph.c (lto_output_edge): Add indirect multiple targets >> logic. Stream out speculative_id. >> (input_edge): Likewise. >> * predict.c (dump_prediction): Revome edges count assert to be >> precise. >> * symtab.c (symtab_node::create_reference): Init speculative_id. >> (symtab_node::clone_references): Clone speculative_id. >> (symtab_node::clone_referring): Clone speculative_id. >> (symtab_node::clone_reference): Clone speculative_id. >> (symtab_node::clear_stmts_in_references): Clear speculative_id. >> * tree-inline.c (copy_bb): Duplicate all the speculative edges >> if indirect call contains multiple speculative targets. >> * tree-profile.c (gimple_gen_ic_profiler): Use the new variable >> __gcov_indirect_call.counters and __gcov_indirect_call.callee. >> (gimple_gen_ic_func_profiler): Likewise. >> (pass_ipa_tree_profile::gate): Fix comment typos. >> * value-prof.h (check_ic_target): Remove. >> * value-prof.c (gimple_value_profile_transformations): >> Use void function gimple_ic_transform. >> * value-prof.c (gimple_ic_transform): Handle topn case. >> Fix comment typos. Change it to a void function. >> >> gcc/testsuite/ChangeLog >> >> 2019-11-14 Xiong Hu Luo <luoxhu@linux.ibm.com> >> >> PR ipa/69678 >> * gcc.dg/tree-prof/indir-call-prof-topn.c: New testcase. >> * gcc.dg/tree-prof/crossmodule-indir-call-topn-1.c: New testcase. >> * gcc.dg/tree-prof/crossmodule-indir-call-topn-1a.c: New testcase. >> * gcc.dg/tree-prof/crossmodule-indir-call-topn-2.c: New testcase. >> * lib/scandump.exp: Dump executable file name. >> * lib/scanwpaipa.exp: New scan-pgo-wap-ipa-dump. >> @@ -1089,6 +1093,38 @@ cgraph_edge::make_speculative (cgraph_node *n2, profile_count direct_count) >> call) and if one of them exists, all of them must exist. >> >> Given speculative call edge, return all three components. >> + >> + For some indirect edge, it may maps to multiple direct edges, i.e. 1:N. >> + check the speculative_id to return all the three components for specified >> + direct edge or indirect edge. >> + If input is indirect, caller of this function will get the direct edge one by >> + one, get_edge will just return one of the direct edge mapped to the indirect >> + edge, the returned direct edge will be resolved or redirected by the caller, >> + then number of indirect calls (speculations) is deceased in each access. >> + If input is direct, this function will get the indirect edge and reference >> + with matched speculative_id, the returned edge will also be resolved or >> + redirected, decrease the speculations accordingly. >> + Speculations of indirect edge will be dropped only if all direct edges >> + be handled. >> + >> + e.g. for indirect edge E statement "call call_dest": >> + >> + Redirect N3 after redirected N2: >> + >> + if (call_dest == N2) >> + n2 (); >> + else if (call_dest == N3) >> + n3 (); >> + else >> + call call_dest >> + >> + Resolve N3 and only redirect N2: >> + >> + if (call_dest == N2) >> + n2 (); >> + else >> + call call_dest >> + > > I find this comment hard to read. Reader probably does not know what > speculative edges are and we only want to describe speculative_call_info > function not also the way we resolve calls. So what about something > like this: > > Speculative calls represent a transformation of indirect calls > which may be later inserted into gimple in the following form: > > if (call_dest == target1) > target1 (); > else if (call_dest == target2) > target (); > else > call_dest (); > > This is a win in case target1 and target2 are common values for > call_dest as determined by ipa-devirt or indirect call profiling. > In particular this may enable inlining and other optimizations. > > Speculative call consists of the following main components: > > 1) One or more direct call > 2) One or more IPA_REF_ADDR references (representing the fact that > code above takes address of target1 and target2) > 3) The fallback indirect call > > Direct calls and corresponidng references are linked by > speculative_id. > > speculative_call_info returns tripple > (direct_call, IPA_REF_ADDR reference, indirect call) > when called on one edge participating in the speculative call. > > If called on direct call its corresponding IPA_REF_ADDR is returned. > > If caled on indirect call it will return one of direct edges and its > matching IPA_REF_ADDR. > > All direct calls corresponding to a given speculative call can be > enumerated by indirect_edge->indirect_info->indirect_call_targets Will update. >> */ >> >> void >> @@ -1128,7 +1164,7 @@ cgraph_edge::speculative_call_info (cgraph_edge *&direct, >> >> reference = NULL; >> for (i = 0; e->caller->iterate_reference (i, ref); i++) >> - if (ref->speculative >> + if (ref->speculative && ref->speculative_id == e->speculative_id >> && ((ref->stmt && ref->stmt == e->call_stmt) >> || (!ref->stmt && ref->lto_stmt_uid == e->lto_stmt_uid))) >> { >> @@ -1189,7 +1225,21 @@ cgraph_edge::resolve_speculation (tree callee_decl) > > Rewrite comment here to make clear what resolve_speculation does for > multiple targets. OK and please be patient with my English :) >> in the functions inlined through it. */ >> } >> edge->count += e2->count; >> - edge->speculative = false; >> + /* edge is indirect, e2 is direct. If edge contains multiple speculations, >> + remove one of speculations for this indirect edge, then if edge still >> + contains direct target, keep the speculation, next direct target >> + will continue use it. Give up speculation completely if no direct >> + target is left for this indirect edge. */ >> + if (edge->has_indirect_call_p ()) >> + { >> + /* As the direct targets are sorted by decrease, delete the first target >> + when it is resolved. */ >> + edge->indirect_info->indirect_call_targets->ordered_remove (0); >> + if (edge->indirect_info->indirect_call_targets->is_empty ()) >> + edge->speculative = false; >> + } >> + else >> + edge->speculative = false; >> e2->speculative = false; >> ref->remove_reference (); >> if (e2->indirect_unknown_callee || e2->inline_failed) >> @@ -1297,7 +1347,21 @@ cgraph_edge::redirect_call_stmt_to_callee (void) >> e->caller->set_call_stmt_including_clones (e->call_stmt, new_stmt, >> false); >> e->count = gimple_bb (e->call_stmt)->count; >> - e2->speculative = false; >> + /* edge is direct, e2 is indirect here. If e2 contains multiple >> + speculations, remove one of speculations for this indirect edge, >> + then if e2 still contains direct target, keep the speculation, >> + next direct target will continue use it. Give up speculation >> + completely if no direct target is left for this indirect e2. */ >> + if (e2->has_indirect_call_p ()) >> + { >> + /* As the direct targets are sorted by decrease, delete the first >> + target when it is redirected. */ >> + e2->indirect_info->indirect_call_targets->ordered_remove (0); >> + if (e2->indirect_info->indirect_call_targets->is_empty ()) >> + e2->speculative = false; >> + } >> + else >> + e2->speculative = false; > > I think this also needs better explanation (and especially in the > comment in the front of function) >> @@ -3696,6 +3760,22 @@ cgraph_edge::possibly_call_in_translation_unit_p (void) >> return node->get_availability () >= AVAIL_INTERPOSABLE; >> } >> >> +/* Return true if this edge has multiple indirect call targets. */ >> +bool >> +cgraph_edge::has_multiple_indirect_call_p (void) > Probaby better as > multiple_speculative_call_targets_p > > It seems to me that here you use indirect_call_targets array which does > not make much sense once the indirect edge is turned into speculative > call. This vector should be freed after that and as I write late > it should not be in the callgraph itself but rather in a ipa-profile > summary. > > (I know that common_target_probability and common_target_id is there > right now but it is code predating summaries and it does not belong there > anymore) > > I would simply add "num_speculative_call_targets" into > cgraph_indirect_call_info datastructure. OK. I used num_of_ics(i.e. num_speculative_call_targets) in early version. Will try restore it. >> +{ >> + return (indirect_info && indirect_info->indirect_call_targets >> + && indirect_info->indirect_call_targets->length () > 1); > > Maybe it would make sense to call this speculative_call_targets? > Since they are after all direct calls... >> +} >> + >> +/* Return true if this edge has at least one indirect call target. */ >> +bool >> +cgraph_edge::has_indirect_call_p (void) >> +{ >> + return (indirect_info && indirect_info->indirect_call_targets >> + && !indirect_info->indirect_call_targets->is_empty ()); > > How this is different from speculative flag on an indirect call? >> +} >> + >> /* A stashed copy of "symtab" for use by selftest::symbol_table_test. >> This needs to be a global so that it can be a GC root, and thus >> prevent the stashed copy from being garbage-collected if the GC runs >> diff --git a/gcc/cgraph.h b/gcc/cgraph.h >> index a4f14743f00..1299702788d 100644 >> --- a/gcc/cgraph.h >> +++ b/gcc/cgraph.h >> @@ -1636,6 +1636,21 @@ private: >> void make_speculative (tree otr_type = NULL); >> }; >> >> +/* Structure containing indirect target information from profile. */ >> + >> +struct GTY (()) indirect_target_info > Probably better as speculative_call_target? >> +{ >> + indirect_target_info (unsigned int id, int prob) >> + : common_target_id (id), common_target_probability (prob) >> + { >> + } >> + >> + /* Profile_id of common target obtained from profile. */ >> + unsigned int common_target_id; >> + /* Probability that call will land in function with COMMON_TARGET_ID. */ >> + int common_target_probability; > > I think this would be more like frequent then common, but perhaps > target_id and target_probablity is better. >> +}; >> + >> /* Structure containing additional information about an indirect call. */ >> >> class GTY(()) cgraph_indirect_call_info >> @@ -1654,10 +1669,9 @@ public: >> int param_index; >> /* ECF flags determined from the caller. */ >> int ecf_flags; >> - /* Profile_id of common target obtained from profile. */ >> - int common_target_id; >> - /* Probability that call will land in function with COMMON_TARGET_ID. */ >> - int common_target_probability; >> + >> + /* An indirect call may contain one or multiple call targets. */ >> + vec<indirect_target_info, va_gc> *indirect_call_targets; > > And speculative_call_targets? > Please put these into cgraph_edge_summary template. > > This is a short lived annotation which is private to ipa-profile and > does not need to be in the core datastructures. So you can then also > delcare it within ipa-profile.c as no one lese cares about it. > Also stream in ipa_profile_write_summary rather than lto-cgraph. Sorry that I don't quite understand your meanning here. I didn't grep the word "cgraph_edge_summary" in source code, do you mean add new structure and related functions like "static ipa_sra_call_summaries *call_sums;" in ipa-sra.c and stream in/out in ipa_profile_write_summary/ipa_profile_read_summary? BTW, there is already an ipa_call_summaries in ipa-profile.c:355, feel apologetic as not familiar with many summaries in it, will take time to go through it: ipa_call_summary *s = ipa_call_summaries->get (edge); > >> + /* speculative id is used by multiple indirect targets when the function is > ... is used to link direct calls with their corresponding IPA_REF_ADDR > references when representing speculative cals. */ > > Even if there is only one speculation the values must match, right? :) >> + speculated. */ >> + unsigned int speculative_id; > > The bitfields will pack well with the speculative_id if we make it > 16bit rather than 32bit. I do not think we will ever want to support > more than 64K speculative targets. OK. >> diff --git a/gcc/ipa-comdats.c b/gcc/ipa-comdats.c >> index b496497ff66..0672d6d6575 100644 >> --- a/gcc/ipa-comdats.c >> +++ b/gcc/ipa-comdats.c >> @@ -18,7 +18,7 @@ along with GCC; see the file COPYING3. If not see >> <http://www.gnu.org/licenses/>. */ >> >> /* This is very simple pass that looks for static symbols that are used >> - exlusively by symbol within one comdat group. In this case it makes >> + exclusively by symbol within one comdat group. In this case it makes > This can go in separately as obovius. >> sense to bring the symbol itself into the group to avoid dead code >> that would arrise when the comdat group from current unit is replaced >> by a different copy. Consider for example: >> diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c >> index 78ec0ec685f..8fc67ef88a5 100644 >> --- a/gcc/ipa-inline.c >> +++ b/gcc/ipa-inline.c >> @@ -1945,12 +1945,15 @@ inline_small_functions (void) >> } >> if (has_speculative) >> for (edge = node->callees; edge; edge = next) >> - if (edge->speculative && !speculation_useful_p (edge, >> - edge->aux != NULL)) >> - { >> - edge->resolve_speculation (); >> - update = true; >> - } >> + { >> + next = edge->next_callee; >> + if (edge->speculative >> + && !speculation_useful_p (edge, edge->aux != NULL)) >> + { >> + edge->resolve_speculation (); >> + update = true; >> + } >> + } > > This looks like pasto. So next is initialized only in the loop before > and it happens that it never has speculative flag on it?> > If so, this is also independent fix, so please send it as separate > patch. >> @@ -563,7 +573,7 @@ ipa_profile (void) >> histogram.release (); >> histogram_pool.release (); >> >> - /* Produce speculative calls: we saved common traget from porfiling into >> + /* Produce speculative calls: we saved common target from profiling into > > Also just commit this as obvious. Done. >> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c >> index bdc332dcc23..6c8ebffa11d 100644 >> --- a/gcc/tree-inline.c >> +++ b/gcc/tree-inline.c >> @@ -2194,6 +2194,25 @@ copy_bb (copy_body_data *id, basic_block bb, >> >> gcc_assert (!edge->indirect_unknown_callee); >> old_edge->speculative_call_info (direct, indirect, ref); >> + while (old_edge->next_callee >> + && old_edge->next_callee->speculative >> + && indirect->has_multiple_indirect_call_p ()) >> + { >> + /* Some speculative calls may contain more than >> + one direct target, loop iterate it to clone all > I gues better as "Iterate through all direct calls associated to the > speculative call and clone all..." >> + related direct edges before cloning the related >> + indirect edge. */ >> + id->dst_node->clone_reference (ref, stmt); >> + >> + edge = old_edge->next_callee; >> + edge = edge->clone (id->dst_node, call_stmt, >> + gimple_uid (stmt), num, den, >> + true); >> + old_edge = old_edge->next_callee; >> + gcc_assert (!edge->indirect_unknown_callee); >> + old_edge->speculative_call_info (direct, indirect, >> + ref); > Why do you call this at the end of loop? And how do you know that al the > edges are associated to the given speculative call? Speculative targets are sorted by decrease order of execution count in tree-profile. speculative_id in each direct target will ensure the identity in speculative_call_info. Here, speculative_call_info is called first before the while loop, if direct target1 exists, it will be cloned to dst_node. Then speculative_call_info is called second time, if direct target2 exsits, also clone it to dst_node, else the while condition is false, the indirect edge is cloned to dst_node in following code out of the while loop. We all know that you are very busy:), it's so nice of you take the time to review patch to improve code quality. Thanks! Xiong Hu BR >> + } >> >> profile_count indir_cnt = indirect->count; >> indirect = indirect->clone (id->dst_node, call_stmt, >> diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c >> index 6a4e62f5bae..b4435b9b2a8 100644 >> --- a/gcc/tree-profile.c >> +++ b/gcc/tree-profile.c >> @@ -73,8 +73,8 @@ static GTY(()) tree ic_tuple_callee_field; >> /* Do initialization work for the edge profiler. */ >> >> /* Add code: >> - __thread gcov* __gcov_indirect_call_counters; // pointer to actual counter >> - __thread void* __gcov_indirect_call_callee; // actual callee address >> + __thread gcov* __gcov_indirect_call.counters; // pointer to actual counter >> + __thread void* __gcov_indirect_call.callee; // actual callee address >> __thread int __gcov_function_counter; // time profiler function counter >> */ >> static void >> @@ -381,7 +381,7 @@ gimple_gen_ic_profiler (histogram_value value, unsigned tag) >> f_1 = foo; >> __gcov_indirect_call.counters = &__gcov4.main[0]; >> PROF_9 = f_1; >> - __gcov_indirect_call_callee = PROF_9; >> + __gcov_indirect_call.callee = PROF_9; >> _4 = f_1 (); >> */ >> >> @@ -444,11 +444,11 @@ gimple_gen_ic_func_profiler (void) >> >> /* Insert code: >> >> - if (__gcov_indirect_call_callee != NULL) >> + if (__gcov_indirect_call.callee != NULL) >> __gcov_indirect_call_profiler_v3 (profile_id, ¤t_function_decl); >> >> The function __gcov_indirect_call_profiler_v3 is responsible for >> - resetting __gcov_indirect_call_callee to NULL. */ >> + resetting __gcov_indirect_call.callee to NULL. */ >> >> gimple_stmt_iterator gsi = gsi_start_bb (cond_bb); >> void0 = build_int_cst (ptr_type_node, 0); >> @@ -890,7 +890,7 @@ pass_ipa_tree_profile::gate (function *) >> { >> /* When profile instrumentation, use or test coverage shall be performed. >> But for AutoFDO, this there is no instrumentation, thus this pass is >> - diabled. */ >> + disabled. */ >> return (!in_lto_p && !flag_auto_profile >> && (flag_branch_probabilities || flag_test_coverage >> || profile_arc_flag)); > > Also commit this independently? > > Patch looks really nice overall. It will be easy to teach ipa-devirt to > use multiple targets too. > > I apologize for very late response and thanks for all the pings > (I will try to get more timely) > Honza >> diff --git a/gcc/value-prof.c b/gcc/value-prof.c >> index cc3542f0295..f64f515c1ee 100644 >> --- a/gcc/value-prof.c >> +++ b/gcc/value-prof.c >> @@ -106,7 +106,7 @@ static bool gimple_divmod_fixed_value_transform (gimple_stmt_iterator *); >> static bool gimple_mod_pow2_value_transform (gimple_stmt_iterator *); >> static bool gimple_mod_subtract_transform (gimple_stmt_iterator *); >> static bool gimple_stringops_transform (gimple_stmt_iterator *); >> -static bool gimple_ic_transform (gimple_stmt_iterator *); >> +static void gimple_ic_transform (gimple_stmt_iterator *); >> >> /* Allocate histogram value. */ >> >> @@ -616,8 +616,7 @@ gimple_value_profile_transformations (void) >> if (gimple_mod_subtract_transform (&gsi) >> || gimple_divmod_fixed_value_transform (&gsi) >> || gimple_mod_pow2_value_transform (&gsi) >> - || gimple_stringops_transform (&gsi) >> - || gimple_ic_transform (&gsi)) >> + || gimple_stringops_transform (&gsi)) >> { >> stmt = gsi_stmt (gsi); >> changed = true; >> @@ -628,6 +627,9 @@ gimple_value_profile_transformations (void) >> gsi = gsi_for_stmt (stmt); >> } >> } >> + >> + /* The function never thansforms a GIMPLE statement. */ >> + gimple_ic_transform (&gsi); >> } >> } >> >> @@ -1386,13 +1388,12 @@ gimple_ic (gcall *icall_stmt, struct cgraph_node *direct_call, >> return dcall_stmt; >> } >> >> -/* >> - For every checked indirect/virtual call determine if most common pid of >> - function/class method has probability more than 50%. If yes modify code of >> - this call to: >> - */ >> +/* There maybe multiple indirect targets in histogram. Check every >> + indirect/virtual call if callee function exists, if not exist, leave it to >> + LTO stage for later process. Modify code of this indirect call to an if-else >> + structure in ipa-profile finally. */ >> >> -static bool >> +static void >> gimple_ic_transform (gimple_stmt_iterator *gsi) >> { >> gcall *stmt; >> @@ -1402,52 +1403,58 @@ gimple_ic_transform (gimple_stmt_iterator *gsi) >> >> stmt = dyn_cast <gcall *> (gsi_stmt (*gsi)); >> if (!stmt) >> - return false; >> + return; >> >> if (gimple_call_fndecl (stmt) != NULL_TREE) >> - return false; >> + return; >> >> if (gimple_call_internal_p (stmt)) >> - return false; >> + return; >> >> histogram = gimple_histogram_value_of_type (cfun, stmt, HIST_TYPE_INDIR_CALL); >> if (!histogram) >> - return false; >> + return; >> >> - if (!get_nth_most_common_value (NULL, "indirect call", histogram, &val, >> - &count, &all)) >> - return false; >> + count = 0; >> + all = histogram->hvalue.counters[0]; >> >> - if (4 * count <= 3 * all) >> - return false; >> + for (unsigned j = 0; j < GCOV_TOPN_VALUES; j++) >> + { >> + if (!get_nth_most_common_value (NULL, "indirect call", histogram, &val, >> + &count, &all, j)) >> + return; >> >> - direct_call = find_func_by_profile_id ((int)val); >> + /* Minimum probability. should be higher than 25%. */ >> + if (4 * count <= all) >> + return; >> >> - if (direct_call == NULL) >> - { >> - if (val) >> + direct_call = find_func_by_profile_id ((int) val); >> + >> + if (direct_call == NULL) >> { >> - if (dump_enabled_p ()) >> - dump_printf_loc (MSG_MISSED_OPTIMIZATION, stmt, >> - "Indirect call -> direct call from other " >> - "module %T=> %i (will resolve only with LTO)\n", >> - gimple_call_fn (stmt), (int)val); >> + if (val) >> + { >> + if (dump_enabled_p ()) >> + dump_printf_loc ( >> + MSG_MISSED_OPTIMIZATION, stmt, >> + "Indirect call -> direct call from other " >> + "module %T=> %i (will resolve only with LTO)\n", >> + gimple_call_fn (stmt), (int) val); >> + } >> + return; >> } >> - return false; >> - } >> >> - if (dump_enabled_p ()) >> - { >> - dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, stmt, >> - "Indirect call -> direct call " >> - "%T => %T transformation on insn postponed\n", >> - gimple_call_fn (stmt), direct_call->decl); >> - dump_printf_loc (MSG_NOTE, stmt, >> - "hist->count %" PRId64 >> - " hist->all %" PRId64"\n", count, all); >> + if (dump_enabled_p ()) >> + { >> + dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, stmt, >> + "Indirect call -> direct call " >> + "%T => %T transformation on insn postponed\n", >> + gimple_call_fn (stmt), direct_call->decl); >> + dump_printf_loc (MSG_NOTE, stmt, >> + "hist->count %" PRId64 " hist->all %" PRId64 "\n", >> + count, all); >> + } >> } >> - >> - return true; >> } >> >> /* Return true if the stringop CALL shall be profiled. SIZE_ARG be >> diff --git a/gcc/value-prof.h b/gcc/value-prof.h >> index 77c06f60096..b3eeb57d37d 100644 >> --- a/gcc/value-prof.h >> +++ b/gcc/value-prof.h >> @@ -89,7 +89,6 @@ void verify_histograms (void); >> void free_histograms (function *); >> void stringop_block_profile (gimple *, unsigned int *, HOST_WIDE_INT *); >> gcall *gimple_ic (gcall *, struct cgraph_node *, profile_probability); >> -bool check_ic_target (gcall *, struct cgraph_node *); >> bool get_nth_most_common_value (gimple *stmt, const char *counter_type, >> histogram_value hist, gcov_type *value, >> gcov_type *count, gcov_type *all, >> -- >> 2.21.0.777.g83232e3864 >> >>
On 11/16/19 10:59 AM, luoxhu wrote: > Sorry that I don't quite understand your meanning here. I didn't grep the > word "cgraph_edge_summary" in source code, do you mean add new structure Hello. He wanted to write call_summary class and so you need something similar to ipa-sra.c:431. It's a data structure which associate a data to cgraph_edge. Is it understandable please? Martin > and related functions like "static ipa_sra_call_summaries *call_sums;" > in ipa-sra.c and stream in/out in ipa_profile_write_summary/ipa_profile_read_summary? > > BTW, there is already an ipa_call_summaries in ipa-profile.c:355, feel apologetic as > not familiar with many summaries in it, will take time to go through it: > ipa_call_summary *s = ipa_call_summaries->get (edge);
diff --git a/gcc/cgraph.c b/gcc/cgraph.c index 1f7a5c58d98..16e92267152 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -864,6 +864,7 @@ symbol_table::create_edge (cgraph_node *caller, cgraph_node *callee, edge->prev_callee = NULL; edge->next_callee = NULL; edge->lto_stmt_uid = 0; + edge->speculative_id = 0; edge->count = count; edge->call_stmt = call_stmt; @@ -1053,7 +1054,8 @@ cgraph_edge::remove (void) Return direct edge created. */ cgraph_edge * -cgraph_edge::make_speculative (cgraph_node *n2, profile_count direct_count) +cgraph_edge::make_speculative (cgraph_node *n2, profile_count direct_count, + unsigned int speculative_id) { cgraph_node *n = caller; ipa_ref *ref = NULL; @@ -1071,11 +1073,13 @@ cgraph_edge::make_speculative (cgraph_node *n2, profile_count direct_count) else e2->can_throw_external = can_throw_external; e2->lto_stmt_uid = lto_stmt_uid; + e2->speculative_id = speculative_id; e2->in_polymorphic_cdtor = in_polymorphic_cdtor; count -= e2->count; symtab->call_edge_duplication_hooks (this, e2); ref = n->create_reference (n2, IPA_REF_ADDR, call_stmt); ref->lto_stmt_uid = lto_stmt_uid; + ref->speculative_id = speculative_id; ref->speculative = speculative; n2->mark_address_taken (); return e2; @@ -1089,6 +1093,38 @@ cgraph_edge::make_speculative (cgraph_node *n2, profile_count direct_count) call) and if one of them exists, all of them must exist. Given speculative call edge, return all three components. + + For some indirect edge, it may maps to multiple direct edges, i.e. 1:N. + check the speculative_id to return all the three components for specified + direct edge or indirect edge. + If input is indirect, caller of this function will get the direct edge one by + one, get_edge will just return one of the direct edge mapped to the indirect + edge, the returned direct edge will be resolved or redirected by the caller, + then number of indirect calls (speculations) is deceased in each access. + If input is direct, this function will get the indirect edge and reference + with matched speculative_id, the returned edge will also be resolved or + redirected, decrease the speculations accordingly. + Speculations of indirect edge will be dropped only if all direct edges + be handled. + + e.g. for indirect edge E statement "call call_dest": + + Redirect N3 after redirected N2: + + if (call_dest == N2) + n2 (); + else if (call_dest == N3) + n3 (); + else + call call_dest + + Resolve N3 and only redirect N2: + + if (call_dest == N2) + n2 (); + else + call call_dest + */ void @@ -1128,7 +1164,7 @@ cgraph_edge::speculative_call_info (cgraph_edge *&direct, reference = NULL; for (i = 0; e->caller->iterate_reference (i, ref); i++) - if (ref->speculative + if (ref->speculative && ref->speculative_id == e->speculative_id && ((ref->stmt && ref->stmt == e->call_stmt) || (!ref->stmt && ref->lto_stmt_uid == e->lto_stmt_uid))) { @@ -1189,7 +1225,21 @@ cgraph_edge::resolve_speculation (tree callee_decl) in the functions inlined through it. */ } edge->count += e2->count; - edge->speculative = false; + /* edge is indirect, e2 is direct. If edge contains multiple speculations, + remove one of speculations for this indirect edge, then if edge still + contains direct target, keep the speculation, next direct target + will continue use it. Give up speculation completely if no direct + target is left for this indirect edge. */ + if (edge->has_indirect_call_p ()) + { + /* As the direct targets are sorted by decrease, delete the first target + when it is resolved. */ + edge->indirect_info->indirect_call_targets->ordered_remove (0); + if (edge->indirect_info->indirect_call_targets->is_empty ()) + edge->speculative = false; + } + else + edge->speculative = false; e2->speculative = false; ref->remove_reference (); if (e2->indirect_unknown_callee || e2->inline_failed) @@ -1297,7 +1347,21 @@ cgraph_edge::redirect_call_stmt_to_callee (void) e->caller->set_call_stmt_including_clones (e->call_stmt, new_stmt, false); e->count = gimple_bb (e->call_stmt)->count; - e2->speculative = false; + /* edge is direct, e2 is indirect here. If e2 contains multiple + speculations, remove one of speculations for this indirect edge, + then if e2 still contains direct target, keep the speculation, + next direct target will continue use it. Give up speculation + completely if no direct target is left for this indirect e2. */ + if (e2->has_indirect_call_p ()) + { + /* As the direct targets are sorted by decrease, delete the first + target when it is redirected. */ + e2->indirect_info->indirect_call_targets->ordered_remove (0); + if (e2->indirect_info->indirect_call_targets->is_empty ()) + e2->speculative = false; + } + else + e2->speculative = false; e2->count = gimple_bb (e2->call_stmt)->count; ref->speculative = false; ref->stmt = NULL; @@ -3359,7 +3423,7 @@ cgraph_node::verify_node (void) for (e = callees; e; e = e->next_callee) { - if (!e->aux) + if (!e->aux && !e->speculative) { error ("edge %s->%s has no corresponding call_stmt", identifier_to_locale (e->caller->name ()), @@ -3696,6 +3760,22 @@ cgraph_edge::possibly_call_in_translation_unit_p (void) return node->get_availability () >= AVAIL_INTERPOSABLE; } +/* Return true if this edge has multiple indirect call targets. */ +bool +cgraph_edge::has_multiple_indirect_call_p (void) +{ + return (indirect_info && indirect_info->indirect_call_targets + && indirect_info->indirect_call_targets->length () > 1); +} + +/* Return true if this edge has at least one indirect call target. */ +bool +cgraph_edge::has_indirect_call_p (void) +{ + return (indirect_info && indirect_info->indirect_call_targets + && !indirect_info->indirect_call_targets->is_empty ()); +} + /* A stashed copy of "symtab" for use by selftest::symbol_table_test. This needs to be a global so that it can be a GC root, and thus prevent the stashed copy from being garbage-collected if the GC runs diff --git a/gcc/cgraph.h b/gcc/cgraph.h index a4f14743f00..1299702788d 100644 --- a/gcc/cgraph.h +++ b/gcc/cgraph.h @@ -1636,6 +1636,21 @@ private: void make_speculative (tree otr_type = NULL); }; +/* Structure containing indirect target information from profile. */ + +struct GTY (()) indirect_target_info +{ + indirect_target_info (unsigned int id, int prob) + : common_target_id (id), common_target_probability (prob) + { + } + + /* Profile_id of common target obtained from profile. */ + unsigned int common_target_id; + /* Probability that call will land in function with COMMON_TARGET_ID. */ + int common_target_probability; +}; + /* Structure containing additional information about an indirect call. */ class GTY(()) cgraph_indirect_call_info @@ -1654,10 +1669,9 @@ public: int param_index; /* ECF flags determined from the caller. */ int ecf_flags; - /* Profile_id of common target obtained from profile. */ - int common_target_id; - /* Probability that call will land in function with COMMON_TARGET_ID. */ - int common_target_probability; + + /* An indirect call may contain one or multiple call targets. */ + vec<indirect_target_info, va_gc> *indirect_call_targets; /* Set when the call is a virtual call with the parameter being the associated object pointer rather than a simple direct call. */ @@ -1714,7 +1728,8 @@ public: /* Turn edge into speculative call calling N2. Update the profile so the direct call is taken COUNT times with FREQUENCY. */ - cgraph_edge *make_speculative (cgraph_node *n2, profile_count direct_count); + cgraph_edge *make_speculative (cgraph_node *n2, profile_count direct_count, + unsigned int speculative_id = 0); /* Given speculative call edge, return all three components. */ void speculative_call_info (cgraph_edge *&direct, cgraph_edge *&indirect, @@ -1773,6 +1788,12 @@ public: be internal to the current translation unit. */ bool possibly_call_in_translation_unit_p (void); + /* Return true if this edge has multiple indirect call targets. */ + bool has_multiple_indirect_call_p (void); + + /* Return true if this edge has at least one indirect call target. */ + bool has_indirect_call_p (void); + /* Expected number of executions: calculated in profile.c. */ profile_count count; cgraph_node *caller; @@ -1792,6 +1813,9 @@ public: /* The stmt_uid of call_stmt. This is used by LTO to recover the call_stmt when the function is serialized in. */ unsigned int lto_stmt_uid; + /* speculative id is used by multiple indirect targets when the function is + speculated. */ + unsigned int speculative_id; /* Whether this edge was made direct by indirect inlining. */ unsigned int indirect_inlining_edge : 1; /* Whether this edge describes an indirect call with an undetermined diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c index bfcebb20495..4c8cb82bd20 100644 --- a/gcc/cgraphclones.c +++ b/gcc/cgraphclones.c @@ -128,6 +128,7 @@ 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; + new_edge->speculative_id = speculative_id; /* Clone flags that depend on call_stmt availability manually. */ new_edge->can_throw_external = can_throw_external; new_edge->call_stmt_cannot_inline_p = call_stmt_cannot_inline_p; diff --git a/gcc/ipa-comdats.c b/gcc/ipa-comdats.c index b496497ff66..0672d6d6575 100644 --- a/gcc/ipa-comdats.c +++ b/gcc/ipa-comdats.c @@ -18,7 +18,7 @@ along with GCC; see the file COPYING3. If not see <http://www.gnu.org/licenses/>. */ /* This is very simple pass that looks for static symbols that are used - exlusively by symbol within one comdat group. In this case it makes + exclusively by symbol within one comdat group. In this case it makes sense to bring the symbol itself into the group to avoid dead code that would arrise when the comdat group from current unit is replaced by a different copy. Consider for example: diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c index 78ec0ec685f..8fc67ef88a5 100644 --- a/gcc/ipa-inline.c +++ b/gcc/ipa-inline.c @@ -1945,12 +1945,15 @@ inline_small_functions (void) } if (has_speculative) for (edge = node->callees; edge; edge = next) - if (edge->speculative && !speculation_useful_p (edge, - edge->aux != NULL)) - { - edge->resolve_speculation (); - update = true; - } + { + next = edge->next_callee; + if (edge->speculative + && !speculation_useful_p (edge, edge->aux != NULL)) + { + edge->resolve_speculation (); + update = true; + } + } if (update) { struct cgraph_node *where = node->inlined_to diff --git a/gcc/ipa-profile.c b/gcc/ipa-profile.c index 61fd5e86681..6f66ee1afd1 100644 --- a/gcc/ipa-profile.c +++ b/gcc/ipa-profile.c @@ -191,23 +191,34 @@ ipa_profile_generate_summary (void) if (h) { gcov_type val, count, all; - if (get_nth_most_common_value (NULL, "indirect call", h, - &val, &count, &all)) + struct cgraph_edge *e = node->get_edge (stmt); + if (e && !e->indirect_unknown_callee) + continue; + + for (unsigned j = 0; j < GCOV_TOPN_VALUES; j++) { - struct cgraph_edge * e = node->get_edge (stmt); - if (e && !e->indirect_unknown_callee) + if (!get_nth_most_common_value (NULL, "indirect call", + h, &val, &count, &all, + j)) + continue; + + if (val == 0) continue; - e->indirect_info->common_target_id = val; - e->indirect_info->common_target_probability - = GCOV_COMPUTE_SCALE (count, all); - if (e->indirect_info->common_target_probability > REG_BR_PROB_BASE) + indirect_target_info item (val, + GCOV_COMPUTE_SCALE (count, + all)); + if (item.common_target_probability > REG_BR_PROB_BASE) { if (dump_file) - fprintf (dump_file, "Probability capped to 1\n"); - e->indirect_info->common_target_probability = REG_BR_PROB_BASE; + fprintf (dump_file, + "Probability capped to 1\n"); + item.common_target_probability = REG_BR_PROB_BASE; } + vec_safe_push ( + e->indirect_info->indirect_call_targets, item); } + gimple_remove_histogram_value (DECL_STRUCT_FUNCTION (node->decl), stmt, h); } @@ -512,6 +523,7 @@ ipa_profile (void) int nindirect = 0, ncommon = 0, nunknown = 0, nuseless = 0, nconverted = 0; int nmismatch = 0, nimpossible = 0; bool node_map_initialized = false; + gcov_type threshold; if (dump_file) dump_histogram (dump_file, histogram); @@ -520,14 +532,12 @@ ipa_profile (void) overall_time += histogram[i]->count * histogram[i]->time; overall_size += histogram[i]->size; } + threshold = 0; if (overall_time) { - gcov_type threshold; - gcc_assert (overall_size); cutoff = (overall_time * param_hot_bb_count_ws_permille + 500) / 1000; - threshold = 0; for (i = 0; cumulated < cutoff; i++) { cumulated += histogram[i]->count * histogram[i]->time; @@ -563,7 +573,7 @@ ipa_profile (void) histogram.release (); histogram_pool.release (); - /* Produce speculative calls: we saved common traget from porfiling into + /* Produce speculative calls: we saved common target from profiling into e->common_target_id. Now, at link time, we can look up corresponding function node and produce speculative call. */ @@ -578,13 +588,37 @@ ipa_profile (void) { if (n->count.initialized_p ()) nindirect++; - if (e->indirect_info->common_target_id) + if (e->has_indirect_call_p ()) { if (!node_map_initialized) - init_node_map (false); + init_node_map (false); node_map_initialized = true; ncommon++; - n2 = find_func_by_profile_id (e->indirect_info->common_target_id); + + if (in_lto_p) + { + if (dump_file) + { + fprintf (dump_file, + "Updating hotness threshold in LTO mode.\n"); + fprintf (dump_file, "Updated min count: %" PRId64 "\n", + (int64_t) threshold); + } + set_hot_bb_threshold (threshold + / e->indirect_info->indirect_call_targets->length ()); + } + + unsigned speculative_id = 0; + indirect_target_info *item; + /* The code below is not formatted yet for review convenience. + Move to a seprate small function is not easy as too many local + variables used in it. Need format and remove this comments + once got approved. */ + FOR_EACH_VEC_SAFE_ELT (e->indirect_info->indirect_call_targets, i, + item) + { + bool speculative_found = false; + n2 = find_func_by_profile_id (item->common_target_id); if (n2) { if (dump_file) @@ -593,11 +627,10 @@ ipa_profile (void) " other module %s => %s, prob %3.2f\n", n->dump_name (), n2->dump_name (), - e->indirect_info->common_target_probability - / (float)REG_BR_PROB_BASE); + item->common_target_probability + / (float) REG_BR_PROB_BASE); } - if (e->indirect_info->common_target_probability - < REG_BR_PROB_BASE / 2) + if (item->common_target_probability < REG_BR_PROB_BASE / 2) { nuseless++; if (dump_file) @@ -653,20 +686,30 @@ ipa_profile (void) n2 = alias; } nconverted++; - e->make_speculative - (n2, - e->count.apply_probability - (e->indirect_info->common_target_probability)); + e->make_speculative (n2, + e->count.apply_probability ( + item->common_target_probability), + speculative_id); update = true; + speculative_id++; + speculative_found = true; } } else { if (dump_file) fprintf (dump_file, "Function with profile-id %i not found.\n", - e->indirect_info->common_target_id); + item->common_target_id); nunknown++; } + if (!speculative_found) + { + /* Remove item from indirect_call_targets if no + speculative edge generated, rollback the iteration. */ + e->indirect_info->indirect_call_targets->ordered_remove (i); + i--; + } + } } } if (update) diff --git a/gcc/ipa-ref.h b/gcc/ipa-ref.h index 0d8e509c932..3e6562ec9d1 100644 --- a/gcc/ipa-ref.h +++ b/gcc/ipa-ref.h @@ -59,6 +59,7 @@ public: symtab_node *referred; gimple *stmt; unsigned int lto_stmt_uid; + unsigned int speculative_id; unsigned int referred_index; ENUM_BITFIELD (ipa_ref_use) use:3; unsigned int speculative:1; diff --git a/gcc/ipa.c b/gcc/ipa.c index 0c92980db46..b5d3769b6fd 100644 --- a/gcc/ipa.c +++ b/gcc/ipa.c @@ -166,7 +166,7 @@ process_references (symtab_node *snode, devirtualization happens. After inlining still keep their declarations around, so we can devirtualize to a direct call. - Also try to make trivial devirutalization when no or only one target is + Also try to make trivial devirtualization when no or only one target is possible. */ static void diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c index a4a70e7848c..ef66f128ff4 100644 --- a/gcc/lto-cgraph.c +++ b/gcc/lto-cgraph.c @@ -235,6 +235,7 @@ lto_output_edge (struct lto_simple_output_block *ob, struct cgraph_edge *edge, unsigned int uid; intptr_t ref; struct bitpack_d bp; + unsigned len; if (edge->indirect_unknown_callee) streamer_write_enum (ob->main_stream, LTO_symtab_tags, LTO_symtab_last_tag, @@ -262,6 +263,7 @@ lto_output_edge (struct lto_simple_output_block *ob, struct cgraph_edge *edge, bp_pack_enum (&bp, cgraph_inline_failed_t, CIF_N_REASONS, edge->inline_failed); bp_pack_var_len_unsigned (&bp, uid); + bp_pack_var_len_unsigned (&bp, edge->speculative_id); bp_pack_value (&bp, edge->indirect_inlining_edge, 1); bp_pack_value (&bp, edge->speculative, 1); bp_pack_value (&bp, edge->call_stmt_cannot_inline_p, 1); @@ -288,11 +290,27 @@ lto_output_edge (struct lto_simple_output_block *ob, struct cgraph_edge *edge, streamer_write_bitpack (&bp); if (edge->indirect_unknown_callee) { - streamer_write_hwi_stream (ob->main_stream, - edge->indirect_info->common_target_id); - if (edge->indirect_info->common_target_id) - streamer_write_hwi_stream - (ob->main_stream, edge->indirect_info->common_target_probability); + indirect_target_info *item; + unsigned int i; + len = edge->has_indirect_call_p () + ? edge->indirect_info->indirect_call_targets->length () + : 0; + gcc_assert (len <= GCOV_TOPN_VALUES); + + streamer_write_hwi_stream (ob->main_stream, len); + + if (len) + { + FOR_EACH_VEC_SAFE_ELT (edge->indirect_info->indirect_call_targets, i, + item) + { + gcc_assert (item->common_target_id); + streamer_write_hwi_stream (ob->main_stream, + item->common_target_id); + streamer_write_hwi_stream (ob->main_stream, + item->common_target_probability); + } + } } } @@ -685,6 +703,7 @@ lto_output_ref (struct lto_simple_output_block *ob, struct ipa_ref *ref, if (ref->stmt) uid = gimple_uid (ref->stmt) + 1; streamer_write_hwi_stream (ob->main_stream, uid); + streamer_write_hwi_stream (ob->main_stream, ref->speculative_id); } } @@ -1416,7 +1435,10 @@ input_ref (class lto_input_block *ib, ref = referring_node->create_reference (node, use); ref->speculative = speculative; if (is_a <cgraph_node *> (referring_node)) - ref->lto_stmt_uid = streamer_read_hwi (ib); + { + ref->lto_stmt_uid = streamer_read_hwi (ib); + ref->speculative_id = streamer_read_hwi (ib); + } } /* Read an edge from IB. NODES points to a vector of previously read nodes for @@ -1430,11 +1452,12 @@ input_edge (class lto_input_block *ib, vec<symtab_node *> nodes, { struct cgraph_node *caller, *callee; struct cgraph_edge *edge; - unsigned int stmt_id; + unsigned int stmt_id, speculative_id; profile_count count; cgraph_inline_failed_t inline_failed; struct bitpack_d bp; int ecf_flags = 0; + unsigned i, len; caller = dyn_cast<cgraph_node *> (nodes[streamer_read_hwi (ib)]); if (caller == NULL || caller->decl == NULL_TREE) @@ -1454,6 +1477,7 @@ input_edge (class lto_input_block *ib, vec<symtab_node *> nodes, bp = streamer_read_bitpack (ib); inline_failed = bp_unpack_enum (&bp, cgraph_inline_failed_t, CIF_N_REASONS); stmt_id = bp_unpack_var_len_unsigned (&bp); + speculative_id = bp_unpack_var_len_unsigned (&bp); if (indirect) edge = caller->create_indirect_edge (NULL, 0, count); @@ -1463,6 +1487,7 @@ input_edge (class lto_input_block *ib, vec<symtab_node *> nodes, edge->indirect_inlining_edge = bp_unpack_value (&bp, 1); edge->speculative = bp_unpack_value (&bp, 1); edge->lto_stmt_uid = stmt_id; + edge->speculative_id = speculative_id; edge->inline_failed = inline_failed; edge->call_stmt_cannot_inline_p = bp_unpack_value (&bp, 1); edge->can_throw_external = bp_unpack_value (&bp, 1); @@ -1482,9 +1507,20 @@ input_edge (class lto_input_block *ib, vec<symtab_node *> nodes, if (bp_unpack_value (&bp, 1)) ecf_flags |= ECF_RETURNS_TWICE; edge->indirect_info->ecf_flags = ecf_flags; - edge->indirect_info->common_target_id = streamer_read_hwi (ib); - if (edge->indirect_info->common_target_id) - edge->indirect_info->common_target_probability = streamer_read_hwi (ib); + + len = streamer_read_hwi (ib); + + gcc_assert (len <= GCOV_TOPN_VALUES); + + if (len) + { + for (i = 0; i < len; i++) + { + indirect_target_info item (streamer_read_hwi (ib), + streamer_read_hwi (ib)); + vec_safe_push (edge->indirect_info->indirect_call_targets, item); + } + } } } diff --git a/gcc/predict.c b/gcc/predict.c index 8c66a27d8b6..73be9e7c357 100644 --- a/gcc/predict.c +++ b/gcc/predict.c @@ -761,7 +761,6 @@ dump_prediction (FILE *file, enum br_predictor predictor, int probability, && bb->count.precise_p () && reason == REASON_NONE) { - gcc_assert (e->count ().precise_p ()); fprintf (file, ";;heuristics;%s;%" PRId64 ";%" PRId64 ";%.1f;\n", predictor_info[predictor].name, bb->count.to_gcov_type (), e->count ().to_gcov_type (), diff --git a/gcc/symtab.c b/gcc/symtab.c index 3e634e22c86..0254524dabc 100644 --- a/gcc/symtab.c +++ b/gcc/symtab.c @@ -603,6 +603,7 @@ symtab_node::create_reference (symtab_node *referred_node, ref->referred = referred_node; ref->stmt = stmt; ref->lto_stmt_uid = 0; + ref->speculative_id = 0; ref->use = use_type; ref->speculative = 0; @@ -660,6 +661,7 @@ symtab_node::clone_references (symtab_node *node) ref2 = create_reference (ref->referred, ref->use, ref->stmt); ref2->speculative = speculative; ref2->lto_stmt_uid = stmt_uid; + ref2->speculative_id = ref->speculative_id; } } @@ -678,6 +680,7 @@ symtab_node::clone_referring (symtab_node *node) ref2 = ref->referring->create_reference (this, ref->use, ref->stmt); ref2->speculative = speculative; ref2->lto_stmt_uid = stmt_uid; + ref2->speculative_id = ref->speculative_id; } } @@ -693,6 +696,7 @@ symtab_node::clone_reference (ipa_ref *ref, gimple *stmt) ref2 = create_reference (ref->referred, ref->use, stmt); ref2->speculative = speculative; ref2->lto_stmt_uid = stmt_uid; + ref2->speculative_id = ref->speculative_id; return ref2; } @@ -747,6 +751,7 @@ symtab_node::clear_stmts_in_references (void) { r->stmt = NULL; r->lto_stmt_uid = 0; + r->speculative_id = 0; } } diff --git a/gcc/testsuite/gcc.dg/tree-prof/crossmodule-indir-call-topn-1.c b/gcc/testsuite/gcc.dg/tree-prof/crossmodule-indir-call-topn-1.c new file mode 100644 index 00000000000..a13b08cd60e --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-prof/crossmodule-indir-call-topn-1.c @@ -0,0 +1,33 @@ +/* { dg-require-effective-target lto } */ +/* { dg-additional-sources "crossmodule-indir-call-topn-1a.c" } */ +/* { dg-require-profiling "-fprofile-generate" } */ +/* { dg-options "-O2 -flto -DDOJOB=1 -fdump-ipa-profile_estimate" } */ + +#include <stdio.h> + +typedef int (*fptr) (int); +int +one (int a); + +int +two (int a); + +fptr table[] = {&one, &two}; + +int +main() +{ + int i, x; + fptr p = &one; + + x = one (3); + + for (i = 0; i < 350000000; i++) + { + x = (*p) (3); + p = table[x]; + } + printf ("done:%d\n", x); +} + +/* { dg-final-use-not-autofdo { scan-pgo-wpa-ipa-dump "2 \\(200.00%\\) speculations produced." "profile_estimate" } } */ diff --git a/gcc/testsuite/gcc.dg/tree-prof/crossmodule-indir-call-topn-1a.c b/gcc/testsuite/gcc.dg/tree-prof/crossmodule-indir-call-topn-1a.c new file mode 100644 index 00000000000..a8c6e365fb9 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-prof/crossmodule-indir-call-topn-1a.c @@ -0,0 +1,22 @@ +/* It seems there is no way to avoid the other source of mulitple + source testcase from being compiled independently. Just avoid + error. */ +#ifdef DOJOB +int +one (int a) +{ + return 1; +} + +int +two (int a) +{ + return 0; +} +#else +int +main() +{ + return 0; +} +#endif diff --git a/gcc/testsuite/gcc.dg/tree-prof/crossmodule-indir-call-topn-2.c b/gcc/testsuite/gcc.dg/tree-prof/crossmodule-indir-call-topn-2.c new file mode 100644 index 00000000000..9b996fcf0ed --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-prof/crossmodule-indir-call-topn-2.c @@ -0,0 +1,40 @@ +/* { dg-require-effective-target lto } */ +/* { dg-additional-sources "crossmodule-indir-call-topn-1a.c" } */ +/* { dg-require-profiling "-fprofile-generate" } */ +/* { dg-options "-O2 -flto -DDOJOB=1 -fdump-ipa-profile_estimate" } */ + +#include <stdio.h> + +typedef int (*fptr) (int); +int +one (int a); + +int +two (int a); + +fptr table[] = {&one, &two}; + +int foo () +{ + int i, x; + fptr p = &one; + + x = one (3); + + for (i = 0; i < 350000000; i++) + { + x = (*p) (3); + p = table[x]; + } + return x; +} + +int +main() +{ + int x = foo (); + printf ("done:%d\n", x); +} + +/* { dg-final-use-not-autofdo { scan-pgo-wpa-ipa-dump "2 \\(200.00%\\) speculations produced." "profile_estimate" } } */ + diff --git a/gcc/testsuite/gcc.dg/tree-prof/indir-call-prof-topn.c b/gcc/testsuite/gcc.dg/tree-prof/indir-call-prof-topn.c new file mode 100644 index 00000000000..063996c71df --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-prof/indir-call-prof-topn.c @@ -0,0 +1,37 @@ +/* { dg-require-profiling "-fprofile-generate" } */ +/* { dg-options "-O2 -fdump-ipa-profile_estimate" } */ + +#include <stdio.h> + +typedef int (*fptr) (int); +int +one (int a) +{ + return 1; +} + +int +two (int a) +{ + return 0; +} + +fptr table[] = {&one, &two}; + +int +main() +{ + int i, x; + fptr p = &one; + + one (3); + + for (i = 0; i < 350000000; i++) + { + x = (*p) (3); + p = table[x]; + } + printf ("done:%d\n", x); +} + +/* { dg-final-use-not-autofdo { scan-ipa-dump "2 \\(200.00%\\) speculations produced." "profile_estimate" } } */ diff --git a/gcc/testsuite/lib/scandump.exp b/gcc/testsuite/lib/scandump.exp index 42f5c01aa60..8b7cd7cfc16 100644 --- a/gcc/testsuite/lib/scandump.exp +++ b/gcc/testsuite/lib/scandump.exp @@ -70,6 +70,7 @@ proc scan-dump { args } { set output_file "[glob -nocomplain $dumpbase.[lindex $args 2]]" if { $output_file == "" } { verbose -log "$testcase: dump file does not exist" + verbose -log "dump file: $dumpbase.$suf" unresolved "$testname" return } diff --git a/gcc/testsuite/lib/scanwpaipa.exp b/gcc/testsuite/lib/scanwpaipa.exp index b5549fd688e..8aafd6c82e8 100644 --- a/gcc/testsuite/lib/scanwpaipa.exp +++ b/gcc/testsuite/lib/scanwpaipa.exp @@ -45,6 +45,29 @@ proc scan-wpa-ipa-dump { args } { } } +# Argument 0 is the regexp to match +# Argument 1 is the name of the dumped ipa pass +# Argument 2 handles expected failures and the like +proc scan-pgo-wpa-ipa-dump { args } { + + if { [llength $args] < 2 } { + error "scan-pgo-wpa-ipa-dump: too few arguments" + return + } + if { [llength $args] > 3 } { + error "scan-pgo-wpa-ipa-dump: too many arguments" + return + } + if { [llength $args] >= 3 } { + scan-dump "pgo-wpa-ipa" [lindex $args 0] \ + "\[0-9\]\[0-9\]\[0-9\]i.[lindex $args 1]" ".x02.wpa" \ + [lindex $args 2] + } else { + scan-dump "pgo-wpa-ipa" [lindex $args 0] \ + "\[0-9\]\[0-9\]\[0-9\]i.[lindex $args 1]" ".x02.wpa" + } +} + # Call pass if pattern is present given number of times, otherwise fail. # Argument 0 is the regexp to match # Argument 1 is number of times the regexp must be found diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index bdc332dcc23..6c8ebffa11d 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -2194,6 +2194,25 @@ copy_bb (copy_body_data *id, basic_block bb, gcc_assert (!edge->indirect_unknown_callee); old_edge->speculative_call_info (direct, indirect, ref); + while (old_edge->next_callee + && old_edge->next_callee->speculative + && indirect->has_multiple_indirect_call_p ()) + { + /* Some speculative calls may contain more than + one direct target, loop iterate it to clone all + related direct edges before cloning the related + indirect edge. */ + id->dst_node->clone_reference (ref, stmt); + + edge = old_edge->next_callee; + edge = edge->clone (id->dst_node, call_stmt, + gimple_uid (stmt), num, den, + true); + old_edge = old_edge->next_callee; + gcc_assert (!edge->indirect_unknown_callee); + old_edge->speculative_call_info (direct, indirect, + ref); + } profile_count indir_cnt = indirect->count; indirect = indirect->clone (id->dst_node, call_stmt, diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c index 6a4e62f5bae..b4435b9b2a8 100644 --- a/gcc/tree-profile.c +++ b/gcc/tree-profile.c @@ -73,8 +73,8 @@ static GTY(()) tree ic_tuple_callee_field; /* Do initialization work for the edge profiler. */ /* Add code: - __thread gcov* __gcov_indirect_call_counters; // pointer to actual counter - __thread void* __gcov_indirect_call_callee; // actual callee address + __thread gcov* __gcov_indirect_call.counters; // pointer to actual counter + __thread void* __gcov_indirect_call.callee; // actual callee address __thread int __gcov_function_counter; // time profiler function counter */ static void @@ -381,7 +381,7 @@ gimple_gen_ic_profiler (histogram_value value, unsigned tag) f_1 = foo; __gcov_indirect_call.counters = &__gcov4.main[0]; PROF_9 = f_1; - __gcov_indirect_call_callee = PROF_9; + __gcov_indirect_call.callee = PROF_9; _4 = f_1 (); */ @@ -444,11 +444,11 @@ gimple_gen_ic_func_profiler (void) /* Insert code: - if (__gcov_indirect_call_callee != NULL) + if (__gcov_indirect_call.callee != NULL) __gcov_indirect_call_profiler_v3 (profile_id, ¤t_function_decl); The function __gcov_indirect_call_profiler_v3 is responsible for - resetting __gcov_indirect_call_callee to NULL. */ + resetting __gcov_indirect_call.callee to NULL. */ gimple_stmt_iterator gsi = gsi_start_bb (cond_bb); void0 = build_int_cst (ptr_type_node, 0); @@ -890,7 +890,7 @@ pass_ipa_tree_profile::gate (function *) { /* When profile instrumentation, use or test coverage shall be performed. But for AutoFDO, this there is no instrumentation, thus this pass is - diabled. */ + disabled. */ return (!in_lto_p && !flag_auto_profile && (flag_branch_probabilities || flag_test_coverage || profile_arc_flag)); diff --git a/gcc/value-prof.c b/gcc/value-prof.c index cc3542f0295..f64f515c1ee 100644 --- a/gcc/value-prof.c +++ b/gcc/value-prof.c @@ -106,7 +106,7 @@ static bool gimple_divmod_fixed_value_transform (gimple_stmt_iterator *); static bool gimple_mod_pow2_value_transform (gimple_stmt_iterator *); static bool gimple_mod_subtract_transform (gimple_stmt_iterator *); static bool gimple_stringops_transform (gimple_stmt_iterator *); -static bool gimple_ic_transform (gimple_stmt_iterator *); +static void gimple_ic_transform (gimple_stmt_iterator *); /* Allocate histogram value. */ @@ -616,8 +616,7 @@ gimple_value_profile_transformations (void) if (gimple_mod_subtract_transform (&gsi) || gimple_divmod_fixed_value_transform (&gsi) || gimple_mod_pow2_value_transform (&gsi) - || gimple_stringops_transform (&gsi) - || gimple_ic_transform (&gsi)) + || gimple_stringops_transform (&gsi)) { stmt = gsi_stmt (gsi); changed = true; @@ -628,6 +627,9 @@ gimple_value_profile_transformations (void) gsi = gsi_for_stmt (stmt); } } + + /* The function never thansforms a GIMPLE statement. */ + gimple_ic_transform (&gsi); } } @@ -1386,13 +1388,12 @@ gimple_ic (gcall *icall_stmt, struct cgraph_node *direct_call, return dcall_stmt; } -/* - For every checked indirect/virtual call determine if most common pid of - function/class method has probability more than 50%. If yes modify code of - this call to: - */ +/* There maybe multiple indirect targets in histogram. Check every + indirect/virtual call if callee function exists, if not exist, leave it to + LTO stage for later process. Modify code of this indirect call to an if-else + structure in ipa-profile finally. */ -static bool +static void gimple_ic_transform (gimple_stmt_iterator *gsi) { gcall *stmt; @@ -1402,52 +1403,58 @@ gimple_ic_transform (gimple_stmt_iterator *gsi) stmt = dyn_cast <gcall *> (gsi_stmt (*gsi)); if (!stmt) - return false; + return; if (gimple_call_fndecl (stmt) != NULL_TREE) - return false; + return; if (gimple_call_internal_p (stmt)) - return false; + return; histogram = gimple_histogram_value_of_type (cfun, stmt, HIST_TYPE_INDIR_CALL); if (!histogram) - return false; + return; - if (!get_nth_most_common_value (NULL, "indirect call", histogram, &val, - &count, &all)) - return false; + count = 0; + all = histogram->hvalue.counters[0]; - if (4 * count <= 3 * all) - return false; + for (unsigned j = 0; j < GCOV_TOPN_VALUES; j++) + { + if (!get_nth_most_common_value (NULL, "indirect call", histogram, &val, + &count, &all, j)) + return; - direct_call = find_func_by_profile_id ((int)val); + /* Minimum probability. should be higher than 25%. */ + if (4 * count <= all) + return; - if (direct_call == NULL) - { - if (val) + direct_call = find_func_by_profile_id ((int) val); + + if (direct_call == NULL) { - if (dump_enabled_p ()) - dump_printf_loc (MSG_MISSED_OPTIMIZATION, stmt, - "Indirect call -> direct call from other " - "module %T=> %i (will resolve only with LTO)\n", - gimple_call_fn (stmt), (int)val); + if (val) + { + if (dump_enabled_p ()) + dump_printf_loc ( + MSG_MISSED_OPTIMIZATION, stmt, + "Indirect call -> direct call from other " + "module %T=> %i (will resolve only with LTO)\n", + gimple_call_fn (stmt), (int) val); + } + return; } - return false; - } - if (dump_enabled_p ()) - { - dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, stmt, - "Indirect call -> direct call " - "%T => %T transformation on insn postponed\n", - gimple_call_fn (stmt), direct_call->decl); - dump_printf_loc (MSG_NOTE, stmt, - "hist->count %" PRId64 - " hist->all %" PRId64"\n", count, all); + if (dump_enabled_p ()) + { + dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, stmt, + "Indirect call -> direct call " + "%T => %T transformation on insn postponed\n", + gimple_call_fn (stmt), direct_call->decl); + dump_printf_loc (MSG_NOTE, stmt, + "hist->count %" PRId64 " hist->all %" PRId64 "\n", + count, all); + } } - - return true; } /* Return true if the stringop CALL shall be profiled. SIZE_ARG be diff --git a/gcc/value-prof.h b/gcc/value-prof.h index 77c06f60096..b3eeb57d37d 100644 --- a/gcc/value-prof.h +++ b/gcc/value-prof.h @@ -89,7 +89,6 @@ void verify_histograms (void); void free_histograms (function *); void stringop_block_profile (gimple *, unsigned int *, HOST_WIDE_INT *); gcall *gimple_ic (gcall *, struct cgraph_node *, profile_probability); -bool check_ic_target (gcall *, struct cgraph_node *); bool get_nth_most_common_value (gimple *stmt, const char *counter_type, histogram_value hist, gcov_type *value, gcov_type *count, gcov_type *all,