The test case is in another patch so LGTM.

Thanks for refactoring LGTM.

Rebase

Rebase against master.

Thanks for rebasing the patch.

In D32140#1725238, @xbolva00 wrote:

GVNHoist was recently reverted maily due to big regressions:
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20181001/591519.html

In D32140#1725210, @fhahn wrote:

Two high level comments after a quick glance

1. gnv-sink & gvn-hoist are still disabled by default AFAIK and there are still a few known bugs that need to be addressed (https://bugs.llvm.org/buglist.cgi?quicksearch=gvn-hoist, https://bugs.llvm.org/buglist.cgi?quicksearch=gvn-sink). It would be good to guard this change by a new flag that's off by default, otherwise it might hinder work towards weeding out the existing bugs.

In D32140#1724853, @lkail wrote:

Hi @hiraditya , thanks for your work. I want to test your patch on PowerPC, however current patch seems unable to be applied to current master branch.

Add testcase for sinking

Remove unused variable and use function for updating local stats.

Make MaxSinkChainLength a flag.

Remove VNs already handled to reduce redundant lookups.

Sink iteratively.

Sink iteratively

Rebase against master and use profitability to reduce liveness

ping.

Add check for both hot and cold prefixes.

btw, I had this patch in mind:

It is happening without my changes with -codegenprepare. So just keeping the testcase.

test case

Added testcase

Because my goal is to improve outlining without PGO (with static analysis), I'm wondering what would be the best place to put this other than HotColdSplitting.

WIP

Will add test case soon.

rebase against latest changes

Good point, there's something about updating the users that copying maybe required. make_early_inc_range didn't work, I searched for uses cases of replaceUsesOfWith and I couldn't find any instance where we use 'users'.

Tried the following, I get test failures and crashes.

LGTM as the comments have been addressed.

Nice!

Reviewed and committed: https://reviews.llvm.org/D67941

Reviewed here: https://reviews.llvm.org/D67941

Closed in: https://reviews.llvm.org/rL373807

remove XFAIL now that fix had landed

Splitting the verifier into a separate diff: https://reviews.llvm.org/D68345

It depends on the verifier function (CodeExtractor::verifyAssumptionCache) from D67941 to fail with the test case in: llvm/test/Transforms/HotColdSplit/assumption-cache-invalidation.ll

Relevant test case: llvm/test/Transforms/HotColdSplit/assumption-cache-invalidation.ll is already in trunk.

passed ninja check-all

addressed comments

ninja check passed