615159 - nanojit: test both SSE2 and non-SSE2 code on tinderbox

Status: RESOLVED FIXED

(Core Graveyard :: Nanojit, defect)

Description

[Nicholas Nethercote [inactive]]

On i386, lirasm defaults to generating non-SSE2 code;  you have to specify the --sse flag to get SSE2 code.  This is silly and should be changed so that SSE2 is the default.

Furthermore, the Nanojit tinderbox tests should be changed so that both kinds of code are tested.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: patch (against TM 58210:25fd3451c0ae)

This patch overhauls NJ testing, particularly w.r.t. how arch-specific
tests are handled.

In lirasm itself:

- Adds a --show-arch option, which gives us a simpler way of knowing the
  architecture than using 'uname'.  Renames --word-size and --endianness as
  --show-word-size and --show-endianness accordingly.  (Nb: the "show"
  prefix was necessary because --arch was already in use as an ARM-specific
  flag.)

- Changes i386 lirasm builds to generate SSE2 code by default.  This is now
  turned off with --nosse.

In testlirc.sh:

- Avoids putting the 'do' and 'then' keywords on a separate line, just to
  make things more compact.

- Adds the 'runtests' function which makes running all the tests in a
  particular directory much easier.

- Instead of doing piecemeal test selection (are we 32-bit?  are we
  big-endian?  do we have hardfloat?) it now selects the tests entirely on a
  per-arch basis.  This introduces a small amount of code duplication but it
  makes it much clearer what is tested for each arch.  Indeed, previously
  some combinations were missed because the test selection was spread around
  more (eg. the big-endian and 32-bit tests weren't tested on ARMv6 and
  ARMv5).

- Changes i386 to test both SSE2 and non-SSE2 code.

Checking of the endian-ness values would be appreciated, as several of the
arches (PPC, SPARC, MIPS, SH4) support both and I think they default to
big-endian.  Also, I'm not certain I've handled PPC32/PPC64 correctly.

Nb: the --show-endianness option is currently unused, but I've kept it in
around in case it's needed again.

Comment 2

[Jacob Bramley [:jbramley]]

Comment on attachment 495423 [details] [diff] [review]
patch (against TM 58210:25fd3451c0ae)

Good idea! This is much easier to read than it was.

(In reply to comment #1)
> Checking of the endian-ness values would be appreciated, as several of the
> arches (PPC, SPARC, MIPS, SH4) support both and I think they default to
> big-endian.  Also, I'm not certain I've handled PPC32/PPC64 correctly.

ARM should probably be running the little-endian tests. ARM's endianness can be configured (at run-time even), but little-endian is far more common on ARM. (Of course, if you decide to test --show-endianness, we can enable big-endian tests for ARM.)

Comment 3

[Edwin Smith]

Comment on attachment 495423 [details] [diff] [review]
patch (against TM 58210:25fd3451c0ae)

This all looks like grade-A good stuff to me.

Comment 4

[Nicholas Nethercote [inactive]]

Pushed with an ARM bustage fix follow-up:

http://hg.mozilla.org/projects/nanojit-central/rev/a54c3a65fa26
http://hg.mozilla.org/projects/nanojit-central/rev/1789b94e3300

http://hg.mozilla.org/tracemonkey/rev/79f9bf0c6146
http://hg.mozilla.org/tracemonkey/rev/9c48c587352a

Comment 5

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/79f9bf0c6146