[PATCH] D156722: [CSSPGO] Support stale profile matching for LTO

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2130
+          if (FunctionSamples::ProfileIsProbeBased &&
+              isa<PseudoProbeInst>(&I)) {
+            // For pseudo probe mode, extracting inline info from the probe inst
----------------
wlei wrote:
> hoy wrote:
> > hoy wrote:
> > > Do we also want to flatten for callsite probes?
> > Also should this be done under `FlattenProfileForMatching`?
> > Do we also want to flatten for callsite probes?
> 
> Block probe should be sufficient. for every callsite, there should be a block probe in the same BB, so their debug info inline stack is the same. 
> 
> > Also should this be done under FlattenProfileForMatching?
> Not sure if this will works with FlattenProfileForMatching off. Maybe we even don't need this ``FlattenProfileForMatching` switch, I'm thinking FlattenIR or FlattenProfile should be always beneficial to the matching. Or maybe we need another switch `FlattenIRForMatching` 
> Block probe should be sufficient. for every callsite, there should be a block probe in the same BB, so their debug info inline stack is the same.

Sounds good, as long as the inlinee has one block probe left, the original callsite should be recoverd.

> Not sure if this will works with FlattenProfileForMatching off. Maybe we even don't need this `FlattenProfileForMatching switch, I'm thinking FlattenIR or FlattenProfile should be always beneficial to the matching. Or maybe we need another switch FlattenIRForMatching

I mean as long as the IR flattening and profile flattening are synchronized it should be fine. Maybe we should just retire the `FlattenProfileForMatching` switch as we always use flattened profile.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156722