Closed Bug 530754 Opened 15 years ago Closed 14 years ago

lirasm: Automate testing with multiple configurations.

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 573998

People

(Reporter: jbramley, Assigned: n.nethercote)

References

Details

Attachments

(1 file)

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.
Depends on: 543144
Assignee: general → nnethercote
Thinking about this some more, I don't understand how the --novfp option works when lirasm doesn't have a SoftFloatFilter implementation...
Attached patch patch (deleted) — Splinter Review
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)
Attachment #424540 - Flags: review?(Jacob.Bramley)
Attachment #424540 - Flags: review?(Jacob.Bramley) → review+
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!
(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 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+
(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.
Depends on: 543401
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).
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.

Attachment

General

Created:
Updated:
Size: