Closed Bug 818889 Opened 10 years ago Closed 10 years ago

BaselineCompiler: Add stubs for double arithmetic

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

No description provided.
Attached patch PatchSplinter Review
Attachment #689721 - Flags: review?(kvijayan)
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?
Attachment #689721 - Flags: review?(kvijayan) → review+
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.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.