Skip to content

Commit 77eaa4a

Browse files
author
Mark Lam
committed
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): Canonical link: https://commits.webkit.org/169569@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@192599 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent f777a4d commit 77eaa4a

8 files changed

Lines changed: 62 additions & 7 deletions

File tree

‎Source/JavaScriptCore/ChangeLog‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,43 @@
1+
2015-11-18 Mark Lam <mark.lam@apple.com>
2+
3+
Remove some unnecessary jumps in snippet code.
4+
https://bugs.webkit.org/show_bug.cgi?id=151415
5+
6+
Reviewed by Geoffrey Garen.
7+
8+
Previously, the snippet generators always emit a jump at the end of the fast
9+
path. In the baseline JIT and FTL, this results in a jump to the very next
10+
instruction. I've change it to assume that the fast path will just fall thru,
11+
and let the client decide instead if it wants/needs a jump or not after the fast
12+
path.
13+
14+
I also changed the generators to provide a didEmitFastPath() query explicitly
15+
declare if they actually generated the fast path, instead of having the client
16+
infer it from an empty endJumpList.
17+
18+
Benchmarks show that perf is neutral with this change (tested on x86_64).
19+
20+
* dfg/DFGSpeculativeJIT.cpp:
21+
(JSC::DFG::SpeculativeJIT::compileValueAdd):
22+
(JSC::DFG::SpeculativeJIT::compileArithSub):
23+
* ftl/FTLCompile.cpp:
24+
(JSC::FTL::mmAllocateDataSection):
25+
* jit/JITAddGenerator.cpp:
26+
(JSC::JITAddGenerator::generateFastPath):
27+
* jit/JITAddGenerator.h:
28+
(JSC::JITAddGenerator::didEmitFastPath):
29+
(JSC::JITAddGenerator::endJumpList):
30+
(JSC::JITAddGenerator::slowPathJumpList):
31+
* jit/JITArithmetic.cpp:
32+
(JSC::JIT::emit_op_add):
33+
(JSC::JIT::emit_op_sub):
34+
* jit/JITSubGenerator.cpp:
35+
(JSC::JITSubGenerator::generateFastPath):
36+
* jit/JITSubGenerator.h:
37+
(JSC::JITSubGenerator::didEmitFastPath):
38+
(JSC::JITSubGenerator::endJumpList):
39+
(JSC::JITSubGenerator::slowPathJumpList):
40+
141
2015-11-18 Commit Queue <commit-queue@webkit.org>
242

343
Unreviewed, rolling out r192436 and r192586.

‎Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2880,6 +2880,9 @@ void SpeculativeJIT::compileValueAdd(Node* node)
28802880
leftFPR, rightFPR, scratchGPR, scratchFPR);
28812881
gen.generateFastPath(m_jit);
28822882

2883+
ASSERT(gen.didEmitFastPath());
2884+
gen.endJumpList().append(m_jit.jump());
2885+
28832886
gen.slowPathJumpList().link(&m_jit);
28842887

28852888
silentSpillAllRegisters(resultRegs);
@@ -3240,6 +3243,9 @@ void SpeculativeJIT::compileArithSub(Node* node)
32403243
leftFPR, rightFPR, scratchGPR, scratchFPR);
32413244
gen.generateFastPath(m_jit);
32423245

3246+
ASSERT(gen.didEmitFastPath());
3247+
gen.endJumpList().append(m_jit.jump());
3248+
32433249
gen.slowPathJumpList().link(&m_jit);
32443250
silentSpillAllRegisters(resultRegs);
32453251
callOperation(operationValueSub, resultRegs, leftRegs, rightRegs);

