Closed Bug 1249493 Opened 6 years ago Closed 6 years ago

IonMonkey: MIPS: Fix crash after enable shared stubs

Categories

(Core :: JavaScript Engine: JIT, defect)

Other
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: hev, Assigned: hev)

Details

Attachments

(2 files)

Implement EmitIon* methods in SharedICHelpers for MIPS.
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?
Attachment #8721112 - Flags: review?(arai.unmht) → review+
(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 on attachment 8721113 [details] [diff] [review]
0002-IonMonkey-MIPS-Fix-crash-after-enable-shared-stubs.patch

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

Thanks!
Attachment #8721113 - Flags: review?(hv1989) → review+
https://hg.mozilla.org/mozilla-central/rev/1a35a3779c7f
https://hg.mozilla.org/mozilla-central/rev/48d6e2969e6a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.