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

Reply via email to