Bug 805918 (BaselineARM)

BaselineCompiler: ARM support

NEW
Assigned to

Status

()

6 years ago
5 years ago

People

(Reporter: djvj, Assigned: djvj)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Fix baseline compiler to work on ARM.
(Assignee)

Updated

6 years ago
Assignee: general → kvijayan
(Assignee)

Comment 1

6 years ago
Created attachment 682320 [details] [diff] [review]
Support ARM.

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+
(Assignee)

Updated

6 years ago
Blocks: 815155

Updated

6 years ago
Depends on: 815700

Updated

6 years ago
Depends on: 816015

Updated

6 years ago
Depends on: 816661
(Assignee)

Updated

6 years ago
Depends on: 840162
(Assignee)

Updated

6 years ago
Depends on: 849909
(Assignee)

Updated

6 years ago
Depends on: 858083

Comment 4

5 years ago
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)?
You need to log in before you can comment on or make changes to this bug.