Make FDO more tolerant to source changes

From-SVN: r213068

gcc/ChangeLog

@@ -1,3 +1,14 @@
+2014-07-25  Xinliang David Li  <davidxl@google.com>
+
+	* 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  <hubicka@ucw.cz>
 	    Richard Biener <rguenther@suse.de>
 

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<coverage_data> ();
-	  
-	  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;
 

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

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

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",

gcc/testsuite/ChangeLog

@@ -1,3 +1,11 @@
+2014-07-25  Xinliang David Li  <davidxl@google.com>
+
+	* 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

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" } } */
+

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" } } */
+

gcc/testsuite/g++.dg/tree-prof/reorder_class1.h

@@ -0,0 +1,11 @@
+struct A {
+  virtual int foo();
+};
+
+int A::foo()
+{
+  return 1;
+}
+
+
+

gcc/testsuite/g++.dg/tree-prof/reorder_class2.h

@@ -0,0 +1,12 @@
+
+struct B {
+  virtual int foo();
+};
+
+int B::foo()
+{
+  return 2;
+}
+
+
+

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.

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)