This is an archive of the discontinued LLVM Phabricator instance.

introducing profi flags
ClosedPublic

Authored by spupyrev on Feb 24 2022, 11:37 AM.

Details

Summary

There are two changes in the diff:

  • adding cmd options for profi weights, so we can experiment/tune different combinations; default values stay the same so this does not impact results;
  • modifying the jump distance in FlowAdjuster.

The former doesn't change the logic but simplifies flag tuning. The latter is useful for "large" profile datasets; the older version works well only for smaller binaries from the SPEC dataset but has scaling issues with larger datasets

Diff Detail

Event Timeline

spupyrev created this revision.Feb 24 2022, 11:37 AM
spupyrev requested review of this revision.Feb 24 2022, 11:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 24 2022, 11:37 AM
spupyrev edited the summary of this revision. (Show Details)Feb 24 2022, 11:42 AM
spupyrev added reviewers: wenlei, hoy.
wlei added a subscriber: wlei.Feb 24 2022, 11:47 AM
spupyrev updated this revision to Diff 413062.Mar 4 2022, 10:24 AM

modifying island connectors + removed one (controversial) option

Herald added a project: Restricted Project. · View Herald TranscriptMar 4 2022, 10:24 AM
spupyrev retitled this revision from introducing some profi flags to introducing profi flags.Mar 4 2022, 10:27 AM
spupyrev edited the summary of this revision. (Show Details)
wenlei added inline comments.Mar 5 2022, 12:18 PM
llvm/lib/Transforms/Utils/SampleProfileInference.cpp
744–746

Could you explain this a bit more, how does this minimizes total multiplicative Flow increase for the remaining edges?

hoy added inline comments.Mar 7 2022, 12:09 PM
llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
923

Remove this since it's not used anywhere?

llvm/lib/Transforms/Utils/SampleProfileInference.cpp
744–746

Same question here. Looks like we are turning the previous subtraction into a division. So it's not a linear reduction anymore. For edges with smaller flow, the cost now will be much larger than previously, which would prevent adding more flow to that edge?

spupyrev updated this revision to Diff 413800.Mar 8 2022, 7:26 AM

added back the option for overwriting entry count (ON by default)

hoy added inline comments.Mar 8 2022, 8:52 AM
llvm/lib/Transforms/Utils/SampleProfileLoaderBaseUtil.cpp
46 ↗(On Diff #413800)

Will there be a separate change to flip the default value?

spupyrev added inline comments.Mar 8 2022, 12:16 PM
llvm/lib/Transforms/Utils/SampleProfileLoaderBaseUtil.cpp
46 ↗(On Diff #413800)

I suggest we keep it as is and test on a couple of prod binaries. If confirmed, we may change the default value

hoy accepted this revision.Mar 8 2022, 12:17 PM
hoy added inline comments.
llvm/lib/Transforms/Utils/SampleProfileLoaderBaseUtil.cpp
46 ↗(On Diff #413800)

sgtm, thanks.

This revision is now accepted and ready to land.Mar 8 2022, 12:17 PM
This revision was automatically updated to reflect the committed changes.