Closed
Bug 530754
Opened 15 years ago
Closed 14 years ago
lirasm: Automate testing with multiple configurations.
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
DUPLICATE
of bug 573998
People
(Reporter: jbramley, Assigned: n.nethercote)
References
Details
Attachments
(1 file)
(deleted),
patch
|
graydon
:
review+
jbramley
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Updated•15 years ago
|
Assignee: general → nnethercote
Assignee | ||
Comment 1•15 years ago
|
||
Thinking about this some more, I don't understand how the --novfp option works when lirasm doesn't have a SoftFloatFilter implementation...
Assignee | ||
Comment 2•15 years ago
|
||
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.
Attachment #424540 -
Flags: review?(graydon)
Assignee | ||
Updated•15 years ago
|
Attachment #424540 -
Flags: review?(Jacob.Bramley)
Reporter | ||
Updated•15 years ago
|
Attachment #424540 -
Flags: review?(Jacob.Bramley) → review+
Reporter | ||
Comment 3•15 years ago
|
||
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!
Reporter | ||
Comment 4•15 years ago
|
||
(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•15 years ago
|
||
Comment on attachment 424540 [details] [diff] [review]
patch
Sorry for the slow review, the notification email accidentally got spam-binned.
Attachment #424540 -
Flags: review?(graydon) → review+
Assignee | ||
Comment 6•15 years ago
|
||
(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•15 years ago
|
||
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).
Reporter | ||
Updated•14 years ago
|
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•