Re: [PATCH] profile feedback: -fprofile-use= and -fprofile-correction, correctness fixes and option semantic changes.

["Seongbae Park (박성배, 朴成培)" <seongbae dot park at gmail dot com>]

On Sat, Mar 29, 2008 at 9:23 PM, Seongbae Park (박성배, 朴成培)
<seongbae.park@gmail.com> wrote:
...
>  >  > @@ -335,6 +338,16 @@ tree_gen_ic_func_profiler (void)
>  >  >                              cur_func,
>  >  >                              ic_void_ptr_var);
>  >  >        bsi_insert_after (&bsi, stmt1, BSI_NEW_STMT);
>  >  > +
>  >  > +      gcc_assert (EDGE_COUNT (bb->succs) == 1);
>  >  > +      bb = split_edge (EDGE_I (bb->succs, 0));
>  >  > +      bsi = bsi_start (bb);
>  >  > +      /* Set __gcov_indirect_call_callee to 0,
>  >  > +         so that calls from other modules won't get misattributed
>  >  > +      to the last caller of the current callee. */
>  >  > +      void0 = build_int_cst (build_pointer_type (void_type_node), 0);
>  >  > +      stmt2 = build_gimple_modify_stmt (ic_void_ptr_var, void0);
>  >  > +      bsi_insert_after (&bsi, stmt2, BSI_NEW_STMT);
>  >
>  >  This is unrealted, right?  Please put it in as separate patch. It is OK.
>
>  Yes. I'll check in as a separate patch.

Attached please find the patch for the above hunk, with a testcase.
The hunk was tested as part of the other patch. The extra test was tested
by running make check-gcc RUNTESTFLAGS=...
The one minor problem is that gcda files and tree_profile dump files
generated for ic-misattribution-1.c file are not cleaned up.
Looks like I'll have to modify cleanup-tree-dump to consider additional sources
but it looks quite ugly to do at this point...

Seongbae

gcc/ChangeLog:

2008-04-01  Seongbae Park <seongbae.park@gmail.com>

        * value-prof.c (tree_ic_transform): Print counts.
        * tree-profile.c (tree_gen_ic_func_profiler):
        Clear __gcov_indreict_call_callee variable
        to avoid misattribution of the profile.

gcc/testsuite/ChangeLog:

2008-04-01  Seongbae Park <seongbae.park@gmail.com>

        * gcc.dg/tree-prof/ic-misattribution-1.c: New test.
        * gcc.dg/tree-prof/ic-misattribution-2.c: New test.
        * lib/profopt.exp (profopt-get-options): Support dg-additional-sources.
        (profopt-execute): Handle additional sources.

Index: gcc/value-prof.c
===================================================================
--- gcc/value-prof.c	(revision 133772)
+++ gcc/value-prof.c	(working copy)
@@ -1198,6 +1198,7 @@ tree_ic_transform (tree stmt)
       print_generic_stmt (dump_file, stmt, TDF_SLIM);
       fprintf (dump_file, " to ");
       print_generic_stmt (dump_file, modify, TDF_SLIM);
+      fprintf (dump_file, "hist->count %llu hist->all %llu\n", count, all);
     }
 
   return true;
