[PATCH] D86502: [CSSPGO] A Clang switch -fpseudo-probe-for-profiling for pseudo-probe instrumentation.

[Hongtao Yu via Phabricator via llvm-commits]

hoy marked an inline comment as done.
hoy added a comment.

In D86502#2245460 <https://reviews.llvm.org/D86502#2245460>, @wmi wrote:

>> The early instrumentation also allows the inliner to duplicate probes for inlined instances. When a probe along with the other instructions of a callee function are inlined into its caller function, the GUID of the callee function goes with the probe. This allows samples collected on inlined probes to be reported for the original callee function.
>
> Just get a question from reading the above. Suppose `A` only has one BB and the BB has one PseudoProbe in it. If function `A` is inlined into `B1` and `B2` and both `B1` and `B2` inlined into `C`, the PseudoProbe from `A` will have two copies in `C` both carrying GUID of `A`. How the samples collected from `A` inlined into `B1` inlined into `C` are categorized differently from `A` inlined into `B2` inlined into `C`, especially when debug information is not enabled (so no inline stack information in the binary)?

This is a very good question. Inlined functions are differentiated by their original callsites. A pseudo probe is allocated for each callsite in the `SampleProfileProbe` pass. Nested inlining will produce a stack of pseudo probes, similar with the Dwarf inline stack. The work is not included in the first set of patches.



================
Comment at: llvm/include/llvm/Passes/PassBuilder.h:67-69
+    // Pseudo probe instrumentation should only work with autoFDO or no FDO.
+    assert(!this->PseudoProbeForProfiling || this->Action == NoAction ||
+           this->Action == SampleUse);
----------------
wmi wrote:
> Need it to work with more types of action for example instrumentation FDO or cs instrumentation FDO. For instrumentation FDO optimized binary, we may want to collect AutoFDO profile for it for performance comparison, enhance the intrumentation profile with AutoFDO profile to make the profile more production representative, ...
> 
> Currently debug information based AutoFDO supports it.
I see. I just removed this assert and the let assert above handle both `DebugInfoForProfiling` and `PseudoProbeForProfiling`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86502