[PATCH] D121862: [ProfSampleLoader] When disable-sample-loader-inlining is true, merge profiles of inlined instances to outlining versions.

[Wenlei He via Phabricator via llvm-commits]

wenlei added a comment.

Thanks for sharing your insights @kazu.

We actually tried cost-benefit inlining for both IRPGO and Sample PGO. We didn't observe measureable gain with IRPGO on HHVM workload, but .text size was 5% smaller with same perf.

For sample PGO, we tried to annotate callsite weight first and then invoke call analyzer for cost-benefit analysis based inlining, all within sample loader, but also didn't get better results.

We still believe cost-benefit analysis is the way to go, but we just haven't look deep into it to understand why it didn't bring benefit for our workload.

For module inliner vs sample loader inliner, I understand that it'd be more accurate to estimate size and simplification effect of inlining when done later in the pipeline, but from the current analysis, it looks like we won't miss much if we do cost-benefit analysis in sample loader, have you considered that? This way you get to leverage context sensitivity and also benefit from cost-benefit analysis.

About context similarity, IIUC you experiments indicates majority of function profiles are actually not context sensitive? That is interesting. For CSSPGO on one internal workload, we see noticeable perf degradation if we try to cap context profile depth -- that indicates there's enough context sensitivity on critical path for it to matter. Maybe this is workload dependent.

The prolog/epilog analysis would need to be done a bit late. It'd be interesting to see to how accurately we can model them. Maybe you have seen pathological cases where it's important, but I wonder how much they help the general cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121862