[llvm] r183624 - ARM FastISel fix load register classes

[Chandler Carruth]

On Sat, Jun 8, 2013 at 9:41 PM, JF Bastien <jfb at google.com> wrote:

>
> > Is this something you can add a regression test that specifically
> exercises the issue so we don't regress? (I realize that you can likely
> reproduce this just be enabling various verifiers on existing tests, or
> enabling fast isel on them, but it'd be good to explicitly test the things
> you end up fixing.
>
> All that's needed is emitting LDRBi12 and enabling MI validation, so yes.
> I can add a test for just that if you think it's useful.
>
My dim memory is that there was a plan to enable MI validation by default
in debug builds once the failures are cleaned up in the test suite. Jakob
or Andy would know better though. If my memory is correct, and there are
already tests that fail with MI validation due to this bug, then that's
more than enough IMO.

>  In the end I think register classes are kind of broken on ARM, I'd like
> to fix it once I'm done with a few PNaCl things.
>
OK, but I think generally speaking you should add test cases as you go
unless there is a good reason not to.

A good reason is what I described above -- we already have the tests, just
that we need to enable the checking that triggers the error and currently
it triggers too many errors. We don't want to add what would eventually be
redundant testing unless there is some serious problem with regressions.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130608/2df439ad/attachment.html>