[PATCH] D17173: [X86] Update test cases that would be affected when X86FixupBWInsts is turned on.

[Kevin B. Smith via llvm-commits]

kbsmith1 added a comment.

In http://reviews.llvm.org/D17173#351515, @spatel wrote:

> This feels similar to http://reviews.llvm.org/D12656, and I don't think it's the right choice for the same reasons that were cited in that review.
>
> I'd prefer that we just change the checks in these tests as part of flipping/removing the "-fixup-byte-word-insts" switch. Alternatively, we could add another RUN to these tests to show the exact codegen difference produced by -fixup-byte-word-insts for these cases.
>
> Masking the default output could hide codegen differences that we would actually like to observe. As an example, we may want to limit the FixupBW pass based on micro-arch (should an in-order CPU get the same set of transforms as as OoO?). If that happens, then these tests become more important as verification that the default behavior is not changing.


I think that having additional RUN commands that show what the results should be in the presence of fixup-byte-word-insts would be OK, although many of the checks would be the same and therefore simply duplicated, making the tests harder to maintain.  I think that using the option to turn off this transformation does the best job of leaving these tests as good unit tests of what they were intended to unit test, and that we should be adding specific tests as needed to adequately unit tests fixup-byte-word-inst option. I think this is different than http://reviews.llvm.org/D12656 in that I am not changing the patterns to allow "either" movw/movzwl (for example), and so these tests continue to have very specific expected output results.  Due to the inability to have MachineIR specific unit tests, this seems like a very reasonable way to continue to have existing unit tests work as expected.  If both you and Quentin disagree with my position on this specific change, I'll abandon it, but I think doing so dilutes the value of these tests as unit tests.


http://reviews.llvm.org/D17173