Closed Bug 578245 Opened 14 years ago Closed 14 years ago

JM: Port method JIT to x64

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dvander, Assigned: sstangl)

References

Details

Attachments

(6 files, 1 obsolete file)

This is part of our six-week plan to shipping, so very high priority. We're estimating that this will probably take about three weeks, from whenever it's started.
Blocks: JaegerBrowser
No longer blocks: Jaeger
Assignee: general → sstangl
Attached patch MethodJIT on 64-bit (deleted) — Splinter Review
Permits running JM on x86_64, passing all trace-tests and jstests. All fastpaths are shared with existing 32-bit code. The port is not complete; this patch merely establishes a fully-functional baseline. Still to come: - Smart register allocation, requiring changing the way registers are allocated - MethodJIT + TraceJIT support - PICs / MICs
Attachment #459756 - Flags: review?(dvander)
Comment on attachment 459756 [details] [diff] [review] MethodJIT on 64-bit >+#if defined(JS_CPU_X86) or defined(JS_CPU_ARM) > masm.subPtr(ImmIntPtr(1), FrameAddress(offsetof(VMFrame, inlineCallCount))); >+#elif defined (JS_CPU_X64) >+ /* Register is clobbered later, so it's safe to use. */ >+ masm.loadPtr(FrameAddress(offsetof(VMFrame, inlineCallCount)), JSReturnReg_Data); >+ masm.subPtr(ImmIntPtr(1), JSReturnReg_Data); >+ masm.storePtr(JSReturnReg_Data, FrameAddress(offsetof(VMFrame, inlineCallCount))); >+#endif Would be nice if we could abstract this a little better. If it helps, I can't remember any reason inlineCallCount is ptr-sized. It's guaranteed not to overflow anyway. You can either add to it as if it were 32-bit, or just shrink it in VMFrame (and pad to make stack happy). >diff --git a/js/src/methodjit/nunbox/Assembler64.h b/js/src/methodjit/nunbox/Assembler64.h Are we calling our boxing scheme on x64 "nunboxing"? For consistency we should call this file "AssemblerX64.h" Another thought: There's a lot of #ifdef X64 in the FastOps. That's fine as we bootstrap the port. Are we going to try and separate or abstract these cases later? Awesome work so far.
Attachment #459756 - Flags: review?(dvander) → review+
inlineCallCount will be changed to uint32 in a subsequent patch, since that touches files shared with tracejit. I'll ask Luke about what name to use for 64-bit boxing. The #ifdefs in FastOps.cpp will go away by abstraction, but I wanted to leave that for the register allocation patch.
Attached patch Remove nunbox folder. (deleted) — Splinter Review
Removes the methodjit/nunbox folder, dumping contents into methodjit/. 32-bit boxing format is known as "nunbox". 64-bit boxing format is known as "punbox". Assembler.h is therefore renamed "NunboxAssembler.h"; Assembler64.h is therefore renamed "PunboxAssembler.h"
Attachment #459940 - Flags: review?(dvander)
Attachment #459940 - Flags: review?(dvander) → review+
Attached patch Fix build on OSX. (deleted) — Splinter Review
The assembler contains a special-case for OSX to use a different system allocation policy. Since we use the standard policy on all platforms, the x86_64 port failed to build. This patch causes OSX x86_64 to use the regular policy.
Attachment #459957 - Flags: review?(dvander)
Attachment #459957 - Flags: review?(dvander) → review+
Attached patch Register Allocation Patch (deleted) — Splinter Review
I implemented changes to the x86_64 register allocation policy that permit register-to-register moves instead of memory-to-register moves on guards in certain circumstances. As it turns out, this intricate cleverness only results in a ~3ms performance boost on SunSpider; differences on v8-v4 are lost in the noise. Since this patch pokes the fragile register allocator and does not result in a meaningful speedup, it will be abandoned to work on PICs. We might try making stores faster, but the difference will also likely be negligible.
x86_64 MIC support is finished; PIC support is coming along nicely. I am temporarily switching contexts to fix a number of potential PIC issues on x86 that surfaced while porting. The port must then be rebased on top of these changes. As an extra bonus, PICs will soon assert the correctness of their patching.
Attached patch MonoIC support for x86_64. (obsolete) (deleted) — Splinter Review
Implements MonoIC support for x86_64. Gets rid of the push(Address) call by MICs, which is nice.
Attachment #461875 - Flags: review?(dvander)
Same as the above patch, minus some leftover changes from darker times.
Attachment #461875 - Attachment is obsolete: true
Attachment #461876 - Flags: review?(dvander)
Attachment #461875 - Flags: review?(dvander)
Comment on attachment 461876 [details] [diff] [review] MonoIC support for x86_64 (minus debugging cruft). Very nice. One question: what does the phrase "loadByteLen" mean? (I can see what it does from the code, but the name threw me off.)
Attachment #461876 - Flags: review?(dvander) → review+
I meant it as "the length from the 'load' label, in bytes." Is 'patchValueOffset' a better name?
Attached patch PolyIC support for x86_64. (deleted) — Splinter Review
Implements PolyIC support for x86_64. Locally, this is about a 10% speedup on SunSpider. This patch was surprisingly simple to write once PIC assertions were in place :).
Attachment #462984 - Flags: review?(dvander)
Comment on attachment 462984 [details] [diff] [review] PolyIC support for x86_64. x64 port brings a tear to mine eye. Great work.
Attachment #462984 - Flags: review?(dvander) → review+
The port is complete (and on schedule!). Only optimizations remain, which should be tracked separately. Dvander says that he will look into tracejit integration.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: