This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [GOOGLE] Fix gcda build info support
- From: Teresa Johnson <tejohnson at google dot com>
- To: Xinliang David Li <davidxl at google dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 11 Sep 2014 10:09:37 -0700
- Subject: Re: [GOOGLE] Fix gcda build info support
- Authentication-results: sourceware.org; auth=none
- References: <CAAe5K+XK4BhUX65BcX=XfYHfHHD8XobvB20F1P6ut28JomXp2w at mail dot gmail dot com> <CAAkRFZJsnrpUU1-n-L+Ab8EJtb7jN7ni5SjVn_i+MzPHS5U8_g at mail dot gmail dot com>
On Wed, Sep 10, 2014 at 3:31 PM, Xinliang David Li <davidxl@google.com> wrote:
> Can you share the buildinfo reader code with the merger by defininig
> some hooks for different callbacks?
Do you mean the two blobs of code guarded by 'if (tag ==
GCOV_TAG_BUILD_INFO)' that I added here and the existing one in
gcov_exit_merge_gcda further down in the same file? Sure, I could
outline that and pass in the gi_ptr for the merger case. Let me know
if you meant something else.
Teresa
>
> David
>
> On Wed, Sep 10, 2014 at 10:24 AM, Teresa Johnson <tejohnson@google.com> wrote:
>> While porting recent support for a build info section in the gcda from
>> google/4_8 to 4_9 and doing manual testing, I discovered that it does
>> not interact well with the COMDAT fixup handling. This patch fixes the
>> issue, and adds a test case that exposes the problem without the fix.
>>
>> Here is the google/4_8 patch - I plan to commit there first then port
>> it along with the original build info patch to 4_9.
>>
>> Passes regression tests - ok for google branches?
>>
>> Thanks,
>> Teresa
>>
>> 2014-09-10 Teresa Johnson <tejohnson@google.com>
>>
>> libgcc:
>> * libgcov-driver.c (gcov_scan_to_function_data): Rename from
>> gcov_scan_summary_end, scan past BUILD_INFO section.
>> (gcov_dump_module_info): Rename gcov_scan_summary_end to
>> gcov_scan_to_function_data.
>>
>> gcc/testsuite:
>> * g++.dg/tree-prof/lipo/buildinfo.txt: Input for
>> -fprofile-generate-buildinfo option.
>> * g++.dg/tree-prof/lipo/comdat_fixup_0.C: New test.
>> * g++.dg/tree-prof/lipo/comdat_fixup_1.C: Ditto.
>> * g++.dg/tree-prof/lipo/comdat_fixup_2.C: Ditto.
>> * g++.dg/tree-prof/lipo/comdat_fixup.h: Ditto.
>> * lib/profopt.exp: Declare srcdir for use in test options.
>>
>> Index: libgcc/libgcov-driver.c
>> ===================================================================
>> --- libgcc/libgcov-driver.c (revision 214976)
>> +++ libgcc/libgcov-driver.c (working copy)
>> @@ -428,13 +428,15 @@ struct gcov_filename_aux{
>> #include "libgcov-driver-system.c"
>>
>> /* Scan through the current open gcda file corresponding to GI_PTR
>> - to locate the end position of the last summary, returned in
>> - SUMMARY_END_POS_P. Return 0 on success, -1 on error. */
>> + to locate the end position just before function data should be rewritten,
>> + returned in SUMMARY_END_POS_P. E.g. scan past the last summary and other
>> + sections that won't be rewritten, like the build info. Return 0 on success,
>> + -1 on error. */
>> static int
>> -gcov_scan_summary_end (struct gcov_info *gi_ptr,
>> - gcov_position_t *summary_end_pos_p)
>> +gcov_scan_to_function_data (struct gcov_info *gi_ptr,
>> + gcov_position_t *summary_end_pos_p)
>> {
>> - gcov_unsigned_t tag, version, stamp;
>> + gcov_unsigned_t tag, version, stamp, i, length;
>> tag = gcov_read_unsigned ();
>> if (tag != GCOV_DATA_MAGIC)
>> {
>> @@ -467,6 +469,28 @@ static int
>> return -1;
>> }
>>
>> + /* If there is a build info section, scan past it as well. */
>> + if (tag == GCOV_TAG_BUILD_INFO)
>> + {
>> + length = gcov_read_unsigned ();
>> + gcov_unsigned_t num_strings = 0;
>> + char **build_info_strings = gcov_read_build_info (length, &num_strings);
>> + if (!build_info_strings)
>> + {
>> + gcov_error ("profiling:%s:Error reading build info\n", gi_filename);
>> + return -1;
>> + }
>> +
>> + for (i = 0; i < num_strings; i++)
>> + free (build_info_strings[i]);
>> + free (build_info_strings);
>> +
>> + *summary_end_pos_p = gcov_position ();
>> + tag = gcov_read_unsigned ();
>> + }
>> + /* The next section should be the function counters. */
>> + gcc_assert (tag == GCOV_TAG_FUNCTION);
>> +
>> return 0;
>> }
>>
>> @@ -1031,10 +1055,10 @@ gcov_dump_module_info (struct gcov_filename_aux *g
>>
>> if (changed)
>> {
>> - /* Scan file to find the end of the summary section, which is
>> + /* Scan file to find the start of the function section, which is
>> where we will start re-writing the counters. */
>> gcov_position_t summary_end_pos;
>> - if (gcov_scan_summary_end (gi_ptr, &summary_end_pos) == -1)
>> + if (gcov_scan_to_function_data (gi_ptr, &summary_end_pos) == -1)
>> gcov_error ("profiling:%s:Error scanning summaries\n",
>> gi_filename);
>> else
>> Index: gcc/testsuite/g++.dg/tree-prof/lipo/buildinfo.txt
>> ===================================================================
>> --- gcc/testsuite/g++.dg/tree-prof/lipo/buildinfo.txt (revision 0)
>> +++ gcc/testsuite/g++.dg/tree-prof/lipo/buildinfo.txt (revision 0)
>> @@ -0,0 +1 @@
>> +Test -fprofile-generate-buildinfo option
>> Index: gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_0.C
>> ===================================================================
>> --- gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_0.C (revision 0)
>> +++ gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_0.C (revision 0)
>> @@ -0,0 +1,9 @@
>> +/* { dg-options "-O2 -fno-inline
>> -fprofile-generate-buildinfo=$srcdir/g++.dg/tree-prof/lipo/buildinfo.txt"
>> } */
>> +#include <stdio.h>
>> +
>> +extern int foo1(int x);
>> +extern int foo2(int x);
>> +int main()
>> +{
>> + printf ("Result = %d\n", foo1(1) + foo2(1));
>> +}
>> Index: gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_1.C
>> ===================================================================
>> --- gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_1.C (revision 0)
>> +++ gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_1.C (revision 0)
>> @@ -0,0 +1,7 @@
>> +/* { dg-options "-O2 -fno-inline" } */
>> +#include "comdat_fixup.h"
>> +int foo1(int x)
>> +{
>> + Foo f;
>> + return f.foo(x);
>> +}
>> Index: gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_2.C
>> ===================================================================
>> --- gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_2.C (revision 0)
>> +++ gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_2.C (revision 0)
>> @@ -0,0 +1,7 @@
>> +/* { dg-options "-O2 -fno-inline" } */
>> +#include "comdat_fixup.h"
>> +int foo2(int x)
>> +{
>> + Foo f;
>> + return f.foo(x);
>> +}
>> Index: gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup.h
>> ===================================================================
>> --- gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup.h (revision 0)
>> +++ gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup.h (revision 0)
>> @@ -0,0 +1,5 @@
>> +class Foo
>> +{
>> + public:
>> + int foo(int x) { return x; }
>> +};
>> Index: gcc/testsuite/lib/profopt.exp
>> ===================================================================
>> --- gcc/testsuite/lib/profopt.exp (revision 214976)
>> +++ gcc/testsuite/lib/profopt.exp (working copy)
>> @@ -169,6 +169,8 @@ proc profopt-final-code { which final_code name }
>> # SRC is the full pathname of the testcase.
>> #
>> proc profopt-get-options { src } {
>> + global srcdir
>> +
>> # dg-options sets a variable called dg-extra-tool-flags.
>> set dg-extra-tool-flags ""
>>
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
--
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413