Changeset 69743 in webkit

Timestamp:

Oct 13, 2010 11:45:26 PM (14 years ago)

Author:

commit-queue@webkit.org

Message:

2010-10-13 David Goodwin <​david_goodwin@apple.com>

Reviewed by Oliver Hunt.

ARMv7 JIT should generated conditional branches when possible
​https://bugs.webkit.org/show_bug.cgi?id=47384

Use different jump padding sizes for conditional and unconditional
jumps (12 bytes and 10 bytes respectively). This allows the JIT to
include the IT instruction as part of the conditional jump sequence
which in turn allows it to optimize away the IT using an ARMv7
conditional branch instruction. Use 2-byte B(T1) and 4-byte B(T3) for
conditional branches when displacement is in range. Also use IT/B(T4)
for conditional branch when displacement does not fit in B(T3).

For unconditional jump, instruction selection options are:
B(T2), B(T4), MOVW/MOVT/BX. For conditional jump, instruction selection
options are: B(T1), B(T3), IT/B(T4), ITTT/MOVW/MOVT/BX.

• assembler/ARMv7Assembler.cpp:
• assembler/ARMv7Assembler.h: (JSC::ARMv7Assembler::JmpSrc::JmpSrc): (JSC::ARMv7Assembler::ifThenElse): (JSC::ARMv7Assembler::jumpSizeDelta): (JSC::ARMv7Assembler::canCompact): (JSC::ARMv7Assembler::computeJumpType): (JSC::ARMv7Assembler::link): (JSC::ARMv7Assembler::canBeJumpT1): (JSC::ARMv7Assembler::canBeJumpT3): (JSC::ARMv7Assembler::canBeJumpT4): (JSC::ARMv7Assembler::linkJumpT1): (JSC::ARMv7Assembler::linkJumpT3): (JSC::ARMv7Assembler::linkJumpT4): (JSC::ARMv7Assembler::linkConditionalJumpT4): (JSC::ARMv7Assembler::linkBX): (JSC::ARMv7Assembler::linkConditionalBX): (JSC::ARMv7Assembler::linkJumpAbsolute):
• assembler/LinkBuffer.h: (JSC::LinkBuffer::linkCode):
• assembler/MacroAssemblerARMv7.h: (JSC::MacroAssemblerARMv7::canCompact): (JSC::MacroAssemblerARMv7::computeJumpType): (JSC::MacroAssemblerARMv7::jumpSizeDelta): (JSC::MacroAssemblerARMv7::jump): (JSC::MacroAssemblerARMv7::nearCall): (JSC::MacroAssemblerARMv7::call): (JSC::MacroAssemblerARMv7::ret): (JSC::MacroAssemblerARMv7::tailRecursiveCall): (JSC::MacroAssemblerARMv7::makeJump): (JSC::MacroAssemblerARMv7::makeBranch):

Location:

