Subzero, MIPS32: Remove --skip-unimplemented from lit tests

Subzero, MIPS32: Remove --skip-unimplemented from lit tests

R=stichnot@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=0dabe18f9251b797cbc6aa9c014a7a79e58fae07

[Stefan Maksimovic, 2016-10-20 08:13:48]

Description was changed from

==========
Subzero, MIPS32: Remove --skip-unimplemented from lit tests

BUG=
==========

to

==========

==========

[Stefan Maksimovic, 2016-10-20 08:13:48]

makdstefan@gmail.com changed reviewers:
+ jaydeep.patil@imgtec.com, sagar.thakur@imgtec.com, stichnot@chromium.org

[Jim Stichnoth, 2016-10-20 13:08:59]

Very nice, thanks!

The endgame here is that "git grep skip-unimplemented" shows no results in .ll
files.

There are still a few references in TODO comments (as I pointed out separately).

The tests_lit/assembler/mips32/ directory still uses it; I don't know if those
are still needed.

The tests_lit/llvm2ice_tests/nop-insertion-no-vectors.ll file still uses it,
that is probably not necessary.

https://codereview.chromium.org/2433243003/diff/1/tests_lit/llvm2ice_tests/64...
File tests_lit/llvm2ice_tests/64bit.pnacl.ll (right):

https://codereview.chromium.org/2433243003/diff/1/tests_lit/llvm2ice_tests/64...
tests_lit/llvm2ice_tests/64bit.pnacl.ll:26: ; TODO(rkotler): Stop skipping
unimplemented parts (via --skip-unimplemented)
Can you also remove the --skip-unimplemented TODO from this and several other
files?

(though switching to --filetype=obj is still a TODO here)

[Stefan Maksimovic, 2016-10-21 13:54:02]

On 2016/10/20 13:08:59, Jim Stichnoth wrote:
> Very nice, thanks!
> 
> The endgame here is that "git grep skip-unimplemented" shows no results in .ll
> files.
> 
> There are still a few references in TODO comments (as I pointed out
separately).
> 
> The tests_lit/assembler/mips32/ directory still uses it; I don't know if those
> are still needed.
> 
> The tests_lit/llvm2ice_tests/nop-insertion-no-vectors.ll file still uses it,
> that is probably not necessary.
> 
>
https://codereview.chromium.org/2433243003/diff/1/tests_lit/llvm2ice_tests/64...
> File tests_lit/llvm2ice_tests/64bit.pnacl.ll (right):
> 
>
https://codereview.chromium.org/2433243003/diff/1/tests_lit/llvm2ice_tests/64...
> tests_lit/llvm2ice_tests/64bit.pnacl.ll:26: ; TODO(rkotler): Stop skipping
> unimplemented parts (via --skip-unimplemented)
> Can you also remove the --skip-unimplemented TODO from this and several other
> files?
> 
> (though switching to --filetype=obj is still a TODO here)

In addition to that, I'll add mips support in some of the tests as well, so it
may take me some time.

[Jim Stichnoth, 2016-10-22 14:13:25]

On 2016/10/21 13:54:02, Stefan Maksimovic wrote:
> On 2016/10/20 13:08:59, Jim Stichnoth wrote:
> > Very nice, thanks!
> > 
> > The endgame here is that "git grep skip-unimplemented" shows no results in
.ll
> > files.
> > 
> > There are still a few references in TODO comments (as I pointed out
> separately).
> > 
> > The tests_lit/assembler/mips32/ directory still uses it; I don't know if
those
> > are still needed.
> > 
> > The tests_lit/llvm2ice_tests/nop-insertion-no-vectors.ll file still uses it,
> > that is probably not necessary.
> > 
> >
>
https://codereview.chromium.org/2433243003/diff/1/tests_lit/llvm2ice_tests/64...
> > File tests_lit/llvm2ice_tests/64bit.pnacl.ll (right):
> > 
> >
>
https://codereview.chromium.org/2433243003/diff/1/tests_lit/llvm2ice_tests/64...
> > tests_lit/llvm2ice_tests/64bit.pnacl.ll:26: ; TODO(rkotler): Stop skipping
> > unimplemented parts (via --skip-unimplemented)
> > Can you also remove the --skip-unimplemented TODO from this and several
other
> > files?
> > 
> > (though switching to --filetype=obj is still a TODO here)
> 
> In addition to that, I'll add mips support in some of the tests as well, so it
> may take me some time.

