[PATCH] D96811: [CSSPGO][llvm-profgen] Change sample count of dangling probe in llvm-profgen
Lei Wang via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 17 09:58:57 PST 2021
wlei added inline comments.
================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:505
+
FunctionProfile.addTotalSamples(Count);
if (Probe->isEntry()) {
----------------
hoy wrote:
> wlei wrote:
> > hoy wrote:
> > > wenlei wrote:
> > > > Do we still want to count samples on dangling probe towards total samples? Without deduplication for dangling probes, we could have multiple dangling probes in the same block and counting samples covering these probe repeatedly may cause bloated total samples?
> > > Good point. Since the samples collected on dangling probes are invalid, I would not count it against total samples.
> > I see, thanks for your suggestion.
> >
> > How about the count for `isEntry` for line506, I guess it's the original count not the zero? Or we won't have dangling probe which is the entry probe?
> That's a good point. It's possible to have a dangling entry probe and we should bail out here. Or we can just return right after adding body samples for dangling probes.
I see. Since the sample count are invalid, we shouldn't count for total sample nor using it to infer inliner's sample count. Thanks for your clarification.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96811/new/
https://reviews.llvm.org/D96811
More information about the llvm-commits
mailing list