Last Comment Bug 814179 - BaselineCompiler: Fix bitwise ops on arm
: BaselineCompiler: Fix bitwise ops on arm
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: ARM All
: -- normal (vote)
: mozilla23
Assigned To: Douglas Crosher [:dougc]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on: 811314
Blocks: BaselineInlineCaches 811766 811767
  Show dependency treegraph
 
Reported: 2012-11-21 13:44 PST by Tom Schuster [:evilpie]
Modified: 2013-04-23 18:23 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Optimize BC ARM JSOP_URSH for a double type result. (2.60 KB, patch)
2013-04-19 05:45 PDT, Douglas Crosher [:dougc]
jdemooij: review+
Details | Diff | Splinter Review

Description Tom Schuster [:evilpie] 2012-11-21 13:44:47 PST
+++ 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.
Comment 1 Douglas Crosher [:dougc] 2013-04-19 05:45:33 PDT
Created attachment 739558 [details] [diff] [review]
Optimize BC ARM JSOP_URSH for a double type result.
Comment 2 Jan de Mooij [:jandem] 2013-04-22 08:42:59 PDT
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 {
Comment 4 Ryan VanderMeulen [:RyanVM] 2013-04-23 18:23:03 PDT
https://hg.mozilla.org/mozilla-central/rev/068fa0fc9064

Note You need to log in before you can comment on or make changes to this bug.