trunk/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• assembler/ARMv7Assembler.cpp (modified) (1 diff)
• assembler/ARMv7Assembler.h (modified) (20 diffs)
• assembler/LinkBuffer.h (modified) (2 diffs)
• assembler/MacroAssemblerARMv7.h (modified) (11 diffs)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2010-10-13  David Goodwin  <david_goodwin@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        ARMv7 JIT should generated conditional branches when possible
+        https://bugs.webkit.org/show_bug.cgi?id=47384
+
+        Use different jump padding sizes for conditional and unconditional
+        jumps (12 bytes and 10 bytes respectively). This allows the JIT to
+        include the IT instruction as part of the conditional jump sequence
+        which in turn allows it to optimize away the IT using an ARMv7 
+        conditional branch instruction. Use 2-byte B(T1) and 4-byte B(T3) for
+        conditional branches when displacement is in range. Also use IT/B(T4)
+        for conditional branch when displacement does not fit in B(T3).
+
+        For unconditional jump, instruction selection options are:
+        B(T2), B(T4), MOVW/MOVT/BX. For conditional jump, instruction selection
+        options are: B(T1), B(T3), IT/B(T4), ITTT/MOVW/MOVT/BX.
+
+        * assembler/ARMv7Assembler.cpp:
+        * assembler/ARMv7Assembler.h:
+        (JSC::ARMv7Assembler::JmpSrc::JmpSrc):
+        (JSC::ARMv7Assembler::ifThenElse):
+        (JSC::ARMv7Assembler::jumpSizeDelta):
+        (JSC::ARMv7Assembler::canCompact):
+        (JSC::ARMv7Assembler::computeJumpType):
+        (JSC::ARMv7Assembler::link):
+        (JSC::ARMv7Assembler::canBeJumpT1):
+        (JSC::ARMv7Assembler::canBeJumpT3):
+        (JSC::ARMv7Assembler::canBeJumpT4):
+        (JSC::ARMv7Assembler::linkJumpT1):
+        (JSC::ARMv7Assembler::linkJumpT3):
+        (JSC::ARMv7Assembler::linkJumpT4):
+        (JSC::ARMv7Assembler::linkConditionalJumpT4):
+        (JSC::ARMv7Assembler::linkBX):
+        (JSC::ARMv7Assembler::linkConditionalBX):
+        (JSC::ARMv7Assembler::linkJumpAbsolute):
+        * assembler/LinkBuffer.h:
+        (JSC::LinkBuffer::linkCode):
+        * assembler/MacroAssemblerARMv7.h:
+        (JSC::MacroAssemblerARMv7::canCompact):
+        (JSC::MacroAssemblerARMv7::computeJumpType):
+        (JSC::MacroAssemblerARMv7::jumpSizeDelta):
+        (JSC::MacroAssemblerARMv7::jump):
+        (JSC::MacroAssemblerARMv7::nearCall):
+        (JSC::MacroAssemblerARMv7::call):
+        (JSC::MacroAssemblerARMv7::ret):
+        (JSC::MacroAssemblerARMv7::tailRecursiveCall):
+        (JSC::MacroAssemblerARMv7::makeJump):
+        (JSC::MacroAssemblerARMv7::makeBranch):
+
 2010-10-13  Fridrich Strba  <fridrich.strba@bluewin.ch>
 

trunk/JavaScriptCore/assembler/ARMv7Assembler.cpp

@@ -33 +33 @@
 
 const int ARMv7Assembler::JumpSizes[] = { 0xffffffff, sizeof(uint16_t), sizeof(uint16_t),
-    2 * sizeof(uint16_t), 2 * sizeof(uint16_t), 5 * sizeof(uint16_t) };
+    2 * sizeof(uint16_t), 2 * sizeof(uint16_t), 3 * sizeof(uint16_t), 5 * sizeof(uint16_t), 6 * sizeof(uint16_t) };
+const int ARMv7Assembler::JumpPaddingSizes[] = { 0, 5 * sizeof(uint16_t), 6 * sizeof(uint16_t),
+    5 * sizeof(uint16_t), 6 * sizeof(uint16_t) };
 
 }

trunk/JavaScriptCore/assembler/ARMv7Assembler.h

@@ -479 +479 @@
     } Condition;
 
-    enum JumpType { JumpNoCondition, JumpCondition, JumpFullSize };
-    enum JumpLinkType { LinkInvalid, LinkJumpT2, LinkConditionalJumpT2, 
-        LinkJumpT4, LinkConditionalJumpT4, LinkBX, JumpTypeCount };
-    static const int JumpSizes[JumpTypeCount];
-    enum { JumpPaddingSize = 5 * sizeof(uint16_t) };
+    enum JumpType { JumpFixed, JumpNoCondition, JumpCondition, JumpNoConditionFixedSize, JumpConditionFixedSize, JumpTypeCount };
+    enum JumpLinkType { LinkInvalid, LinkJumpT1, LinkJumpT2, LinkJumpT3,
+        LinkJumpT4, LinkConditionalJumpT4, LinkBX, LinkConditionalBX, JumpLinkTypeCount };
+    static const int JumpSizes[JumpLinkTypeCount];
+    static const int JumpPaddingSizes[JumpTypeCount];
     class LinkRecord {
     public:
@@ -504 +504 @@
         intptr_t m_from : 31;
         intptr_t m_to : 31;
-        JumpType m_type : 2;
-        JumpLinkType m_linkType : 3;
+        JumpType m_type : 3;
+        JumpLinkType m_linkType : 4;
         Condition m_condition : 16;
     };
