[PATCH] D120335: [llvm-profgen] Generating probe-based non-CS profile.

[Lei Wang via Phabricator via llvm-commits]

wlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:944
+    // doesn't belong to current context, filter them out.
+    if (!Probe->isBlock() || Count == 0)
+      continue;
----------------
hoy wrote:
> wlei wrote:
> > `Probe.isBlock()` check is hoisted from `extractProbesFromRange` to this for CS profile but Why doesn't add this check back for non-CS profile?
> So we don't count callsite probes here since they are handled in `populateBoundarySamplesWithProbes`. Otherwise they will be double counted.
> 
> We don't do this for non-CS probe profile. This is to be consistent with non-CS dwarf implementation where we report zero count in `populateBoundarySamplesForAllFunctions`. I guess I can also do some refactoring to unify the two implementations. 
> 
Yeah, I understand for `count==0`. Here I'm asking for whether we should add `Probe->isBlock()` for non-CS probe profile. My understanding is  for body sample, we only accumulate total sample for Block profile? otherwise, the count from callsite probes will affect the total samples.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120335