Closed Bug 615159 Opened 9 years ago Closed 9 years ago

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

Categories

(Core Graveyard :: Nanojit, defect)

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: njn, Assigned: njn)

References

Details

(Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey)

Attachments

(1 file)

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.
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.
Attachment #495423 - Flags: review?(Jacob.Bramley)
Attachment #495423 - Flags: feedback?(edwsmith)
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.)
Attachment #495423 - Flags: review?(Jacob.Bramley) → review+
Comment on attachment 495423 [details] [diff] [review]
patch (against TM 58210:25fd3451c0ae)

This all looks like grade-A good stuff to me.
Attachment #495423 - Flags: feedback?(edwsmith) → feedback+
http://hg.mozilla.org/mozilla-central/rev/79f9bf0c6146
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.