@@ -525 +525 @@
             , m_type(type)
         {
-            ASSERT(m_type != JumpCondition);
+            ASSERT(m_type == JumpFixed || m_type == JumpNoCondition || m_type == JumpNoConditionFixedSize);
         }
 
@@ -533 +533 @@
             , m_type(type)
         {
-            ASSERT(m_type == JumpCondition || m_type == JumpFullSize);
+            ASSERT(m_type == JumpFixed || m_type == JumpCondition || m_type == JumpConditionFixedSize);
         }
 
@@ -630 +630 @@
 
     typedef enum {
+        OP_B_T1         = 0xD000,
         OP_B_T2         = 0xE000,
         OP_AND_reg_T2   = 0xEA00,
@@ -658 +659 @@
         OP_VMOV_IMM_T2  = 0xEEB0,
         OP_VMRS         = 0xEEB0,
+        OP_B_T3a        = 0xF000,
         OP_B_T4a        = 0xF000,
         OP_AND_imm_T1   = 0xF000,
@@ -710 +712 @@
         OP_VSUB_T2b     = 0x0A40,
         OP_NOP_T2b      = 0x8000,
+        OP_B_T3b        = 0x8000,
         OP_B_T4b        = 0x9000,
     } OpcodeID2;
@@ -746 +749 @@
             | (ifThenElseConditionBit(condition, inst4if) << 1)
             | 1;
-        ASSERT((condition != ConditionAL) || (mask & (mask - 1)));
+        ASSERT((condition != ConditionAL) || !(mask & (mask - 1)));
         return (condition << 4) | mask;
     }
@@ -754 +757 @@
             | (ifThenElseConditionBit(condition, inst3if) << 2)
             | 2;
-        ASSERT((condition != ConditionAL) || (mask & (mask - 1)));
+        ASSERT((condition != ConditionAL) || !(mask & (mask - 1)));
         return (condition << 4) | mask;
     }
@@ -761 +764 @@
         int mask = (ifThenElseConditionBit(condition, inst2if) << 3)
             | 4;
-        ASSERT((condition != ConditionAL) || (mask & (mask - 1)));
+        ASSERT((condition != ConditionAL) || !(mask & (mask - 1)));
         return (condition << 4) | mask;
     }
@@ -768 +771 @@
     {
         int mask = 8;
-        ASSERT((condition != ConditionAL) || (mask & (mask - 1)));
         return (condition << 4) | mask;
     }
@@ -1665 +1667 @@
     }
     
-    int jumpSizeDelta(JumpLinkType jumpLinkType) { return JumpPaddingSize - JumpSizes[jumpLinkType]; }
+    int jumpSizeDelta(JumpType jumpType, JumpLinkType jumpLinkType) { return JumpPaddingSizes[jumpType] - JumpSizes[jumpLinkType]; }
     
     // Assembler admin methods:
@@ -1679 +1681 @@
     }
 
