Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PseudoProbe] Use probe id as the base dwarf discriminator for callsites #65685

Merged
merged 1 commit into from
Sep 8, 2023

Conversation

htyu
Copy link
Contributor

@htyu htyu commented Sep 7, 2023

With -fpseudo-probe-for-profiling, the dwarf discriminator for a callsite will be overwritten to pseudo probe related information for that callsite. The probe information is encoded in a special format (i.e., with all lowest three digits be one) in order to be distinguished from regular dwarf discriminator. The special encoding format will be decoded to zero by the regular discriminator logic. This means all callsites would have a zero discriminator in both the sample profile and the compiler, for classic AutoFDO. This is inconvenient in that no decent classic AutoFDO can be generated from a pseudo probe build. I'm mitigating the issue by allowing callsite probe id to be used as the base dwarf discriminator for classic AutoFDO, since probe id is also unique and can be used to differentiate callsites on the same source line.

@htyu htyu requested a review from WenleiHe September 7, 2023 22:01
@htyu htyu requested a review from a team as a code owner September 7, 2023 22:01
Comment on lines 2079 to 2080
// Return the probe id instead of zero for a pseudo probe discriminator.
// This should help differenciate callsites with same line numbers.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest to make it clear that this is so we can still generate good quality AutoFDO profile when -fpseudo-probe-for-profiling is on, in which case we will use probe-id for calls as AutoFDO's discriminator for calls locations to avoid losing all discriminators for callsites.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, comment updated.

@WenleiHe WenleiHe requested a review from a team September 8, 2023 15:49
@htyu htyu requested a review from a team as a code owner September 8, 2023 16:37
Copy link
Member

@WenleiHe WenleiHe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks.

@htyu htyu merged commit 6b856ab into llvm:main Sep 8, 2023
0 of 2 checks passed
@htyu htyu deleted the probeIdAsDiscriminator branch September 8, 2023 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants