LGTM!

Committed revision 286632.

ping?

I added a unit test for FunctionComparator. It mainly test the API of the class (I didn't want to replicate the test for functionality, which are covered by the MergeFunction pass tests).

In D25385#566415, @dblaikie wrote:

Might be worth having some unit tests if it's nicely abstracted as a reusable utility.

Thanks for reviewing!
Committed in r271299

ping (2)

ping?

forgot to add the test file

LGTM (but please wait for other reviewers)

Mitch, Erik, David, Bruno,
Are you planning to attend the LLVM developers meeting on Thu/Fri? That would be a good opportunity to discuss this patch in person if you would like to.

Some comments:

BTW, would it make sense to add a test to check that we properly use MinScheduleRegionSize too?

Thanks for reviewing!
I committed it in r248917.

Add an option to set the scheduling budget + a test which uses that option.

committed in r244901

Here is an updated version with a bug fix in MemoryIsNotModifiedBetween.
I also added a few tests.

Thanks for reviewing!

Correction to the static data:

Some data from the lnt test suite:

Philip, thanks for your comments on the first version. Please let me know if your questions are still relevant with this updated patch.

This updated version uses a work-list algorithm to do the safety checking. This has 2 advantages compared to MD->getNonLocalPointerDependency:

1. It ignores load instructions which should not prevent the optimization
2. It's faster because it bails out at the first memory alias it finds.

Hi Karthik,
Daniel pointed me to your patch, because I'm also working on something in DeadStoreElimination: http://reviews.llvm.org/D11854

Thanks for looking at this patch.

renamed variables in test
diff with full context

Committed in r231276.

You are mentioning 5% perf improvement in single-threaded release build, can you tell how to reproduce?

New version using llvm::Optional

Hi Owen, Chandler,

Committed in r226792 (with a minor change in comment).

Committed in r226439

Arnold, thanks for your comments.
I renamed the const to "AliasedCheckLimit"

Committed in rL225977 with a few changes/corrections:

committed in r224417

I updated the comments.

I did a split of the change:
I committed the refactoring in r224006.

commited in r223224

commited in r222891. Note that I added a fix which checks that the guarding branch dominates the phi-block.

This is already the third version of the change. Previous version were discussed per email:

This is already the second version of the patch. The first version was discussed by email.

OK, thanks!

yes, the test passes.
Please let me know if it also gives the same improvement for the benchmark.

Hi Chad,