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