[sh4] Refactor jumps in baseline JIT to return label after the jump.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 4 Nov 2013 18:21:37 +0000 (18:21 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 4 Nov 2013 18:21:37 +0000 (18:21 +0000)
https://bugs.webkit.org/show_bug.cgi?id=123734

Patch by Julien Brianceau <jbriance@cisco.com> on 2013-11-04
Reviewed by Michael Saboff.

Current implementation of jumps in sh4 baseline JIT returns a label on the jump itself
and not after it. This is not correct and leads to issues like infinite loop the DFG
(https://bugs.webkit.org/show_bug.cgi?id=122597 for instance). This refactor fixes this
and also simplifies the link and relink procedures for sh4 jumps.

* assembler/MacroAssemblerSH4.h:
(JSC::MacroAssemblerSH4::branchDouble):
(JSC::MacroAssemblerSH4::branchTrue):
(JSC::MacroAssemblerSH4::branchFalse):
* assembler/SH4Assembler.h:
(JSC::SH4Assembler::jmp):
(JSC::SH4Assembler::extraInstrForBranch):
(JSC::SH4Assembler::jne):
(JSC::SH4Assembler::je):
(JSC::SH4Assembler::bra):
(JSC::SH4Assembler::linkJump):
(JSC::SH4Assembler::relinkJump):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@158580 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/assembler/MacroAssemblerSH4.h
Source/JavaScriptCore/assembler/SH4Assembler.h

index 64bad07..0e55ce7 100644 (file)
@@ -1,3 +1,28 @@
+2013-11-04  Julien Brianceau  <jbriance@cisco.com>
+
+        [sh4] Refactor jumps in baseline JIT to return label after the jump.
+        https://bugs.webkit.org/show_bug.cgi?id=123734
+
+        Reviewed by Michael Saboff.
+
+        Current implementation of jumps in sh4 baseline JIT returns a label on the jump itself
+        and not after it. This is not correct and leads to issues like infinite loop the DFG
+        (https://bugs.webkit.org/show_bug.cgi?id=122597 for instance). This refactor fixes this
+        and also simplifies the link and relink procedures for sh4 jumps.
+
+        * assembler/MacroAssemblerSH4.h:
+        (JSC::MacroAssemblerSH4::branchDouble):
+        (JSC::MacroAssemblerSH4::branchTrue):
+        (JSC::MacroAssemblerSH4::branchFalse):
+        * assembler/SH4Assembler.h:
+        (JSC::SH4Assembler::jmp):
+        (JSC::SH4Assembler::extraInstrForBranch):
+        (JSC::SH4Assembler::jne):
+        (JSC::SH4Assembler::je):
+        (JSC::SH4Assembler::bra):
+        (JSC::SH4Assembler::linkJump):
+        (JSC::SH4Assembler::relinkJump):
+
 2013-11-03  Filip Pizlo  <fpizlo@apple.com>
 
         Generated color wheel displays incorrectly (regressed in r155567)
index ceb614c..09e7c21 100644 (file)
@@ -1454,10 +1454,9 @@ public:
             m_assembler.dcmppeq(right, right);
             takeBranch.append(Jump(m_assembler.jne(), SH4Assembler::JumpNear));
             m_assembler.dcmppeq(left, right);
-            Jump m_jump = Jump(m_assembler.je());
+            m_assembler.branch(BF_OPCODE, 2);
             takeBranch.link(this);
-            m_assembler.extraInstrForBranch(scratchReg3);
-            return m_jump;
+            return Jump(m_assembler.extraInstrForBranch(scratchReg3));
         }
 
         if (cond == DoubleGreaterThanOrUnordered) {
@@ -1468,10 +1467,9 @@ public:
             m_assembler.dcmppeq(right, right);
             takeBranch.append(Jump(m_assembler.jne(), SH4Assembler::JumpNear));
             m_assembler.dcmppgt(right, left);
-            Jump m_jump = Jump(m_assembler.je());
+            m_assembler.branch(BF_OPCODE, 2);
             takeBranch.link(this);
-            m_assembler.extraInstrForBranch(scratchReg3);
-            return m_jump;
+            return Jump(m_assembler.extraInstrForBranch(scratchReg3));
         }
 
         if (cond == DoubleGreaterThanOrEqualOrUnordered) {
@@ -1487,10 +1485,9 @@ public:
             m_assembler.dcmppeq(right, right);
             takeBranch.append(Jump(m_assembler.jne(), SH4Assembler::JumpNear));
             m_assembler.dcmppgt(left, right);
-            Jump m_jump = Jump(m_assembler.je());
+            m_assembler.branch(BF_OPCODE, 2);
             takeBranch.link(this);
-            m_assembler.extraInstrForBranch(scratchReg3);
-            return m_jump;
+            return Jump(m_assembler.extraInstrForBranch(scratchReg3));
         }
 
         if (cond == DoubleLessThanOrEqualOrUnordered) {
@@ -1506,17 +1503,15 @@ public:
     Jump branchTrue()
     {
         m_assembler.ensureSpace(m_assembler.maxInstructionSize + 6, sizeof(uint32_t));
-        Jump m_jump = Jump(m_assembler.je());
-        m_assembler.extraInstrForBranch(scratchReg3);
-        return m_jump;
+        m_assembler.branch(BF_OPCODE, 2);
+        return Jump(m_assembler.extraInstrForBranch(scratchReg3));
     }
 
     Jump branchFalse()
     {
         m_assembler.ensureSpace(m_assembler.maxInstructionSize + 6, sizeof(uint32_t));
-        Jump m_jump = Jump(m_assembler.jne());
-        m_assembler.extraInstrForBranch(scratchReg3);
-        return m_jump;
+        m_assembler.branch(BT_OPCODE, 2);
+        return Jump(m_assembler.extraInstrForBranch(scratchReg3));
     }
 
     Jump branch32(RelationalCondition cond, BaseIndex left, TrustedImm32 right)
index 0d81a13..92f1a8f 100644 (file)
@@ -1251,19 +1251,19 @@ public:
     {
         RegisterID scr = claimScratch();
         m_buffer.ensureSpace(maxInstructionSize + 4, sizeof(uint32_t));
-        AssemblerLabel label = m_buffer.label();
         loadConstantUnReusable(0x0, scr);
         branch(BRAF_OPCODE, scr);
         nop();
         releaseScratch(scr);
-        return label;
+        return m_buffer.label();
     }
 
-    void extraInstrForBranch(RegisterID dst)
+    AssemblerLabel extraInstrForBranch(RegisterID dst)
     {
         loadConstantUnReusable(0x0, dst);
+        branch(BRAF_OPCODE, dst);
         nop();
-        nop();
+        return m_buffer.label();
     }
 
     AssemblerLabel jmp(RegisterID dst)
@@ -1281,23 +1281,20 @@ public:
 
     AssemblerLabel jne()
     {
-        AssemblerLabel label = m_buffer.label();
         branch(BF_OPCODE, 0);
-        return label;
+        return m_buffer.label();
     }
 
     AssemblerLabel je()
     {
-        AssemblerLabel label = m_buffer.label();
         branch(BT_OPCODE, 0);
-        return label;
+        return m_buffer.label();
     }
 
     AssemblerLabel bra()
     {
-        AssemblerLabel label = m_buffer.label();
         branch(BRA_OPCODE, 0);
-        return label;
+        return m_buffer.label();
     }
 
     void ret()
@@ -1371,33 +1368,16 @@ public:
     {
         ASSERT(from.isSet());
 
-        uint16_t* instructionPtr = getInstructionPtr(code, from.m_offset);
-        uint16_t instruction = *instructionPtr;
+        uint16_t* instructionPtr = getInstructionPtr(code, from.m_offset) - 3;
         int offsetBits = (reinterpret_cast<uint32_t>(to) - reinterpret_cast<uint32_t>(code)) - from.m_offset;
 
-        if (((instruction & 0xff00) == BT_OPCODE) || ((instruction & 0xff00) == BF_OPCODE)) {
-            /* BT label ==> BF 2
-               nop          LDR reg
-               nop          braf @reg
-               nop          nop
-            */
-            offsetBits -= 8;
-            instruction ^= 0x0202;
-            *instructionPtr++ = instruction;
-            changePCrelativeAddress((*instructionPtr & 0xff), instructionPtr, offsetBits);
-            instruction = (BRAF_OPCODE | (*instructionPtr++ & 0xf00));
-            *instructionPtr = instruction;
-            printBlockInstr(instructionPtr - 2, from.m_offset, 3);
-            return;
-        }
-
-         /* MOV #imm, reg => LDR reg
-            braf @reg        braf @reg
-            nop              nop
-         */
+        /* MOV #imm, reg => LDR reg
+           braf @reg        braf @reg
+           nop              nop
+        */
         ASSERT((instructionPtr[0] & 0xf000) == MOVL_READ_OFFPC_OPCODE);
         ASSERT((instructionPtr[1] & 0xf0ff) == BRAF_OPCODE);
-        changePCrelativeAddress((*instructionPtr & 0xff), instructionPtr, offsetBits - 6);
+        changePCrelativeAddress((*instructionPtr & 0xff), instructionPtr, offsetBits);
         printInstr(*instructionPtr, from.m_offset + 2);
     }
 
@@ -1500,24 +1480,10 @@ public:
     static void relinkJump(void* from, void* to)
     {
         uint16_t* instructionPtr = reinterpret_cast<uint16_t*> (from);
-        uint16_t instruction = *instructionPtr;
-        int32_t offsetBits = (reinterpret_cast<uint32_t>(to) - reinterpret_cast<uint32_t>(from));
-
-        if (((*instructionPtr & 0xff00) == BT_OPCODE) || ((*instructionPtr & 0xff00) == BF_OPCODE)) {
-            offsetBits -= 8;
-            instructionPtr++;
-            ASSERT((instructionPtr[0] & 0xf000) == MOVL_READ_OFFPC_OPCODE);
-            changePCrelativeAddress((*instructionPtr & 0xff), instructionPtr, offsetBits);
-            instruction = (BRAF_OPCODE | (*instructionPtr++ & 0xf00));
-            *instructionPtr = instruction;
-            printBlockInstr(instructionPtr, reinterpret_cast<uint32_t>(from) + 1, 3);
-            cacheFlush(instructionPtr, sizeof(SH4Word));
-            return;
-        }
-
+        instructionPtr -= 3;
+        ASSERT((instructionPtr[0] & 0xf000) == MOVL_READ_OFFPC_OPCODE);
         ASSERT((instructionPtr[1] & 0xf0ff) == BRAF_OPCODE);
-        changePCrelativeAddress((*instructionPtr & 0xff), instructionPtr, offsetBits - 6);
-        printInstr(*instructionPtr, reinterpret_cast<uint32_t>(from));
+        changePCrelativeAddress((*instructionPtr & 0xff), instructionPtr, reinterpret_cast<uint32_t>(to) - reinterpret_cast<uint32_t>(from));
     }
 
     // Linking & patching
@@ -1569,12 +1535,12 @@ public:
         ASSERT(to.isSet());
         ASSERT(from.isSet());
 
-        uint16_t* instructionPtr = getInstructionPtr(data(), from.m_offset);
-        uint16_t instruction = *instructionPtr;
-        int offsetBits;
+        uint16_t* instructionPtr = getInstructionPtr(data(), from.m_offset) - 1;
+        int offsetBits = (to.m_offset - from.m_offset);
 
         if (type == JumpNear) {
-            int offset = (codeSize() - from.m_offset) - 4;
+            uint16_t instruction = instructionPtr[0];
+            int offset = (offsetBits - 2);
             ASSERT((((instruction == BT_OPCODE) || (instruction == BF_OPCODE)) && (offset >= -256) && (offset <= 254))
                 || ((instruction == BRA_OPCODE) && (offset >= -4096) && (offset <= 4094)));
             *instructionPtr++ = instruction | (offset >> 1);
@@ -1582,35 +1548,14 @@ public:
             return;
         }
 
-        if (((instruction & 0xff00) == BT_OPCODE) || ((instruction & 0xff00) == BF_OPCODE)) {
-            /* BT label => BF 2
-               nop        LDR reg
-               nop        braf @reg
-               nop        nop
-            */
-            offsetBits = (to.m_offset - from.m_offset) - 8;
-            instruction ^= 0x0202;
-            *instructionPtr++ = instruction;
-            if ((*instructionPtr & 0xf000) == MOVIMM_OPCODE) {
-                uint32_t* addr = getLdrImmAddressOnPool(instructionPtr, m_buffer.poolAddress());
-                *addr = offsetBits;
-            } else
-                changePCrelativeAddress((*instructionPtr & 0xff), instructionPtr, offsetBits);
-            instruction = (BRAF_OPCODE | (*instructionPtr++ & 0xf00));
-            *instructionPtr = instruction;
-            printBlockInstr(instructionPtr - 2, from.m_offset, 3);
-            return;
-        }
-
         /* MOV # imm, reg => LDR reg
            braf @reg         braf @reg
            nop               nop
         */
-        ASSERT((*(instructionPtr + 1) & BRAF_OPCODE) == BRAF_OPCODE);
-        offsetBits = (to.m_offset - from.m_offset) - 6;
+        instructionPtr -= 2;
+        ASSERT((instructionPtr[1] & 0xf0ff) == BRAF_OPCODE);
 
-        instruction = *instructionPtr;
-        if ((instruction & 0xf000) == MOVIMM_OPCODE) {
+        if ((instructionPtr[0] & 0xf000) == MOVIMM_OPCODE) {
             uint32_t* addr = getLdrImmAddressOnPool(instructionPtr, m_buffer.poolAddress());
             *addr = offsetBits;
             printInstr(*instructionPtr, from.m_offset + 2);