[PATCH] D20379: Codegen: Fix broken assumption in Tail Merge.

[Kyle Butt via llvm-commits]

iteratee added a comment.

OK, the two failing tests are gone, I don't think they were valid to begin with. Are there any other concerns about this change?

In http://reviews.llvm.org/D20379#458728, @mcrosier wrote:

> In http://reviews.llvm.org/D20379#457684, @iteratee wrote:
>
> > OK, the two failing tests are gone, I don't think they were valid to begin with. Are there any other concerns about this change?
>
>
> In general, we don't want to reduce our test coverage.  Please provide additional details as to why you believe these two tests aren't valid and why it's reasonable to delete them.


In general, we want coverage by correct tests. Incorrect tests are a liability, not an asset.

OK, let's take them one at a time:
licm-dominance:
This test has NEVER been correct. I've checked the entire version history. The goal of the test is to verify that LICM checks the dominance relation to make sure a load is guaranteed to be executed. The problem with the test is that with all the undefineds, the compiler is free to make sure the load is guaranteed to be executed. The test also relies on the particular behavior of undefined as setting eax to 0

Thumb2/v8_IT_5.ll:
There are 4 other v8_it tests, but this one suffers from a similar problem as the one above. It has too many undefineds for the assumptions about the resulting code to ever be correct.

I feel I've given plenty of time for people to respond to the tests in question. On May 19, almost a month ago I said:
"""
That only leaves licm-dominance and Thumb2/v8_IT_5.ll

The optimizer seems to be doing reasonable things in both of those cases.

I don't really want to remove tests from the regression suite, but the tests seem to be relying on bad assumptions, and I'm not the best person to fix them.
I could disable the FileCheck lines and open bugs for them.
"""
and a week later on the 25th I said:
"""
Barring comment from anyone else, I'm going to go ahead and remove

LLVM :: CodeGen/Thumb2/v8_IT_5.ll
LLVM :: CodeGen/X86/licm-dominance.ll
In licm-dominance it's clear that bugpoint has gone too far and the optimizer is being perfectly reasonable,
and it looks pretty reasonable for v8_IT_5.ll as well.
"""
A comment from you then would have been very helpful.


http://reviews.llvm.org/D20379