643109 - TI+JM: add JSOP_RSH to jsop_bitop

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

JSOP_RSH has its own function because IIRC (?) it had to support double operands. Now that jsop_bitop (used for the other binary bitwise ops) also supports double operands, we can remove the special treatment for JSOP_RSH.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Patch

Removes about 100 lines of code.

Comment 2

[Brian Hackett [Laid off!]]

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

>-RegisterID
>-mjit::Compiler::rightRegForShift(FrameEntry *rhs)
>-{
>-#if defined(JS_CPU_X86) || defined(JS_CPU_X64)
>-    /*
>-     * Gross: RHS _must_ be in ECX, on x86.
>-     * Note that we take this first so that we can't up with other register
>-     * allocations (below) owning ecx before rhs.
>-     */
>-    RegisterID reg = JSC::X86Registers::ecx;
>-    if (!rhs->isConstant())
>-        frame.copyDataIntoReg(rhs, reg);
>-    return reg;
>-#else
>-    if (rhs->isConstant())
>-        return frame.allocReg();
>-    return frame.copyDataIntoReg(rhs);
>-#endif
>-}

Isn't this code still necessary?  How do we ensure the RHS in is in ECX for an 'x >> y' op?

Comment 3

[Jan de Mooij [:jandem]]

(In reply to comment #2)
> Comment on attachment 521579 [details] [diff] [review]
> 
> Isn't this code still necessary?  How do we ensure the RHS in is in ECX for an
> 'x >> y' op?

jsop_bitop calls tempRegInMaskForData(rhs, Registers::maskReg(JSC::X86Registers::ecx)) for all shift ops.

Comment 4

[Brian Hackett [Laid off!]]

http://hg.mozilla.org/projects/jaegermonkey/rev/7bfbc13e500a