Changeset 50595 in webkit for trunk/JavaScriptCore

Timestamp:

Nov 6, 2009, 1:35:03 AM (16 years ago)

Author:

[email protected]

Message:

Tidy up the shift methods on the macro-assembler interface.

Patch by Gavin Barraclough <​[email protected]> on 2009-11-06
Reviewed by Oliver Hunt.

Currently behaviour of shifts of a magnitude > 0x1f is undefined.
Instead defined that all shifts are masked to this range. This makes a lot of
practical sense, both since having undefined behaviour is not particularly
desirable, and because this behaviour is commonly required (particularly since
it is required bt ECMA-262 for shifts).

Update the ARM assemblers to provide this behaviour. Remove (now) redundant
masks from JITArithmetic, and remove rshiftPtr (this was used in case that
could be rewritten in a simpler form using rshift32, only optimized JSVALUE32
on x86-64, which uses JSVALUE64!)

• assembler/MacroAssembler.h:
• assembler/MacroAssemblerARM.h:

(JSC::MacroAssemblerARM::lshift32):
(JSC::MacroAssemblerARM::rshift32):

• assembler/MacroAssemblerARMv7.h:

(JSC::MacroAssemblerARMv7::lshift32):
(JSC::MacroAssemblerARMv7::rshift32):

• assembler/MacroAssemblerX86_64.h:
• jit/JITArithmetic.cpp:

(JSC::JIT::emit_op_lshift):
(JSC::JIT::emit_op_rshift):

Location:

trunk/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• assembler/MacroAssembler.h (modified) (1 diff)
• assembler/MacroAssemblerARM.h (modified) (2 diffs)
• assembler/MacroAssemblerARMv7.h (modified) (2 diffs)
• assembler/MacroAssemblerX86Common.h (modified) (1 diff)
• assembler/MacroAssemblerX86_64.h (modified) (1 diff)
• jit/JITArithmetic.cpp (modified) (2 diffs)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2009-11-06  Gavin Barraclough  <[email protected]>
+
+        Reviewed by Oliver Hunt.
+
+        Tidy up the shift methods on the macro-assembler interface.
+
+        Currently behaviour of shifts of a magnitude > 0x1f is undefined.
+        Instead defined that all shifts are masked to this range.  This makes a lot of
+        practical sense, both since having undefined behaviour is not particularly
+        desirable, and because this behaviour is commonly required (particularly since
+        it is required bt ECMA-262 for shifts).
+
+        Update the ARM assemblers to provide this behaviour.  Remove (now) redundant
+        masks from JITArithmetic, and remove rshiftPtr (this was used in case that
+        could be rewritten in a simpler form using rshift32, only optimized JSVALUE32
+        on x86-64, which uses JSVALUE64!)
+
+        * assembler/MacroAssembler.h:
+        * assembler/MacroAssemblerARM.h:
+        (JSC::MacroAssemblerARM::lshift32):
+        (JSC::MacroAssemblerARM::rshift32):
+        * assembler/MacroAssemblerARMv7.h:
+        (JSC::MacroAssemblerARMv7::lshift32):
+        (JSC::MacroAssemblerARMv7::rshift32):
+        * assembler/MacroAssemblerX86_64.h:
+        * jit/JITArithmetic.cpp:
+        (JSC::JIT::emit_op_lshift):
+        (JSC::JIT::emit_op_rshift):
+
 2009-11-05  Gavin Barraclough  <[email protected]>
 

trunk/JavaScriptCore/assembler/MacroAssembler.h

@@ -180 +180 @@
     }
 
-    void rshiftPtr(RegisterID shift_amount, RegisterID dest)
-    {
-        rshift32(shift_amount, dest);
-    }
-
-    void rshiftPtr(Imm32 imm, RegisterID dest)
-    {
-        rshift32(imm, dest);
-    }
-
     void subPtr(RegisterID src, RegisterID dest)
     {

trunk/JavaScriptCore/assembler/MacroAssemblerARM.h

@@ -119 +119 @@
     }
 
+    void lshift32(RegisterID shift_amount, RegisterID dest)
+    {
+        ARMWord w = m_assembler.getImm(0x1f, ARMRegisters::S0, true);
+        ASSERT(!(w & ARMAssembler::OP2_INV_IMM));
+        m_assembler.ands_r(ARMRegisters::S0, shift_amount, w);
+
+        m_assembler.movs_r(dest, m_assembler.lsl_r(dest, ARMRegisters::S0));
+    }
+
     void lshift32(Imm32 imm, RegisterID dest)
     {
         m_assembler.movs_r(dest, m_assembler.lsl(dest, imm.m_value & 0x1f));
-    }
-
-    void lshift32(RegisterID shift_amount, RegisterID dest)
-    {
-        m_assembler.movs_r(dest, m_assembler.lsl_r(dest, shift_amount));
     }
 
@@ -161 +165 @@
     void rshift32(RegisterID shift_amount, RegisterID dest)
     {
-        m_assembler.movs_r(dest, m_assembler.asr_r(dest, shift_amount));
+        ARMWord w = m_assembler.getImm(0x1f, ARMRegisters::S0, true);
+        ASSERT(!(w & ARMAssembler::OP2_INV_IMM));
+        m_assembler.ands_r(ARMRegisters::S0, shift_amount, w);
+
+        m_assembler.movs_r(dest, m_assembler.asr_r(dest, ARMRegisters::S0));
     }
 