‎Source/JavaScriptCore/ftl/FTLCompile.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,7 @@ static void generateArithSubICFastPath(
462462
context.initializeRegisters(fastPathJIT);
463463
gen.generateFastPath(fastPathJIT);
464464

465+
ASSERT(gen.didEmitFastPath());
465466
gen.endJumpList().link(&fastPathJIT);
466467
context.restoreRegisters(fastPathJIT);
467468
allocator.restoreReusedRegistersByPopping(fastPathJIT, numberOfBytesUsedToPreserveReusedRegisters,

‎Source/JavaScriptCore/jit/JITAddGenerator.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,12 @@ void JITAddGenerator::generateFastPath(CCallHelpers& jit)
4444
ASSERT(!m_leftIsConstInt32 || !m_rightIsConstInt32);
4545

4646
if (!m_leftType.mightBeNumber() || !m_rightType.mightBeNumber()) {
47-
m_slowPathJumpList.append(jit.jump());
47+
ASSERT(!m_didEmitFastPath);
4848
return;
4949
}
5050

51+
m_didEmitFastPath = true;
52+
5153
if (m_leftIsConstInt32 || m_rightIsConstInt32) {
5254
JSValueRegs var;
5355
ResultType varType = ResultType::unknownType();
@@ -137,8 +139,6 @@ void JITAddGenerator::generateFastPath(CCallHelpers& jit)
137139
// Do doubleVar + doubleVar.
138140
jit.addDouble(m_rightFPR, m_leftFPR);
139141
jit.boxDouble(m_leftFPR, m_result);
140-
141-
m_endJumpList.append(jit.jump());
142142
}
143143

144144
} // namespace JSC

‎Source/JavaScriptCore/jit/JITAddGenerator.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ class JITAddGenerator {
5858

5959
void generateFastPath(CCallHelpers&);
6060

61+
bool didEmitFastPath() const { return m_didEmitFastPath; }
6162
CCallHelpers::JumpList endJumpList() { return m_endJumpList; }
6263
CCallHelpers::JumpList slowPathJumpList() { return m_slowPathJumpList; }
6364

@@ -75,6 +76,7 @@ class JITAddGenerator {
7576
FPRReg m_rightFPR;
7677
GPRReg m_scratchGPR;
7778
FPRReg m_scratchFPR;
79+
bool m_didEmitFastPath { false };
7880

7981
CCallHelpers::JumpList m_endJumpList;
8082
CCallHelpers::JumpList m_slowPathJumpList;

‎Source/JavaScriptCore/jit/JITArithmetic.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -958,13 +958,14 @@ void JIT::emit_op_add(Instruction* currentInstruction)
958958

959959
gen.generateFastPath(*this);
960960

961-
if (!gen.endJumpList().empty()) {
961+
if (gen.didEmitFastPath()) {
962962
gen.endJumpList().link(this);
963963
emitPutVirtualRegister(result, resultRegs);
964964

965965
addSlowCase(gen.slowPathJumpList());
966966
} else {
967-
gen.slowPathJumpList().link(this);
967+
ASSERT(gen.endJumpList().empty());
968+
ASSERT(gen.slowPathJumpList().empty());
968969
JITSlowPathCall slowPathCall(this, currentInstruction, slow_path_add);
969970
slowPathCall.call();
970971
}
@@ -1006,6 +1007,8 @@ void JIT::emit_op_sub(Instruction* currentInstruction)
10061007
fpRegT0, fpRegT1, scratchGPR, scratchFPR);
10071008

10081009
gen.generateFastPath(*this);
1010+
1011+
ASSERT(gen.didEmitFastPath());
10091012
gen.endJumpList().link(this);
10101013
emitPutVirtualRegister(result, resultRegs);
10111014

‎Source/JavaScriptCore/jit/JITSubGenerator.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ void JITSubGenerator::generateFastPath(CCallHelpers& jit)
4040
ASSERT(m_scratchGPR != m_right.tagGPR());
4141
ASSERT(m_scratchFPR != InvalidFPRReg);
4242
#endif
43+
44+
m_didEmitFastPath = true;
45+
4346
CCallHelpers::Jump leftNotInt = jit.branchIfNotInt32(m_left);
4447
CCallHelpers::Jump rightNotInt = jit.branchIfNotInt32(m_right);
4548

@@ -81,8 +84,6 @@ void JITSubGenerator::generateFastPath(CCallHelpers& jit)
8184

8285
jit.subDouble(m_rightFPR, m_leftFPR);
8386
jit.boxDouble(m_leftFPR, m_result);
84-
85-
m_endJumpList.append(jit.jump());
8687
}
8788

8889
} // namespace JSC

‎Source/JavaScriptCore/jit/JITSubGenerator.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ class JITSubGenerator {
5151

5252
void generateFastPath(CCallHelpers&);
5353

54+
bool didEmitFastPath() const { return m_didEmitFastPath; }
5455
CCallHelpers::JumpList endJumpList() { return m_endJumpList; }
5556
CCallHelpers::JumpList slowPathJumpList() { return m_slowPathJumpList; }
5657

@@ -64,6 +65,7 @@ class JITSubGenerator {
6465
FPRReg m_rightFPR;
6566
GPRReg m_scratchGPR;
6667
FPRReg m_scratchFPR;
68+
bool m_didEmitFastPath { false };
6769

6870
CCallHelpers::JumpList m_endJumpList;
6971
CCallHelpers::JumpList m_slowPathJumpList;

0 commit comments

Comments
 (0)