Closed Bug 504202 Opened 15 years ago Closed 15 years ago

MIPS target support

Categories

(Tamarin Graveyard :: Virtual Machine, defect, P2)

Other
Linux
defect

Tracking

(Not tracked)

VERIFIED FIXED
flash10.1

People

(Reporter: chris, Assigned: rreitmai)

References

Details

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

Attachments

(5 files, 9 obsolete files)

(deleted), patch
rreitmai
: review+
edwsmith
: superreview+
Details | Diff | Splinter Review
(deleted), patch
rreitmai
: review+
brbaker
: review+
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), application/octet-stream
Details
(deleted), patch
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.11) Gecko/2009060309 Ubuntu/9.04 (jaunty) Firefox/3.0.11 Build Identifier: The attached patch adds build support for the MIPS target. At present there is no JIT support for MIPS, but it is possible to build the interpreter. Reproducible: Always After prebuilding all of the .abc files on an x86 host, the MIPS interpreter passes all of the acceptance tests except: abcasm/syntaxErrors.abs: My test system does not have a native java compiler installed ecma3/String/e15_5_4_7_2.as: I've tracked this down to core/StringObject.cpp where it casts an out of range float value to an integer which generates an undefined value. On MIPS the undefined value is positive, on x86 it is negative. I'll file a separate bug report for this.
Attached patch Enable MIPS support (obsolete) (deleted) — Splinter Review
Comment on attachment 388588 [details] [diff] [review] Enable MIPS support Looks good. Since MMGC_MIPS and AVMPLUS_MIPS are not used in any of the code at present, there should be no need to introduce them in avmfeatures.as.
("Build Config" component is more for the test harness etc; makefiles and configuration are part of the vm component.)
Status: UNCONFIRMED → NEW
Component: Build Config → Virtual Machine
Ever confirmed: true
QA Contact: build-config → vm
2: AVMPLUS_MIPS was being used in platform/unix/unixcpuid.cpp to select an implementation of P4Available() that didn't involve x86 assembler. The only user of P4Available() is shell/ShellCore.cpp iff JIT && (IA32 || AMD64), so there is really no need for P4Available() to be defined at all. I've split this patch into 2 parts. Part 1 restricts P4Available() so that it is only defined and used for targets that need it (IA32 & AMD64). Part 2 contains the MIPS changes without MMGC_MIPS and AVMPLUS_MIPS defined. Strictly speaking Part1 is no longer part of the MIPS target support. If you prefer, I could file the P4Available change as a separate bug and make this one dependent on it?
separate bug isn't required, you can just attach the separate patches here
Attached patch Enable MIPS support (obsolete) (deleted) — Splinter Review
Removed unused legacy definitions
Attachment #388588 - Attachment is obsolete: true
Attachment #388961 - Flags: review?(rreitmai)
Attachment #388962 - Flags: review?(rreitmai)
Comment on attachment 388961 [details] [diff] [review] Enable MIPS support +'ing but should runtests.py use lower case - 'mips' not 'MIPS'
Attachment #388961 - Flags: review?(rreitmai) → review+
Attachment #388962 - Flags: review?(rreitmai) → review+
(In reply to comment #8) > (From update of attachment 388961 [details] [diff] [review]) > +'ing but should runtests.py use lower case - 'mips' not 'MIPS' file(1) returns MIPS in uppercase for most formats. The exceptions are Mach-O and NetBSD. I can change this to elif re.search('(mips)', f.lower()): to catch those too. Incidentally I noticed a problem with the way this detection works. You can get a false positive from the file/directory name which file(1) prints out by default. Eg passing --avm=/opt/appcore/tamarin-redux/objdir-mips/shell/avmshell would convince the script it was running on ppc(!). Modern versions of file(1) have a -b option to suppress the filename output, but for portability there should be something like this in runtestBase.py: f = ' '.join(f) + f = f.replace(self.avm, ''); self.verbose_print('determineConfig: %s' % f)
Tracking update for config detection in runtests.py in comment #9 as bug #512934
Rick, you approved a couple of these patches, are they ready to land? Can you land them if you have time?
ok, will look into landing them next week.
Assignee: nobody → rreitmai
Priority: -- → P3
Target Milestone: --- → flash10.1
Attached patch Update: Enable MIPS support (obsolete) (deleted) — Splinter Review
Ignore case when checking binary type. Refreshed to head of tree
Attached patch MIPS JIT (obsolete) (deleted) — Splinter Review
Basic JIT support for MIPS
Attachment #388961 - Attachment is obsolete: true
Priority: P3 → P4
Raising priority for the MIPS implementation to a must-have for 10.1. Rick is on the hook for review in the next week or so.
Flags: flashplayer-qrb+
Priority: P4 → P2
Attachment #404635 - Flags: review?(rreitmai)
The patch looks good so far (I'll do a more extensive review tomorrow or early next week), but I assume that we don't want to land it unless we have a h/w platform as part of our nightly builds or at least until we can run it on something.
Depends on: 520080
Attached patch Update: recent codeAlloc changes (obsolete) (deleted) — Splinter Review
Attachment #405736 - Flags: review?(rreitmai)
Attached patch Update: placeholder for asm_inc_m32 (obsolete) (deleted) — Splinter Review
Attachment #405737 - Flags: review?(rreitmai)
Attachment #405737 - Flags: review?(rreitmai) → review+
Attachment #405736 - Flags: review?(rreitmai) → review+
Comment on attachment 388962 [details] [diff] [review] P4Available is only needed for IA32 and AMD64 JIT builds P4Avail change can be landed now.
Attachment #388962 - Flags: superreview?(edwsmith)
Attachment #388962 - Flags: superreview?(edwsmith) → superreview+
Attachment #404632 - Flags: review?(brbaker)
Comment on attachment 404632 [details] [diff] [review] Update: Enable MIPS support Also adding an additional patch to enable: ../configure.py --enable-shell --target=mips-linux
Attachment #404632 - Flags: review?(brbaker) → review+
Attached patch MIPS x-platform compile patch (obsolete) (deleted) — Splinter Review
Apply after Attachment 404632 [details] [diff], this enables: ../configure.py --enable-shell --target=mips-linux Need to either link gcc, g++, ar to MIPS tool chain or export the following CXX=mips-linux-gnu-g++ CC=mips-linux-gnu-gcc LD=mips-linux-gnu-ar AR=mips-linux-gnu-ar There is a hack involved with the current patch to enable the same switch combination as gcc 4.2 since there are currently too many errors to be able to compile with -Werror
Attachment #409763 - Flags: review?(rreitmai)
Blocks: 524129
Attachment #409763 - Flags: review?(rreitmai) → review+
No longer depends on: 520080
Attached patch Add MIPS target to configuration/build system (obsolete) (deleted) — Splinter Review
This patch does not include a JIT. Use the --enable-wordcode-interp option when configuring.
Attachment #404632 - Attachment is obsolete: true
Attachment #410410 - Flags: review?(rreitmai)
Attached patch Nanojit support for MIPS (obsolete) (deleted) — Splinter Review
Nanojit support for MIPS. This patch only modifies the nanojit directory It combines all of the previous JIT related patches and adds the following: updated to head of tree (2977:4c213156e2d5) bugfix for passing mixed float/integer arguments suppressed unused argument warnings cosmetic (whitespace & stylistic) changes It does not enable JIT support
Attachment #404635 - Attachment is obsolete: true
Attachment #405736 - Attachment is obsolete: true
Attachment #405737 - Attachment is obsolete: true
Attachment #410414 - Flags: review?(rreitmai)
Attachment #404635 - Flags: review?(rreitmai)
Attached patch Build nanojit for MIPS targets (deleted) — Splinter Review
This patch enables nanojit support for MIPS
Attachment #410416 - Flags: review?(rreitmai)
(In reply to comment #21) > There is a hack involved with the current patch to enable the same switch > combination as gcc 4.2 since there are currently too many errors to be able to > compile with -Werror I've posted a bugreport for some alignment warnings https://bugzilla.mozilla.org/show_bug.cgi?id=526666 With these patches in place the code compiles cleanly with the CS gcc-4.3-154 compiler
Depends on: 526666
Comment on attachment 410414 [details] [diff] [review] Nanojit support for MIPS asking graydon to review the NJ specific for landing in nanojit-central
Attachment #410414 - Flags: review?(graydon)
Comment on attachment 409763 [details] [diff] [review] MIPS x-platform compile patch Looks good to me...asking Brent to review (and land?) configure patches.
Attachment #409763 - Flags: review?(brbaker)
Attachment #410416 - Flags: review?(rreitmai)
Attachment #410416 - Flags: review?(brbaker)
Attachment #410416 - Flags: review+
Comment on attachment 409763 [details] [diff] [review] MIPS x-platform compile patch Pushed changeset: 3007:c35559a0d468
Attachment #409763 - Attachment is obsolete: true
Attachment #409763 - Flags: review?(brbaker)
Comment on attachment 410410 [details] [diff] [review] Add MIPS target to configuration/build system Pushed changeset: 3007:c35559a0d468
Attachment #410410 - Flags: review?(rreitmai) → review+
Attachment #410410 - Attachment is obsolete: true
Comment on attachment 410416 [details] [diff] [review] Build nanojit for MIPS targets Pushed changeset: 3007:c35559a0d468
Attachment #410416 - Flags: review?(brbaker) → review+
Attachment #410414 - Flags: review?(rreitmai) → review+
Comment on attachment 410414 [details] [diff] [review] Nanojit support for MIPS Brief comments: - There's no support here for configure.in to notice we're targeting MIPS and define AVMPLUS_MIPS and NANOJIT_ARCH=MIPS; patch-as-proposed won't build out of nanojit-central. - Predicate on AVMPLUS_MIPS in nanojit.h, not AVMSYSTEM_PLUS. - As with all the other mode-selections, hard-wiring to LE/BE mode with CPP symbols seems undesirable. Would prefer a runtime flag. - The const flags (fpu, cmov, etc.) should similarly be runtime flags in the AVM config. Generally we want to make it possible to build 1 binary for a given arch that runs on a reasonable set of variants by auto-detecting capabilities during startup. - Hard tabs! Verboten in nanojit. Spaces only, 4-space indent. Otherwise, well, I don't really know MIPS assembly at all; it looks a bit incomplete and/or broken in some logic, but it's not like we have anything better at the moment. Good first step! May as well land once the above are fixed.
Attachment #410414 - Flags: review?(graydon) → review+
(In reply to comment #32) > (From update of attachment 410414 [details] [diff] [review]) > Brief comments: > > - There's no support here for configure.in to notice we're targeting MIPS and > define AVMPLUS_MIPS and NANOJIT_ARCH=MIPS; patch-as-proposed won't build out of > nanojit-central. The final patch was submitted just as the changeover was happening. I've checked out the nanojit-central tree. Should I resubmit the JIT patch against that tree? > - Predicate on AVMPLUS_MIPS in nanojit.h, not AVMSYSTEM_PLUS. OK. The AVMPLUS_{cpu} flags are marked as legacy names in core/avmfeatures.as so I assumed they were deprecated. > - As with all the other mode-selections, hard-wiring to LE/BE mode with CPP > symbols seems undesirable. Would prefer a runtime flag. Hmm... I thought that 64 bit values were stored in native format and the JIT needs to use the same ordering so I'm not clear why this decision should be made at runtime? > - The const flags (fpu, cmov, etc.) should similarly be runtime flags in the > AVM config. Generally we want to make it possible to build 1 binary for a given > arch that runs on a reasonable set of variants by auto-detecting capabilities > during startup. I'll define the cpu_has_* flags as variables. I assume it's ok to preinitialise the values based on the compilation flags? The rationale is that compiler is generating code based on the same information so the JIT should use the same defaults. If some external code wants to override those choices it would be free to do so. > - Hard tabs! Verboten in nanojit. Spaces only, 4-space indent. Understood. I'll reformat the code. > > Otherwise, well, I don't really know MIPS assembly at all; it looks a bit > incomplete and/or broken in some logic, but it's not like we have anything > better at the moment. Good first step! May as well land once the above are > fixed. Thanks for taking a look at it. If there are areas you're concerned about I'd like to fix them, so please let me know.
Chris let me know when you have the code ready (or if you need a hand). We'll be able to land without another review.
See also, bug 521215. both implementations appear relatively complete but unrelated code and authors... doh?
(In reply to comment #35) > See also, bug 521215. both implementations appear relatively complete but > unrelated code and authors... doh? correction: see bug 531215
> > - As with all the other mode-selections, hard-wiring to LE/BE mode with CPP > > symbols seems undesirable. Would prefer a runtime flag. > > Hmm... I thought that 64 bit values were stored in native format and the JIT > needs to use the same ordering so I'm not clear why this decision should be > made at runtime? I tend to agree with Chris. runtime detection of optional cpu features or processor revisions is desireable. But, do we need a build of the VM compiled for LE and the JIT generating BE code (or vice versa)? To follow convention, runtime code that tests the setting (with the setting declared as a static const) keeps the generator code ifdef-free while still making it a compile-time choice.
Attached patch Nanojit support for MIPS32 (deleted) — Splinter Review
Updated MIPS backend for nanojit-central/tamarin-redux Follows the NativeARM implementation where possible to ease maintenance
Attachment #410414 - Attachment is obsolete: true
Attached file Add AVMPLUS_MIPS definitions (deleted) —
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: fixed-in-nanojit
Is there a plan for a MIPS machine in the TR buildbot setup? This back-end will break early and often if there isn't one.
Yes, bug 524129 addresses the build scripts, and we have the hardware in house here at Adobe.
Hi Chris, great work! One thing is that regexp-dna will be rather slow without patch below. It is due to loading word from a 2-byte-aligned address, which causes slow exception processing. On Loongson machine, regexp-dna gets 9X faster with patch below. Another way to fix this is probably to explicitly use unaligned load/store. --- a/js/src/jsregexp.cpp Mon Jan 04 15:51:13 2010 +0800 +++ b/js/src/jsregexp.cpp Thu Jan 07 11:50:45 2010 +0800 @@ -2322,7 +2322,7 @@ return pos; } -#if defined(AVMPLUS_ARM) || defined(AVMPLUS_SPARC) +#if defined(AVMPLUS_ARM) || defined(AVMPLUS_SPARC) || defined (AVMPLUS_MIPS) /* We can't do this on ARM or SPARC, since it relies on doing a 32-bit load from * a pointer which is only 2-byte aligned. */
Perhaps we should add a generic flag to nanojit (NJ_UNALIGNED_ACCESS) so we don't have to sniff for specific processors.
Chris, I assume you'll be the goto person for any MIPS issues in the future? Eg. the interface between the backends and the rest of Nanojit changes reasonably often. When I'm the one causing such a change I try to fix all the back-ends appropriately, but for the case where I don't have machines (PPC, Sparc, MIPS) I sometimes cause breakage. We have a goto person at SUN for Sparc. (I admit to not being thrilled at the addition of the MIPS back-end. To me it's just extra maintenance work with no upside. Who will actually be using this back-end?)
(In reply to comment #46) > (I admit to not being thrilled at the addition of the MIPS back-end. To me > it's just extra maintenance work with no upside. Who will actually be using > this back-end?) Supposedly MIPS is heavily used in some classes of consumer devices, like television sets.
(In reply to comment #47) > > Supposedly MIPS is heavily used in some classes of consumer devices, like > television sets. Is Nanojit running on these machines?
IIRC Bloomberg was doing a MIPS nanojit/TM port. Also Sparc, w00t. Nick, don't be busting on MIPS, my favorite RISC architecture! *nostalgic sigh for SGI iron...* ;-) /be
(In reply to comment #48) > Is Nanojit running on these machines? Obviously not, since (up to now) MIPS support wasn't present... :-)
(In reply to comment #48) > (In reply to comment #47) > > > > Supposedly MIPS is heavily used in some classes of consumer devices, like > > television sets. > > Is Nanojit running on these machines? Nanojit is not running on MIPS yet to my knowledge, but variants of the Flash Platform are running on these types of devices: http://www.adobe.com/aboutadobe/pressroom/pressreleases/200904/042009FlashDigitalHome.html
(In reply to comment #46) Yes. I am the tech contact at MIPS. We have an active interest in keeping the backend current. Chris
BTW, we don't marked bugs as RESOLVED until they've been merged into Tamarin-Redux and mozilla-central. http://hg.mozilla.org/tracemonkey/rev/fee1af2ffc8f
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: fixed-in-nanojit → fixed-in-nanojit, fixed-in-tracemonkey
Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey → fixed-in-nanojit, fixed-in-tracemonkey, fixed-in-tamarin
Depends on: 545113
(In reply to comment #46) > > (I admit to not being thrilled at the addition of the MIPS back-end. To me > it's just extra maintenance work with no upside. Who will actually be using > this back-end?) MIPS-based CPUs ship in > 500MU annually. Much of this is in DTV, STB, DVD/BD, and home networking gear. Belated, but thought this question should be answered ;<)
(In reply to comment #55) > > MIPS-based CPUs ship in > 500MU annually. Much of this is in DTV, STB, DVD/BD, > and home networking gear. Belated, but thought this question should be answered > ;<) How many of those will be running Nanojit?
SoCs based on the MIPS architecture account for #1 market share in each of those segments. If you believe Flash player technology migrates into the living room, it's a large number of those shipments, dependent on the adoption rate of Flash and other apps using the JIT.
We don't have test infrastructure for mips, but if there are contributors willing to keep it working, I think we should take a patch. If we add LIR opcodes, it might break. We will need someone to watch out for breakage. Sun does this for SPARC. They run a tinderbox and if something break, Leon comes around and fixes it. On the upside, this is probably the last relevant architecture we have to add =)
Chris Dearman is the MIPS champion responsible for fixing broken builds. MIPS will be added to the build system once Chris addresses some recent breakage.
Status: REOPENED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: