From d9e50003cdcadbf7e4f2564ebee9ad108633afd8 Mon Sep 17 00:00:00 2001 From: davidxl Date: Sat, 26 Jul 2014 00:06:56 +0000 Subject: [PATCH] Make FDO more tolerant to source changes git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@213068 138bc75d-0d04-0410-961f-82ee72b054a4 --- gcc/ChangeLog | 11 +++++ gcc/coverage.c | 39 ++++++++++++++---- gcc/coverage.h | 1 + gcc/doc/invoke.texi | 8 ++++ gcc/params.def | 8 ++++ gcc/testsuite/ChangeLog | 8 ++++ gcc/testsuite/g++.dg/tree-prof/morefunc.C | 55 +++++++++++++++++++++++++ gcc/testsuite/g++.dg/tree-prof/reorder.C | 48 +++++++++++++++++++++ gcc/testsuite/g++.dg/tree-prof/reorder_class1.h | 11 +++++ gcc/testsuite/g++.dg/tree-prof/reorder_class2.h | 12 ++++++ gcc/testsuite/g++.dg/tree-prof/tree-prof.exp | 4 +- gcc/value-prof.c | 15 +++++-- 12 files changed, 207 insertions(+), 13 deletions(-) create mode 100644 gcc/testsuite/g++.dg/tree-prof/morefunc.C create mode 100644 gcc/testsuite/g++.dg/tree-prof/reorder.C create mode 100644 gcc/testsuite/g++.dg/tree-prof/reorder_class1.h create mode 100644 gcc/testsuite/g++.dg/tree-prof/reorder_class2.h diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 5a50820..b2d5532 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,14 @@ +2014-07-25 Xinliang David Li + + * params.def: New parameter. + * coverage.c (get_coverage_counts): Check new flag. + (coverage_compute_profile_id): Check new flag. + (coverage_begin_function): Check new flag. + (coverage_end_function): Check new flag. + * value-prof.c (coverage_node_map_initialized_p): New function. + (init_node_map): Populate map with all functions. + * doc/invoke.texi: Document new parameter. + 2014-07-25 Jan Hubicka Richard Biener diff --git a/gcc/coverage.c b/gcc/coverage.c index 8ac1d5f..dd7655d 100644 --- a/gcc/coverage.c +++ b/gcc/coverage.c @@ -54,6 +54,7 @@ along with GCC; see the file COPYING3. If not see #include "intl.h" #include "filenames.h" #include "target.h" +#include "params.h" #include "gcov-io.h" #include "gcov-io.c" @@ -369,8 +370,13 @@ get_coverage_counts (unsigned counter, unsigned expected, da_file_name); return NULL; } - - elt.ident = current_function_funcdef_no + 1; + if (PARAM_VALUE (PARAM_PROFILE_FUNC_INTERNAL_ID)) + elt.ident = current_function_funcdef_no + 1; + else + { + gcc_assert (coverage_node_map_initialized_p ()); + elt.ident = cgraph_node::get (cfun->decl)->profile_id; + } elt.ctr = counter; entry = counts_hash->find (&elt); if (!entry || !entry->summary.num) @@ -416,7 +422,8 @@ get_coverage_counts (unsigned counter, unsigned expected, } else if (entry->lineno_checksum != lineno_checksum) { - warning (0, "source locations for function %qE have changed," + warning (OPT_Wcoverage_mismatch, + "source locations for function %qE have changed," " the profile data may be out of date", DECL_ASSEMBLER_NAME (current_function_decl)); } @@ -581,12 +588,13 @@ coverage_compute_profile_id (struct cgraph_node *n) { expanded_location xloc = expand_location (DECL_SOURCE_LOCATION (n->decl)); + bool use_name_only = (PARAM_VALUE (PARAM_PROFILE_FUNC_INTERNAL_ID) == 0); - chksum = xloc.line; + chksum = (use_name_only ? 0 : xloc.line); chksum = coverage_checksum_string (chksum, xloc.file); chksum = coverage_checksum_string (chksum, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (n->decl))); - if (first_global_object_name) + if (!use_name_only && first_global_object_name) chksum = coverage_checksum_string (chksum, first_global_object_name); chksum = coverage_checksum_string @@ -645,7 +653,15 @@ coverage_begin_function (unsigned lineno_checksum, unsigned cfg_checksum) /* Announce function */ offset = gcov_write_tag (GCOV_TAG_FUNCTION); - gcov_write_unsigned (current_function_funcdef_no + 1); + if (PARAM_VALUE (PARAM_PROFILE_FUNC_INTERNAL_ID)) + gcov_write_unsigned (current_function_funcdef_no + 1); + else + { + gcc_assert (coverage_node_map_initialized_p ()); + gcov_write_unsigned ( + cgraph_node::get (current_function_decl)->profile_id); + } + gcov_write_unsigned (lineno_checksum); gcov_write_unsigned (cfg_checksum); gcov_write_string (IDENTIFIER_POINTER @@ -682,8 +698,15 @@ coverage_end_function (unsigned lineno_checksum, unsigned cfg_checksum) if (!DECL_EXTERNAL (current_function_decl)) { item = ggc_alloc (); - - item->ident = current_function_funcdef_no + 1; + + if (PARAM_VALUE (PARAM_PROFILE_FUNC_INTERNAL_ID)) + item->ident = current_function_funcdef_no + 1; + else + { + gcc_assert (coverage_node_map_initialized_p ()); + item->ident = cgraph_node::get (cfun->decl)->profile_id; + } + item->lineno_checksum = lineno_checksum; item->cfg_checksum = cfg_checksum; diff --git a/gcc/coverage.h b/gcc/coverage.h index a144e0b..2ea5293 100644 --- a/gcc/coverage.h +++ b/gcc/coverage.h @@ -56,5 +56,6 @@ extern gcov_type *get_coverage_counts (unsigned /*counter*/, const struct gcov_ctr_summary **); extern tree get_gcov_type (void); +extern bool coverage_node_map_initialized_p (void); #endif diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index c1b37f1..aaa5a68 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -9659,6 +9659,14 @@ Deeper chains are still handled by late inlining. Probability (in percent) that C++ inline function with comdat visibility are shared across multiple compilation units. The default value is 20. +@item profile-func-internal-id +@itemx profile-func-internal-id +A parameter to control whether to use function internal id in profile +database lookup. If the value is 0, the compiler will use id that +is based on function assembler name and filename, which makes old profile +data more tolerant to source changes such as function reordering etc. +The default value is 0. + @item min-vect-loop-bound The minimum number of iterations under which loops are not vectorized when @option{-ftree-vectorize} is used. The number of iterations after diff --git a/gcc/params.def b/gcc/params.def index 595a093..cad00e2 100644 --- a/gcc/params.def +++ b/gcc/params.def @@ -874,6 +874,14 @@ DEFPARAM (PARAM_LOOP_INVARIANT_MAX_BBS_IN_LOOP, "Max basic blocks number in loop for loop invariant motion", 10000, 0, 0) +/* When the parameter is 1, use the internal function id + to look up for profile data. Otherwise, use a more stable + external id based on assembler name and source location. */ +DEFPARAM (PARAM_PROFILE_FUNC_INTERNAL_ID, + "profile-func-internal-id", + "use internal function id in profile lookup", + 0, 0, 1) + /* Avoid SLP vectorization of large basic blocks. */ DEFPARAM (PARAM_SLP_MAX_INSNS_IN_BB, "slp-max-insns-in-bb", diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 92d1f96..1b842d8 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,11 @@ +2014-07-25 Xinliang David Li + + * g++.dg/tree-prof/tree-prof.exp: Define macros. + * g++.dg/tree-prof/reorder_class1.h: New file. + * g++.dg/tree-prof/reorder_class2.h: New file. + * g++.dg/tree-prof/reorder.C: New test. + * g++.dg/tree-prof/morefunc.C: New test. + 2014-07-25 Edward Smith-Rowland <3dw4rd@verizon.net> Implement N4051 - Allow typename in a template template parameter diff --git a/gcc/testsuite/g++.dg/tree-prof/morefunc.C b/gcc/testsuite/g++.dg/tree-prof/morefunc.C new file mode 100644 index 0000000..d5cee40 --- /dev/null +++ b/gcc/testsuite/g++.dg/tree-prof/morefunc.C @@ -0,0 +1,55 @@ +/* { dg-options "-O2 -fno-devirtualize --param=profile-func-internal-id=0 -fdump-ipa-profile -Wno-attributes -Wno-coverage-mismatch" } */ +#include "reorder_class1.h" +#include "reorder_class2.h" + +int g; + +#ifdef _PROFILE_USE +/* Another function not existing + * in profile-gen */ + +__attribute__((noinline)) void +new_func (int i) +{ + g += i; +} +#endif + +static __attribute__((always_inline)) +void test1 (A *tc) +{ + int i; + for (i = 0; i < 1000; i++) + g += tc->foo(); + if (g<100) g++; +} + +static __attribute__((always_inline)) +void test2 (B *tc) +{ + int i; + for (i = 0; i < 1000; i++) + g += tc->foo(); +} + + +__attribute__((noinline)) void test_a(A *ap) { test1 (ap); } +__attribute__((noinline)) void test_b(B *bp) { test2 (bp); } + + +int main() +{ + A* ap = new A(); + B* bp = new B(); + + test_a(ap); + test_b(bp); + +#ifdef _PROFILE_USE + new_func(10); +#endif + +} + +/* { dg-final-use { scan-ipa-dump-times "Indirect call -> direct call" 2 "profile" } } */ + diff --git a/gcc/testsuite/g++.dg/tree-prof/reorder.C b/gcc/testsuite/g++.dg/tree-prof/reorder.C new file mode 100644 index 0000000..223bcf9 --- /dev/null +++ b/gcc/testsuite/g++.dg/tree-prof/reorder.C @@ -0,0 +1,48 @@ +/* { dg-options "-O2 -fno-devirtualize --param=profile-func-internal-id=0 -fdump-ipa-profile -Wno-coverage-mismatch -Wno-attributes" } */ + +#ifdef _PROFILE_USE +#include "reorder_class1.h" +#include "reorder_class2.h" +#else +#include "reorder_class2.h" +#include "reorder_class1.h" +#endif + +int g; +static __attribute__((always_inline)) +void test1 (A *tc) +{ + int i; + for (i = 0; i < 1000; i++) + g += tc->foo(); + if (g<100) g++; +} + +static __attribute__((always_inline)) +void test2 (B *tc) +{ + int i; + for (i = 0; i < 1000; i++) + g += tc->foo(); +} + + +#ifdef _PROFILE_USE +__attribute__((noinline)) void test_a(A *ap) { test1 (ap); } +__attribute__((noinline)) void test_b(B *bp) { test2 (bp); } +#else +__attribute__((noinline)) void test_b(B *bp) { test2 (bp); } +__attribute__((noinline)) void test_a(A *ap) { test1 (ap); } +#endif + +int main() +{ + A* ap = new A(); + B* bp = new B(); + + test_a(ap); + test_b(bp); +} + +/* { dg-final-use { scan-ipa-dump-times "Indirect call -> direct call" 2 "profile" } } */ + diff --git a/gcc/testsuite/g++.dg/tree-prof/reorder_class1.h b/gcc/testsuite/g++.dg/tree-prof/reorder_class1.h new file mode 100644 index 0000000..62a1e92 --- /dev/null +++ b/gcc/testsuite/g++.dg/tree-prof/reorder_class1.h @@ -0,0 +1,11 @@ +struct A { + virtual int foo(); +}; + +int A::foo() +{ + return 1; +} + + + diff --git a/gcc/testsuite/g++.dg/tree-prof/reorder_class2.h b/gcc/testsuite/g++.dg/tree-prof/reorder_class2.h new file mode 100644 index 0000000..ee3ed10 --- /dev/null +++ b/gcc/testsuite/g++.dg/tree-prof/reorder_class2.h @@ -0,0 +1,12 @@ + +struct B { + virtual int foo(); +}; + +int B::foo() +{ + return 2; +} + + + diff --git a/gcc/testsuite/g++.dg/tree-prof/tree-prof.exp b/gcc/testsuite/g++.dg/tree-prof/tree-prof.exp index 2c96ee3..f12ddaf 100644 --- a/gcc/testsuite/g++.dg/tree-prof/tree-prof.exp +++ b/gcc/testsuite/g++.dg/tree-prof/tree-prof.exp @@ -42,8 +42,8 @@ set PROFOPT_OPTIONS [list {}] # These are globals used by profopt-execute. The first is options # needed to generate profile data, the second is options to use the # profile data. -set profile_option "-fprofile-generate" -set feedback_option "-fprofile-use" +set profile_option "-fprofile-generate -D_PROFILE_GENERATE" +set feedback_option "-fprofile-use -D_PROFILE_USE" foreach src [lsort [glob -nocomplain $srcdir/$subdir/*.C]] { # If we're only testing specific files and this isn't one of them, skip it. diff --git a/gcc/value-prof.c b/gcc/value-prof.c index 54a7feb..3e51539 100644 --- a/gcc/value-prof.c +++ b/gcc/value-prof.c @@ -1210,7 +1210,17 @@ gimple_mod_subtract_transform (gimple_stmt_iterator *si) return true; } -static pointer_map_t *cgraph_node_map; +static pointer_map_t *cgraph_node_map = 0; + +/* Returns true if node graph is initialized. This + is used to test if profile_id has been created + for cgraph_nodes. */ + +bool +coverage_node_map_initialized_p (void) +{ + return cgraph_node_map != 0; +} /* Initialize map from PROFILE_ID to CGRAPH_NODE. When LOCAL is true, the PROFILE_IDs are computed. when it is false we assume @@ -1223,8 +1233,7 @@ init_node_map (bool local) cgraph_node_map = pointer_map_create (); FOR_EACH_DEFINED_FUNCTION (n) - if (n->has_gimple_body_p () - && !n->only_called_directly_p ()) + if (n->has_gimple_body_p ()) { void **val; if (local) -- 2.7.4