Skip to content

Commit 3a4c621

Browse files
author
Gavin Barraclough
committed
Bug 58701 - DFG JIT - add GetLocal/SetLocal nodes
Reviewed by Geoff Garen. Use these for both access to arguments & local variables, adds ability to set locals, such that values will persist between basic blocks. * dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::ByteCodeParser): (JSC::DFG::ByteCodeParser::get): (JSC::DFG::ByteCodeParser::set): (JSC::DFG::ByteCodeParser::getVariable): (JSC::DFG::ByteCodeParser::setVariable): (JSC::DFG::ByteCodeParser::getArgument): (JSC::DFG::ByteCodeParser::setArgument): (JSC::DFG::ByteCodeParser::getThis): (JSC::DFG::ByteCodeParser::setThis): (JSC::DFG::ByteCodeParser::VariableRecord::VariableRecord): (JSC::DFG::ByteCodeParser::parseBlock): * dfg/DFGGraph.cpp: (JSC::DFG::Graph::dump): (JSC::DFG::Graph::derefChildren): * dfg/DFGGraph.h: (JSC::DFG::Graph::ref): (JSC::DFG::Graph::deref): * dfg/DFGNode.h: (JSC::DFG::Node::hasLocal): (JSC::DFG::Node::local): * dfg/DFGNonSpeculativeJIT.cpp: (JSC::DFG::NonSpeculativeJIT::compile): * dfg/DFGSpeculativeJIT.cpp: (JSC::DFG::SpeculativeJIT::compile): Canonical link: https://commits.webkit.org/73782@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@84041 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 7e00585 commit 3a4c621

7 files changed

Lines changed: 177 additions & 55 deletions

File tree

‎Source/JavaScriptCore/ChangeLog‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,38 @@
1+
2011-04-15 Gavin Barraclough <barraclough@apple.com>
2+
3+
Reviewed by Geoff Garen.
4+
5+
Bug 58701 - DFG JIT - add GetLocal/SetLocal nodes
6+
7+
Use these for both access to arguments & local variables, adds ability
8+
to set locals, such that values will persist between basic blocks.
9+
10+
* dfg/DFGByteCodeParser.cpp:
11+
(JSC::DFG::ByteCodeParser::ByteCodeParser):
12+
(JSC::DFG::ByteCodeParser::get):
13+
(JSC::DFG::ByteCodeParser::set):
14+
(JSC::DFG::ByteCodeParser::getVariable):
15+
(JSC::DFG::ByteCodeParser::setVariable):
16+
(JSC::DFG::ByteCodeParser::getArgument):
17+
(JSC::DFG::ByteCodeParser::setArgument):
18+
(JSC::DFG::ByteCodeParser::getThis):
19+
(JSC::DFG::ByteCodeParser::setThis):
20+
(JSC::DFG::ByteCodeParser::VariableRecord::VariableRecord):
21+
(JSC::DFG::ByteCodeParser::parseBlock):
22+
* dfg/DFGGraph.cpp:
23+
(JSC::DFG::Graph::dump):
24+
(JSC::DFG::Graph::derefChildren):
25+
* dfg/DFGGraph.h:
26+
(JSC::DFG::Graph::ref):
27+
(JSC::DFG::Graph::deref):
28+
* dfg/DFGNode.h:
29+
(JSC::DFG::Node::hasLocal):
30+
(JSC::DFG::Node::local):
31+
* dfg/DFGNonSpeculativeJIT.cpp:
32+
(JSC::DFG::NonSpeculativeJIT::compile):
33+
* dfg/DFGSpeculativeJIT.cpp:
34+
(JSC::DFG::SpeculativeJIT::compile):
35+
136
2011-04-15 Gavin Barraclough <barraclough@apple.com>
237

338
Reviewed by Sam Weinig.

‎Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp‎

