On Tue, May 20, 2014 at 8:39 AM, Xinliang David Li <davi...@google.com> wrote:
> On Tue, May 20, 2014 at 6:32 AM, Teresa Johnson <tejohn...@google.com> wrote:
>> On Mon, May 19, 2014 at 11:51 PM, Xinliang David Li <davi...@google.com> 
>> 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.

Done.

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

Fixed the issues below and others uncovered by contrib/check_GNU_style.sh.

Patch committed to google/4_8 as r210652.

Will port to google/4_9.

Teresa

>
> 1) calller --> caller
> 2) many places where there is missing space: e.g,
> gcov_fixup_counters_checksum( ..
>
> David
>
>> Thanks,
>> Teresa
>>
>>>
>>> David
>>>
>>> On Mon, May 19, 2014 at 10:08 PM, Teresa Johnson <tejohn...@google.com> 
>>> wrote:
>>>> Ping.
>>>> Teresa
>>>>
>>>> On Wed, May 14, 2014 at 4:39 PM, Teresa Johnson <tejohn...@google.com> 
>>>> 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
>>>>> option.
>>>>>
>>>>> 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
>>>>> lineno_checksum.
>>>>>
>>>>> For the counter fixup, we merge all non-zero profiles with matching
>>>>> checksums and copy the merged profile into copies with all-zero
>>>>> profiles.
>>>>>
>>>>> 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
>>>>> that.
>>>>>
>>>>> 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?
>>>>>
>>>>> Thanks,
>>>>> Teresa
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
>>>>
>>>>
>>>>
>>>> --
>>>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
>>
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413



-- 
Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413

Reply via email to