aaupov wrote:
> > > > > Ping @wlei-llvm
> > > >
> > > >
> > > > Sorry for the delay. The new version addressed my last comment (with
> > > > just minor nits). However, I didn't fully follow the new features
> > > > related to `ProbeMatchSpecs` stuffs. Could you add more descriptions to
> > > > the diff summary? Or if it’s not a lot of work, could we split it into
> > > > two patches? We could commit the first part, and I will review the
> > > > second part separately.
> > >
> > >
> > > NVM, I think now I get what `ProbeMatchSpecs` does, it's a vector because
> > > a function can have multiple sections(function split)
> >
> >
> > Thank you for reviewing and sorry for the delay from my end, was busy with
> > profile quality work.
> > ProbeMatchSpecs is a mechanism to match probes belonging to another binary
> > function. I'm going to utilize it in probe-based function matching
> > (#100446). For example:
> > source function:
> > ```
> > void foo() {
> > bar();
> > }
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > profiled binary: bar is not inlined => have top-level function bar new
> > binary (where the profile is applied to): bar is inlined into foo.
> > Right now, BOLT does 1:1 matching between profile functions and binary
> > functions based on the name. #100446 will extend this to N:M where multiple
> > profiles can be matched to one binary function (as in the example above
> > where binary function foo would use profiles for foo and bar), and one
> > profile can be matched to multiple binary functions (eg if bar was inlined
> > into multiple functions).
> > In this diff, ProbeMatchSpecs would only have one BinaryFunctionProfile
> > (existing name-based matching).
>
> Thanks for the explanation! I was confused of the use of `ProbeMatchSpecs`,
> it would be great to add more descriptions in the diff summary or somewhere
> in the comments, or in the another patch when it's used(IMO if we add a
> feature, but we doesn't use it in the patch, it should be in the future patch
> when it's used)
Thank you. Added description of ProbeMatchSpecs to the summary.
I decided to introduce it in this diff because there's tight coupling between
probe-based block matching and function matching. This way, probe-based
function matching would not need to change how block matching works.
https://github.com/llvm/llvm-project/pull/99891
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