Lines changed: 73 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,6 @@ class ByteCodeParser {
6060
, m_variables(codeBlock->m_numVars)
6161
, m_temporaries(codeBlock->m_numCalleeRegisters - codeBlock->m_numVars)
6262
{
63-
for (unsigned i = 0; i < m_arguments.size(); ++i)
64-
m_arguments[i] = NoNode;
65-
for (unsigned i = 0; i < m_variables.size(); ++i)
66-
m_variables[i] = NoNode;
6763
for (unsigned i = 0; i < m_temporaries.size(); ++i)
6864
m_temporaries[i] = NoNode;
6965
}
@@ -86,11 +82,8 @@ class ByteCodeParser {
8682
}
8783

8884
// Is this an argument?
89-
if (operand < 0) {
90-
unsigned argument = operand + m_codeBlock->m_numParameters + RegisterFile::CallFrameHeaderSize;
91-
ASSERT(argument < m_arguments.size());
92-
return getArgument(argument);
93-
}
85+
if (operand < 0)
86+
return getArgument(operand);
9487

9588
// Is this a variable?
9689
unsigned numVariables = m_variables.size();
@@ -106,9 +99,8 @@ class ByteCodeParser {
10699
{
107100
// Is this an argument?
108101
if (operand < 0) {
109-
unsigned argument = operand + m_codeBlock->m_numParameters + RegisterFile::CallFrameHeaderSize;
110-
ASSERT(argument < m_arguments.size());
111-
return setArgument(argument, value);
102+
setArgument(operand, value);
103+
return;
112104
}
113105

114106
// Is this a variable?
@@ -127,19 +119,24 @@ class ByteCodeParser {
127119
// Used in implementing get/set, above, where the operand is a local variable.
128120
NodeIndex getVariable(unsigned operand)
129121
{
130-
NodeIndex index = m_variables[operand];
131-
if (index != NoNode)
132-
return index;
133-
// We have not yet seen a definition for this value in this block.
134-
// For now, since we are only generating single block functions,
135-
// this value must be undefined.
136-
// For example:
137-
// function f() { var x; return x; }
138-
return constantUndefined();
122+
NodeIndex setNode = m_variables[operand].set;
123+
if (setNode != NoNode)
124+
return m_graph[setNode].child1;
125+
126+
NodeIndex getNode = m_variables[operand].get;
127+
if (getNode != NoNode)
128+
return getNode;
129+
130+
getNode = addToGraph(GetLocal, OpInfo(operand));
131+
m_variables[operand].get = getNode;
132+
return getNode;
139133
}
140-
void setVariable(int operand, NodeIndex value)
134+
void setVariable(unsigned operand, NodeIndex value)
141135
{
142-
m_variables[operand] = value;
136+
NodeIndex priorSet = m_variables[operand].set;
137+
m_variables[operand].set = addToGraph(SetLocal, OpInfo(operand), value);
138+
if (priorSet != NoNode)
139+
m_graph.deref(priorSet);
143140
}
144141

145142
// Used in implementing get/set, above, where the operand is a temporary.
@@ -159,17 +156,32 @@ class ByteCodeParser {
159156
}
160157

161158
// Used in implementing get/set, above, where the operand is an argument.
162-
NodeIndex getArgument(unsigned argument)
159+
NodeIndex getArgument(unsigned operand)
163160
{
164-
NodeIndex index = m_arguments[argument];
165-
if (index != NoNode)
166-
return index;
167-
NodeIndex resultIndex = addToGraph(Argument, OpInfo(argument));
168-
return m_arguments[argument] = resultIndex;
161+
unsigned argument = operand + m_codeBlock->m_numParameters + RegisterFile::CallFrameHeaderSize;
162+
ASSERT(argument < m_arguments.size());
163+
164+
NodeIndex setNode = m_arguments[argument].set;
165+
if (setNode != NoNode)
166+
return m_graph[setNode].child1;
167+
168+
NodeIndex getNode = m_arguments[argument].get;
169+
if (getNode != NoNode)
170+
return getNode;
171+
172+
getNode = addToGraph(GetLocal, OpInfo(operand));
173+
m_arguments[argument].get = getNode;
174+
return getNode;
169175
}
170176
void setArgument(int operand, NodeIndex value)
171177
{
172-
m_arguments[operand] = value;
178+
unsigned argument = operand + m_codeBlock->m_numParameters + RegisterFile::CallFrameHeaderSize;
179+
ASSERT(argument < m_arguments.size());
180+
181+
NodeIndex priorSet = m_arguments[argument].set;
182+
m_arguments[argument].set = addToGraph(SetLocal, OpInfo(operand), value);
183+
if (priorSet != NoNode)
184+
m_graph.deref(priorSet);
173185
}
174186

175187
// Get an operand, and perform a ToInt32/ToNumber conversion on it.
@@ -300,11 +312,11 @@ class ByteCodeParser {
300312
// Helper functions to get/set the this value.
301313
NodeIndex getThis()
302314
{
303-
return getArgument(0);
315+
return getArgument(m_codeBlock->thisRegister());
304316
}
305317
void setThis(NodeIndex value)
306318
{
307-
setArgument(0, value);
319+
setArgument(m_codeBlock->thisRegister(), value);
308320
}
309321

310322
// Convenience methods for checking nodes for constants.
@@ -469,12 +481,26 @@ class ByteCodeParser {
469481
NodeIndex asNumeric;
470482
NodeIndex asJSValue;
471483
};
472-
Vector <ConstantRecord, 32> m_constants;
484+
485+
// For every local variable we track any existing get or set of the value.
486+
// We track the get so that these may be shared, and we track the set to
487+
// retrieve the current value, and to reference the final definition.
488+
struct VariableRecord {
489+
VariableRecord()
490+
: get(NoNode)
491+
, set(NoNode)
492+
{
493+
}
494+
495+
NodeIndex get;
496+
NodeIndex set;
497+
};
473498

474499
// Track the index of the node whose result is the current value for every
475500
// register value in the bytecode - argument, local, and temporary.
476-
Vector <NodeIndex, 32> m_arguments;
477-
Vector <NodeIndex, 32> m_variables;
501+
Vector <ConstantRecord, 32> m_constants;
502+
Vector <VariableRecord, 32> m_arguments;
503+
Vector <VariableRecord, 32> m_variables;
478504
Vector <NodeIndex, 32> m_temporaries;
479505

480506
// These maps are used to unique ToNumber and ToInt32 operations.
@@ -505,8 +531,9 @@ bool ByteCodeParser::parseBlock()
505531
// === Function entry opcodes ===
506532

507533
case op_enter:
508-
// This is a no-op for now - may need to initialize locals, if
509-
// DCE analysis cannot determine that the values are never read.
534+
// Initialize all locals to undefined.
535+
for (int i = 0; i < m_codeBlock->m_numVars; ++i)
536+
set(i, constantUndefined());
510537
NEXT_OPCODE(op_enter);
511538

512539
case op_convert_this: {
@@ -818,6 +845,15 @@ bool ByteCodeParser::parseBlock()
818845

819846
case op_ret: {
820847
addToGraph(Return, get(currentInstruction[1].u.operand));
848+
849+
// FIXME: throw away terminal definitions of variables;
850+
// should not be necessary once we have proper DCE!
851+
for (unsigned i = 0; i < m_variables.size(); ++i) {
852+
NodeIndex priorSet = m_variables[i].set;
853+
if (priorSet != NoNode)
854+
m_graph.deref(priorSet);
855+
}
856+
821857
LAST_OPCODE(op_ret);
822858
}
823859

‎Source/JavaScriptCore/dfg/DFGGraph.cpp‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,12 @@ void Graph::dump(NodeIndex nodeIndex, CodeBlock* codeBlock)
9090
printf("%sid%u", hasPrinted ? ", " : "", node.identifierNumber());
9191
hasPrinted = true;
9292
}
93-
if (node.isArgument()) {
94-
printf("%sarg%u", hasPrinted ? ", " : "", node.argumentNumber());
93+
if (node.hasLocal()) {
94+
int local = node.local();
95+
if (local < 0)
96+
printf("%sarg%u", hasPrinted ? ", " : "", local - codeBlock->thisRegister());
97+
else
98+
printf("%sr%u", hasPrinted ? ", " : "", local);
9599
hasPrinted = true;
96100
}
97101
if (op == Int32Constant) {
@@ -118,7 +122,7 @@ void Graph::dump(CodeBlock* codeBlock)
118122

119123
#endif
120124

121-
// FIXME: Convert this method to be iterative, not recursive.
125+
// FIXME: Convert these methods to be iterative, not recursive.
122126
void Graph::refChildren(NodeIndex op)
123127
{
124128
Node& node = at(op);
@@ -139,6 +143,26 @@ void Graph::refChildren(NodeIndex op)
139143
return;
140144
ref(node.child3);
141145
}
146+
void Graph::derefChildren(NodeIndex op)
147+
{
148+
Node& node = at(op);
149+
150+
if (node.child1 == NoNode) {
151+
ASSERT(node.child2 == NoNode && node.child3 == NoNode);
152+
return;
153+
}
154+
deref(node.child1);
155+
156+
if (node.child2 == NoNode) {
157+
ASSERT(node.child3 == NoNode);
158+
return;
159+
}
160+
deref(node.child2);
161+
162+
if (node.child3 == NoNode)
163+
return;
164+
deref(node.child3);
165+
}
142166

143167
} } // namespace JSC::DFG
144168

‎Source/JavaScriptCore/dfg/DFGGraph.h‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,19 @@ class Graph : public Vector<Node, 64> {
4848
// Mark a node as being referenced.
4949
void ref(NodeIndex nodeIndex)
5050
{
51-
// If the value (before incrementing) was at reCount zero then we need to ref its children.
52-
if (!at(nodeIndex).refCount++)
51+
Node& node = at(nodeIndex);
52+
// If the value (before incrementing) was at refCount zero then we need to ref its children.
53+
if (!node.refCount++)
5354
refChildren(nodeIndex);
5455
}
56+
void deref(NodeIndex nodeIndex)
57+
{
58+
Node& node = at(nodeIndex);
59+
ASSERT(node.refCount);
60+
// If the value (after decrementing) becomes refCount zero then we need to deref its children.
61+
if (!--node.refCount)
62+
derefChildren(nodeIndex);
63+
}
5564

5665
#ifndef NDEBUG
5766
// CodeBlock is optional, but may allow additional information to be dumped (e.g. Identifier names).
@@ -60,8 +69,9 @@ class Graph : public Vector<Node, 64> {
6069
#endif
6170

6271
private:
63-
// When a node's refCount goes from 0 to 1, it must (logically) recursively ref all of its children.
72+
// When a node's refCount goes from 0 to 1, it must (logically) recursively ref all of its children, and vice versa.
6473
void refChildren(NodeIndex);
74+
void derefChildren(NodeIndex);
6575
};
6676

6777
} } // namespace JSC::DFG

‎Source/JavaScriptCore/dfg/DFGNode.h‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,12 @@ typedef uint32_t ExceptionInfo;
7575
macro(JSConstant, NodeResultJS | NodeIsConstant) \
7676
macro(Int32Constant, NodeResultJS | NodeIsConstant) \
7777
macro(DoubleConstant, NodeResultJS | NodeIsConstant) \
78-
macro(Argument, NodeResultJS) \
7978
macro(ConvertThis, NodeResultJS) \
8079
\
80+
/* Nodes for local variable access. */\
81+
macro(GetLocal, NodeResultJS) \
82+
macro(SetLocal, NodeMustGenerate) \
83+
\
8184
/* Nodes for bitwise operations. */\
8285
macro(BitAnd, NodeResultInt32) \
8386
macro(BitOr, NodeResultInt32) \
@@ -197,15 +200,15 @@ struct Node {
197200
return m_opInfo;
198201
}
199202

200-
bool isArgument()
203+
bool hasLocal()
201204
{
202-
return op == Argument;
205+
return op == GetLocal || op == SetLocal;
203206
}
204207

205-
unsigned argumentNumber()
208+
VirtualRegister local()
206209
{
207-
ASSERT(isArgument());
208-
return m_opInfo;
210+
ASSERT(hasLocal());
211+
return (VirtualRegister)m_opInfo;
209212
}
210213

211214
bool hasIdentifier()

‎Source/JavaScriptCore/dfg/DFGNonSpeculativeJIT.cpp‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -198,14 +198,21 @@ void NonSpeculativeJIT::compile(SpeculationCheckIndexIterator& checkIterator, No
198198
case JSConstant:
199199
initConstantInfo(m_compileIndex);
200200
break;
201-
202-
case Argument: {
201+
202+
case GetLocal: {
203203
GPRTemporary result(this);
204-
m_jit.loadPtr(m_jit.addressForArgument(node.argumentNumber()), result.registerID());
204+
m_jit.loadPtr(JITCompiler::addressFor(node.local()), result.registerID());
205205
jsValueResult(result.gpr(), m_compileIndex);
206206
break;
207207
}
208208

209+
case SetLocal: {
210+
JSValueOperand value(this, node.child1);
211+
m_jit.storePtr(value.registerID(), JITCompiler::addressFor(node.local()));
212+
noResult(m_compileIndex);
213+
break;
214+
}
215+
209216
case BitAnd:
210217
case BitOr:
211218
case BitXor:

‎Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,14 +251,21 @@ bool SpeculativeJIT::compile(Node& node)
251251
case JSConstant:
252252
initConstantInfo(m_compileIndex);
253253
break;
254-
255-
case Argument: {
254+
255+
case GetLocal: {
256256
GPRTemporary result(this);
257-
m_jit.loadPtr(m_jit.addressForArgument(node.argumentNumber()), result.registerID());
257+
m_jit.loadPtr(JITCompiler::addressFor(node.local()), result.registerID());
258258
jsValueResult(result.gpr(), m_compileIndex);
259259
break;
260260
}
261261

262+
case SetLocal: {
263+
JSValueOperand value(this, node.child1);
264+
m_jit.storePtr(value.registerID(), JITCompiler::addressFor(node.local()));
265+
noResult(m_compileIndex);
266+
break;
267+
}
268+
262269
case BitAnd:
263270
case BitOr:
264271
case BitXor:

0 commit comments

Comments
 (0)