Index: gcc/testsuite/gcc.dg/tree-prof/ic-misattribution-1.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-prof/ic-misattribution-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/tree-prof/ic-misattribution-1.c	(revision 0)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+extern void callee (void);
+extern void caller (void (*func) (void));
+
+typedef void (*func_t) (void);
+func_t func;
+
+int
+main ()
+{
+  func = callee;
+  caller (callee);
+  func ();
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/tree-prof/ic-misattribution-2.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-prof/ic-misattribution-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/tree-prof/ic-misattribution-2.c	(revision 0)
@@ -0,0 +1,19 @@
+/* { dg-options "-O2 -fdump-tree-tree_profile" } */
+/* { dg-additional-sources "ic-misattribution-1.c" } */
+
+extern void other_caller (void);
+
+void
+callee (void)
+{
+  return;
+}
+
+void
+caller(void (*func) (void))
+{
+  func ();
+}
+
+/* { dg-final-use { scan-tree-dump "hist->count 1 hist->all 1" "tree_profile" } } */
+/* { dg-final-use { cleanup-tree-dump "tree_profile" } } */
Index: gcc/testsuite/lib/profopt.exp
===================================================================
--- gcc/testsuite/lib/profopt.exp	(revision 133772)
+++ gcc/testsuite/lib/profopt.exp	(working copy)
@@ -184,6 +184,7 @@ proc profopt-get-options { src } {
 	     || ![string compare "dg-skip-if" $cmd] \
 	     || ![string compare "dg-final-generate" $cmd] \
 	     || ![string compare "dg-final-use" $cmd] \
+             || ![string compare "dg-additional-sources" $cmd] \
 	     || [string match "dg-require-*" $cmd]  } {
 	    set status [catch "$op" errmsg]
 	    if { $status != 0 } {
@@ -266,9 +267,11 @@ proc profopt-execute { src } {
 	    return
 	}
 
+        set extra_options [dg-additional-files-options "" "$src"]
+
 	# Compile for profiling.
 
-	set options ""
+	set options "$extra_options"
 	lappend options "additional_flags=$option $extra_flags $profile_option"
 	set optstr "$option $profile_option"
 	set comp_output [${tool}_target_compile "$src" "$execname1" executable $options]
@@ -317,7 +320,7 @@ proc profopt-execute { src } {
 
 	# Compile with feedback-directed optimizations.
 
-	set options ""
+	set options "$extra_options"
 	lappend options "additional_flags=$option $extra_flags $feedback_option"
 	set optstr "$option $feedback_option"
 	set comp_output [${tool}_target_compile "$src" "$execname2" "executable" $options]
@@ -364,7 +367,7 @@ proc profopt-execute { src } {
 
 	# Compile with normal optimizations.
 
-	set options ""
+	set options "$extra_options"
 	lappend options "additional_flags=$option"
 	set optstr "$option"
 	set comp_output [${tool}_target_compile "$src" "$execname3" "executable" $options]
Index: gcc/tree-profile.c
===================================================================
--- gcc/tree-profile.c	(revision 133772)
+++ gcc/tree-profile.c	(working copy)
@@ -308,7 +308,7 @@ tree_gen_ic_func_profiler (void)
   edge e;
   basic_block bb;
   edge_iterator ei;
-  tree stmt1;
+  tree stmt1, stmt2;
   tree tree_uid, cur_func;
 
   if (flag_unit_at_a_time)
@@ -321,8 +321,11 @@ tree_gen_ic_func_profiler (void)
   
   FOR_EACH_EDGE (e, ei, ENTRY_BLOCK_PTR->succs)
     {
+      tree void0;
+
       bb = split_edge (e);
       bsi = bsi_start (bb);
+
       cur_func = force_gimple_operand_bsi (&bsi,
 					   build_addr (current_function_decl, 
 						       current_function_decl),
@@ -335,6 +338,16 @@ tree_gen_ic_func_profiler (void)
 			       cur_func,
 			       ic_void_ptr_var);
       bsi_insert_after (&bsi, stmt1, BSI_NEW_STMT);
+
+      gcc_assert (EDGE_COUNT (bb->succs) == 1);
+      bb = split_edge (EDGE_I (bb->succs, 0));
+      bsi = bsi_start (bb);
+      /* Set __gcov_indirect_call_callee to 0,
+         so that calls from other modules won't get misattributed
+	 to the last caller of the current callee. */
+      void0 = build_int_cst (build_pointer_type (void_type_node), 0);
+      stmt2 = build_gimple_modify_stmt (ic_void_ptr_var, void0);
+      bsi_insert_after (&bsi, stmt2, BSI_NEW_STMT);
     }
 }