Commit d7113640 authored by fpizlo@apple.com's avatar fpizlo@apple.com

DFG does not speculate aggressively enough on put_by_id

https://bugs.webkit.org/show_bug.cgi?id=69114

Reviewed by Oliver Hunt.

This adds new nodes along with optimizations for those nodes:
        
GetPropertyStorage: CheckStructure used to do both the structure
check and retrieve the storage pointer. Now CheckStructure just
checks the structure, and GetPropertyStorage retrieves the
storage pointer.
        
PutStructure: Changes the structure, and has the expected store
to load optimization with CheckStructure.
        
PutByOffset: Directly sets the value. Has store to load
optimization with GetByOffset.

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::cellConstant):
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::dump):
* dfg/DFGJITCodeGenerator.cpp:
(JSC::DFG::JITCodeGenerator::writeBarrier):
* dfg/DFGJITCodeGenerator.h:
* dfg/DFGNode.h:
(JSC::DFG::Node::hasStructure):
(JSC::DFG::Node::hasStorageAccessData):
* dfg/DFGPropagator.cpp:
(JSC::DFG::Propagator::propagateNodePredictions):
(JSC::DFG::Propagator::impureCSE):
(JSC::DFG::Propagator::checkStructureLoadElimination):
(JSC::DFG::Propagator::getByOffsetLoadElimination):
(JSC::DFG::Propagator::getPropertyStorageLoadElimination):
(JSC::DFG::Propagator::eliminate):
(JSC::DFG::Propagator::performNodeCSE):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@96443 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent ea04bf36
2011-09-30 Filip Pizlo <fpizlo@apple.com>
DFG does not speculate aggressively enough on put_by_id
https://bugs.webkit.org/show_bug.cgi?id=69114
Reviewed by Oliver Hunt.
This adds new nodes along with optimizations for those nodes:
GetPropertyStorage: CheckStructure used to do both the structure
check and retrieve the storage pointer. Now CheckStructure just
checks the structure, and GetPropertyStorage retrieves the
storage pointer.
PutStructure: Changes the structure, and has the expected store
to load optimization with CheckStructure.
PutByOffset: Directly sets the value. Has store to load
optimization with GetByOffset.
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::cellConstant):
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::dump):
* dfg/DFGJITCodeGenerator.cpp:
(JSC::DFG::JITCodeGenerator::writeBarrier):
* dfg/DFGJITCodeGenerator.h:
* dfg/DFGNode.h:
(JSC::DFG::Node::hasStructure):
(JSC::DFG::Node::hasStorageAccessData):
* dfg/DFGPropagator.cpp:
(JSC::DFG::Propagator::propagateNodePredictions):
(JSC::DFG::Propagator::impureCSE):
(JSC::DFG::Propagator::checkStructureLoadElimination):
(JSC::DFG::Propagator::getByOffsetLoadElimination):
(JSC::DFG::Propagator::getPropertyStorageLoadElimination):
(JSC::DFG::Propagator::eliminate):
(JSC::DFG::Propagator::performNodeCSE):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
2011-09-30 Gavin Barraclough <barraclough@apple.com>
StringRecursionChecker should not work in terms of EncodedJSValue
......@@ -30,6 +30,7 @@
#include "DFGCapabilities.h"
#include "CodeBlock.h"
#include <wtf/HashMap.h>
#include <wtf/MathExtras.h>
namespace JSC { namespace DFG {
......@@ -404,6 +405,19 @@ private:
return getJSConstant(m_constantNaN);
}
NodeIndex cellConstant(JSCell* cell)
{
HashMap<JSCell*, unsigned>::iterator iter = m_cellConstants.find(cell);
if (iter != m_cellConstants.end())
return getJSConstant(iter->second);
m_codeBlock->addConstant(cell);
m_constants.append(ConstantRecord());
ASSERT(m_constants.size() == m_codeBlock->numberOfConstantRegisters());
return getJSConstant(m_codeBlock->numberOfConstantRegisters() - 1);
}
CodeOrigin currentCodeOrigin()
{
return CodeOrigin(m_currentIndex);
......@@ -576,6 +590,7 @@ private:
unsigned m_constantNull;
unsigned m_constantNaN;
unsigned m_constant1;
HashMap<JSCell*, unsigned> m_cellConstants;
// 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.
......@@ -1154,7 +1169,8 @@ bool ByteCodeParser::parseBlock(unsigned limit)
size_t offset = structure->get(*m_globalData, identifier);
if (offset != notFound) {
getById = addToGraph(GetByOffset, OpInfo(m_graph.m_storageAccessData.size()), OpInfo(prediction), addToGraph(CheckStructure, OpInfo(structure), base));
addToGraph(CheckStructure, OpInfo(structure), base);
getById = addToGraph(GetByOffset, OpInfo(m_graph.m_storageAccessData.size()), OpInfo(prediction), addToGraph(GetPropertyStorage, base));
StorageAccessData storageAccessData;
storageAccessData.offset = offset;
......@@ -1174,13 +1190,84 @@ bool ByteCodeParser::parseBlock(unsigned limit)
case op_put_by_id: {
NodeIndex value = get(currentInstruction[3].u.operand);
NodeIndex base = get(currentInstruction[1].u.operand);
unsigned identifier = currentInstruction[2].u.operand;
unsigned identifierNumber = currentInstruction[2].u.operand;
bool direct = currentInstruction[8].u.operand;
if (direct)
addToGraph(PutByIdDirect, OpInfo(identifier), base, value);
else
addToGraph(PutById, OpInfo(identifier), base, value);
StructureStubInfo& stubInfo = m_profiledBlock->getStubInfo(m_currentIndex);
if (!stubInfo.seen)
addToGraph(ForceOSRExit);
bool alreadyGenerated = false;
if (stubInfo.seen && !m_profiledBlock->likelyToTakeSlowCase(m_currentIndex)) {
switch (stubInfo.accessType) {
case access_put_by_id_replace: {
Structure* structure = stubInfo.u.putByIdReplace.baseObjectStructure.get();
Identifier identifier = m_codeBlock->identifier(identifierNumber);
size_t offset = structure->get(*m_globalData, identifier);
if (offset != notFound) {
addToGraph(CheckStructure, OpInfo(structure), base);
addToGraph(PutByOffset, OpInfo(m_graph.m_storageAccessData.size()), base, addToGraph(GetPropertyStorage, base), value);
StorageAccessData storageAccessData;
storageAccessData.offset = offset;
storageAccessData.identifierNumber = identifierNumber;
m_graph.m_storageAccessData.append(storageAccessData);
alreadyGenerated = true;
}
break;
}
case access_put_by_id_transition: {
Structure* previousStructure = stubInfo.u.putByIdTransition.previousStructure.get();
Structure* newStructure = stubInfo.u.putByIdTransition.structure.get();
if (previousStructure->propertyStorageCapacity() != newStructure->propertyStorageCapacity())
break;
StructureChain* structureChain = stubInfo.u.putByIdTransition.chain.get();
Identifier identifier = m_codeBlock->identifier(identifierNumber);
size_t offset = newStructure->get(*m_globalData, identifier);
if (offset != notFound) {
addToGraph(CheckStructure, OpInfo(previousStructure), base);
if (!direct) {
for (WriteBarrier<Structure>* it = structureChain->head(); *it; ++it) {
JSValue prototype = (*it)->storedPrototype();
if (prototype.isNull())
continue;
ASSERT(prototype.isCell());
addToGraph(CheckStructure, OpInfo(prototype.asCell()->structure()), cellConstant(prototype.asCell()));
}
}
addToGraph(PutStructure, OpInfo(newStructure), base);
addToGraph(PutByOffset, OpInfo(m_graph.m_storageAccessData.size()), base, addToGraph(GetPropertyStorage, base), value);
StorageAccessData storageAccessData;
storageAccessData.offset = offset;
storageAccessData.identifierNumber = identifierNumber;
m_graph.m_storageAccessData.append(storageAccessData);
alreadyGenerated = true;
}
break;
}
default:
break;
}
}
if (!alreadyGenerated) {
if (direct)
addToGraph(PutByIdDirect, OpInfo(identifierNumber), base, value);
else
addToGraph(PutById, OpInfo(identifierNumber), base, value);
}
NEXT_OPCODE(op_put_by_id);
}
......
......@@ -149,6 +149,20 @@ void Graph::dump(NodeIndex nodeIndex, CodeBlock* codeBlock)
printf("%sid%u", hasPrinted ? ", " : "", node.identifierNumber());
hasPrinted = true;
}
if (node.hasStructure()) {
printf("%sstruct(%p)", hasPrinted ? ", " : "", node.structure());
hasPrinted = true;
}
if (node.hasStorageAccessData()) {
StorageAccessData& storageAccessData = m_storageAccessData[node.storageAccessDataIndex()];
if (codeBlock)
printf("%sid%u{%s}", hasPrinted ? ", " : "", storageAccessData.identifierNumber, codeBlock->identifier(storageAccessData.identifierNumber).ustring().utf8().data());
else
printf("%sid%u", hasPrinted ? ", " : "", storageAccessData.identifierNumber);
printf(", %lu", storageAccessData.offset);
hasPrinted = true;
}
ASSERT(node.hasVariableAccessData() == node.hasLocal());
if (node.hasVariableAccessData()) {
VariableAccessData* variableAccessData = node.variableAccessData();
......
......@@ -267,6 +267,39 @@ void JITCodeGenerator::writeBarrier(GPRReg ownerGPR, GPRReg valueGPR, NodeIndex
#endif
}
void JITCodeGenerator::writeBarrier(GPRReg ownerGPR, JSCell* value, WriteBarrierUseKind useKind, GPRReg scratch1, GPRReg scratch2)
{
UNUSED_PARAM(ownerGPR);
UNUSED_PARAM(value);
UNUSED_PARAM(scratch1);
UNUSED_PARAM(scratch2);
UNUSED_PARAM(useKind);
if (Heap::isMarked(value))
return;
#if ENABLE(WRITE_BARRIER_PROFILING)
JITCompiler::emitCount(jit, WriteBarrierCounters::jitCounterFor(useKind));
#endif
#if ENABLE(GGC)
GPRTemporary temp1;
GPRTemporary temp2;
if (scratch1 == InvalidGPRReg) {
GPRTemporary scratchGPR(this);
temp1.adopt(scratchGPR);
scratch1 = temp1.gpr();
}
if (scratch2 == InvalidGPRReg) {
GPRTemporary scratchGPR(this);
temp2.adopt(scratchGPR);
scratch2 = temp2.gpr();
}
markCellCard(m_jit, ownerGPR, scratch1, scratch2);
#endif
}
void JITCodeGenerator::writeBarrier(JSCell* owner, GPRReg valueGPR, NodeIndex valueIndex, WriteBarrierUseKind useKind, GPRReg scratch)
{
UNUSED_PARAM(owner);
......
......@@ -214,6 +214,7 @@ public:
static void writeBarrier(MacroAssembler&, GPRReg ownerGPR, GPRReg scratchGPR1, GPRReg scratchGPR2, WriteBarrierUseKind);
void writeBarrier(GPRReg ownerGPR, GPRReg valueGPR, NodeIndex valueIndex, WriteBarrierUseKind, GPRReg scratchGPR1 = InvalidGPRReg, GPRReg scratchGPR2 = InvalidGPRReg);
void writeBarrier(GPRReg ownerGPR, JSCell* value, WriteBarrierUseKind, GPRReg scratchGPR1 = InvalidGPRReg, GPRReg scratchGPR2 = InvalidGPRReg);
void writeBarrier(JSCell* owner, GPRReg valueGPR, NodeIndex valueIndex, WriteBarrierUseKind, GPRReg scratchGPR1 = InvalidGPRReg);
static GPRReg selectScratchGPR(GPRReg preserve1 = InvalidGPRReg, GPRReg preserve2 = InvalidGPRReg, GPRReg preserve3 = InvalidGPRReg)
......
......@@ -313,8 +313,11 @@ static inline const char* arithNodeFlagsAsString(ArithNodeFlags flags)
macro(GetById, NodeResultJS | NodeMustGenerate | NodeClobbersWorld) \
macro(PutById, NodeMustGenerate | NodeClobbersWorld) \
macro(PutByIdDirect, NodeMustGenerate | NodeClobbersWorld) \
macro(CheckStructure, NodeResultStorage | NodeMustGenerate) \
macro(CheckStructure, NodeMustGenerate) \
macro(PutStructure, NodeMustGenerate | NodeClobbersWorld) \
macro(GetPropertyStorage, NodeResultStorage) \
macro(GetByOffset, NodeResultJS) \
macro(PutByOffset, NodeMustGenerate | NodeClobbersWorld) \
macro(GetArrayLength, NodeResultInt32) \
macro(GetMethod, NodeResultJS | NodeMustGenerate) \
macro(CheckMethod, NodeResultJS | NodeMustGenerate) \
......@@ -782,7 +785,7 @@ struct Node {
bool hasStructure()
{
return op == CheckStructure;
return op == CheckStructure || op == PutStructure;
}
Structure* structure()
......@@ -792,7 +795,7 @@ struct Node {
bool hasStorageAccessData()
{
return op == GetByOffset;
return op == GetByOffset || op == PutByOffset;
}
unsigned storageAccessDataIndex()
......
......@@ -448,7 +448,7 @@ private:
break;
}
case CheckStructure: {
case GetPropertyStorage: {
changed |= setPrediction(PredictOther);
break;
}
......@@ -583,6 +583,9 @@ private:
case PutByValAlias:
case PutById:
case PutByIdDirect:
case CheckStructure:
case PutStructure:
case PutByOffset:
break;
// This gets ignored because it doesn't do anything.
......@@ -888,6 +891,39 @@ private:
}
}
NodeIndex impureCSE(Node& node)
{
NodeIndex child1 = canonicalize(node.child1());
NodeIndex child2 = canonicalize(node.child2());
NodeIndex child3 = canonicalize(node.child3());
NodeIndex start = startIndex();
for (NodeIndex index = m_compileIndex; index-- > start;) {
Node& otherNode = m_graph[index];
if (node.op == otherNode.op
&& node.arithNodeFlagsForCompare() == otherNode.arithNodeFlagsForCompare()) {
NodeIndex otherChild = canonicalize(otherNode.child1());
if (otherChild == NoNode)
return index;
if (otherChild == child1) {
otherChild = canonicalize(otherNode.child2());
if (otherChild == NoNode)
return index;
if (otherChild == child2) {
otherChild = canonicalize(otherNode.child3());
if (otherChild == NoNode)
return index;
if (otherChild == child3)
return index;
}
}
}
if (clobbersWorld(index))
break;
}
return NoNode;
}
NodeIndex globalVarLoadElimination(unsigned varNumber)
{
NodeIndex start = startIndexForChildren();
......@@ -950,19 +986,35 @@ private:
return NoNode;
}
NodeIndex checkStructureLoadElimination(Structure* structure, NodeIndex child1)
bool checkStructureLoadElimination(Structure* structure, NodeIndex child1)
{
NodeIndex start = startIndexForChildren(child1);
for (NodeIndex index = m_compileIndex; index-- > start;) {
Node& node = m_graph[index];
if (node.op == CheckStructure
&& node.child1() == child1
&& node.structure() == structure)
return index;
if (clobbersWorld(index))
switch (node.op) {
case CheckStructure:
if (node.child1() == child1
&& node.structure() == structure)
return true;
break;
case PutStructure:
if (node.child1() == child1
&& node.structure() == structure)
return true;
return false;
case PutByOffset:
// Setting a property cannot change the structure.
break;
default:
if (clobbersWorld(index))
return false;
break;
}
}
return NoNode;
return false;
}
NodeIndex getByOffsetLoadElimination(unsigned identifierNumber, NodeIndex child1)
......@@ -970,12 +1022,56 @@ private:
NodeIndex start = startIndexForChildren(child1);
for (NodeIndex index = m_compileIndex; index-- > start;) {
Node& node = m_graph[index];
if (node.op == GetByOffset
&& node.child1() == child1
&& m_graph.m_storageAccessData[node.storageAccessDataIndex()].identifierNumber == identifierNumber)
return index;
if (clobbersWorld(index))
switch (node.op) {
case GetByOffset:
if (node.child1() == child1
&& m_graph.m_storageAccessData[node.storageAccessDataIndex()].identifierNumber == identifierNumber)
return index;
break;
case PutByOffset:
if (m_graph.m_storageAccessData[node.storageAccessDataIndex()].identifierNumber == identifierNumber) {
if (node.child2() == child1)
return node.child3();
return NoNode;
}
break;
case PutStructure:
// Changing the structure cannot change the outcome of a property get.
break;
default:
if (clobbersWorld(index))
return NoNode;
break;
}
}
return NoNode;
}
NodeIndex getPropertyStorageLoadElimination(NodeIndex child1)
{
NodeIndex start = startIndexForChildren(child1);
for (NodeIndex index = m_compileIndex; index-- > start;) {
Node& node = m_graph[index];
switch (node.op) {
case GetPropertyStorage:
if (node.child1() == child1)
return index;
break;
case PutByOffset:
case PutStructure:
// Changing the structure or putting to the storage cannot
// change the property storage pointer.
break;
default:
if (clobbersWorld(index))
return NoNode;
break;
}
}
return NoNode;
}
......@@ -1034,6 +1130,18 @@ private:
m_replacements[m_compileIndex] = replacement;
}
void eliminate()
{
#if ENABLE(DFG_DEBUG_PROPAGATION_VERBOSE)
printf(" Eliminating @%u", m_compileIndex);
#endif
Node& node = m_graph[m_compileIndex];
ASSERT(node.refCount() == 1);
ASSERT(node.mustGenerate());
node.op = Phantom;
}
void performNodeCSE(Node& node)
{
if (node.op & NodeHasVarArgs) {
......@@ -1133,7 +1241,12 @@ private:
break;
case CheckStructure:
setReplacement(checkStructureLoadElimination(node.structure(), node.child1()));
if (checkStructureLoadElimination(node.structure(), node.child1()))
eliminate();
break;
case GetPropertyStorage:
setReplacement(getPropertyStorageLoadElimination(node.child1()));
break;
case GetByOffset:
......
......@@ -1657,13 +1657,37 @@ void SpeculativeJIT::compile(Node& node)
case CheckStructure: {
SpeculateCellOperand base(this, node.child1());
GPRTemporary result(this, base);
GPRReg baseGPR = base.gpr();
GPRReg resultGPR = result.gpr();
speculationCheck(m_jit.branchPtr(JITCompiler::NotEqual, JITCompiler::Address(baseGPR, JSCell::structureOffset()), JITCompiler::TrustedImmPtr(node.structure())));
noResult(m_compileIndex);
break;
}
case PutStructure: {
SpeculateCellOperand base(this, node.child1());
GPRReg baseGPR = base.gpr();
#if ENABLE(GGC) || ENABLE(WRITE_BARRIER_PROFILING)
// Must always emit this write barrier as the structure transition itself requires it
writeBarrier(baseGPR, node.structure(), WriteBarrierForGenericAccess);
#endif
m_jit.storePtr(MacroAssembler::TrustedImmPtr(node.structure()), MacroAssembler::Address(baseGPR, JSCell::structureOffset()));
noResult(m_compileIndex);
break;
}
case GetPropertyStorage: {
SpeculateCellOperand base(this, node.child1());
GPRTemporary result(this, base);
GPRReg baseGPR = base.gpr();
GPRReg resultGPR = result.gpr();
m_jit.loadPtr(JITCompiler::Address(baseGPR, JSObject::offsetOfPropertyStorage()), resultGPR);
storageResult(resultGPR, m_compileIndex);
......@@ -1688,6 +1712,30 @@ void SpeculativeJIT::compile(Node& node)
break;
}
case PutByOffset: {
#if ENABLE(GGC) || ENABLE(WRITE_BARRIER_PROFILING)
SpeculateCellOperand base(this, node.child1());
#endif
StorageOperand storage(this, node.child2());
JSValueOperand value(this, node.child3());
GPRReg storageGPR = storage.gpr();
GPRReg valueTagGPR = value.tagGPR();
GPRReg valuePayloadGPR = value.payloadGPR();
#if ENABLE(GGC) || ENABLE(WRITE_BARRIER_PROFILING)
writeBarrier(base.gpr(), valueTagGPR, node.child3(), WriteBarrierForPropertyAccess);
#endif
StorageAccessData& storageAccessData = m_jit.graph().m_storageAccessData[node.storageAccessDataIndex()];
m_jit.storePtr(valueTagGPR, JITCompiler::Address(storageGPR, storageAccessData.offset * sizeof(EncodedJSValue) + OBJECT_OFFSETOF(EncodedValueDescriptor, asBits.tag)));
m_jit.storePtr(valuePayloadGPR, JITCompiler::Address(storageGPR, storageAccessData.offset * sizeof(EncodedJSValue) + OBJECT_OFFSETOF(EncodedValueDescriptor, asBits.payload)));
noResult(m_compileIndex);
break;
}
case GetMethod: {
SpeculateCellOperand base(this, node.child1());
GPRTemporary resultTag(this, base);
......
......@@ -1717,13 +1717,37 @@ void SpeculativeJIT::compile(Node& node)
case CheckStructure: {
SpeculateCellOperand base(this, node.child1());
GPRTemporary result(this, base);
GPRReg baseGPR = base.gpr();
GPRReg resultGPR = result.gpr();
speculationCheck(m_jit.branchPtr(JITCompiler::NotEqual, JITCompiler::Address(baseGPR, JSCell::structureOffset()), JITCompiler::TrustedImmPtr(node.structure())));
noResult(m_compileIndex);
break;
}
case PutStructure: {
SpeculateCellOperand base(this, node.child1());
GPRReg baseGPR = base.gpr();
#if ENABLE(GGC) || ENABLE(WRITE_BARRIER_PROFILING)
// Must always emit this write barrier as the structure transition itself requires it
writeBarrier(baseGPR, node.structure(), WriteBarrierForGenericAccess);
#endif
m_jit.storePtr(MacroAssembler::TrustedImmPtr(node.structure()), MacroAssembler::Address(baseGPR, JSCell::structureOffset()));
noResult(m_compileIndex);
break;
}
case GetPropertyStorage: {
SpeculateCellOperand base(this, node.child1());
GPRTemporary result(this, base);
GPRReg baseGPR = base.gpr();
GPRReg resultGPR = result.gpr();
m_jit.loadPtr(JITCompiler::Address(baseGPR, JSObject::offsetOfPropertyStorage()), resultGPR);
storageResult(resultGPR, m_compileIndex);
......@@ -1745,6 +1769,28 @@ void SpeculativeJIT::compile(Node& node)
break;
}
case PutByOffset: {
#if ENABLE(GGC) || ENABLE(WRITE_BARRIER_PROFILING)
SpeculateCellOperand base(this, node.child1());
#endif
StorageOperand storage(this, node.child2());
JSValueOperand value(this, node.child3());
GPRReg storageGPR = storage.gpr();
GPRReg valueGPR = value.gpr();
#if ENABLE(GGC) || ENABLE(WRITE_BARRIER_PROFILING)
writeBarrier(base.gpr(), value.gpr(), node.child3(), WriteBarrierForPropertyAccess);
#endif
StorageAccessData& storageAccessData = m_jit.graph().m_storageAccessData[node.storageAccessDataIndex()];
m_jit.storePtr(valueGPR, JITCompiler::Address(storageGPR, storageAccessData.offset * sizeof(EncodedJSValue)));
noResult(m_compileIndex);
break;
}