<div dir="ltr">On Sat, Jun 8, 2013 at 9:41 PM, JF Bastien <span dir="ltr"><<a href="mailto:jfb@google.com" target="_blank" class="cremed">jfb@google.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"><p dir="ltr"><br>
> 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.</p>


</div><p dir="ltr">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.</p></blockquote><div>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.</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr"> </p>
<p dir="ltr">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.</p></blockquote><div>OK, but I think generally speaking you should add test cases as you go unless there is a good reason not to.</div>
<div><br></div><div>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. </div>
</div></div></div>