wlei-llvm 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)
https://github.com/llvm/llvm-project/pull/99891
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
