[PATCH] D92998: [CSSPGO][llvm-profgen] Pseudo probe based CS profile generation

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/test/tools/llvm-profgen/noinline-cs-pseudoprobe.test:13-14
+; CHECK-NEXT: 1: 15
+; CHECK-NEXT: 2: 0
+; CHECK-NEXT: 3: 0
+; CHECK-NEXT: 4: 15
----------------
wmi wrote:
> wlei wrote:
> > wmi wrote:
> > > In [main:2 @ foo], probes with 0 count are not dumped. In which case probe with 0 count will be dumped?
> > The 0 count is for a dangling probe case to distinguish a missing count, see the ProfileGenerator.cpp:420
> > 
> > Copied the comments for you to refer:
> > 
> > ```
> >     // Drop the samples collected for a dangling probe since it's misleading.
> >     // We still report the probe but with a special zero count. The compiler
> >     // won't trust the zero count and will rely on the counts inference
> >     // algorithm to get the probe a reasonable count. Note that a zero count is
> >     // different from a missing count, where the latter really tells the
> >     // compiler that a probe is never executed.
> > ```
> > Note that a zero count is different from a missing count, where the latter really tells the compiler that a probe is never executed.
> 
> That is contrary to the debug info based profile where zero count for a line means the line is never executed. Missing count for a line means compiler has to infer the count. Why the probe based profile is implemented in such way?
Good question. The current counts inference algorithm in sample profile loader doesn't seem to differentiate a missing sample from a zero sample. For example, in `SampleProfileLoader::propagateThroughEdges`, accesses to `BlockWeights` are not preceded with a check. Therefore a missing entry in `BlockWeights` will be created as a zero entry. 

With pseudo probes, a missing sample really means the probe is not executed. A zero sample, on the other hand, wouldn't be created by the profile generator. We are leveraging zero sample to represent a special type of probe, called `dangling` probe, which will need the compiler to infer its count. We were thinking about using UINT64_MAX instead, but UINT64_MAX literally is a legal number of samples.

The compiler change that dangles probes haven't been sent out yet. @wlei I think we should exclude this detail from this change for now. But it's better to discuss here if this approach makes sense.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92998/new/

https://reviews.llvm.org/D92998