[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373

barraclough at apple.com barraclough at apple.com
Wed Apr 7 23:14:04 UTC 2010


The following commit has been merged in the webkit-1.2 branch:
commit a24f02b9b7e98131a067f8e79d3cd3ec00cb24ba
Author: barraclough at apple.com <barraclough at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Thu Oct 29 01:40:09 2009 +0000

    JSC JIT on ARMv7 cannot link jumps >16Mb range
    https://bugs.webkit.org/show_bug.cgi?id=30891
    
    Patch by Gavin Barraclough <barraclough at apple.com> on 2009-10-28
    Reviewed by Oliver Hunt.
    
    Start planing all relative jumps as move-32-bit-immediate-to-register-BX.
    In the cases where the jump would fall within a relative jump range, use a relative jump.
    
    * JavaScriptCore.xcodeproj/project.pbxproj:
    * assembler/ARMv7Assembler.h:
    (JSC::ARMv7Assembler::~ARMv7Assembler):
    (JSC::ARMv7Assembler::LinkRecord::LinkRecord):
    (JSC::ARMv7Assembler::):
    (JSC::ARMv7Assembler::executableCopy):
    (JSC::ARMv7Assembler::linkJump):
    (JSC::ARMv7Assembler::relinkJump):
    (JSC::ARMv7Assembler::setInt32):
    (JSC::ARMv7Assembler::isB):
    (JSC::ARMv7Assembler::isBX):
    (JSC::ARMv7Assembler::isMOV_imm_T3):
    (JSC::ARMv7Assembler::isMOVT):
    (JSC::ARMv7Assembler::isNOP_T1):
    (JSC::ARMv7Assembler::isNOP_T2):
    (JSC::ARMv7Assembler::linkJumpAbsolute):
    (JSC::ARMv7Assembler::twoWordOp5i6Imm4Reg4EncodedImmFirst):
    (JSC::ARMv7Assembler::twoWordOp5i6Imm4Reg4EncodedImmSecond):
    (JSC::ARMv7Assembler::ARMInstructionFormatter::twoWordOp5i6Imm4Reg4EncodedImm):
    * assembler/MacroAssemblerARMv7.h:
    (JSC::MacroAssemblerARMv7::makeJump):
    (JSC::MacroAssemblerARMv7::makeBranch):
    * jit/JIT.h:
    * wtf/Platform.h:
    
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@50255 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 0ddae8d..fb09372 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,38 @@
+2009-10-28  Gavin Barraclough  <barraclough at apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        JSC JIT on ARMv7 cannot link jumps >16Mb range
+        https://bugs.webkit.org/show_bug.cgi?id=30891
+
+        Start planing all relative jumps as move-32-bit-immediate-to-register-BX.
+        In the cases where the jump would fall within a relative jump range, use a relative jump.
+
+        * JavaScriptCore.xcodeproj/project.pbxproj:
+        * assembler/ARMv7Assembler.h:
+        (JSC::ARMv7Assembler::~ARMv7Assembler):
+        (JSC::ARMv7Assembler::LinkRecord::LinkRecord):
+        (JSC::ARMv7Assembler::):
+        (JSC::ARMv7Assembler::executableCopy):
+        (JSC::ARMv7Assembler::linkJump):
+        (JSC::ARMv7Assembler::relinkJump):
+        (JSC::ARMv7Assembler::setInt32):
+        (JSC::ARMv7Assembler::isB):
+        (JSC::ARMv7Assembler::isBX):
+        (JSC::ARMv7Assembler::isMOV_imm_T3):
+        (JSC::ARMv7Assembler::isMOVT):
+        (JSC::ARMv7Assembler::isNOP_T1):
+        (JSC::ARMv7Assembler::isNOP_T2):
+        (JSC::ARMv7Assembler::linkJumpAbsolute):
+        (JSC::ARMv7Assembler::twoWordOp5i6Imm4Reg4EncodedImmFirst):
+        (JSC::ARMv7Assembler::twoWordOp5i6Imm4Reg4EncodedImmSecond):
+        (JSC::ARMv7Assembler::ARMInstructionFormatter::twoWordOp5i6Imm4Reg4EncodedImm):
+        * assembler/MacroAssemblerARMv7.h:
+        (JSC::MacroAssemblerARMv7::makeJump):
+        (JSC::MacroAssemblerARMv7::makeBranch):
+        * jit/JIT.h:
+        * wtf/Platform.h:
+
 2009-10-28  Oliver Hunt  <oliver at apple.com>
 
         Reviewed by Geoff Garen.
diff --git a/JavaScriptCore/assembler/ARMv7Assembler.h b/JavaScriptCore/assembler/ARMv7Assembler.h
index 078de44..02ce2e9 100644
--- a/JavaScriptCore/assembler/ARMv7Assembler.h
+++ b/JavaScriptCore/assembler/ARMv7Assembler.h
@@ -407,6 +407,11 @@ register writeback
 
 class ARMv7Assembler {
 public:
+    ~ARMv7Assembler()
+    {
+        ASSERT(m_jumpsToLink.isEmpty());
+    }
+
     typedef ARMRegisters::RegisterID RegisterID;
     typedef ARMRegisters::FPRegisterID FPRegisterID;
 
@@ -477,6 +482,17 @@ public:
 
 private:
 
+    struct LinkRecord {
+        LinkRecord(intptr_t from, intptr_t to)
+            : from(from)
+            , to(to)
+        {
+        }
+
+        intptr_t from;
+        intptr_t to;
+    };
+
     // ARMv7, Appx-A.6.3
     bool BadReg(RegisterID reg)
     {
@@ -574,6 +590,7 @@ private:
         OP_SUB_SP_imm_T1    = 0xB080,
         OP_BKPT             = 0xBE00,
         OP_IT               = 0xBF00,
+        OP_NOP_T1           = 0xBF00,
     } OpcodeID;
 
     typedef enum {
@@ -608,6 +625,7 @@ private:
         OP_MOV_imm_T3   = 0xF240,
         OP_SUB_imm_T4   = 0xF2A0,
         OP_MOVT         = 0xF2C0,
+        OP_NOP_T2a      = 0xF3AF,
         OP_LDRH_reg_T2  = 0xF830,
         OP_LDRH_imm_T3  = 0xF830,
         OP_STR_imm_T4   = 0xF840,
@@ -626,6 +644,7 @@ private:
 
     typedef enum {
         OP_B_T4b        = 0x9000,
+        OP_NOP_T2b      = 0x8000,
     } OpcodeID2;
 
     struct FourFours {
@@ -1481,6 +1500,15 @@ public:
     void* executableCopy(ExecutablePool* allocator)
     {
         void* copy = m_formatter.executableCopy(allocator);
+
+        unsigned jumpCount = m_jumpsToLink.size();
+        for (unsigned i = 0; i < jumpCount; ++i) {
+            uint16_t* location = reinterpret_cast<uint16_t*>(reinterpret_cast<intptr_t>(copy) + m_jumpsToLink[i].from);
+            uint16_t* target = reinterpret_cast<uint16_t*>(reinterpret_cast<intptr_t>(copy) + m_jumpsToLink[i].to);
+            linkJumpAbsolute(location, target);
+        }
+        m_jumpsToLink.clear();
+
         ASSERT(copy);
         return copy;
     }
@@ -1503,11 +1531,7 @@ public:
     {
         ASSERT(to.m_offset != -1);
         ASSERT(from.m_offset != -1);
-
-        uint16_t* location = reinterpret_cast<uint16_t*>(reinterpret_cast<intptr_t>(m_formatter.data()) + from.m_offset);
-        intptr_t relative = to.m_offset - from.m_offset;
-
-        linkWithOffset(location, relative);
+        m_jumpsToLink.append(LinkRecord(from.m_offset, to.m_offset));
     }
 
     static void linkJump(void* code, JmpSrc from, void* to)
@@ -1515,9 +1539,7 @@ public:
         ASSERT(from.m_offset != -1);
         
         uint16_t* location = reinterpret_cast<uint16_t*>(reinterpret_cast<intptr_t>(code) + from.m_offset);
-        intptr_t relative = reinterpret_cast<intptr_t>(to) - reinterpret_cast<intptr_t>(location);
-
-        linkWithOffset(location, relative);
+        linkJumpAbsolute(location, to);
     }
 
     // bah, this mathod should really be static, since it is used by the LinkBuffer.
@@ -1541,10 +1563,9 @@ public:
         ASSERT(!(reinterpret_cast<intptr_t>(from) & 1));
         ASSERT(!(reinterpret_cast<intptr_t>(to) & 1));
 
-        intptr_t relative = reinterpret_cast<intptr_t>(to) - reinterpret_cast<intptr_t>(from);
-        linkWithOffset(reinterpret_cast<uint16_t*>(from), relative);
+        linkJumpAbsolute(reinterpret_cast<uint16_t*>(from), to);
 
-        ExecutableAllocator::cacheFlush(reinterpret_cast<uint16_t*>(from) - 2, 2 * sizeof(uint16_t));
+        ExecutableAllocator::cacheFlush(reinterpret_cast<uint16_t*>(from) - 5, 5 * sizeof(uint16_t));
     }
     
     static void relinkCall(void* from, void* to)
@@ -1613,14 +1634,14 @@ private:
     static void setInt32(void* code, uint32_t value)
     {
         uint16_t* location = reinterpret_cast<uint16_t*>(code);
+        ASSERT(isMOV_imm_T3(location - 4) && isMOVT(location - 2));
 
-        uint16_t lo16 = value;
-        uint16_t hi16 = value >> 16;
-
-        spliceHi5(location - 4, lo16);
-        spliceLo11(location - 3, lo16);
-        spliceHi5(location - 2, hi16);
-        spliceLo11(location - 1, hi16);
+        ARMThumbImmediate lo16 = ARMThumbImmediate::makeUInt16(static_cast<uint16_t>(value));
+        ARMThumbImmediate hi16 = ARMThumbImmediate::makeUInt16(static_cast<uint16_t>(value >> 16));
+        location[-4] = twoWordOp5i6Imm4Reg4EncodedImmFirst(OP_MOV_imm_T3, lo16);
+        location[-3] = twoWordOp5i6Imm4Reg4EncodedImmSecond((location[-3] >> 8) & 0xf, lo16);
+        location[-2] = twoWordOp5i6Imm4Reg4EncodedImmFirst(OP_MOVT, hi16);
+        location[-1] = twoWordOp5i6Imm4Reg4EncodedImmSecond((location[-1] >> 8) & 0xf, hi16);
 
         ExecutableAllocator::cacheFlush(location - 4, 4 * sizeof(uint16_t));
     }
@@ -1630,41 +1651,89 @@ private:
         setInt32(code, reinterpret_cast<uint32_t>(value));
     }
 
-    // Linking & patching:
-    // This method assumes that the JmpSrc being linked is a T4 b instruction.
-    static void linkWithOffset(uint16_t* instruction, intptr_t relative)
-    {
-        // Currently branches > 16m = mostly deathy.
-        if (((relative << 7) >> 7) != relative) {
-            // FIXME: This CRASH means we cannot turn the JIT on by default on arm-v7.
-            fprintf(stderr, "Error: Cannot link T4b.\n");
-            CRASH();
-        }
-        
-        // ARM encoding for the top two bits below the sign bit is 'peculiar'.
-        if (relative >= 0)
-            relative ^= 0xC00000;
+    static bool isB(void* address)
+    {
+        uint16_t* instruction = static_cast<uint16_t*>(address);
+        return ((instruction[0] & 0xf800) == OP_B_T4a) && ((instruction[1] & 0xd000) == OP_B_T4b);
+    }
+
+    static bool isBX(void* address)
+    {
+        uint16_t* instruction = static_cast<uint16_t*>(address);
+        return (instruction[0] & 0xff87) == OP_BX;
+    }
 
-        // All branch offsets should be an even distance.
-        ASSERT(!(relative & 1));
+    static bool isMOV_imm_T3(void* address)
+    {
+        uint16_t* instruction = static_cast<uint16_t*>(address);
+        return ((instruction[0] & 0xFBF0) == OP_MOV_imm_T3) && ((instruction[1] & 0x8000) == 0);
+    }
 
-        int word1 = ((relative & 0x1000000) >> 14) | ((relative & 0x3ff000) >> 12);
-        int word2 = ((relative & 0x800000) >> 10) | ((relative & 0x400000) >> 11) | ((relative & 0xffe) >> 1);
+    static bool isMOVT(void* address)
+    {
+        uint16_t* instruction = static_cast<uint16_t*>(address);
+        return ((instruction[0] & 0xFBF0) == OP_MOVT) && ((instruction[1] & 0x8000) == 0);
+    }
 
-        instruction[-2] = OP_B_T4a | word1;
-        instruction[-1] = OP_B_T4b | word2;
+    static bool isNOP_T1(void* address)
+    {
+        uint16_t* instruction = static_cast<uint16_t*>(address);
+        return instruction[0] == OP_NOP_T1;
     }
 
-    // These functions can be used to splice 16-bit immediates back into previously generated instructions.
-    static void spliceHi5(uint16_t* where, uint16_t what)
+    static bool isNOP_T2(void* address)
     {
-        uint16_t pattern = (what >> 12) | ((what & 0x0800) >> 1);
-        *where = (*where & 0xFBF0) | pattern;
+        uint16_t* instruction = static_cast<uint16_t*>(address);
+        return (instruction[0] == OP_NOP_T2a) && (instruction[1] == OP_NOP_T2b);
     }
-    static void spliceLo11(uint16_t* where, uint16_t what)
+
+    static void linkJumpAbsolute(uint16_t* instruction, void* target)
     {
-        uint16_t pattern = ((what & 0x0700) << 4) | (what & 0x00FF);
-        *where = (*where & 0x8F00) | pattern;
+        // FIMXE: this should be up in the MacroAssembler layer. :-(
+        const uint16_t JUMP_TEMPORARY_REGISTER = ARMRegisters::ip;
+
+        ASSERT(!(reinterpret_cast<intptr_t>(instruction) & 1));
+        ASSERT(!(reinterpret_cast<intptr_t>(target) & 1));
+
+        ASSERT( (isMOV_imm_T3(instruction - 5) && isMOVT(instruction - 3) && isBX(instruction - 1))
+            || (isNOP_T1(instruction - 5) && isNOP_T2(instruction - 4) && isB(instruction - 2)) );
+
+        intptr_t relative = reinterpret_cast<intptr_t>(target) - (reinterpret_cast<intptr_t>(instruction));
+        if (((relative << 7) >> 7) == relative) {
+            // ARM encoding for the top two bits below the sign bit is 'peculiar'.
+            if (relative >= 0)
+                relative ^= 0xC00000;
+
+            // All branch offsets should be an even distance.
+            ASSERT(!(relative & 1));
+            // There may be a better way to fix this, but right now put the NOPs first, since in the
+            // case of an conditional branch this will be coming after an ITTT predicating *three*
+            // instructions!  Looking backwards to modify the ITTT to an IT is not easy, due to
+            // variable wdith encoding - the previous instruction might *look* like an ITTT but
+            // actually be the second half of a 2-word op.
+            instruction[-5] = OP_NOP_T1;
+            instruction[-4] = OP_NOP_T2a;
+            instruction[-3] = OP_NOP_T2b;
+            instruction[-2] = OP_B_T4a | ((relative & 0x1000000) >> 14) | ((relative & 0x3ff000) >> 12);
+            instruction[-1] = OP_B_T4b | ((relative & 0x800000) >> 10) | ((relative & 0x400000) >> 11) | ((relative & 0xffe) >> 1);
+        } else {
+            ARMThumbImmediate lo16 = ARMThumbImmediate::makeUInt16(static_cast<uint16_t>(reinterpret_cast<uint32_t>(target) + 1));
+            ARMThumbImmediate hi16 = ARMThumbImmediate::makeUInt16(static_cast<uint16_t>(reinterpret_cast<uint32_t>(target) >> 16));
+            instruction[-5] = twoWordOp5i6Imm4Reg4EncodedImmFirst(OP_MOV_imm_T3, lo16);
+            instruction[-4] = twoWordOp5i6Imm4Reg4EncodedImmSecond(JUMP_TEMPORARY_REGISTER, lo16);
+            instruction[-3] = twoWordOp5i6Imm4Reg4EncodedImmFirst(OP_MOVT, hi16);
+            instruction[-2] = twoWordOp5i6Imm4Reg4EncodedImmSecond(JUMP_TEMPORARY_REGISTER, hi16);
+            instruction[-1] = OP_BX | (JUMP_TEMPORARY_REGISTER << 3);
+        }
+    }
+
+    static uint16_t twoWordOp5i6Imm4Reg4EncodedImmFirst(uint16_t op, ARMThumbImmediate imm)
+    {
+        return op | (imm.m_value.i << 10) | imm.m_value.imm4;
+    }
+    static uint16_t twoWordOp5i6Imm4Reg4EncodedImmSecond(uint16_t rd, ARMThumbImmediate imm)
+    {
+        return (imm.m_value.imm3 << 12) | (rd << 8) | imm.m_value.imm8;
     }
 
     class ARMInstructionFormatter {
@@ -1723,8 +1792,11 @@ private:
 
         void twoWordOp5i6Imm4Reg4EncodedImm(OpcodeID1 op, int imm4, RegisterID rd, ARMThumbImmediate imm)
         {
-            m_buffer.putShort(op | (imm.m_value.i << 10) | imm4);
-            m_buffer.putShort((imm.m_value.imm3 << 12) | (rd << 8) | imm.m_value.imm8);
+            ARMThumbImmediate newImm = imm;
+            newImm.m_value.imm4 = imm4;
+
+            m_buffer.putShort(ARMv7Assembler::twoWordOp5i6Imm4Reg4EncodedImmFirst(op, newImm));
+            m_buffer.putShort(ARMv7Assembler::twoWordOp5i6Imm4Reg4EncodedImmSecond(rd, newImm));
         }
 
         void twoWordOp12Reg4Reg4Imm12(OpcodeID1 op, RegisterID reg1, RegisterID reg2, uint16_t imm)
@@ -1749,6 +1821,8 @@ private:
     private:
         AssemblerBuffer m_buffer;
     } m_formatter;
+
+    Vector<LinkRecord> m_jumpsToLink;
 };
 
 } // namespace JSC
