530754 - lirasm: Automate testing with multiple configurations.

Status: RESOLVED DUPLICATE of bug 573998

(Core :: JavaScript Engine, defect)

Description

[Jacob Bramley [:jbramley]]

Bug 526974 adds the ability to configure Nanojit when running lirasm. For example, on ARM you can select "--arm-arch 6" to tell Nanojit to use ARMv6 instructions, and on i386 you can select "--sse" to tell Nanojit to use SSE2 instructions.

We should automate "make check" (or something similar) so that it checks multiple possible configurations and ensures that they all work. Each configuration represents a different code path through Nanojit, so this should improve our test coverage.

Comment 1

[Nicholas Nethercote [inactive]]

Thinking about this some more, I don't understand how the --novfp option works when lirasm doesn't have a SoftFloatFilter implementation...

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: patch

Here's a patch that achieves this goal.

Instead of going to the trouble of working out what kind of machine we're
on and then selecting the configs accordingly, it just tries all of them,
and if any don't work, it skips them.  This is possible because I changed
lirasm to return a special exit code (99) if it's given an arch-specific 
option that doesn't apply to the current arch.

This is definitely a hack.  But I can't decide if it's an inspired 
worse-is-better kind of hack, or just a garden variety
I'll-live-to-regret-this hack.

Alternative suggestions are welcome, but I couldn't bear to do it by parsing
the output of 'uname' or anything like that.

Comment 3

[Jacob Bramley [:jbramley]]

Comment on attachment 424540 [details] [diff] [review]
patch

Yep, it looks good to me!

>+#if defined NANOJIT_IA32
>+            i386_sse = false;
>+#else
>+            errMsgAndQuit(opts.progname, BAD_ARCH_EXITCODE, "--nosse only valid on i386");

You missed the extra "-" in "--no-sse".

----

+# Alternative i386 configs.
+#go --sse       (covered by default, above)
+go --no-sse
+
+# Alternative ARM configs.
+#go --arch 7    (covered by default, above)
+go --arch 6
+go --arch 5
+go --arch 7 --no-vfp
+go --arch 6 --no-vfp
+go --arch 5 --no-vfp

One question here: What happens if you're running the tests on an ARMv5 or ARMv6 platform (or one without VFP)? The ARMv7 tests will fail in that case. Parsing uname or auxv may be the only way around that. I think we generally test on ARMv7 platforms anyway, but it's worth noting that some tests will fail on older platforms.

Trace Monkey parses auxv to determine the platform variant, and I think Tamarin will do the same thing shortly, so can we not use a similar mechanism here?

It doesn't matter too much for now; the current default is to run ARMv7+vfp anyway, so the default test run will already fail on older platforms.

----

Thanks!

Comment 4

[Jacob Bramley [:jbramley]]

(In reply to comment #1)
> Thinking about this some more, I don't understand how the --novfp option works
> when lirasm doesn't have a SoftFloatFilter implementation...

It just sets AvmCore::config.vfp to 'false', so it doesn't change the filter configurations at all. (It probably should.) Indeed, I've just tried it on my ARM board and the tests using floating-point do fail if I use --novfp. I hadn't noticed that before, though it's fairly obvious really.

Perhaps for now we should disable the --no-vfp case in the automatic script. We should keep the lirasm switches because they're useful for local testing, and if a SoftFloatFilter implementation is added in the future we can add --no-vfp to the automatic tests.

Comment 5

[Graydon Hoare :graydon]

Comment on attachment 424540 [details] [diff] [review]
patch

Sorry for the slow review, the notification email accidentally got spam-binned.

Comment 6

[Nicholas Nethercote [inactive]]

(In reply to comment #3)
> 
> One question here: What happens if you're running the tests on an ARMv5 or
> ARMv6 platform (or one without VFP)? The ARMv7 tests will fail in that case.
> Parsing uname or auxv may be the only way around that. I think we generally
> test on ARMv7 platforms anyway, but it's worth noting that some tests will fail
> on older platforms.

I have a virtual ARM5 machine, for example.  This patch doesn't entirely work for such machines, as you say, but it's a start.  (Graydon, is the tinderbox ARM machine ARM7?)

> Trace Monkey parses auxv to determine the platform variant, and I think Tamarin
> will do the same thing shortly, so can we not use a similar mechanism here?

That detection code is 210 lines, a fair chunk.  If TR will use it then it can be pulled into NJ itself.  (Can any Adobe people weigh in here?)  Then we just need a way to communicate supported versions between NJ and the testlirc.sh script that runs the tests.

Comment 7

[Edwin Smith]

We'd like to use it instead of reinventing the wheel; only speed bumps might be release branching or platform dependencies in the detection code (signal handlers? stuff like that).