trunk/JavaScriptCore/assembler/MacroAssemblerARMv7.h

@@ -198 +198 @@
     }
 
+    void lshift32(RegisterID shift_amount, RegisterID dest)
+    {
+        // Clamp the shift to the range 0..31
+        ARMThumbImmediate armImm = ARMThumbImmediate::makeEncodedImm(0x1f);
+        ASSERT(armImm.isValid());
+        m_assembler.ARM_and(dataTempRegister, shift_amount, armImm);
+
+        m_assembler.lsl(dest, dest, dataTempRegister);
+    }
+
     void lshift32(Imm32 imm, RegisterID dest)
     {
-        m_assembler.lsl(dest, dest, imm.m_value);
-    }
-
-    void lshift32(RegisterID shift_amount, RegisterID dest)
-    {
-        m_assembler.lsl(dest, dest, shift_amount);
+        m_assembler.lsl(dest, dest, imm.m_value & 0x1f);
     }
 
@@ -242 +247 @@
     void rshift32(RegisterID shift_amount, RegisterID dest)
     {
-        m_assembler.asr(dest, dest, shift_amount);
+        // Clamp the shift to the range 0..31
+        ARMThumbImmediate armImm = ARMThumbImmediate::makeEncodedImm(0x1f);
+        ASSERT(armImm.isValid());
+        m_assembler.ARM_and(dataTempRegister, shift_amount, armImm);
+
+        m_assembler.asr(dest, dest, dataTempRegister);
     }
 
     void rshift32(Imm32 imm, RegisterID dest)
     {
-        m_assembler.asr(dest, dest, imm.m_value);
+        m_assembler.asr(dest, dest, imm.m_value & 0x1f);
     }
 

trunk/JavaScriptCore/assembler/MacroAssemblerX86Common.h

@@ -559 +559 @@
     void swap(RegisterID reg1, RegisterID reg2)
     {
-        m_assembler.xchgq_rr(reg1, reg2);
+        if (reg1 != reg2)
+            m_assembler.xchgq_rr(reg1, reg2);
     }
 

trunk/JavaScriptCore/assembler/MacroAssemblerX86_64.h

@@ -193 +193 @@
     }
 
-    void rshiftPtr(RegisterID shift_amount, RegisterID dest)
-    {
-        // On x86 we can only shift by ecx; if asked to shift by another register we'll
-        // need rejig the shift amount into ecx first, and restore the registers afterwards.
-        if (shift_amount != X86Registers::ecx) {
-            swap(shift_amount, X86Registers::ecx);
-
-            // E.g. transform "shll %eax, %eax" -> "xchgl %eax, %ecx; shll %ecx, %ecx; xchgl %eax, %ecx"
-            if (dest == shift_amount)
-                m_assembler.sarq_CLr(X86Registers::ecx);
-            // E.g. transform "shll %eax, %ecx" -> "xchgl %eax, %ecx; shll %ecx, %eax; xchgl %eax, %ecx"
-            else if (dest == X86Registers::ecx)
-                m_assembler.sarq_CLr(shift_amount);
-            // E.g. transform "shll %eax, %ebx" -> "xchgl %eax, %ecx; shll %ecx, %ebx; xchgl %eax, %ecx"
-            else
-                m_assembler.sarq_CLr(dest);
-        
-            swap(shift_amount, X86Registers::ecx);
-        } else
-            m_assembler.sarq_CLr(dest);
-    }
-
-    void rshiftPtr(Imm32 imm, RegisterID dest)
-    {
-        m_assembler.sarq_i8r(imm.m_value, dest);
-    }
-
     void subPtr(RegisterID src, RegisterID dest)
     {

trunk/JavaScriptCore/jit/JITArithmetic.cpp

@@ -1144 +1144 @@
     emitFastArithImmToInt(regT0);
     emitFastArithImmToInt(regT2);
-#if !PLATFORM(X86)
-    // Mask with 0x1f as per ecma-262 11.7.2 step 7.
-    // On 32-bit x86 this is not necessary, since the shift anount is implicitly masked in the instruction.
-    and32(Imm32(0x1f), regT2);
-#endif
     lshift32(regT2, regT0);
-#if !USE(JSVALUE64)
+#if USE(JSVALUE32)
     addSlowCase(branchAdd32(Overflow, regT0, regT0));
     signExtend32ToPtr(regT0, regT0);
@@ -1226 +1221 @@
         }
         emitFastArithImmToInt(regT2);
-#if !PLATFORM(X86)
-        // Mask with 0x1f as per ecma-262 11.7.2 step 7.
-        // On 32-bit x86 this is not necessary, since the shift anount is implicitly masked in the instruction.
-        and32(Imm32(0x1f), regT2);
-#endif
-#if USE(JSVALUE64)
         rshift32(regT2, regT0);
-#else
-        rshiftPtr(regT2, regT0);
+#if USE(JSVALUE32)
+        signExtend32ToPtr(regT0, regT0);
 #endif
     }