Page MenuHomePhabricator

[PGO] Change hardcoded thresholds for cold/inlinehint to use summary
ClosedPublic

Authored by tejohnson on Sep 17 2019, 12:00 PM.

Details

Summary

The PGO counter reading will add cold and inlinehint (hot) attributes
to functions that are very cold or hot. This was using hardcoded
thresholds, instead of the profile summary cutoffs which are used in
other hot/cold detection and are more dynamic and adaptable. Switch
to using the summary-based cold/hot detection.

The hardcoded limits were causing some code that had a medium level of
hotness (per the summary) to be incorrectly marked with a cold
attribute, blocking inlining.

Diff Detail

Repository
rL LLVM

Event Timeline

tejohnson created this revision.Sep 17 2019, 12:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 17 2019, 12:01 PM
tejohnson marked an inline comment as done.Sep 17 2019, 12:07 PM
tejohnson added inline comments.
test/Transforms/PGOProfile/func_entry.ll
9 ↗(On Diff #220547)

I forgot to add the checking of the attribute, updating shortly.

Fix test and actually check the attributes

davidxl added inline comments.Sep 17 2019, 1:59 PM
lib/Transforms/Instrumentation/PGOInstrumentation.cpp
1626 ↗(On Diff #220550)

Conceptually, this should be set even earlier before getAnalysis at line 1754 below.

tejohnson marked an inline comment as done.Sep 17 2019, 2:08 PM
tejohnson added inline comments.
lib/Transforms/Instrumentation/PGOInstrumentation.cpp
1626 ↗(On Diff #220550)

I'm not sure what you mean - we can't do so until we create the IndexedInstrProfReader which is just above here, since that reads the header and therefore the summary. Or do you mean that the IndexedInstrProfReader should be created before we call getAnalysis? That would take more restructuring.

davidxl accepted this revision.Sep 17 2019, 2:15 PM

you are right. lgtm

This revision is now accepted and ready to land.Sep 17 2019, 2:15 PM
This revision was automatically updated to reflect the committed changes.