diff --git a/JavaScriptCore/assembler/MacroAssemblerARMv7.h b/JavaScriptCore/assembler/MacroAssemblerARMv7.h
index a549604..c479517 100644
--- a/JavaScriptCore/assembler/MacroAssemblerARMv7.h
+++ b/JavaScriptCore/assembler/MacroAssemblerARMv7.h
@@ -990,13 +990,15 @@ public:
 protected:
     ARMv7Assembler::JmpSrc makeJump()
     {
-        return m_assembler.b();
+        moveFixedWidthEncoding(Imm32(0), dataTempRegister);
+        return m_assembler.bx(dataTempRegister);
     }
 
     ARMv7Assembler::JmpSrc makeBranch(ARMv7Assembler::Condition cond)
     {
-        m_assembler.it(cond);
-        return m_assembler.b();
+        m_assembler.it(cond, true, true);
+        moveFixedWidthEncoding(Imm32(0), dataTempRegister);
+        return m_assembler.bx(dataTempRegister);
     }
     ARMv7Assembler::JmpSrc makeBranch(Condition cond) { return makeBranch(armV7Condition(cond)); }
     ARMv7Assembler::JmpSrc makeBranch(DoubleCondition cond) { return makeBranch(armV7Condition(cond)); }
diff --git a/JavaScriptCore/jit/JIT.h b/JavaScriptCore/jit/JIT.h
index 1452c0e..e19ea17 100644
--- a/JavaScriptCore/jit/JIT.h
+++ b/JavaScriptCore/jit/JIT.h
@@ -586,26 +586,26 @@ namespace JSC {
 #elif PLATFORM(ARM_THUMB2)
         // These architecture specific value are used to enable patching - see comment on op_put_by_id.
         static const int patchOffsetPutByIdStructure = 10;
-        static const int patchOffsetPutByIdExternalLoad = 20;
+        static const int patchOffsetPutByIdExternalLoad = 26;
         static const int patchLengthPutByIdExternalLoad = 12;
-        static const int patchOffsetPutByIdPropertyMapOffset = 40;
+        static const int patchOffsetPutByIdPropertyMapOffset = 46;
         // These architecture specific value are used to enable patching - see comment on op_get_by_id.
         static const int patchOffsetGetByIdStructure = 10;
-        static const int patchOffsetGetByIdBranchToSlowCase = 20;
-        static const int patchOffsetGetByIdExternalLoad = 20;
+        static const int patchOffsetGetByIdBranchToSlowCase = 26;
+        static const int patchOffsetGetByIdExternalLoad = 26;
         static const int patchLengthGetByIdExternalLoad = 12;
-        static const int patchOffsetGetByIdPropertyMapOffset = 40;
-        static const int patchOffsetGetByIdPutResult = 44;
+        static const int patchOffsetGetByIdPropertyMapOffset = 46;
+        static const int patchOffsetGetByIdPutResult = 50;
 #if ENABLE(OPCODE_SAMPLING)
         static const int patchOffsetGetByIdSlowCaseCall = 0; // FIMXE
 #else
         static const int patchOffsetGetByIdSlowCaseCall = 28;
 #endif
-        static const int patchOffsetOpCallCompareToJump = 10;
+        static const int patchOffsetOpCallCompareToJump = 16;
 
-        static const int patchOffsetMethodCheckProtoObj = 18;
-        static const int patchOffsetMethodCheckProtoStruct = 28;
-        static const int patchOffsetMethodCheckPutFunction = 46;
+        static const int patchOffsetMethodCheckProtoObj = 24;
+        static const int patchOffsetMethodCheckProtoStruct = 34;
+        static const int patchOffsetMethodCheckPutFunction = 58;
 #elif PLATFORM(ARM_TRADITIONAL)
         // These architecture specific value are used to enable patching - see comment on op_put_by_id.
         static const int patchOffsetPutByIdStructure = 4;
diff --git a/JavaScriptCore/wtf/Platform.h b/JavaScriptCore/wtf/Platform.h
index aa85d0e..7632435 100644
--- a/JavaScriptCore/wtf/Platform.h
+++ b/JavaScriptCore/wtf/Platform.h
@@ -730,8 +730,7 @@ on MinGW. See https://bugs.webkit.org/show_bug.cgi?id=29268 */
     #define ENABLE_JIT 1
     #define WTF_USE_JIT_STUB_ARGUMENT_VA_LIST 1
 #elif PLATFORM(ARM_THUMB2) && PLATFORM(IPHONE)
-    /* Under development, temporarily disabled until 16Mb link range limit in assembler is fixed. */
-    #define ENABLE_JIT 0
+    #define ENABLE_JIT 1
     #define ENABLE_JIT_OPTIMIZE_NATIVE_CALL 0
 /* The JIT is tested & working on x86 Windows */
 #elif PLATFORM(X86) && PLATFORM(WIN)
@@ -797,8 +796,7 @@ on MinGW. See https://bugs.webkit.org/show_bug.cgi?id=29268 */
 /* YARR supports x86 & x86-64, and has been tested on Mac and Windows. */
 #if (PLATFORM(X86) && PLATFORM(MAC)) \
  || (PLATFORM(X86_64) && PLATFORM(MAC)) \
- /* Under development, temporarily disabled until 16Mb link range limit in assembler is fixed. */ \
- || (PLATFORM(ARM_THUMB2) && PLATFORM(IPHONE) && 0) \
+ || (PLATFORM(ARM_THUMB2) && PLATFORM(IPHONE)) \
  || (PLATFORM(X86) && PLATFORM(WIN))
 #define ENABLE_YARR 1
 #define ENABLE_YARR_JIT 1

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list