[Webkit-unassigned] [Bug 40179] Re-enable JIT_OPTIMIZE_NATIVE_CALL on MIPS

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=40179


Oliver Hunt <oliver at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #57924|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #9 from Oliver Hunt <oliver at apple.com>  2010-06-21 17:48:28 PST ---
(From update of attachment 57924)
Sorry for the delay in review

> +#if CPU(MIPS)
> +    // move the returned address to regT1
> +    move(MIPSRegisters::ra, regT1);
> +#else
>      peek(regT1);
> +#endif
This is bad, we should simply have a loadReturnAddress(RegisterID dst) function to do this, rather than having an architecture check everywhere we need to do this (eg. ARM, PPC, etc would need to have a branch as well)

>      emitPutToCallFrameHeader(regT1, RegisterFile::ReturnPC);
>  
>  #if CPU(X86_64)
> @@ -204,6 +209,31 @@ JIT::Label JIT::privateCompileCTINativeC
>  
>      addPtr(Imm32(16 - sizeof(void*)), stackPointerRegister);
>  
> +#elif CPU(MIPS)
> +    // Calling convention:      f(a0, a1, a2, a3);
> +    // Host function signature: f(ExecState*);
> +
> +    // Allocate stack space for 24 bytes (8-byte aligned)
> +    // 16 bytes (unused) for 4 arguments
> +    // 4 bytes (unused)
> +    // 4 bytes for the returned address 
> +    subPtr(Imm32(24), stackPointerRegister);
> +
> +    // Save the returned address to 20($sp)
> +    storePtr(MIPSRegisters::ra, Address(stackPointerRegister, 20));
> +
> +    // Setup arg0
> +    move(callFrameRegister, MIPSRegisters::a0);
> +
> +    // Call
> +    emitGetFromCallFrameHeaderPtr(RegisterFile::Callee, MIPSRegisters::a2);
> +    loadPtr(Address(MIPSRegisters::a2, OBJECT_OFFSETOF(JSFunction, m_executable)), regT2);
> +    move(regT0, callFrameRegister); // Eagerly restore caller frame register to avoid loading from stack.
> +    call(Address(regT2, executableOffsetToFunction));
> +
> +    // Restore stack space
> +    addPtr(Imm32(24), stackPointerRegister);
> +

Our normal model for a callframe this complicated is to make a struct that describes it and then use OBJECT_OFFSETOF to do the stores

> +#if CPU(MIPS)
> +    loadPtr(Address(stackPointerRegister, -4), MIPSRegisters::ra);
> +#endif
-4? should be -sizeof(void*)?


> +
>      // Return.
>      ret();
>  
>      // Handle an exception
>      exceptionHandler.link(this);
>      // Grab the return address.
> +#if CPU(MIPS)
> +    loadPtr(Address(stackPointerRegister, -4), regT1);
> +#else
>      peek(regT1);
> +#endif

why doesn't peek() work here?

> +#if CPU(MIPS)
> +    poke(callFrameRegister, OBJECT_OFFSETOF(struct JITStackFrame, callFrame) / sizeof(void*));
> +    move(ImmPtr(FunctionPtr(ctiVMThrowTrampoline).value()), MIPSRegisters::ra);
> +#else
>      poke(callFrameRegister, 1 + OBJECT_OFFSETOF(struct JITStackFrame, callFrame) / sizeof (void*));
>      poke(ImmPtr(FunctionPtr(ctiVMThrowTrampoline).value()));
> +#endif
a setReturnAddress function is probably better here

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.