[PATCH] D12656: PR 23155: Change test to allow movzbl,movzwl in place of movb,movw instructions

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 9 09:33:39 PDT 2015


> On Sep 8, 2015, at 8:25 PM, Kevin B. Smith <kevin.b.smith at intel.com> wrote:
> 
> kbsmith1 added a comment.
> 
> There are a handful (6-8) tests that currently are checking for movw, where either movzwl or movw would be legal choices. There are quite a few more that specifically expect movb where movzbl would also be legal.  My intent was to add a test that tested for the specific movb vs movzbl or movw vs movzwl when the transformations to change these opcodes got added.
> 
> My intent with this change-set was to get started changing tests where the test wasn't specifically trying to test ffor  one or the other, where the test was simply testing for what the compiler currently did, rather than what it was legal to do.  It seems like the principal of these unit tests should be to accept all legal outputs, except for the key place where they were checking for a specific piece of the implementation.  That way the tests are not so fragile as heuristic changes are made.  It seems wrong to have so many tests dependent on this choice, when that is really tangential to what they seem to be intending to check.

Although I sympathize with the argument of having less fragile tests, like I said, allowing to match this kind of legal but different variant may hide non-detertiministic behavior in the isel process. Therefore, if we need to change the test output when we change some heuristics, then so be it. I do not expect this to happen often enough to be worth bothering being more permissive in those.

Cheers,
Q.


> I was doing this to try to make these changes in small, easy to review pieces.
> 
> 
> http://reviews.llvm.org/D12656
> 
> 
> 



More information about the llvm-commits mailing list