This is an archive of the discontinued LLVM Phabricator instance.

profilie inference changes for stale profile matching
ClosedPublic

Authored by spupyrev on May 12 2023, 10:50 AM.

Details

Summary

This diff facilitates a new stale profile matching in BOLT: D144500

This is a no-op for existing usages of profi (CSSPGO).

Diff Detail

Event Timeline

spupyrev created this revision.May 12 2023, 10:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 12 2023, 10:50 AM
spupyrev requested review of this revision.May 12 2023, 10:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 12 2023, 10:50 AM
spupyrev edited the summary of this revision. (Show Details)May 12 2023, 10:52 AM
spupyrev added reviewers: wenlei, hoy, Amir.
hoy added inline comments.May 12 2023, 11:03 AM
llvm/lib/Transforms/Utils/SampleProfileInference.cpp
1236

nit: you can just assert on the return value of the insert operation, which returns a pair consisting of an iterator to the inserted element (or to the element that prevented the insertion) and a bool denoting whether the insertion took place.

1338

Wondering in what cases a function doesn't come in with samples? Thought we should have bailed out on a higher level in the sample loader.

spupyrev marked an inline comment as done.May 12 2023, 11:19 AM
spupyrev added inline comments.
llvm/lib/Transforms/Utils/SampleProfileInference.cpp
1338

This is going to be called from BOLT in addition to sample loader. One scenario when a function doesn't have samples is when its profile data is so stale that we couldn't match a single block/jump/call.
(I agree that check could be verified earlier but i don't see a big advantage.)

hoy accepted this revision.May 12 2023, 11:37 AM

LGTM.

This revision is now accepted and ready to land.May 12 2023, 11:37 AM
This revision was landed with ongoing or failed builds.May 12 2023, 11:58 AM
This revision was automatically updated to reflect the committed changes.
wenlei added inline comments.May 12 2023, 12:17 PM
llvm/lib/Transforms/Utils/SampleProfileInference.cpp
1230

how about noreturn function, or functions with an infinite loop? These are not important functions to optimize, but at least we shouldn't fail on those cases?

1338

SampleProfileInference<BT>::apply has the same logic to check function size and whether there's sample. Better unify and avoid duplication. I also agree that it's better to check at caller side.