> Hi Honza, > > > On 26 May 2025, at 5:28 pm, Jan Hubicka <hubi...@ucw.cz> wrote: > > > > External email: Use caution opening links or attachments > > > > > > Hi, > >> Ping? > > Sorry for the delay. I think I finally got auto-fdo running on my box > > and indeed I see that if function is cloned later, the profile is lost. > > There are .suffixes added before afdo pass (such as openmp offloading or > > nested functions) and there are .suffixes added afer afdo (by ipa > > cloning and LTO privatization). I see we want to merge those created by > > ipa cloning (after afdo pass). But I do not think we want to merge > > those for i.e. nested functions since those are actual different > > functions or for openmp offloading. > > IMO get_original_name should handle this. We want to strip the suffix only > for the functions that are cloned afterwards. > Stripping suffixes for all the functions is not correct. We need to know the > suffixes that are created later and stip only them > > > Clone names we should strip: > * SRA clones (of the form foo.isra.0) > * IPA-CP clonse (in the form bar.constprop.123) > * Partial inlining clones (foo.part.0, foo.cold)
I think we also need to handle .lto_priv > > > Others that should not be stripped: > * target_clones (foo.arch_x86_64_v2, foo.avx2) > * OpenMP related suffixes (.omp_fn.N, .omp_cpyfn.N) Also nested functions ale called with dot suffix. The following produces foo.0 void test() { void foo () { } foo (); } > > Is that what you want? Yes. I suppose we need to list those suffxes created late (isra, constprop, part, cold). Not sure if that is complete list but can't think of anything else. Honza > > > > > I also wonder what happens with LTO privatization - i.e. how we look up > > what static function does the symbol belong? > > This as Dhruv mentioned need change to gcov itself. Private (static) > functions with the same name also will have the same issue. > Dhruv is working on an RFC for this. > > Thanks, > Kugan > > > > > > Overwritting the data by the last clone is definitely bad, so the patch > > is OK, but we should figure out what happens in the cases above. > > > > Also if we merge, it may happen that the clone is noticeably different > > from original - for example with ipa split it may be missing part of the > > body. Merging the tables elementwise is safe then? > > > > Honza > >> > >> Thanks, > >> Kugan > >> > >> > >> > >>> On 9 May 2025, at 11:54 am, Kugan Vivekanandarajah > >>> <kvivekana...@nvidia.com> wrote: > >>> > >>> External email: Use caution opening links or attachments > >>> > >>> > >>> This patch add support for merging profiles from multiple clones. > >>> That is, when optimized binaries have clones such as IPA-CP clone or SRA > >>> clones, genarted gcov will have profiled them spereately. > >>> Currently we pick one and ignore the rest. This patch fixes this by > >>> merging the profiles. > >>> > >>> > >>> Regression tested on aarch64-linux-gnu with no new regression. > >>> Also successfully done autoprofiledbootstrap with the relevant patch. > >>> > >>> Is this OK for trunk? > >>> Thanks, > >>> Kugan > >>> > >> > > > > > > >