Page MenuHomePhabricator

[LICM] Make Loop ICM profile aware again
ClosedPublic

Authored by modimo on Sep 11 2020, 3:33 PM.

Details

Summary

D65060 was reverted because it introduced non-determinism by using BFI counts from already freed blocks. The parent of this revision fixes that by using a VH callback on blocks to prevent this from happening and makes sure BFI data is passed correctly in LoopStandardAnalysisResults.

This re-introduces the previous optimization of using BFI data to prevent LICM from hoisting/sinking if the instruction will end up moving to a colder block.

Internally at Facebook this change results in a ~7% win in a CPU related metric in one of our big services by preventing hoisting cold code into a hot pre-header like the added test case demonstrates.

Testing:
ninja check

Diff Detail

Event Timeline

modimo created this revision.Sep 11 2020, 3:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 11 2020, 3:33 PM
modimo requested review of this revision.Sep 11 2020, 3:33 PM
asbirlea accepted this revision.Sep 14 2020, 2:24 PM

Thank you for the patch!

Could you include in the description the benefits seen from adding this, as a data point?

llvm/include/llvm/Transforms/Utils/LoopUtils.h
127

Nit: update comment to include arguments and not duplicate "information for all".

This revision is now accepted and ready to land.Sep 14 2020, 2:24 PM
modimo updated this revision to Diff 291692.Sep 14 2020, 3:11 PM
modimo edited the summary of this revision. (Show Details)

Fix merge issues with comment, updated description to include benefit we see from this change. Thanks @asbirlea for the quick review!

modimo updated this revision to Diff 291702.Sep 14 2020, 3:15 PM
modimo edited the summary of this revision. (Show Details)

Remove unnecessary period

This revision was automatically updated to reflect the committed changes.

Committed on behalf of @modimo.