Remove some unnecessary jumps in snippet code.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Nov 2015 00:30:37 +0000 (00:30 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Nov 2015 00:30:37 +0000 (00:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=151415

Reviewed by Geoffrey Garen.

Previously, the snippet generators always emit a jump at the end of the fast
path.  In the baseline JIT and FTL, this results in a jump to the very next
instruction.  I've change it to assume that the fast path will just fall thru,
and let the client decide instead if it wants/needs a jump or not after the fast
path.

I also changed the generators to provide a didEmitFastPath() query explicitly
declare if they actually generated the fast path, instead of having the client
infer it from an empty endJumpList.

Benchmarks show that perf is neutral with this change (tested on x86_64).

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileValueAdd):
(JSC::DFG::SpeculativeJIT::compileArithSub):
* ftl/FTLCompile.cpp:
(JSC::FTL::mmAllocateDataSection):
* jit/JITAddGenerator.cpp:
(JSC::JITAddGenerator::generateFastPath):
* jit/JITAddGenerator.h:
(JSC::JITAddGenerator::didEmitFastPath):
(JSC::JITAddGenerator::endJumpList):
(JSC::JITAddGenerator::slowPathJumpList):
* jit/JITArithmetic.cpp:
(JSC::JIT::emit_op_add):
(JSC::JIT::emit_op_sub):
* jit/JITSubGenerator.cpp:
(JSC::JITSubGenerator::generateFastPath):
* jit/JITSubGenerator.h:
(JSC::JITSubGenerator::didEmitFastPath):
(JSC::JITSubGenerator::endJumpList):
(JSC::JITSubGenerator::slowPathJumpList):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/ftl/FTLCompile.cpp
Source/JavaScriptCore/jit/JITAddGenerator.cpp
Source/JavaScriptCore/jit/JITAddGenerator.h
Source/JavaScriptCore/jit/JITArithmetic.cpp
Source/JavaScriptCore/jit/JITSubGenerator.cpp
Source/JavaScriptCore/jit/JITSubGenerator.h

index 2ad35cb..079ab66 100644 (file)
@@ -1,3 +1,43 @@
+2015-11-18  Mark Lam  <mark.lam@apple.com>
+
+        Remove some unnecessary jumps in snippet code.
+        https://bugs.webkit.org/show_bug.cgi?id=151415
+
+        Reviewed by Geoffrey Garen.
+
+        Previously, the snippet generators always emit a jump at the end of the fast
+        path.  In the baseline JIT and FTL, this results in a jump to the very next
+        instruction.  I've change it to assume that the fast path will just fall thru,
+        and let the client decide instead if it wants/needs a jump or not after the fast
+        path.
+
+        I also changed the generators to provide a didEmitFastPath() query explicitly
+        declare if they actually generated the fast path, instead of having the client
+        infer it from an empty endJumpList.
+
+        Benchmarks show that perf is neutral with this change (tested on x86_64).
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileValueAdd):
+        (JSC::DFG::SpeculativeJIT::compileArithSub):
+        * ftl/FTLCompile.cpp:
+        (JSC::FTL::mmAllocateDataSection):
+        * jit/JITAddGenerator.cpp:
+        (JSC::JITAddGenerator::generateFastPath):
+        * jit/JITAddGenerator.h:
+        (JSC::JITAddGenerator::didEmitFastPath):
+        (JSC::JITAddGenerator::endJumpList):
+        (JSC::JITAddGenerator::slowPathJumpList):
+        * jit/JITArithmetic.cpp:
+        (JSC::JIT::emit_op_add):
+        (JSC::JIT::emit_op_sub):
+        * jit/JITSubGenerator.cpp:
+        (JSC::JITSubGenerator::generateFastPath):
+        * jit/JITSubGenerator.h:
+        (JSC::JITSubGenerator::didEmitFastPath):
+        (JSC::JITSubGenerator::endJumpList):
+        (JSC::JITSubGenerator::slowPathJumpList):
+
 2015-11-18  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r192436 and r192586.
index b4a6438..5b4ca34 100755 (executable)
@@ -2880,6 +2880,9 @@ void SpeculativeJIT::compileValueAdd(Node* node)
         leftFPR, rightFPR, scratchGPR, scratchFPR);
     gen.generateFastPath(m_jit);
 
+    ASSERT(gen.didEmitFastPath());
+    gen.endJumpList().append(m_jit.jump());
+
     gen.slowPathJumpList().link(&m_jit);
 
     silentSpillAllRegisters(resultRegs);
@@ -3240,6 +3243,9 @@ void SpeculativeJIT::compileArithSub(Node* node)
             leftFPR, rightFPR, scratchGPR, scratchFPR);
         gen.generateFastPath(m_jit);
 
+        ASSERT(gen.didEmitFastPath());
+        gen.endJumpList().append(m_jit.jump());
+
         gen.slowPathJumpList().link(&m_jit);
         silentSpillAllRegisters(resultRegs);
         callOperation(operationValueSub, resultRegs, leftRegs, rightRegs);