+    bool canCompact(JumpType jumpType)
+    {
+        // The following cannot be compacted:
+        //   JumpFixed: represents custom jump sequence
+        //   JumpNoConditionFixedSize: represents unconditional jump that must remain a fixed size
+        //   JumpConditionFixedSize: represents conditional jump that must remain a fixed size
+        return (jumpType == JumpNoCondition) || (jumpType == JumpCondition);
+    }
+    
+    JumpLinkType computeJumpType(JumpType jumpType, const uint8_t* from, const uint8_t* to)
+    {
+        if (jumpType == JumpFixed)
+            return LinkInvalid;
+        
+        // for patchable jump we must leave space for the longest code sequence
+        if (jumpType == JumpNoConditionFixedSize)
+            return LinkBX;
+        if (jumpType == JumpConditionFixedSize)
+            return LinkConditionalBX;
+        
+        const int paddingSize = JumpPaddingSizes[jumpType];
+        bool mayTriggerErrata = false;
+        
+        if (jumpType == JumpCondition) {
+            // 2-byte conditional T1
+            const uint16_t* jumpT1Location = reinterpret_cast<const uint16_t*>(from - (paddingSize - JumpSizes[LinkJumpT1]));
+            if (canBeJumpT1(jumpT1Location, to))
+                return LinkJumpT1;
+            // 4-byte conditional T3
+            const uint16_t* jumpT3Location = reinterpret_cast<const uint16_t*>(from - (paddingSize - JumpSizes[LinkJumpT3]));
+            if (canBeJumpT3(jumpT3Location, to, mayTriggerErrata)) {
+                if (!mayTriggerErrata)
+                    return LinkJumpT3;
+            }
+            // 4-byte conditional T4 with IT
+            const uint16_t* conditionalJumpT4Location = 
+            reinterpret_cast<const uint16_t*>(from - (paddingSize - JumpSizes[LinkConditionalJumpT4]));
+            if (canBeJumpT4(conditionalJumpT4Location, to, mayTriggerErrata)) {
+                if (!mayTriggerErrata)
+                    return LinkConditionalJumpT4;
+            }
+        } else {
+            // 2-byte unconditional T2
+            const uint16_t* jumpT2Location = reinterpret_cast<const uint16_t*>(from - (paddingSize - JumpSizes[LinkJumpT2]));
+            if (canBeJumpT2(jumpT2Location, to))
+                return LinkJumpT2;
+            // 4-byte unconditional T4
+            const uint16_t* jumpT4Location = reinterpret_cast<const uint16_t*>(from - (paddingSize - JumpSizes[LinkJumpT4]));
+            if (canBeJumpT4(jumpT4Location, to, mayTriggerErrata)) {
+                if (!mayTriggerErrata)
+                    return LinkJumpT4;
+            }
+            // use long jump sequence
+            return LinkBX;
+        }
+        
+        ASSERT(jumpType == JumpCondition);
+        return LinkConditionalBX;
+    }
+    
     JumpLinkType computeJumpType(LinkRecord& record, const uint8_t* from, const uint8_t* to)
     {
-        if (record.type() >= JumpFullSize) {
-            record.setLinkType(LinkBX);
-            return LinkBX;
-        }
-        const uint16_t* jumpT2Location = reinterpret_cast<const uint16_t*>(from  - (JumpPaddingSize - JumpSizes[LinkJumpT2]));
-        if (canBeJumpT2(jumpT2Location, to)) {
-            if (record.type() == JumpCondition) {
-                record.setLinkType(LinkConditionalJumpT2);
-                return LinkConditionalJumpT2;
-            }
-            record.setLinkType(LinkJumpT2);
-            return LinkJumpT2;
-        }
-        bool mayTriggerErrata = false;
-        const uint16_t* jumpT4Location = reinterpret_cast<const uint16_t*>(from  - (JumpPaddingSize - JumpSizes[LinkJumpT4]));
-        if (!canBeJumpT4(jumpT4Location, to, mayTriggerErrata)) {
-            record.setLinkType(LinkBX);
-            return LinkBX;
-        }
-        if (mayTriggerErrata) {
-            record.setLinkType(LinkBX);
-            return LinkBX;
-        }
-        if (record.type() == JumpCondition) {
-            record.setLinkType(LinkConditionalJumpT4);
-            return LinkConditionalJumpT4;
-        }
-        record.setLinkType(LinkJumpT4);
-        return LinkJumpT4;
-    }
-
+        JumpLinkType linkType = computeJumpType(record.type(), from, to);
+        record.setLinkType(linkType);
+        return linkType;
+    }
+    
     void recordLinkOffsets(int32_t regionStart, int32_t regionEnd, int32_t offset)
     {
@@ -1729 +1765 @@
     void link(LinkRecord& record, uint8_t* from, uint8_t* to)
     {
-        JumpLinkType linkType = record.linkType();
-        ASSERT(linkType != LinkInvalid);
-        if ((linkType == LinkConditionalJumpT2) || (linkType == LinkConditionalJumpT4)) {
-            uint16_t* itttLocation = reinterpret_cast<uint16_t*>(from - JumpSizes[linkType] - 2);
-            itttLocation[0] = ifThenElse(record.condition()) | OP_IT;
-        }
-        switch (linkType) {
+        switch (record.linkType()) {
+        case LinkJumpT1:
+            linkJumpT1(record.condition(), reinterpret_cast<uint16_t*>(from), to);
+            break;
         case LinkJumpT2:
-        case LinkConditionalJumpT2:
             linkJumpT2(reinterpret_cast<uint16_t*>(from), to);
             break;
+        case LinkJumpT3:
+            linkJumpT3(record.condition(), reinterpret_cast<uint16_t*>(from), to);
+            break;
         case LinkJumpT4:
+            linkJumpT4(reinterpret_cast<uint16_t*>(from), to);
+            break;
         case LinkConditionalJumpT4:
-            linkJumpT4(reinterpret_cast<uint16_t*>(from), to);
+            linkConditionalJumpT4(record.condition(), reinterpret_cast<uint16_t*>(from), to);
+            break;
+        case LinkConditionalBX:
+            linkConditionalBX(record.condition(), reinterpret_cast<uint16_t*>(from), to);
             break;
         case LinkBX:
@@ -1970 +2010 @@
     }
 
+    static bool canBeJumpT1(const uint16_t* instruction, const void* target)
+    {
+        ASSERT(!(reinterpret_cast<intptr_t>(instruction) & 1));
+        ASSERT(!(reinterpret_cast<intptr_t>(target) & 1));
+        
+        intptr_t relative = reinterpret_cast<intptr_t>(target) - (reinterpret_cast<intptr_t>(instruction));
+        // It does not appear to be documented in the ARM ARM (big surprise), but
+        // for OP_B_T1 the branch displacement encoded in the instruction is 2 
+        // less than the actual displacement.
+        relative -= 2;
+        return ((relative << 23) >> 23) == relative;
+    }
+    
     static bool canBeJumpT2(const uint16_t* instruction, const void* target)
     {
@@ -1981 +2034 @@
         relative -= 2;
         return ((relative << 20) >> 20) == relative;
+    }
+    
+    static bool canBeJumpT3(const uint16_t* instruction, const void* target, bool& mayTriggerErrata)
+    {
+        ASSERT(!(reinterpret_cast<intptr_t>(instruction) & 1));
+        ASSERT(!(reinterpret_cast<intptr_t>(target) & 1));
+        
+        intptr_t relative = reinterpret_cast<intptr_t>(target) - (reinterpret_cast<intptr_t>(instruction));
+        // From Cortex-A8 errata:
+        // If the 32-bit Thumb-2 branch instruction spans two 4KiB regions and
+        // the target of the branch falls within the first region it is
+        // possible for the processor to incorrectly determine the branch
+        // instruction, and it is also possible in some cases for the processor
+        // to enter a deadlock state.
+        // The instruction is spanning two pages if it ends at an address ending 0x002
+        bool spansTwo4K = ((reinterpret_cast<intptr_t>(instruction) & 0xfff) == 0x002);
+        mayTriggerErrata = spansTwo4K;
+        // The target is in the first page if the jump branch back by [3..0x1002] bytes
+        bool targetInFirstPage = (relative >= -0x1002) && (relative < -2);
+        bool wouldTriggerA8Errata = spansTwo4K && targetInFirstPage;
+        return ((relative << 11) >> 11) == relative && !wouldTriggerA8Errata;
     }
     
@@ -2003 +2077 @@
         return ((relative << 7) >> 7) == relative && !wouldTriggerA8Errata;
     }
-
-    static void linkBX(uint16_t* instruction, void* target)
-    {
-        linkJumpAbsolute(instruction, target);
-    }
-    
-    static void linkJumpT4(uint16_t* instruction, void* target)
+    
+    void linkJumpT1(Condition cond, uint16_t* instruction, void* target)
     {
         // FIMXE: this should be up in the MacroAssembler layer. :-(        
         ASSERT(!(reinterpret_cast<intptr_t>(instruction) & 1));
         ASSERT(!(reinterpret_cast<intptr_t>(target) & 1));
+        ASSERT(canBeJumpT1(instruction, target));
         
         intptr_t relative = reinterpret_cast<intptr_t>(target) - (reinterpret_cast<intptr_t>(instruction));
-        bool scratch;
-        UNUSED_PARAM(scratch);
-        ASSERT(canBeJumpT4(instruction, target, scratch));
-        // ARM encoding for the top two bits below the sign bit is 'peculiar'.
-        if (relative >= 0)
-            relative ^= 0xC00000;
-
+        // It does not appear to be documented in the ARM ARM (big surprise), but
+        // for OP_B_T1 the branch displacement encoded in the instruction is 2 
+        // less than the actual displacement.
+        relative -= 2;
+        
         // All branch offsets should be an even distance.
         ASSERT(!(relative & 1));
-        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);
-    }
-
+        instruction[-1] = OP_B_T1 | ((cond & 0xf) << 8) | ((relative & 0x1fe) >> 1);
+    }
+    
     static void linkJumpT2(uint16_t* instruction, void* target)
     {
@@ -2047 +2114 @@
     }
     
