Re: [GOOGLE] Writes annotation info in elf section.

2014-03-12 Thread Xinliang David Li
Dehao explained that the data needs to merged during link time to give
meaningful diagnostics.

Ok for google branch.


David

On Wed, Mar 12, 2014 at 3:55 PM, Xinliang David Li  wrote:
> Why is it not enough to emit warnings during build time when source
> code changes signifcantly?
>
> David
>
> On Tue, Mar 11, 2014 at 4:27 PM, Dehao Chen  wrote:
>> During AutoFDO annotation, we want to record the annotation stats into
>> an elf section, so that we can calculate how much percentage of the
>> profile is annotated, which can be used as an indicator whether code
>> has changed significantly comparing with the profiled source.
>>
>> Bootstrapped and performance test on-going.
>>
>> OK for google-4_8?
>>
>> Thanks,
>> Dehao


Re: [GOOGLE] Writes annotation info in elf section.

2014-03-12 Thread Xinliang David Li
Why is it not enough to emit warnings during build time when source
code changes signifcantly?

David

On Tue, Mar 11, 2014 at 4:27 PM, Dehao Chen  wrote:
> During AutoFDO annotation, we want to record the annotation stats into
> an elf section, so that we can calculate how much percentage of the
> profile is annotated, which can be used as an indicator whether code
> has changed significantly comparing with the profiled source.
>
> Bootstrapped and performance test on-going.
>
> OK for google-4_8?
>
> Thanks,
> Dehao


Re: [GOOGLE] Writes annotation info in elf section.

2014-03-12 Thread Dehao Chen
Thanks Cary for the comments.

Patch updated, an also added a tool in contrib/ to dump the profile
annotation coverage.

Dehao
>
>
> On Wed, Mar 12, 2014 at 9:48 AM, Cary Coutant  wrote:
>>
>> +void autofdo_source_profile::write_annotated_count () const
>> +{
>> +  switch_to_section (get_section (
>> +  ".gnu.switches.text.annotation",
>> +  SECTION_DEBUG | SECTION_MERGE | SECTION_STRINGS | 1, NULL));
>>
>> I think it would be worth a comment explaining the point of setting
>> the SECTION_MERGE and SECTION_STRINGS flags, and why it works for this
>> section. Also, the "1" is clearer if you write is as "(SECTION_ENTSIZE
>> & 1)".
>>
>> -cary
>
>
Index: contrib/autofdo_coverage.py
===
--- contrib/autofdo_coverage.py (revision 0)
+++ contrib/autofdo_coverage.py (revision 0)
@@ -0,0 +1,46 @@
+#!/usr/bin/python
+#
+# Copyright (C) 2013 Free Software Foundation, Inc.
+#
+# This script is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3, or (at your option)
+# any later version.
+
+# This script computes and outputs the percentage of profile that has
+# been used to annotate the AutoFDO optimized binary.
+
+import subprocess
+import sys
+
+if len(sys.argv) != 2:
+  print "Usage: " + sys.argv[0] + " "
+  exit(1)
+
+args = ["readelf", "-p", ".gnu.switches.text.annotation", sys.argv[1]]
+output, _ = subprocess.Popen(args, stdout=subprocess.PIPE).communicate()
+
+profile_map = {}
+
+for l in output.split('\n'):
+  parts = l.split()
+  if len(parts) != 3:
+continue;
+  words = parts[2].split(':')
+  if len(words) != 3:
+continue;
+  function = words[0]
+  total_count = int(words[1])
+  annotated_count = int(words[2])
+  if function not in profile_map:
+profile_map[function] = [total_count, annotated_count]
+  elif annotated_count > profile_map[function][1]:
+profile_map[function][1] = annotated_count
+
+total_sum = 0
+annotated_sum = 0
+for function in profile_map:
+  total_sum += profile_map[function][0]
+  annotated_sum += profile_map[function][1]
+
+print float(annotated_sum) / total_sum

Property changes on: contrib/autofdo_coverage.py
___
Added: svn:executable
   + *

Index: gcc/auto-profile.c
===
--- gcc/auto-profile.c  (revision 208283)
+++ gcc/auto-profile.c  (working copy)
@@ -49,6 +49,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "l-ipo.h"
 #include "ipa-utils.h"
 #include "ipa-inline.h"
+#include "output.h"
+#include "dwarf2asm.h"
 #include "auto-profile.h"
 
 /* The following routines implements AutoFDO optimization.
@@ -100,9 +102,6 @@ typedef std::vector string_vector;
 /* Map from function name's index in function_name_map to target's
execution count.  */
 typedef std::map icall_target_map;
-/* Represent profile count of an inline stack,  profile count is represented as
-   (execution_count, value_profile_histogram).  */
-typedef std::pair count_info;
 
 /* Set of inline_stack. Used to track if the profile is already used to
annotate the program.  */
@@ -112,6 +111,13 @@ typedef std::set location_set;
to direct call.  */
 typedef std::set stmt_set;
 
+struct count_info
+{
+  gcov_type count;
+  icall_target_map targets;
+  bool annotated;
+};
+
 struct string_compare
 {
   bool operator() (const char *a, const char *b) const
@@ -154,7 +160,7 @@ class function_instance {
   /* Read the profile and create a function_instance with head count as
  HEAD_COUNT. Recursively read callsites to create nested function_instances
  too. STACK is used to track the recursive creation process.  */
-  static const function_instance *read_function_instance (
+  static function_instance *read_function_instance (
   function_instance_stack *stack, gcov_type head_count);
 
   /* Recursively deallocate all callsites (nested function_instances).  */
@@ -167,8 +173,8 @@ class function_instance {
 
   /* Recursively traverse STACK starting from LEVEL to find the corresponding
  function_instance.  */
-  const function_instance *get_function_instance (const inline_stack &stack,
- unsigned level) const;
+  function_instance *get_function_instance (const inline_stack &stack,
+   unsigned level);
 
   /* Store the profile info for LOC in INFO. Return TRUE if profile info
  is found.  */
@@ -178,18 +184,23 @@ class function_instance {
   MAP, return the total count for all inlined indirect calls.  */
   gcov_type find_icall_target_map (gimple stmt, icall_target_map *map) const;
 
+  /* Total number of counts that is used during annotation.  */
+  gcov_type total_annotated_count () const;
+
+  /* Mark LOC as annotated.  */
+  void mark_annotated (location_t loc