index b71f800..2c065c0 100644 (file)
@@ -462,6 +462,7 @@ static void generateArithSubICFastPath(
         context.initializeRegisters(fastPathJIT);
         gen.generateFastPath(fastPathJIT);
 
+        ASSERT(gen.didEmitFastPath());
         gen.endJumpList().link(&fastPathJIT);
         context.restoreRegisters(fastPathJIT);
         allocator.restoreReusedRegistersByPopping(fastPathJIT, numberOfBytesUsedToPreserveReusedRegisters,
index 4748fb6..20e0250 100644 (file)
@@ -44,10 +44,12 @@ void JITAddGenerator::generateFastPath(CCallHelpers& jit)
     ASSERT(!m_leftIsConstInt32 || !m_rightIsConstInt32);
     
     if (!m_leftType.mightBeNumber() || !m_rightType.mightBeNumber()) {
-        m_slowPathJumpList.append(jit.jump());
+        ASSERT(!m_didEmitFastPath);
         return;
     }
 
+    m_didEmitFastPath = true;
+
     if (m_leftIsConstInt32 || m_rightIsConstInt32) {
         JSValueRegs var;
         ResultType varType = ResultType::unknownType();
@@ -137,8 +139,6 @@ void JITAddGenerator::generateFastPath(CCallHelpers& jit)
     // Do doubleVar + doubleVar.
     jit.addDouble(m_rightFPR, m_leftFPR);
     jit.boxDouble(m_leftFPR, m_result);
-
-    m_endJumpList.append(jit.jump());
 }
 
 } // namespace JSC
index 6d62a0f..34fb653 100644 (file)
@@ -58,6 +58,7 @@ public:
 
     void generateFastPath(CCallHelpers&);
 
+    bool didEmitFastPath() const { return m_didEmitFastPath; }
     CCallHelpers::JumpList endJumpList() { return m_endJumpList; }
     CCallHelpers::JumpList slowPathJumpList() { return m_slowPathJumpList; }
 
@@ -75,6 +76,7 @@ private:
     FPRReg m_rightFPR;
     GPRReg m_scratchGPR;
     FPRReg m_scratchFPR;
+    bool m_didEmitFastPath { false };
 
     CCallHelpers::JumpList m_endJumpList;
     CCallHelpers::JumpList m_slowPathJumpList;
index 4507ddd..c3c2ae4 100644 (file)
@@ -958,13 +958,14 @@ void JIT::emit_op_add(Instruction* currentInstruction)
 
     gen.generateFastPath(*this);
 
-    if (!gen.endJumpList().empty()) {
+    if (gen.didEmitFastPath()) {
         gen.endJumpList().link(this);
         emitPutVirtualRegister(result, resultRegs);
         
         addSlowCase(gen.slowPathJumpList());
     } else {
-        gen.slowPathJumpList().link(this);
+        ASSERT(gen.endJumpList().empty());
+        ASSERT(gen.slowPathJumpList().empty());
         JITSlowPathCall slowPathCall(this, currentInstruction, slow_path_add);
         slowPathCall.call();
     }
@@ -1006,6 +1007,8 @@ void JIT::emit_op_sub(Instruction* currentInstruction)
         fpRegT0, fpRegT1, scratchGPR, scratchFPR);
 
     gen.generateFastPath(*this);
+
+    ASSERT(gen.didEmitFastPath());
     gen.endJumpList().link(this);
     emitPutVirtualRegister(result, resultRegs);
 
index ca8fd82..ae9bde6 100644 (file)
@@ -40,6 +40,9 @@ void JITSubGenerator::generateFastPath(CCallHelpers& jit)
     ASSERT(m_scratchGPR != m_right.tagGPR());
     ASSERT(m_scratchFPR != InvalidFPRReg);
 #endif
+
+    m_didEmitFastPath = true;
+
     CCallHelpers::Jump leftNotInt = jit.branchIfNotInt32(m_left);
     CCallHelpers::Jump rightNotInt = jit.branchIfNotInt32(m_right);
 
@@ -81,8 +84,6 @@ void JITSubGenerator::generateFastPath(CCallHelpers& jit)
 
     jit.subDouble(m_rightFPR, m_leftFPR);
     jit.boxDouble(m_leftFPR, m_result);
-
-    m_endJumpList.append(jit.jump());
 }
 
 } // namespace JSC
index eb879d6..447ce9d 100644 (file)
@@ -51,6 +51,7 @@ public:
 
     void generateFastPath(CCallHelpers&);
 
+    bool didEmitFastPath() const { return m_didEmitFastPath; }
     CCallHelpers::JumpList endJumpList() { return m_endJumpList; }
     CCallHelpers::JumpList slowPathJumpList() { return m_slowPathJumpList; }
 
@@ -64,6 +65,7 @@ private:
     FPRReg m_rightFPR;
     GPRReg m_scratchGPR;
     FPRReg m_scratchFPR;
+    bool m_didEmitFastPath { false };
 
     CCallHelpers::JumpList m_endJumpList;
     CCallHelpers::JumpList m_slowPathJumpList;