-    static void linkJumpAbsolute(uint16_t* instruction, void* target)
+    void linkJumpT3(Condition cond, uint16_t* instruction, void* target)
     {
         // FIMXE: this should be up in the MacroAssembler layer. :-(
         ASSERT(!(reinterpret_cast<intptr_t>(instruction) & 1));
         ASSERT(!(reinterpret_cast<intptr_t>(target) & 1));
-
+        bool scratch;
+        UNUSED_PARAM(scratch);
+        ASSERT(canBeJumpT3(instruction, target, scratch));
+        
+        intptr_t relative = reinterpret_cast<intptr_t>(target) - (reinterpret_cast<intptr_t>(instruction));
+        
+        // All branch offsets should be an even distance.
+        ASSERT(!(relative & 1));
+        instruction[-2] = OP_B_T3a | ((relative & 0x100000) >> 10) | ((cond & 0xf) << 6) | ((relative & 0x3f000) >> 12);
+        instruction[-1] = OP_B_T3b | ((relative & 0x80000) >> 8) | ((relative & 0x40000) >> 5) | ((relative & 0xffe) >> 1);
+    }
+    
+    static void linkJumpT4(uint16_t* instruction, void* target)
+    {
+        // FIMXE: this should be up in the MacroAssembler layer. :-(        
+        ASSERT(!(reinterpret_cast<intptr_t>(instruction) & 1));
+        ASSERT(!(reinterpret_cast<intptr_t>(target) & 1));
+        bool scratch;
+        UNUSED_PARAM(scratch);
+        ASSERT(canBeJumpT4(instruction, target, scratch));
+        
+        intptr_t relative = reinterpret_cast<intptr_t>(target) - (reinterpret_cast<intptr_t>(instruction));
+        // 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));
+        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);
+    }
+    
+    void linkConditionalJumpT4(Condition cond, uint16_t* instruction, void* target)
+    {
+        // FIMXE: this should be up in the MacroAssembler layer. :-(        
+        ASSERT(!(reinterpret_cast<intptr_t>(instruction) & 1));
+        ASSERT(!(reinterpret_cast<intptr_t>(target) & 1));
+        
+        instruction[-3] = ifThenElse(cond) | OP_IT;
+        linkJumpT4(instruction, target);
+    }
+    
+    static void linkBX(uint16_t* instruction, void* target)
+    {
+        // FIMXE: this should be up in the MacroAssembler layer. :-(
+        ASSERT(!(reinterpret_cast<intptr_t>(instruction) & 1));
+        ASSERT(!(reinterpret_cast<intptr_t>(target) & 1));
+        
+        const uint16_t JUMP_TEMPORARY_REGISTER = ARMRegisters::ip;
+        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);
+    }
+    
+    void linkConditionalBX(Condition cond, uint16_t* instruction, void* target)
+    {
+        // FIMXE: this should be up in the MacroAssembler layer. :-(        
+        ASSERT(!(reinterpret_cast<intptr_t>(instruction) & 1));
+        ASSERT(!(reinterpret_cast<intptr_t>(target) & 1));
+        
+        linkBX(instruction, target);
+        instruction[-6] = ifThenElse(cond, true, true) | OP_IT;
+    }
+    
+    static void linkJumpAbsolute(uint16_t* instruction, void* target)
+    {
+        // FIMXE: this should be up in the MacroAssembler layer. :-(
+        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));
+               || (isNOP_T1(instruction - 5) && isNOP_T2(instruction - 4) && isB(instruction - 2)));
+        
         bool scratch;
         if (canBeJumpT4(instruction, target, scratch)) {
-            // 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*
@@ -2073 +2206 @@
             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);
