818889 - BaselineCompiler: Add stubs for double arithmetic

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Patch

Comment 2

[Kannan Vijayan [:djvj]]

Comment on attachment 689721 [details] [diff] [review]
Patch

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

::: js/src/ion/BaselineIC.cpp
@@ +727,5 @@
> +          case JSOP_MUL:
> +          case JSOP_DIV:
> +          case JSOP_MOD: {
> +            // Unlink int32 stubs, it's faster to always use the double stub.
> +            stub->unlinkStubsWithKind(ICStub::BinaryArith_Int32);

Do we really want to get rid of int32 stubs when we see doubles occurring?

I can accept that it will be faster, but it seems like we may be getting rid of useful information.  In particular if most operations are integer ops.

It seems like there are three kinds of characterizations that are important to distinguish between:
1. Integer inputs leading to integer output.
2. Integer input leading to double output.
3. Double inputs.

::: js/src/ion/arm/BaselineIC-arm.cpp
@@ -120,5 @@
>          masm.passABIArg(R1.payloadReg());
>          masm.callWithABI(JS_FUNC_TO_DATA_PTR(void *, __aeabi_idivmod));
>  
> -        masm.mov(savedLr, lr);
> -

Why is this OK to remove?  Does callWithABI preserve lr?

Comment 3

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/projects/ionmonkey/rev/7ccc81e3c9da

(In reply to Kannan Vijayan [:djvj] from comment #2)
> 
> I can accept that it will be faster, but it seems like we may be getting rid
> of useful information.  In particular if most operations are integer ops.

For e.g. an MSub, Ion only looks at the return type it gets from TI to determine the specialization. So an int32 - int32 -> double will be specialized as double. This seems to work pretty well and we can still provide this information.

> 
> Why is this OK to remove?  Does callWithABI preserve lr?

Yeah, indeed.