This is the mail archive of the
mailing list for the GCC project.
Re: [Google/4_8] LIPO COMDAT profile fixups
- From: Xinliang David Li <davidxl at google dot com>
- To: Teresa Johnson <tejohnson at google dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Rong Xu <xur at google dot com>
- Date: Tue, 20 May 2014 08:39:56 -0700
- Subject: Re: [Google/4_8] LIPO COMDAT profile fixups
- Authentication-results: sourceware.org; auth=none
- References: <CAAe5K+UkRzc42LWqO2kMQ8FUyn-q1yLe6___VvVf4V8i-_ixSA at mail dot gmail dot com> <CAAe5K+Wz2_wiPEJmMfnVbmCe5saFqG1tpkBNxN0gapbeKnv73g at mail dot gmail dot com> <CAAkRFZJLdALK-ToYRShjKxugPsXRQopQCFQ761dOGTezD1XenA at mail dot gmail dot com> <CAAe5K+VW+CaYEBgVpuT+PB7m64W82oarxuLMLQNYCdaYvT4naA at mail dot gmail dot com>
On Tue, May 20, 2014 at 6:32 AM, Teresa Johnson <email@example.com> wrote:
> On Mon, May 19, 2014 at 11:51 PM, Xinliang David Li <firstname.lastname@example.org> wrote:
>> Why duplicating the merger functions in dyn-ipa.c? Should those in
>> libgcov-merge.c be reused?
> The merger functions in libgcov-merge.c use a macro to either read the
> counter to merge from a buffer in memory (when IN_GCOV_TOOL) or from
> the gcda file otherwise. In this case I need to merge from another
> array, and it needs to do so both IN_GCOV_TOOL and otherwise. So to
> reuse the merger functions I would have had to get the counter value
> from a new argument, guarded by a conditional. I didn't want to change
> the interface of those existing merge functions, or more importantly,
> make them less efficient with the added condition since they are
> invoked frequently during gcda file merging.
ok -- something to think about in the future.
For now, please add a comment in libgcov-utils.c before
ctr_merge_functions to point to the dup merge functions in dyn-ipa.c.
>> The refactoring of gcov_exit_write_gcda should probably be done in a
>> separate patch -- preferably submitted to trunk too.
> The refactoring is necessary for this patch since I need to invoke the
> outlined functionality in gcov_dump_module_info as well.
> I could submit that to trunk, but it has to go in to the google
> branches either with or preceeding this patch.
ok for google branch after fixing the formatting issues
1) calller --> caller
2) many places where there is missing space: e.g,
>> On Mon, May 19, 2014 at 10:08 PM, Teresa Johnson <email@example.com> wrote:
>>> On Wed, May 14, 2014 at 4:39 PM, Teresa Johnson <firstname.lastname@example.org> wrote:
>>>> This patch applies profile fixups to COMDATs on the dyn ipa callgraph
>>>> at the end of LIPO module grouping (either in the profile gen run or
>>>> in gcov-tool). This is to address issues with missing profiles in the
>>>> out-of-line COMDAT copies not selected by the linker, and indirect
>>>> call profiles that target a module not included in the module group.
>>>> By default, both fixups are enabled, but can be controlled by a
>>>> profile-gen parameter, an environment variable, and a gcov-tool
>>>> The fixups assume that functions with the same lineno and cfg checksum
>>>> are copies of the same COMDAT. This is made more likely by ensuring
>>>> that in LIPO mode we include the full mangled name in the
>>>> For the counter fixup, we merge all non-zero profiles with matching
>>>> checksums and copy the merged profile into copies with all-zero
>>>> For the indirect counter fixup, if an indirect call profile target is
>>>> not in the module group, we look for a matching checksum copy in the
>>>> primary module and if exactly one is found we change the target to
>>>> If any fixups are applied, the gcda files are rewritten after module grouping.
>>>> This also required a couple of other changes to the optimizer. During
>>>> cgraph node resolution, we were arbitrarily selecting a copy that had
>>>> non-zero profiles. Now there are many more to choose from, so we will
>>>> prefer the primary module copy if it is non-zero. Also, during cloning
>>>> for inlining, we only want to update the profile on the callee node if
>>>> we are inlining into the resolved node caller node. We were already
>>>> doing this for AutoFDO, and need to do this here now that many node
>>>> copies have the same profile.
>>>> Patch attached. Tested with regression tests and internal benchmarks.
>>>> Ok for google branches?
>>>> Teresa Johnson | Software Engineer | email@example.com | 408-460-2413
>>> Teresa Johnson | Software Engineer | firstname.lastname@example.org | 408-460-2413
> Teresa Johnson | Software Engineer | email@example.com | 408-460-2413