[PATCH] D84997: [AutoFDO] Avoid merging inlinee samples multiple times

Thu Jul 30 23:14:16 PDT 2020

hoyFB marked an inline comment as done.
hoyFB added a comment.

In D84997#2186553 <https://reviews.llvm.org/D84997#2186553>, @wmi wrote:

>>> The multiple inline instance copies generated from loop unrolling or jump threading should have their own FunctionSamples objects. Why merging multiple inline instance to the outline profile will trigger assertion? Could you have a testcase for it?
>>
>> It's a slightly difference case. In the training build, we didn't unroll, so there's a single inline site in the loop, but in the PGO optimizing build, the loop got unrolled with call site duplicated. And the duplicated call sites all point to the same `FunctionSamples`, in which case we should merge them only once. Agree that we should add a test case.
>
> Thanks. So it is a combined version of normal PGO and sampleFDO, or the PGO here means the instrument hook for sampleFDO?
>
>>> For the second problem, we got a bug report for the same problem. I create a testcase and a fix in https://reviews.llvm.org/D84994. I think your fix is more concise while mine is a straightforward fix. If you think your fix is better, free to use the testcase in D84994 <https://reviews.llvm.org/D84994>. Otherwise, I can commit that one.
>
> I will commit Hongtao's version in D84994 <https://reviews.llvm.org/D84994> so the two fixes can be separated.

Thanks for committing the change for me!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84997