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

[Mingming Liu via Phabricator via llvm-commits]

luna added a comment.

In D121862#3389529 <https://reviews.llvm.org/D121862#3389529>, @wenlei wrote:

> What's your actual use case that motivated this change?
>
> When this was originally added, we used it to disable only pre-link sample loader inlining, in which case we don't want to merge the profiles even when inlining there is skipped.

If the profiles of inlined instances are not merged, and the instances are not inlined, I'm wondering if the profiles will be inaccurate for other optimization passes (not only inliner itself).

I think the answer is yes, so decide to make this patch.

This is because during post-link, sample loader inlining will still happen, so merging profiles during pre-link as if relevant inlining will never happen may cause over-optimize (it was measurable in code size). + at hoy

Thanks for pointing this out!

For my understanding, how does over-optimization or measurable code size happen when profiles are merged and pre-link inliner skips IR transformation? Does it happen for the following scenario?

1. "merging without inlining" cause some functions (in the module) to become hot functions; these hot functions are imported in post-link sample loader, and cause code size increase
2. If inlining in pre-link sample loader happens, the in-module hot functions are inlined; and if they are not used in other modules, original function definitions are simply eliminated (so not going to be imported)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121862