I think it might be better to keep this as a simple cleanup CL, and add the new
tests as a separate CL.

[Stefan Maksimovic, 2016-10-25 06:05:55]

On 2016/10/22 14:13:25, Jim Stichnoth wrote:
> On 2016/10/21 13:54:02, Stefan Maksimovic wrote:
> > On 2016/10/20 13:08:59, Jim Stichnoth wrote:
> > > Very nice, thanks!
> > > 
> > > The endgame here is that "git grep skip-unimplemented" shows no results in
> .ll
> > > files.
> > > 
> > > There are still a few references in TODO comments (as I pointed out
> > separately).
> > > 
> > > The tests_lit/assembler/mips32/ directory still uses it; I don't know if
> those
> > > are still needed.
> > > 
> > > The tests_lit/llvm2ice_tests/nop-insertion-no-vectors.ll file still uses
it,
> > > that is probably not necessary.
> > > 
> > >
> >
>
https://codereview.chromium.org/2433243003/diff/1/tests_lit/llvm2ice_tests/64...
> > > File tests_lit/llvm2ice_tests/64bit.pnacl.ll (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2433243003/diff/1/tests_lit/llvm2ice_tests/64...
> > > tests_lit/llvm2ice_tests/64bit.pnacl.ll:26: ; TODO(rkotler): Stop skipping
> > > unimplemented parts (via --skip-unimplemented)
> > > Can you also remove the --skip-unimplemented TODO from this and several
> other
> > > files?
> > > 
> > > (though switching to --filetype=obj is still a TODO here)
> > 
> > In addition to that, I'll add mips support in some of the tests as well, so
it
> > may take me some time.
> 
> I think it might be better to keep this as a simple cleanup CL, and add the
new
> tests as a separate CL.

Agreed.

[Jim Stichnoth, 2016-10-25 14:22:19]

https://codereview.chromium.org/2433243003/diff/1/tests_lit/llvm2ice_tests/na...
File tests_lit/llvm2ice_tests/nacl-other-intrinsics.ll (right):

https://codereview.chromium.org/2433243003/diff/1/tests_lit/llvm2ice_tests/na...
tests_lit/llvm2ice_tests/nacl-other-intrinsics.ll:39: ; RUN:   -i %s --args -Om1
\
Unfortunately, this --skip-unimplemented may need to stay.
  make -f Makefile.standalone check-lit
fails (at least on my machine) because of the unimplemented stacksave and
stackrestore intrinsics.

These tests don't currently have any MIPS32 check lines, so it may seem odd that
they cause the test to fail.  I think there are two possibilities:

1. The unimplemented error message gets mixed in with the filetype=asm text
stream and makes the llvm-mc assembler fail.

2. Because of pnacl-sz's default -threads=2 setting, one thread could hit the
unimplemented error and cause an abort before another thread produces sufficient
output for the other tests.

Note that there are still a bunch of other instances of
UnimplementedLoweringError in the MIPS lowering that may not be tested by the
lit tests.  (and a few in the ARM lowering, which need investigation...)

I tried to draw out more failures of this type by changing the default
-threads=2 to -threads=100 (via IceClFlags.def) and didn't find any more, but
they could still be lurking...

[Jim Stichnoth, 2016-10-28 12:21:12]

lgtm

[Jim Stichnoth, 2016-10-28 12:22:14]

Description was changed from

==========

==========

to

==========
Subzero, MIPS32: Remove --skip-unimplemented from lit tests
==========

[Jim Stichnoth, 2016-10-28 12:24:01]

Description was changed from

==========
Subzero, MIPS32: Remove --skip-unimplemented from lit tests
==========

to

==========
Subzero, MIPS32: Remove --skip-unimplemented from lit tests

R=stichnot@chromium.org

Committed:
https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a...
==========

[Jim Stichnoth, 2016-10-28 12:24:02]

Committed patchset #3 (id:40001) manually as
0dabe18f9251b797cbc6aa9c014a7a79e58fae07 (presubmit successful).