This is an archive of the discontinued LLVM Phabricator instance.

[BOLT] computing raw branch count for yaml profiles
ClosedPublic

Authored by spupyrev on Feb 16 2023, 11:48 AM.

Details

Summary

Function.RawBranchCount is initialized for fdata profile but not for yaml one.
The diff adds the computation of the field for yaml profiles

Diff Detail

Event Timeline

spupyrev created this revision.Feb 16 2023, 11:48 AM
Herald added a reviewer: Amir. · View Herald Transcript
Herald added a reviewer: maksfb. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
spupyrev requested review of this revision.Feb 16 2023, 11:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 16 2023, 11:48 AM
spupyrev edited the summary of this revision. (Show Details)Feb 16 2023, 11:50 AM
wlei added a subscriber: wlei.Feb 27 2023, 3:38 PM
Amir added a comment.Feb 27 2023, 4:52 PM

Is that primarily needed for "X out of Y samples in the binary (Z%) belong to functions with invalid (possibly stale) profile" log message? Can you add a test using yaml profile, e.g. based on bolt/test/X86/pre-aggregated-perf.test?

Is that primarily needed for "X out of Y samples in the binary (Z%) belong to functions with invalid (possibly stale) profile" log message? Can you add a test using yaml profile, e.g. based on bolt/test/X86/pre-aggregated-perf.test?

I noticed a few (minor) differences between how yaml and fdata profiles are parsed and processed -- this is one of them and can easily be fixed. I will add a test.

spupyrev updated this revision to Diff 507878.Mar 23 2023, 2:05 PM
  • re-enabled test in branch-data.test
  • added a check to the test verifying the new flag
Amir accepted this revision.Mar 27 2023, 12:16 PM

LGTM with small nits

bolt/lib/Core/BinaryFunction.cpp
474
bolt/lib/Profile/YAMLProfileReader.cpp
87–91
bolt/test/X86/branch-data.test
18
This revision is now accepted and ready to land.Mar 27 2023, 12:16 PM
This revision was automatically updated to reflect the committed changes.