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

[Lei Wang via Phabricator via llvm-commits]

wlei 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
----------------
hoy wrote:
> 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.
> 
> 
Sounds good to retire `FlattenProfileForMatching `, I will do it in https://reviews.llvm.org/D156725, which already made some changes to `FlattenProfileForMatching`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156722