This is an archive of the discontinued LLVM Phabricator instance.

[llvm-profdata] Fixed various issue with Sample Profile Reader
ClosedPublic

Authored by huangjd on Mar 15 2023, 5:25 PM.

Details

Summary

Fixed various undefind behaviors with current Sample Profile Reader when reading unusual input. Furthermore, add the following rule on allowing multiple name table sections (current Reader has conflicted code handling such case):

When a new name table section is read (in the order sections are read), the names in the previous name table are cleared. Any subsequent sections referring to function names will index into the most recent read name table.

Also changed name table index to uint64_t to be consistent since there's a mix of using uint32_t and uint64_t.

Diff Detail

Event Timeline

huangjd created this revision.Mar 15 2023, 5:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2023, 5:25 PM
huangjd requested review of this revision.Mar 15 2023, 5:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2023, 5:25 PM
snehasish requested changes to this revision.Mar 16 2023, 12:37 PM

Can you split out the uint64_t type changes into a separate patch?

llvm/lib/ProfileData/SampleProfReader.cpp
1069

Just to confirm, the reason this is safe for existing profiles on disk is because the underlying number is uleb128 encoded?

This revision now requires changes to proceed.Mar 16 2023, 12:37 PM
huangjd updated this revision to Diff 513026.Apr 12 2023, 5:44 PM

Separate patch into two

  1. Use correct integral type when reading the profile
snehasish accepted this revision.Apr 13 2023, 10:36 AM

lgtm since I think this change is safe for existing profiles. @huangjd please confirm before submitting, thanks!

This revision is now accepted and ready to land.Apr 13 2023, 10:36 AM
huangjd accepted this revision.Apr 13 2023, 2:27 PM
This revision was landed with ongoing or failed builds.Apr 13 2023, 2:32 PM
This revision was automatically updated to reflect the committed changes.