Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (100314 => 100315)
--- trunk/Source/_javascript_Core/ChangeLog 2011-11-15 21:38:35 UTC (rev 100314)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-11-15 21:54:38 UTC (rev 100315)
@@ -1,3 +1,51 @@
+2011-11-15 Filip Pizlo <fpi...@apple.com>
+
+ DFG should distinguish between constants in the constant pool and weak
+ constants added as artifacts of code generation
+ https://bugs.webkit.org/show_bug.cgi?id=72367
+
+ Reviewed by Geoff Garen.
+
+ Added the notion of a WeakJSConstant, which is like a JSConstant except that
+ it can only refer to JSCell*. Currently all WeakJSConstants are also backed
+ by constants in the constant pool, since weak references originated from
+ machine code are not yet properly handled.
+
+ Replaced CheckMethod, and MethodCheckData, with a combination of WeakJSConstant
+ and CheckStructure. This results in improved CSE, leading to a 1% win on V8.
+
+ * dfg/DFGAbstractState.cpp:
+ (JSC::DFG::AbstractState::execute):
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::cellConstant):
+ (JSC::DFG::ByteCodeParser::prepareToParseBlock):
+ (JSC::DFG::ByteCodeParser::parseBlock):
+ * dfg/DFGGraph.cpp:
+ (JSC::DFG::Graph::dump):
+ * dfg/DFGGraph.h:
+ (JSC::DFG::Graph::getJSConstantPrediction):
+ (JSC::DFG::Graph::valueOfJSConstant):
+ (JSC::DFG::Graph::valueOfInt32Constant):
+ (JSC::DFG::Graph::valueOfNumberConstant):
+ (JSC::DFG::Graph::valueOfBooleanConstant):
+ * dfg/DFGNode.h:
+ (JSC::DFG::Node::isWeakConstant):
+ (JSC::DFG::Node::hasConstant):
+ (JSC::DFG::Node::weakConstant):
+ (JSC::DFG::Node::valueOfJSConstant):
+ (JSC::DFG::Node::isInt32Constant):
+ (JSC::DFG::Node::isDoubleConstant):
+ (JSC::DFG::Node::isNumberConstant):
+ (JSC::DFG::Node::isBooleanConstant):
+ (JSC::DFG::Node::hasIdentifier):
+ * dfg/DFGPropagator.cpp:
+ (JSC::DFG::Propagator::propagateNodePredictions):
+ (JSC::DFG::Propagator::performNodeCSE):
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+
2011-11-15 Michael Saboff <msab...@apple.com>
Towards 8 bit Strings - Initial JS String Tuning
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractState.cpp (100314 => 100315)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractState.cpp 2011-11-15 21:38:35 UTC (rev 100314)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractState.cpp 2011-11-15 21:54:38 UTC (rev 100315)
@@ -168,7 +168,8 @@
return true;
switch (node.op) {
- case JSConstant: {
+ case JSConstant:
+ case WeakJSConstant: {
JSValue value = m_graph.valueOfJSConstant(m_codeBlock, nodeIndex);
if (value.isCell())
m_haveStructures = true;
@@ -601,13 +602,6 @@
forNode(node.child1()).filter(PredictCell);
break;
- case CheckMethod:
- // FIXME: We should be able to propagate the structure sets of constants (i.e. prototypes).
- forNode(node.child1()).filter(m_graph.m_methodCheckData[node.methodCheckDataIndex()].structure);
- forNode(nodeIndex).set(PredictFunction);
- m_haveStructures = true;
- break;
-
case CheckFunction:
forNode(node.child1()).filter(PredictFunction);
// FIXME: Should be able to propagate the fact that we know what the function is.
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (100314 => 100315)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2011-11-15 21:38:35 UTC (rev 100314)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2011-11-15 21:54:38 UTC (rev 100315)
@@ -531,7 +531,11 @@
NodeIndex cellConstant(JSCell* cell)
{
- return getJSConstant(getCellConstantIndex(cell));
+ pair<HashMap<JSCell*, NodeIndex>::iterator, bool> iter = m_cellConstantNodes.add(cell, NoNode);
+ if (iter.second)
+ iter.first->second = addToGraph(WeakJSConstant, OpInfo(cell));
+
+ return iter.first->second;
}
CodeOrigin currentCodeOrigin()
@@ -734,6 +738,7 @@
unsigned m_constantNaN;
unsigned m_constant1;
HashMap<JSCell*, unsigned> m_cellConstants;
+ HashMap<JSCell*, NodeIndex> m_cellConstantNodes;
// A constant in the constant pool may be represented by more than one
// node in the graph, depending on the context in which it is being used.
@@ -1255,6 +1260,7 @@
{
for (unsigned i = 0; i < m_constants.size(); ++i)
m_constants[i] = ConstantRecord();
+ m_cellConstantNodes.clear();
}
bool ByteCodeParser::parseBlock(unsigned limit)
@@ -1668,20 +1674,16 @@
// It's monomorphic as far as we can tell, since the method_check was linked
// but the slow path (i.e. the normal get_by_id) never fired.
- pinCell(methodCall.cachedStructure.get());
- pinCell(methodCall.cachedPrototypeStructure.get());
- pinCell(methodCall.cachedFunction.get());
- pinCell(methodCall.cachedPrototype.get());
-
- NodeIndex checkMethod = addToGraph(CheckMethod, OpInfo(identifier), OpInfo(m_graph.m_methodCheckData.size()), base);
- set(getInstruction[1].u.operand, checkMethod);
+ pinCell(methodCall.cachedStructure.get()); // first check
+ pinCell(methodCall.cachedPrototype.get()); // second check
+ pinCell(methodCall.cachedPrototypeStructure.get()); // second check
+ pinCell(methodCall.cachedFunction.get()); // result
- MethodCheckData methodCheckData;
- methodCheckData.structure = methodCall.cachedStructure.get();
- methodCheckData.prototypeStructure = methodCall.cachedPrototypeStructure.get();
- methodCheckData.function = methodCall.cachedFunction.get();
- methodCheckData.prototype = methodCall.cachedPrototype.get();
- m_graph.m_methodCheckData.append(methodCheckData);
+ addToGraph(CheckStructure, OpInfo(m_graph.addStructureSet(methodCall.cachedStructure.get())), base);
+ if (methodCall.cachedPrototype.get() != m_inlineStackTop->m_profiledBlock->globalObject()->methodCallDummy())
+ addToGraph(CheckStructure, OpInfo(m_graph.addStructureSet(methodCall.cachedPrototypeStructure.get())), cellConstant(methodCall.cachedPrototype.get()));
+
+ set(getInstruction[1].u.operand, cellConstant(methodCall.cachedFunction.get()));
} else {
NodeIndex getMethod = addToGraph(GetMethod, OpInfo(identifier), OpInfo(prediction), base);
set(getInstruction[1].u.operand, getMethod);
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (100314 => 100315)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2011-11-15 21:38:35 UTC (rev 100314)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2011-11-15 21:54:38 UTC (rev 100315)
@@ -242,6 +242,10 @@
}
hasPrinted = true;
}
+ if (op == WeakJSConstant) {
+ printf("%s%p", hasPrinted ? ", " : "", node.weakConstant());
+ hasPrinted = true;
+ }
if (node.isBranch() || node.isJump()) {
printf("%sT:#%u", hasPrinted ? ", " : "", node.takenBlockIndex());
hasPrinted = true;
@@ -261,31 +265,6 @@
printf(" predicting %s", predictionToString(getGlobalVarPrediction(node.varNumber())));
else if (node.hasHeapPrediction())
printf(" predicting %s", predictionToString(node.getHeapPrediction()));
- else if (node.hasMethodCheckData()) {
- MethodCheckData& methodCheckData = m_methodCheckData[node.methodCheckDataIndex()];
- JSCell* functionCell = getJSFunction(methodCheckData.function);
- ExecutableBase* executable = 0;
- CodeBlock* primaryForCall = 0;
- CodeBlock* secondaryForCall = 0;
- CodeBlock* primaryForConstruct = 0;
- CodeBlock* secondaryForConstruct = 0;
- if (functionCell) {
- JSFunction* function = asFunction(functionCell);
- executable = function->executable();
- if (!executable->isHostFunction()) {
- FunctionExecutable* functionExecutable = static_cast<FunctionExecutable*>(executable);
- if (functionExecutable->isGeneratedForCall()) {
- primaryForCall = &functionExecutable->generatedBytecodeForCall();
- secondaryForCall = primaryForCall->alternative();
- }
- if (functionExecutable->isGeneratedForConstruct()) {
- primaryForConstruct = &functionExecutable->generatedBytecodeForConstruct();
- secondaryForConstruct = primaryForConstruct->alternative();
- }
- }
- }
- printf(" predicting function %p(%p(%p(%p) %p(%p)))", methodCheckData.function, executable, primaryForCall, secondaryForCall, primaryForConstruct, secondaryForConstruct);
- }
}
printf("\n");
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.h (100314 => 100315)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.h 2011-11-15 21:38:35 UTC (rev 100314)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.h 2011-11-15 21:54:38 UTC (rev 100315)
@@ -45,33 +45,6 @@
namespace DFG {
-struct MethodCheckData {
- // It is safe to refer to these directly because they are shadowed by
- // the old JIT's CodeBlock's MethodCallLinkInfo.
- Structure* structure;
- Structure* prototypeStructure;
- JSObject* function;
- JSObject* prototype;
-
- bool operator==(const MethodCheckData& other) const
- {
- if (structure != other.structure)
- return false;
- if (prototypeStructure != other.prototypeStructure)
- return false;
- if (function != other.function)
- return false;
- if (prototype != other.prototype)
- return false;
- return true;
- }
-
- bool operator!=(const MethodCheckData& other) const
- {
- return !(*this == other);
- }
-};
-
struct StorageAccessData {
size_t offset;
unsigned identifierNumber;
@@ -129,14 +102,9 @@
return m_predictions.getGlobalVarPrediction(varNumber);
}
- PredictedType getMethodCheckPrediction(Node& node)
- {
- return predictionFromCell(m_methodCheckData[node.methodCheckDataIndex()].function);
- }
-
PredictedType getJSConstantPrediction(Node& node, CodeBlock* codeBlock)
{
- return predictionFromValue(node.valueOfJSConstantNode(codeBlock));
+ return predictionFromValue(node.valueOfJSConstant(codeBlock));
}
// Helper methods to check nodes for constants.
@@ -175,21 +143,19 @@
// Helper methods get constant values from nodes.
JSValue valueOfJSConstant(CodeBlock* codeBlock, NodeIndex nodeIndex)
{
- if (at(nodeIndex).hasMethodCheckData())
- return JSValue(m_methodCheckData[at(nodeIndex).methodCheckDataIndex()].function);
- return valueOfJSConstantNode(codeBlock, nodeIndex);
+ return at(nodeIndex).valueOfJSConstant(codeBlock);
}
int32_t valueOfInt32Constant(CodeBlock* codeBlock, NodeIndex nodeIndex)
{
- return valueOfJSConstantNode(codeBlock, nodeIndex).asInt32();
+ return valueOfJSConstant(codeBlock, nodeIndex).asInt32();
}
double valueOfNumberConstant(CodeBlock* codeBlock, NodeIndex nodeIndex)
{
- return valueOfJSConstantNode(codeBlock, nodeIndex).asNumber();
+ return valueOfJSConstant(codeBlock, nodeIndex).asNumber();
}
bool valueOfBooleanConstant(CodeBlock* codeBlock, NodeIndex nodeIndex)
{
- return valueOfJSConstantNode(codeBlock, nodeIndex).asBoolean();
+ return valueOfJSConstant(codeBlock, nodeIndex).asBoolean();
}
JSFunction* valueOfFunctionConstant(CodeBlock* codeBlock, NodeIndex nodeIndex)
{
@@ -256,7 +222,6 @@
Vector< OwnPtr<BasicBlock> , 8> m_blocks;
Vector<NodeIndex, 16> m_varArgChildren;
- Vector<MethodCheckData> m_methodCheckData;
Vector<StorageAccessData> m_storageAccessData;
Vector<ResolveGlobalData> m_resolveGlobalData;
Vector<NodeIndex, 8> m_arguments;
@@ -268,11 +233,6 @@
unsigned m_parameterSlots;
private:
- JSValue valueOfJSConstantNode(CodeBlock* codeBlock, NodeIndex nodeIndex)
- {
- return codeBlock->constantRegister(FirstConstantRegisterIndex + at(nodeIndex).constantNumber()).get();
- }
-
// When a node's refCount goes from 0 to 1, it must (logically) recursively ref all of its children, and vice versa.
void refChildren(NodeIndex);
Modified: trunk/Source/_javascript_Core/dfg/DFGNode.h (100314 => 100315)
--- trunk/Source/_javascript_Core/dfg/DFGNode.h 2011-11-15 21:38:35 UTC (rev 100314)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.h 2011-11-15 21:54:38 UTC (rev 100315)
@@ -157,9 +157,13 @@
// This macro defines a set of information about all known node types, used to populate NodeId, NodeType below.
#define FOR_EACH_DFG_OP(macro) \
- /* Nodes for constants. */\
+ /* A constant in the CodeBlock's constant pool. */\
macro(JSConstant, NodeResultJS) \
\
+ /* A constant not in the CodeBlock's constant pool. Uses get patched to jumps that exit the */\
+ /* code block. */\
+ macro(WeakJSConstant, NodeResultJS) \
+ \
/* Nodes for handling functions (both as call and as construct). */\
macro(ConvertThis, NodeResultJS) \
macro(CreateThis, NodeResultJS) /* Note this is not MustGenerate since we're returning it anyway. */ \
@@ -230,7 +234,6 @@
macro(GetStringLength, NodeResultInt32) \
macro(GetByteArrayLength, NodeResultInt32) \
macro(GetMethod, NodeResultJS | NodeMustGenerate) \
- macro(CheckMethod, NodeResultJS | NodeMustGenerate) \
macro(GetScopeChain, NodeResultJS) \
macro(GetScopedVar, NodeResultJS | NodeMustGenerate) \
macro(PutScopedVar, NodeMustGenerate | NodeClobbersWorld) \
@@ -397,9 +400,14 @@
return op == JSConstant;
}
+ bool isWeakConstant()
+ {
+ return op == WeakJSConstant;
+ }
+
bool hasConstant()
{
- return isConstant() || hasMethodCheckData();
+ return isConstant() || isWeakConstant();
}
unsigned constantNumber()
@@ -408,20 +416,26 @@
return m_opInfo;
}
- // NOTE: this only works for JSConstant nodes.
- JSValue valueOfJSConstantNode(CodeBlock* codeBlock)
+ JSCell* weakConstant()
{
+ return bitwise_cast<JSCell*>(m_opInfo);
+ }
+
+ JSValue valueOfJSConstant(CodeBlock* codeBlock)
+ {
+ if (op == WeakJSConstant)
+ return JSValue(weakConstant());
return codeBlock->constantRegister(FirstConstantRegisterIndex + constantNumber()).get();
}
bool isInt32Constant(CodeBlock* codeBlock)
{
- return isConstant() && valueOfJSConstantNode(codeBlock).isInt32();
+ return isConstant() && valueOfJSConstant(codeBlock).isInt32();
}
bool isDoubleConstant(CodeBlock* codeBlock)
{
- bool result = isConstant() && valueOfJSConstantNode(codeBlock).isDouble();
+ bool result = isConstant() && valueOfJSConstant(codeBlock).isDouble();
if (result)
ASSERT(!isInt32Constant(codeBlock));
return result;
@@ -429,14 +443,14 @@
bool isNumberConstant(CodeBlock* codeBlock)
{
- bool result = isConstant() && valueOfJSConstantNode(codeBlock).isNumber();
+ bool result = isConstant() && valueOfJSConstant(codeBlock).isNumber();
ASSERT(result == (isInt32Constant(codeBlock) || isDoubleConstant(codeBlock)));
return result;
}
bool isBooleanConstant(CodeBlock* codeBlock)
{
- return isConstant() && valueOfJSConstantNode(codeBlock).isBoolean();
+ return isConstant() && valueOfJSConstant(codeBlock).isBoolean();
}
bool hasVariableAccessData()
@@ -477,7 +491,6 @@
case PutById:
case PutByIdDirect:
case GetMethod:
- case CheckMethod:
case Resolve:
case ResolveBase:
case ResolveBaseStrictPut:
@@ -723,17 +736,6 @@
return mergePrediction(m_opInfo2, prediction);
}
- bool hasMethodCheckData()
- {
- return op == CheckMethod;
- }
-
- unsigned methodCheckDataIndex()
- {
- ASSERT(hasMethodCheckData());
- return m_opInfo2;
- }
-
bool hasFunctionCheckData()
{
return op == CheckFunction;
Modified: trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp (100314 => 100315)
--- trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp 2011-11-15 21:38:35 UTC (rev 100314)
+++ trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp 2011-11-15 21:54:38 UTC (rev 100315)
@@ -301,7 +301,8 @@
bool changed = false;
switch (op) {
- case JSConstant: {
+ case JSConstant:
+ case WeakJSConstant: {
changed |= setPrediction(predictionFromValue(m_graph.valueOfJSConstant(m_codeBlock, m_compileIndex)));
break;
}
@@ -473,11 +474,6 @@
break;
}
- case CheckMethod: {
- changed |= setPrediction(m_graph.getMethodCheckPrediction(node));
- break;
- }
-
case Call:
case Construct: {
if (node.getHeapPrediction())
@@ -1041,47 +1037,6 @@
return NoNode;
}
- NodeIndex getMethodLoadElimination(const MethodCheckData& methodCheckData, unsigned identifierNumber, NodeIndex child1)
- {
- NodeIndex start = startIndexForChildren(child1);
- for (NodeIndex index = m_compileIndex; index-- > start;) {
- Node& node = m_graph[index];
- switch (node.op) {
- case CheckMethod:
- if (node.child1() == child1
- && node.identifierNumber() == identifierNumber
- && m_graph.m_methodCheckData[node.methodCheckDataIndex()] == methodCheckData)
- return index;
- break;
-
- case PutByOffset:
- // If a put was optimized to by-offset then it's not changing the structure
- break;
-
- case PutByVal:
- case PutByValAlias:
- if (byValHasIntBase(node)) {
- // If PutByVal speculates that it's accessing an array with an
- // integer index, then it's impossible for it to cause a structure
- // change.
- break;
- }
- return NoNode;
-
- case ArrayPush:
- case ArrayPop:
- // Pushing and popping cannot despecify a function.
- break;
-
- default:
- if (clobbersWorld(index))
- return NoNode;
- break;
- }
- }
- return NoNode;
- }
-
bool checkFunctionElimination(JSFunction* function, NodeIndex child1)
{
NodeIndex start = startIndexForChildren(child1);
@@ -1392,10 +1347,6 @@
node.op = PutByValAlias;
break;
- case CheckMethod:
- setReplacement(getMethodLoadElimination(m_graph.m_methodCheckData[node.methodCheckDataIndex()], node.identifierNumber(), node.child1()));
- break;
-
case CheckStructure:
if (checkStructureLoadElimination(node.structureSet(), node.child1()))
eliminate();
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (100314 => 100315)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2011-11-15 21:38:35 UTC (rev 100314)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2011-11-15 21:54:38 UTC (rev 100315)
@@ -2058,6 +2058,7 @@
switch (op) {
case JSConstant:
+ case WeakJSConstant:
initConstantInfo(m_compileIndex);
break;
@@ -3629,26 +3630,6 @@
break;
}
- case CheckMethod: {
- MethodCheckData& methodCheckData = m_jit.graph().m_methodCheckData[node.methodCheckDataIndex()];
-
- SpeculateCellOperand base(this, node.child1());
- GPRTemporary scratch(this); // this needs to be a separate register, unfortunately.
- GPRReg baseGPR = base.gpr();
- GPRReg scratchGPR = scratch.gpr();
-
- if (!m_state.forNode(node.child1()).m_structure.doesNotContainAnyOtherThan(methodCheckData.structure))
- speculationCheck(JSValueRegs(), NoNode, m_jit.branchPtr(JITCompiler::NotEqual, JITCompiler::Address(baseGPR, JSCell::structureOffset()), JITCompiler::TrustedImmPtr(methodCheckData.structure)));
- if (methodCheckData.prototype != m_jit.globalObjectFor(node.codeOrigin)->methodCallDummy()) {
- m_jit.move(JITCompiler::TrustedImmPtr(methodCheckData.prototype->structureAddress()), scratchGPR);
- speculationCheck(JSValueRegs(), NoNode, m_jit.branchPtr(JITCompiler::NotEqual, JITCompiler::Address(scratchGPR), JITCompiler::TrustedImmPtr(methodCheckData.prototypeStructure)));
- }
-
- useChildren(node);
- initConstantInfo(m_compileIndex);
- break;
- }
-
case PutById: {
SpeculateCellOperand base(this, node.child1());
JSValueOperand value(this, node.child2());
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (100314 => 100315)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2011-11-15 21:38:35 UTC (rev 100314)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2011-11-15 21:54:38 UTC (rev 100315)
@@ -2069,6 +2069,7 @@
switch (op) {
case JSConstant:
+ case WeakJSConstant:
initConstantInfo(m_compileIndex);
break;
@@ -3543,26 +3544,6 @@
break;
}
- case CheckMethod: {
- MethodCheckData& methodCheckData = m_jit.graph().m_methodCheckData[node.methodCheckDataIndex()];
-
- SpeculateCellOperand base(this, node.child1());
- GPRTemporary scratch(this); // this needs to be a separate register, unfortunately.
- GPRReg baseGPR = base.gpr();
- GPRReg scratchGPR = scratch.gpr();
-
- if (!m_state.forNode(node.child1()).m_structure.doesNotContainAnyOtherThan(methodCheckData.structure))
- speculationCheck(JSValueRegs(), NoNode, m_jit.branchPtr(JITCompiler::NotEqual, JITCompiler::Address(baseGPR, JSCell::structureOffset()), JITCompiler::TrustedImmPtr(methodCheckData.structure)));
- if (methodCheckData.prototype != m_jit.globalObjectFor(node.codeOrigin)->methodCallDummy()) {
- m_jit.move(JITCompiler::TrustedImmPtr(methodCheckData.prototype->structureAddress()), scratchGPR);
- speculationCheck(JSValueRegs(), NoNode, m_jit.branchPtr(JITCompiler::NotEqual, JITCompiler::Address(scratchGPR), JITCompiler::TrustedImmPtr(methodCheckData.prototypeStructure)));
- }
-
- useChildren(node);
- initConstantInfo(m_compileIndex);
- break;
- }
-
case PutById: {
SpeculateCellOperand base(this, node.child1());
JSValueOperand value(this, node.child2());
