[SharedStubs] Get arm working

RESOLVED FIXED in Firefox 43

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: h4writer, Assigned: h4writer)

Tracking

unspecified
mozilla43
Points:
---

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(2 attachments)

Comment hidden (empty)
(Assignee)

Updated

3 years ago
Blocks: 1161516
(Assignee)

Comment 1

3 years ago
Created attachment 8657077 [details] [diff] [review]
Part 1: Get tailCallVM working in shared stub embedded in ion
Assignee: nobody → hv1989
Attachment #8657077 - Flags: review?(sstangl)
(Assignee)

Comment 2

3 years ago
Created attachment 8657086 [details] [diff] [review]
Part 2: get callVM to work on ARM
Attachment #8657086 - Flags: review?(sstangl)
Comment on attachment 8657077 [details] [diff] [review]
Part 1: Get tailCallVM working in shared stub embedded in ion

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

::: js/src/jit/CodeGenerator.cpp
@@ +1711,5 @@
>      // Fix up upon return.
>      uint32_t callOffset = masm.currentOffset();
>      masm.freeStack(sizeof(intptr_t));
> +#ifdef JS_USE_LINK_REGISTER
> +    masm.freeStack(sizeof(intptr_t));

Merge with above freeStack():

#ifdef JS_USE_LINK_REGISTER
masm.freeStack(sizeof(intptr_t) * 2);
#else
masm.freeStack(sizeof(intptr_t));
#endif

::: js/src/jit/arm/SharedICHelpers-arm.h
@@ +107,5 @@
>  
>  inline void
>  EmitIonTailCallVM(JitCode* target, MacroAssembler& masm, uint32_t stackSize)
>  {
> +    masm.loadPtr(Address(sp, stackSize), r0);

Note that r0 is safe to use here as scratch because r0 == ReturnReg, and it is already assumed that the CallVM is going to clobber ReturnReg. Or you could use a ScratchRegisterScope for everything but the masm.branch().

@@ +108,5 @@
>  inline void
>  EmitIonTailCallVM(JitCode* target, MacroAssembler& masm, uint32_t stackSize)
>  {
> +    masm.loadPtr(Address(sp, stackSize), r0);
> +    masm.ma_lsr(Imm32(FRAMESIZE_SHIFT), r0, r0);

Prefer masm.rshiftPtr(Imm32(...), r0);

@@ +109,5 @@
>  EmitIonTailCallVM(JitCode* target, MacroAssembler& masm, uint32_t stackSize)
>  {
> +    masm.loadPtr(Address(sp, stackSize), r0);
> +    masm.ma_lsr(Imm32(FRAMESIZE_SHIFT), r0, r0);
> +    masm.ma_add(Imm32(stackSize + JitStubFrameLayout::Size() - sizeof(intptr_t)), r0);

Prefer masm.add32(...);
Attachment #8657077 - Flags: review?(sstangl) → review+
Comment on attachment 8657086 [details] [diff] [review]
Part 2: get callVM to work on ARM

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

::: js/src/jit/arm/SharedICHelpers-arm.h
@@ +150,5 @@
> +    masm.Push(Imm32(descriptor));
> +    masm.callJit(target);
> +
> +    // Remove rest of the frame left on the stack. We remove the return address
> +    // which is implicitly poped when returning.

ultra-nit: "popped"
Attachment #8657086 - Flags: review?(sstangl) → review+
https://hg.mozilla.org/mozilla-central/rev/a1255d7b2c11
https://hg.mozilla.org/mozilla-central/rev/0e15a964f1f9
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.