Page MenuHomePhabricator

Early exit from Hoist() in machine licm pass based on block hotness
Needs ReviewPublic

Authored by NeHuang on Jun 21 2019, 2:57 PM.

Details

Summary

In current Hoist() function of machine licm pass, it will not check the source and destination basic block frequencies that a instruction is hoisted from/to. There is a chance that instruction is hoisted from a cold to a hot basic block.

In this patch, we add options to disable instruction hoisting due to block hotness.

Diff Detail

Event Timeline

NeHuang created this revision.Jun 21 2019, 2:57 PM

Added a couple more reviewers that either recently modified this file or I think may be interested in the proposed change.

In the future, please try to ping more frequently than once a month; we don't want to lose patches, even if there are some areas where it takes a while to get review.

The general idea of avoiding hoisting that would increase the frequency makes sense.

Could you give some idea what effect this has in practice? How often does this trigger on SPEC? Does it affect SPEC performance or size numbers? (Replace SPEC with some similar benchmark if you prefer.)

Is there anything special about the number "100"?

Thanks @efriedma for the review and remind.

This patch adds an option to disable non-profitable hoisting from a cold to a hot basic block.

The stats below is collected based on Google benchmark. (The 3rd column indicates number of hoisting is disabled due to block frequency hotness, results varies from the loads of different benchmarks)

BenchmarkNumber of machine instructions hoisted out of loopsNumber of instructions not hoisted due to block frequency hotness
Protobuf209311706
Brotli993517616
Gipfeli471879
snappy8861154
TCMalloc6435891
BoringSSL6115501
FFMPEG832417

With the patch enabled, we found SPEC performance improvement, e.g. perlbench speeding up by 2.5%.

'100' is the block-freq-ratio-threshold used in our experiment to determine when to disable the hoisting, which is a configurable option.

Whitney added inline comments.Wed, Aug 21, 6:27 AM
llvm/lib/CodeGen/MachineLICM.cpp
54

Please run clang-format on your change. The include is not added at the right place.

1585

Change to ++ NumNotHoistedDueToHotness

NeHuang updated this revision to Diff 216684.Thu, Aug 22, 12:06 PM
NeHuang marked 4 inline comments as done.Thu, Aug 22, 12:09 PM
This comment was removed by NeHuang.
llvm/lib/CodeGen/MachineLICM.cpp
54

Done.

1585

Done

jsji added a comment.Thu, Aug 22, 12:47 PM

machinelicm_clangformat_modv1.cpp ? I think you uploaded wrong patch.

NeHuang updated this revision to Diff 216693.Thu, Aug 22, 12:55 PM
NeHuang marked 2 inline comments as done.

@jsji Yes, please ignore diff 216684. Thanks you for the remind.

Thanks @Whitney for the review. Addressed the comment and uploaded diff 216693.