1046161 - IonMonkey MIPS: Port latest changes in AsmJSFramIterator to MIPS

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Branislav Rankov [:rankov]]

Latest changes in AsmJSFramIterator.cpp break MIPS build. They need to be ported to MIPS.

Comment 1

[Branislav Rankov [:rankov]]

Attachment: AsmJSFrameIterator.patch

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 8464751 [details] [diff] [review]
AsmJSFrameIterator.patch

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

::: js/src/jit/AsmJSFrameIterator.cpp
@@ +517,5 @@
>              callerFP_ = fp;
>              AssertMatchesCallSite(*module_, codeRange, callerPC_, callerFP_, sp - 2);
>          } else
> +#elif defined(JS_CODEGEN_MIPS)
> +        MOZ_ASSUME_UNREACHABLE("NYI");

ooc, why doesn't the ARM path work for MIPS?

::: js/src/jit/AsmJSModule.cpp
@@ +1610,5 @@
> +#elif defined(JS_CODEGEN_MIPS)
> +        Assembler::WriteLuiOriInstructions(instr, instr->next(),
> +                                           ScratchRegister, (uint32_t)newCallee);
> +        instr[2] = InstReg(op_special, ScratchRegister, zero, ra, ff_jalr);
> +        AutoFlushICache::flush(uintptr_t(instr), 3 * sizeof(uint32_t));

Can you reuse the same AutoFlushICache that ARM has that is declared higher up (and just flushes the entire module's code)?

Comment 3

[Branislav Rankov [:rankov]]

(In reply to Luke Wagner [:luke] from comment #2)
> Comment on attachment 8464751 [details] [diff] [review]
> AsmJSFrameIterator.patch
> 
> Review of attachment 8464751 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/jit/AsmJSFrameIterator.cpp
> @@ +517,5 @@
> >              callerFP_ = fp;
> >              AssertMatchesCallSite(*module_, codeRange, callerPC_, callerFP_, sp - 2);
> >          } else
> > +#elif defined(JS_CODEGEN_MIPS)
> > +        MOZ_ASSUME_UNREACHABLE("NYI");
> 
> ooc, why doesn't the ARM path work for MIPS?

I haven't implemented SingleStepCallback yet. Now that I look at it, it should work. On MIPS 'lr' is called 'ra', but that shouldn't be a problem.

Comment 4

[Branislav Rankov [:rankov]]

Attachment: AsmJSFrameIterator.patch

Comment 5

[Luke Wagner [:luke]]

Comment on attachment 8464817 [details] [diff] [review]
AsmJSFrameIterator.patch

Great!

Comment 6

[Branislav Rankov [:rankov]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6f9baccd7e74

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6f9baccd7e74