The default bug view has changed. See this FAQ.

BaselineCompiler: Fix bitwise ops on arm

RESOLVED FIXED in mozilla23

Status

()

Core
JavaScript Engine
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: evilpie, Assigned: dougc)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla23
ARM
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
+++ This bug was initially created as a clone of Bug #811314 +++

Something is going wrong on arm. And we are missing an optimization for JSOP_URSH.
(Assignee)

Updated

4 years ago
OS: Linux → All
Hardware: x86_64 → ARM
(Assignee)

Updated

4 years ago
Assignee: general → dtc-moz
(Assignee)

Comment 1

4 years ago
Created attachment 739558 [details] [diff] [review]
Optimize BC ARM JSOP_URSH for a double type result.
Attachment #739558 - Flags: review?(jdemooij)
Comment on attachment 739558 [details] [diff] [review]
Optimize BC ARM JSOP_URSH for a double type result.

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

Excellent, thanks! I will fix the nits, push this to Try and then land this today or tomorrow (or let me know if you have commit access and I will let you do it).

::: js/src/ion/arm/BaselineIC-arm.cpp
@@ +182,5 @@
> +            Label toUint;
> +            masm.j(Assembler::LessThan, &toUint);
> +
> +            // Move result and box for return.
> +            masm.tagValue(JSVAL_TYPE_INT32, scratchReg, R0);

Nit: this can be

masm.mov(scratchReg, R0.payloadReg());

because R0.typeReg() should still hold the int32 type tag from the LHS. On x86 we need the tagValue because the rhs operand *must* be in ecx (= R0.typeReg()) there, so we clobber it and use tagValue to restore it afterwards.

@@ +189,5 @@
> +            masm.bind(&toUint);
> +            masm.convertUInt32ToDouble(scratchReg, ScratchFloatReg);
> +            masm.boxDouble(ScratchFloatReg, R0);
> +        }
> +        else {

Nit: } else {
Attachment #739558 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/068fa0fc9064
https://hg.mozilla.org/mozilla-central/rev/068fa0fc9064
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.