Closed Bug 805918 (BaselineARM) Opened 10 years ago Closed 4 years ago

BaselineCompiler: ARM support


(Core :: JavaScript Engine, defect)

Not set





(Reporter: djvj, Assigned: djvj)


(Blocks 1 open bug)



(1 file)

Fix baseline compiler to work on ARM.
Assignee: general → kvijayan
Attached patch Support ARM.Splinter Review
Adds support on ARM.

Outside of the usual additions required to support a new arch, here are the notable changes:

1. We were missing proper use of AutoFlushCache in all of our codegen functions.  This is a general issue, but was discovered when trying to get ARM working, so it's fixed here.

2. The masm.pop(BaselineTailCallReg) emission in the fallback stub compilers has been replaced with a new helper |EmitRestoreTailCallReg|, because on ARM this should be a no-op since the return address is not pushed in the first place.

3. All uses of masm.ret() in stubcode that directly returns (as opposed to returning from a tailcall to a VM Function), is replaced with a new helper |EmitReturnFromIC|, which does the right thing.  Needed because masm.ret() for ARM pops the link register from stack, which we don't want when returning from ICs since we do not push lr in the first place when calling them.
Attachment #682320 - Flags: review?(jdemooij)
Comment on attachment 682320 [details] [diff] [review]
Support ARM.

Review of attachment 682320 [details] [diff] [review]:

Great! Good to see the design is ARM-ready now ;)

::: js/src/ion/BaselineIC.cpp
@@ +92,5 @@
>  ICCompare_Fallback::Compiler::generateStubCode()
>  {
>      MacroAssembler masm;
> +    AutoFlushCache afc("ICCompare_Fallback::Compiler::generateStubCode",
> +                       cx->compartment->ionCompartment());

It would be good to move this to getStubCode, we will have many stubs and it will help minimize boilerplate code.

(Unrelated, we could also pass MacroAssembler &masm to generateStubcode and link it in getStubCode?)

::: js/src/ion/arm/BaselineIC-arm.cpp
@@ +23,5 @@
> +ICCompare_Int32::Compiler::generateStubCode()
> +{
> +    // The condition to test on depends on the opcode
> +    Assembler::Condition cond;
> +    Assembler::Condition notcond;

Nit: you can use Assembler::InvertCondition(cond) below.
Attachment #682320 - Flags: review?(jdemooij) → review+
Blocks: 815155
Depends on: 815700
Depends on: 816015
Depends on: 816661
Depends on: 840162
Depends on: 849909
Depends on: 858083
Apart from Bug 816661 (which looks like an optimization), I guess this is fixed now. Close, then (and move the dependancy up to Bug 805241)?
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.