[PATCH] D86193: [CSSPGO] Pseudo probe instrumentation for basic blocks.

[Hongtao Yu via Phabricator via llvm-commits]

hoy added a comment.

In D86193#2232609 <https://reviews.llvm.org/D86193#2232609>, @wmi wrote:

> Thanks for the patch! A few questions:
>
>> probe blocks some CFG transformations that can mess up profile correlation.
>
> Can you enumerate some CFG transformations which be blocked? Is it possible that some CFG transformations being blocked are actually beneficial for later optimizations?

There are some optimizations such as if-convert, tail call elimination, that were initially blocked by the pseudo probe intrinsic but is now unblocked by fixes included in this change. With the current change we do not see perf degradation out of SPEC and one of our internal large services.

The main optimizations left blocked intentionally are those that merge blocks for smaller code size, such as tail merge which is the opposite of jump threading. We believe that those optimizations are not very beneficial for performance and AutoFDO. But if things are changed we can always unblock them.

> Are the intrinsic probes counted when computing bb size and function size?

That's a good question. On the IR level, pseudo probe intrinsics are treated in a similar way of the debug intrinsics and the side-effect intrinsics. On the MIR level, pseudo probe intrinsics are implemented as a `StandardPseudoInstruction`. So they should not be counted towards real code size.

> And could you split the patches into small parts for easier review. For example,  Add the intrinsic support in IR and MIR. SampleProfileProbe pass. -fpseudo-probe-for-profiling support. changes in various passes.

Thanks for the suggestion. Agreed the current patch is too big to review. Will come up with a list of breakdowns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86193