> Hi Honza,
>
> > On 26 May 2025, at 5:28 pm, Jan Hubicka <[email protected]> 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
> >>> <[email protected]> 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
> >>>
> >>
> >
> >
> >
>