578245 - JM: Port method JIT to x64

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

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.

Comment 1

[Sean Stangl [:sstangl]]

Attachment: MethodJIT on 64-bit

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

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

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.

Comment 3

[Sean Stangl [:sstangl]]

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.

Comment 4

[Sean Stangl [:sstangl]]

Attachment: Remove nunbox folder.

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"

Comment 5

[Sean Stangl [:sstangl]]

Pushed:

http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/2e002537851b
http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/e1e99d887dd8

Comment 6

[Sean Stangl [:sstangl]]

Attachment: Fix build on OSX.

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.

Comment 7

[Sean Stangl [:sstangl]]

http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/c93d6ce28aee

Comment 8

[Sean Stangl [:sstangl]]

Attachment: Register Allocation Patch

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.

Comment 9

[Sean Stangl [:sstangl]]

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.

Comment 10

[Sean Stangl [:sstangl]]

Attachment: MonoIC support for x86_64.

Implements MonoIC support for x86_64. Gets rid of the push(Address) call by MICs, which is nice.

Comment 11

[Sean Stangl [:sstangl]]

Attachment: MonoIC support for x86_64 (minus debugging cruft).

Same as the above patch, minus some leftover changes from darker times.

Comment 12

[David Anderson [:dvander] - inactive, e-mail if emergency]

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.)

Comment 13

[Sean Stangl [:sstangl]]

I meant it as "the length from the 'load' label, in bytes." Is 'patchValueOffset' a better name?

Comment 14

[David Anderson [:dvander] - inactive, e-mail if emergency]

Yeah.

Comment 15

[Sean Stangl [:sstangl]]

http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/951a3dbd5541

Comment 16

[Sean Stangl [:sstangl]]

Attachment: PolyIC support for x86_64.

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 :).

Comment 17

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 462984 [details] [diff] [review]
PolyIC support for x86_64.

x64 port brings a tear to mine eye. Great work.

Comment 18

[Sean Stangl [:sstangl]]

http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/6347cf00d3ab

Comment 19

[Sean Stangl [:sstangl]]

The port is complete (and on schedule!). Only optimizations remain, which should be tracked separately. Dvander says that he will look into tracejit integration.