Closed Bug 643109 Opened 9 years ago Closed 9 years ago

TI+JM: add JSOP_RSH to jsop_bitop

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Assignee: general → jandemooij
Status: NEW → ASSIGNED
Attached patch PatchSplinter Review
Removes about 100 lines of code.
Attachment #521579 - Flags: review?(bhackett1024)
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?
(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.
Attachment #521579 - Flags: review?(bhackett1024) → review+
http://hg.mozilla.org/projects/jaegermonkey/rev/7bfbc13e500a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.