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