1249493 - IonMonkey: MIPS: Fix crash after enable shared stubs

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[hev [:hev]]

Implement EmitIon* methods in SharedICHelpers for MIPS.

Comment 1

[hev [:hev]]

Attachment: 0001-IonMonkey-MIPS-Move-SharedICHelpers-mips32-64-to-mip.patch

Comment 2

[hev [:hev]]

Attachment: 0002-IonMonkey-MIPS-Fix-crash-after-enable-shared-stubs.patch

Comment 3

[Tooru Fujisawa [:arai]]

Comment on attachment 8721112 [details] [diff] [review]
0001-IonMonkey-MIPS-Move-SharedICHelpers-mips32-64-to-mip.patch

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

Looks good :)

::: js/src/jit/mips-shared/SharedICHelpers-mips-shared.h
@@ +88,5 @@
> +    masm.subPtr(BaselineStackReg, scratch);
> +
> +    // Store frame size without VMFunction arguments for GC marking.
> +    masm.subPtr(Imm32(argSize), scratch);
> +    masm.storePtr(scratch, Address(BaselineFrameReg, BaselineFrame::reverseOffsetOfFrameSize()));

BaselineFrame::frameSize_ is uint32_t, so this should be store32.

::: js/src/jit/mips32/SharedICHelpers-mips32.h
@@ -177,5 @@
> -                                            offsetof(BaselineStubFrame, savedFrame)));
> -    masm.movePtr(BaselineStackReg, BaselineFrameReg);
> -
> -    // We pushed 4 words, so the stack is still aligned to 8 bytes.
> -    masm.checkStackAlignment();

is this intentionally removed in mips-shared?

Comment 4

[hev [:hev]]

(In reply to Tooru Fujisawa [:arai] from comment #3)
> Comment on attachment 8721112 [details] [diff] [review]
> 0001-IonMonkey-MIPS-Move-SharedICHelpers-mips32-64-to-mip.patch
> 
> Review of attachment 8721112 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good :)
> 
> ::: js/src/jit/mips-shared/SharedICHelpers-mips-shared.h
> @@ +88,5 @@
> > +    masm.subPtr(BaselineStackReg, scratch);
> > +
> > +    // Store frame size without VMFunction arguments for GC marking.
> > +    masm.subPtr(Imm32(argSize), scratch);
> > +    masm.storePtr(scratch, Address(BaselineFrameReg, BaselineFrame::reverseOffsetOfFrameSize()));
> 
> BaselineFrame::frameSize_ is uint32_t, so this should be store32.
Thank you.

> 
> ::: js/src/jit/mips32/SharedICHelpers-mips32.h
> @@ -177,5 @@
> > -                                            offsetof(BaselineStubFrame, savedFrame)));
> > -    masm.movePtr(BaselineStackReg, BaselineFrameReg);
> > -
> > -    // We pushed 4 words, so the stack is still aligned to 8 bytes.
> > -    masm.checkStackAlignment();
> 
> is this intentionally removed in mips-shared?
No, It's my fault.

Comment 5

[Hannes Verschore [:h4writer]]

Comment on attachment 8721113 [details] [diff] [review]
0002-IonMonkey-MIPS-Fix-crash-after-enable-shared-stubs.patch

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

Thanks!

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1a35a3779c7f
https://hg.mozilla.org/integration/mozilla-inbound/rev/48d6e2969e6a

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/1a35a3779c7f
https://hg.mozilla.org/mozilla-central/rev/48d6e2969e6a