Page MenuHomePhabricator

Annotate VP prof on indirect call if it is ICPed in the profiled binary.
ClosedPublic

Authored by danielcdh on Oct 2 2017, 1:35 PM.

Details

Summary

In SamplePGO, when an indirect call is promoted in the profiled binary, before profile annotation, it will be promoted and inlined. For the original indirect call, the current implementation will not mark VP profile on it. This is an issue when profile becomes stale. This patch annotates VP prof on indirect calls during annotation.

Event Timeline

danielcdh created this revision.Oct 2 2017, 1:35 PM
tejohnson edited edge metadata.Oct 3 2017, 11:22 AM

Summary has a couple of typos

lib/Transforms/IPO/SampleProfile.cpp
514–515

How do we know it was inlined in the profile from the below checks?

519

Comment needs update. It isn't clear to me why indirect calls are handled differently. I understand that you want to annotate the fallback indirect call with any VP metadata it collected. But couldn't the promoted/inlined indirect call in the profiled binary have the same situation (inlined callsite had no sample)? I think this goes back to my first question above, about how we know it was inlined in the profile from here.

danielcdh updated this revision to Diff 117553.Oct 3 2017, 11:28 AM

update comment

danielcdh added inline comments.Oct 3 2017, 11:28 AM
lib/Transforms/IPO/SampleProfile.cpp
514–515

Updated the comment to make it clear.

519

Comment updated to make it clear.

Basically this only applies to direct call. For indirect call, even if the call is promoted and inlined, we will still get profile for the indirect call, so should not blindly set it as 0.

tejohnson accepted this revision.Oct 5 2017, 11:16 AM

LGTM
(please just fix the 2 typos in the patch summary before submitting)

This revision is now accepted and ready to land.Oct 5 2017, 11:16 AM
danielcdh edited the summary of this revision. (Show Details)Oct 5 2017, 12:38 PM
danielcdh closed this revision.Oct 5 2017, 1:17 PM