Closed Bug 670819 Opened 13 years ago Closed 13 years ago

IonMonkey: x86 JIT trampolines

Categories

(Core :: JavaScript Engine, defect)

x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dvander, Assigned: ascheff)

References

Details

Attachments

(1 file, 1 obsolete file)

We need some kind of trampoline to transition from the interpreter to JIT code. The trampolines should be generated upon initializing IonMonkey. This can be done using the existing macro assembler, but the code will look pretty different on each platform, so we're not worrying about abstractions.

The input to the trampoline generator is the following C++ signature:
JSBool EnterIonCode(void *code, int argc, Value *argv, Value *vp);

The output of the trampoline generator is a function pointer conforming to that signature, which when called will execute an Ion-generated code pointer with the provided argument vector. The pattern we're going for on all platforms will look something like:

 (1) Prologue.
 (2) Save all non-volatile registers (these are listed in */Architecture-*.h)
 (3) Set up an ion frame: push argument Values in reverse order.
 (4) Call JIT code.
 (5) Store return register(s) into the return value pointer.
 (6) Restore registers, epilogue.
 (7) Return true (we'll figure out failure later).

Right now an ion frame is very simple and all that's required is to push the arguments in reverse order. I.e. the stack from top to bottom is:
   argv[argc - 1]
   ...
   argv[0]
   argv[-1]
   argv[-2]
   <return addr>
   <local variables>
   ...

(Note that the -1 and -2 arguments are always there, implicitly. argv[-1] contains the "this" object. argv[-2] contains the callee and is always a JSObject*, so you only have to copy the object half, not the type half).
This is a completely untested ion trampoline, just throwing it up here to get some early feedback. I'll try to see if it actually works starting... now.
Attachment #545528 - Flags: review?(dvander)
Comment on attachment 545528 [details] [diff] [review]
First attempt at an Ion trampoline on x86

Review of attachment 545528 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, it looks like this is ready for the next step, I'll write a better comment on that shortly.

::: js/src/ion/Ion.cpp
@@ +230,5 @@
> +    MacroAssembler masm;
> +
> +    // Save old stack frame pointer, set new stack fram pointer.
> +    masm.push(X86Registers::ebp);
> +    masm.move(X86Registers::esp, X86Registers::ebp);

We'll also need to save all non-volatile registers, since JIT code can stomp on them.

@@ +240,5 @@
> +    // eax <- 8*(argc) - 4, eax is now the offset betwen argv and the last
> +    // word of parameter data   --argc is in ebp + 12
> +    masm.load32(Address(X86Registers::ebp, 12), X86Registers::eax);
> +    masm.mul32(Imm32(8), X86Registers::eax, X86Registers::eax);
> +    masm.sub32(Imm32(4), X86Registers::eax);

eax*8 can be eax<<3

@@ +256,5 @@
> +    // Push what eax points to on stack
> +    masm.push(Address(X86Registers::eax, 0));
> +
> +    // eax -= 4 -- decrement eax
> +    masm.sub32(Imm32(4), X86Registers::eax);

This works - it might be clearer to iterate by 8 and push two words each time.

@@ +272,5 @@
> +
> +    // Store returned value in vp   -- vp is in ebp + 20
> +    masm.loadPtr(Address(X86Registers::ebp, 20), X86Registers::eax);
> +    masm.store32(X86Registers::ecx, Address(X86Registers::eax, 0)); // Store type
> +    masm.store32(X86Registers::edx, Address(X86Registers::eax, 4)); // Store data

Looks good.

@@ +278,5 @@
> +    /**************************************************************
> +    Return stack to correct state
> +    **************************************************************/
> +    // Pop all the arguments that we pushed from the stack
> +    masm.add32(Imm32(8*argc), X86Registers::esp);

The argc here must be read back in from [ebp+12].
Attachment #545528 - Flags: review?(dvander) → feedback+
Comment on attachment 545528 [details] [diff] [review]
First attempt at an Ion trampoline on x86

Review of attachment 545528 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ion/Ion.cpp
@@ +240,5 @@
> +    // eax <- 8*(argc) - 4, eax is now the offset betwen argv and the last
> +    // word of parameter data   --argc is in ebp + 12
> +    masm.load32(Address(X86Registers::ebp, 12), X86Registers::eax);
> +    masm.mul32(Imm32(8), X86Registers::eax, X86Registers::eax);
> +    masm.sub32(Imm32(4), X86Registers::eax);

Would an LEA work for this?

@@ +281,5 @@
> +    // Pop all the arguments that we pushed from the stack
> +    masm.add32(Imm32(8*argc), X86Registers::esp);
> +
> +    // Restore old stack frame pointer
> +    masm.pop(X86Registers::ebp);

It would be useful to have something that helps verify the property that ebp/esp are the same on entry/exit.
This patch implements IonCompartments and a trampoline on x86.  It's pretty much untested because I don't really know how to test it.
Attachment #545528 - Attachment is obsolete: true
Attachment #546401 - Flags: review?(dvander)
Comment on attachment 546401 [details] [diff] [review]
Ion compartments and x86 trampoline

Review of attachment 546401 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good - I'll put this in my queue for bug 670816 and check it all in at once.
Attachment #546401 - Flags: review?(dvander) → review+
http://hg.mozilla.org/users/danderson_mozilla.com/ionmonkey/rev/aea4907eb793
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.