+            linkJumpT4(instruction, target);
         } else {
             const uint16_t JUMP_TEMPORARY_REGISTER = ARMRegisters::ip;
@@ -2086 +2218 @@
         }
     }
-
+    
     static uint16_t twoWordOp5i6Imm4Reg4EncodedImmFirst(uint16_t op, ARMThumbImmediate imm)
     {

trunk/JavaScriptCore/assembler/LinkBuffer.h

@@ -1 +1 @@
 /*
- * Copyright (C) 2009 Apple Inc. All rights reserved.
+ * Copyright (C) 2009, 2010 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -236 +236 @@
             
             JumpLinkType jumpLinkType = m_assembler->computeJumpType(jumpsToLink[i], linkBase + writePtr, target);
-
-            // Step back in the write stream
-            int32_t delta = m_assembler->jumpSizeDelta(jumpLinkType);
-            if (delta) {
-                writePtr -= delta;
-                m_assembler->recordLinkOffsets(jumpsToLink[i].from() - delta, readPtr, readPtr - writePtr);
+            // Compact branch if we can...
+            if (m_assembler->canCompact(jumpsToLink[i].type())) {
+                // Step back in the write stream
+                int32_t delta = m_assembler->jumpSizeDelta(jumpsToLink[i].type(), jumpLinkType);
+                if (delta) {
+                    writePtr -= delta;
+                    m_assembler->recordLinkOffsets(jumpsToLink[i].from() - delta, readPtr, readPtr - writePtr);
+                }
             }
             jumpsToLink[i].setFrom(writePtr);

trunk/JavaScriptCore/assembler/MacroAssemblerARMv7.h

@@ -1 +1 @@
 /*
- * Copyright (C) 2009 Apple Inc. All rights reserved.
+ * Copyright (C) 2009, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2010 University of Szeged
  *
@@ -47 +47 @@
 public:
     typedef ARMv7Assembler::LinkRecord LinkRecord;
+    typedef ARMv7Assembler::JumpType JumpType;
     typedef ARMv7Assembler::JumpLinkType JumpLinkType;
 
@@ -58 +59 @@
     Vector<LinkRecord>& jumpsToLink() { return m_assembler.jumpsToLink(); }
     void* unlinkedCode() { return m_assembler.unlinkedCode(); }
+    bool canCompact(JumpType jumpType) { return m_assembler.canCompact(jumpType); }
+    JumpLinkType computeJumpType(JumpType jumpType, const uint8_t* from, const uint8_t* to) { return m_assembler.computeJumpType(jumpType, from, to); }
     JumpLinkType computeJumpType(LinkRecord& record, const uint8_t* from, const uint8_t* to) { return m_assembler.computeJumpType(record, from, to); }
     void recordLinkOffsets(int32_t regionStart, int32_t regionEnd, int32_t offset) {return m_assembler.recordLinkOffsets(regionStart, regionEnd, offset); }
-    int jumpSizeDelta(JumpLinkType jumpLinkType) { return m_assembler.jumpSizeDelta(jumpLinkType); }
+    int jumpSizeDelta(JumpType jumpType, JumpLinkType jumpLinkType) { return m_assembler.jumpSizeDelta(jumpType, jumpLinkType); }
     void link(LinkRecord& record, uint8_t* from, uint8_t* to) { return m_assembler.link(record, from, to); }
 
@@ -987 +990 @@
     void jump(RegisterID target)
     {
-        m_assembler.bx(target, inUninterruptedSequence() ? ARMv7Assembler::JumpFullSize : ARMv7Assembler::JumpNoCondition);
+        m_assembler.bx(target, ARMv7Assembler::JumpFixed);
     }
 
@@ -994 +997 @@
     {
         load32(address, dataTempRegister);
-        m_assembler.bx(dataTempRegister, inUninterruptedSequence() ? ARMv7Assembler::JumpFullSize : ARMv7Assembler::JumpNoCondition);
+        m_assembler.bx(dataTempRegister, ARMv7Assembler::JumpFixed);
     }
 
@@ -1083 +1086 @@
     {
         moveFixedWidthEncoding(Imm32(0), dataTempRegister);
-        return Call(m_assembler.blx(dataTempRegister, ARMv7Assembler::JumpFullSize), Call::LinkableNear);
+        return Call(m_assembler.blx(dataTempRegister, ARMv7Assembler::JumpFixed), Call::LinkableNear);
     }
 
@@ -1089 +1092 @@
     {
         moveFixedWidthEncoding(Imm32(0), dataTempRegister);
-        return Call(m_assembler.blx(dataTempRegister, ARMv7Assembler::JumpFullSize), Call::Linkable);
+        return Call(m_assembler.blx(dataTempRegister, ARMv7Assembler::JumpFixed), Call::Linkable);
     }
 
     Call call(RegisterID target)
     {
-        return Call(m_assembler.blx(target, ARMv7Assembler::JumpFullSize), Call::None);
+        return Call(m_assembler.blx(target, ARMv7Assembler::JumpFixed), Call::None);
     }
 
@@ -1100 +1103 @@
     {
         load32(address, dataTempRegister);
-        return Call(m_assembler.blx(dataTempRegister, ARMv7Assembler::JumpFullSize), Call::None);
+        return Call(m_assembler.blx(dataTempRegister, ARMv7Assembler::JumpFixed), Call::None);
     }
 
     void ret()
     {
-        m_assembler.bx(linkRegister, ARMv7Assembler::JumpFullSize);
+        m_assembler.bx(linkRegister, ARMv7Assembler::JumpFixed);
     }
 
@@ -1205 +1208 @@
         // Like a normal call, but don't link.
         moveFixedWidthEncoding(Imm32(0), dataTempRegister);
-        return Call(m_assembler.bx(dataTempRegister, ARMv7Assembler::JumpFullSize), Call::Linkable);
+        return Call(m_assembler.bx(dataTempRegister, ARMv7Assembler::JumpFixed), Call::Linkable);
     }
 
@@ -1229 +1232 @@
     {
         moveFixedWidthEncoding(Imm32(0), dataTempRegister);
-        return m_assembler.bx(dataTempRegister, inUninterruptedSequence() ? ARMv7Assembler::JumpFullSize : ARMv7Assembler::JumpNoCondition);
+        return m_assembler.bx(dataTempRegister, inUninterruptedSequence() ? ARMv7Assembler::JumpNoConditionFixedSize : ARMv7Assembler::JumpNoCondition);
     }
 
@@ -1236 +1239 @@
         m_assembler.it(cond, true, true);
         moveFixedWidthEncoding(Imm32(0), dataTempRegister);
-        return m_assembler.bx(dataTempRegister, inUninterruptedSequence() ? ARMv7Assembler::JumpFullSize : ARMv7Assembler::JumpCondition, cond);
+        return m_assembler.bx(dataTempRegister, inUninterruptedSequence() ? ARMv7Assembler::JumpConditionFixedSize : ARMv7Assembler::JumpCondition, cond);
     }
     ARMv7Assembler::JmpSrc makeBranch(Condition cond) { return makeBranch(armV7Condition(cond)); }