Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
tryGetById should be supported by the DFG/FTL
https://bugs.webkit.org/show_bug.cgi?id=156378

Reviewed by Filip Pizlo.

This patch adds support for tryGetById in the DFG/FTL. It adds a new DFG node
TryGetById, which acts similarly to the normal GetById DFG node. One key
difference between GetById and TryGetById is that in the LLInt and Baseline
we do not profile the result type. This profiling is unnessary for the current
use case of tryGetById, which is expected to be a strict equality comparision
against a specific object or undefined. In either case other DFG optimizations
will make this equally fast with or without the profiling information.

Additionally, this patch adds new reuse modes for JSValueRegsTemporary that take
an operand and attempt to reuse the registers for that operand if they are free
after the current DFG node.

* bytecode/GetByIdStatus.cpp:
(JSC::GetByIdStatus::computeFromLLInt):
(JSC::GetByIdStatus::computeForStubInfoWithoutExitSiteFeedback):
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleGetById):
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGCapabilities.cpp:
(JSC::DFG::capabilityLevel):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGNode.h:
(JSC::DFG::Node::hasIdentifier):
* dfg/DFGNodeType.h:
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::propagate):
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileTryGetById):
(JSC::DFG::JSValueRegsTemporary::JSValueRegsTemporary):
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::GPRTemporary::operator=):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::cachedGetById):
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::cachedGetById):
(JSC::DFG::SpeculativeJIT::compile):
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileNode):
(JSC::FTL::DFG::LowerDFGToB3::compileGetById):
(JSC::FTL::DFG::LowerDFGToB3::getById):
* jit/JITOperations.cpp:
* jit/JITOperations.h:
* tests/stress/try-get-by-id.js:
(tryGetByIdTextStrict):
(get let):
(let.get createBuiltin):
(get throw):
(getCaller.obj.1.throw.new.Error): Deleted.


Canonical link: https://commits.webkit.org/174545@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@199279 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
kmiller68 committed Apr 10, 2016
1 parent 3cc23c5 commit 28643a9
Show file tree
Hide file tree
Showing 21 changed files with 368 additions and 41 deletions.
68 changes: 68 additions & 0 deletions Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,71 @@
2016-04-09 Keith Miller <keith_miller@apple.com>

tryGetById should be supported by the DFG/FTL
https://bugs.webkit.org/show_bug.cgi?id=156378

Reviewed by Filip Pizlo.

This patch adds support for tryGetById in the DFG/FTL. It adds a new DFG node
TryGetById, which acts similarly to the normal GetById DFG node. One key
difference between GetById and TryGetById is that in the LLInt and Baseline
we do not profile the result type. This profiling is unnessary for the current
use case of tryGetById, which is expected to be a strict equality comparision
against a specific object or undefined. In either case other DFG optimizations
will make this equally fast with or without the profiling information.

Additionally, this patch adds new reuse modes for JSValueRegsTemporary that take
an operand and attempt to reuse the registers for that operand if they are free
after the current DFG node.

* bytecode/GetByIdStatus.cpp:
(JSC::GetByIdStatus::computeFromLLInt):
(JSC::GetByIdStatus::computeForStubInfoWithoutExitSiteFeedback):
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleGetById):
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGCapabilities.cpp:
(JSC::DFG::capabilityLevel):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGNode.h:
(JSC::DFG::Node::hasIdentifier):
* dfg/DFGNodeType.h:
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::propagate):
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileTryGetById):
(JSC::DFG::JSValueRegsTemporary::JSValueRegsTemporary):
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::GPRTemporary::operator=):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::cachedGetById):
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::cachedGetById):
(JSC::DFG::SpeculativeJIT::compile):
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileNode):
(JSC::FTL::DFG::LowerDFGToB3::compileGetById):
(JSC::FTL::DFG::LowerDFGToB3::getById):
* jit/JITOperations.cpp:
* jit/JITOperations.h:
* tests/stress/try-get-by-id.js:
(tryGetByIdTextStrict):
(get let):
(let.get createBuiltin):
(get throw):
(getCaller.obj.1.throw.new.Error): Deleted.

2016-04-09 Saam barati <sbarati@apple.com>

Allocation sinking SSA Defs are allowed to have replacements
Expand Down
5 changes: 3 additions & 2 deletions Source/JavaScriptCore/bytecode/GetByIdStatus.cpp
Expand Up @@ -75,7 +75,7 @@ GetByIdStatus GetByIdStatus::computeFromLLInt(CodeBlock* profiledBlock, unsigned

Instruction* instruction = profiledBlock->instructions().begin() + bytecodeIndex;

if (instruction[0].u.opcode == LLInt::getOpcode(op_get_array_length))
if (instruction[0].u.opcode == LLInt::getOpcode(op_get_array_length) || instruction[0].u.opcode == LLInt::getOpcode(op_try_get_by_id))
return GetByIdStatus(NoInformation, false);

StructureID structureID = instruction[4].u.structureID;
Expand Down Expand Up @@ -210,7 +210,8 @@ GetByIdStatus GetByIdStatus::computeForStubInfoWithoutExitSiteFeedback(
JSFunction* intrinsicFunction = nullptr;

switch (access.type()) {
case AccessCase::Load: {
case AccessCase::Load:
case AccessCase::GetGetter: {
break;
}
case AccessCase::IntrinsicGetter: {
Expand Down
8 changes: 7 additions & 1 deletion Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Expand Up @@ -1972,7 +1972,13 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi

case PutToArguments:
break;


case TryGetById:
// FIXME: This should constant fold at least as well as the normal GetById case.
// https://bugs.webkit.org/show_bug.cgi?id=156422
forNode(node).makeHeapTop();
break;

case GetById:
case GetByIdFlush: {
if (!node->prediction()) {
Expand Down
41 changes: 31 additions & 10 deletions Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Expand Up @@ -231,9 +231,10 @@ class ByteCodeParser {
Node* load(SpeculatedType, Node* base, unsigned identifierNumber, const VariantType&);

Node* store(Node* base, unsigned identifier, const PutByIdVariant&, Node* value);


void handleTryGetById(int destinationOperand, Node* base, unsigned identifierNumber, const GetByIdStatus&);
void handleGetById(
int destinationOperand, SpeculatedType, Node* base, unsigned identifierNumber, GetByIdStatus);
int destinationOperand, SpeculatedType, Node* base, unsigned identifierNumber, GetByIdStatus, AccessType);
void emitPutById(
Node* base, unsigned identifierNumber, Node* value, const PutByIdStatus&, bool isDirect);
void handlePutById(
Expand Down Expand Up @@ -2887,7 +2888,7 @@ Node* ByteCodeParser::store(Node* base, unsigned identifier, const PutByIdVarian

void ByteCodeParser::handleGetById(
int destinationOperand, SpeculatedType prediction, Node* base, unsigned identifierNumber,
GetByIdStatus getByIdStatus)
GetByIdStatus getByIdStatus, AccessType type)
{
// Attempt to reduce the set of things in the GetByIdStatus.
if (base->op() == NewObject) {
Expand All @@ -2905,8 +2906,13 @@ void ByteCodeParser::handleGetById(
getByIdStatus.filter(base->structure());
}

NodeType getById = getByIdStatus.makesCalls() ? GetByIdFlush : GetById;

NodeType getById;
if (type == AccessType::Get)
getById = getByIdStatus.makesCalls() ? GetByIdFlush : GetById;
else
getById = TryGetById;

ASSERT(type == AccessType::Get || !getByIdStatus.makesCalls());
if (!getByIdStatus.isSimple() || !getByIdStatus.numVariants() || !Options::useAccessInlining()) {
set(VirtualRegister(destinationOperand),
addToGraph(getById, OpInfo(identifierNumber), OpInfo(prediction), base));
Expand Down Expand Up @@ -2976,6 +2982,7 @@ void ByteCodeParser::handleGetById(
if (m_graph.compilation())
m_graph.compilation()->noticeInlinedGetById();

ASSERT(type == AccessType::Get || !variant.callLinkStatus());
if (!variant.callLinkStatus() && variant.intrinsic() == NoIntrinsic) {
set(VirtualRegister(destinationOperand), loadedValue);
return;
Expand All @@ -2991,8 +2998,7 @@ void ByteCodeParser::handleGetById(
return;
}

if (variant.intrinsic() != NoIntrinsic)
ASSERT(variant.intrinsic() == NoIntrinsic);
ASSERT(variant.intrinsic() == NoIntrinsic);

// Make a call. We don't try to get fancy with using the smallest operand number because
// the stack layout phase should compress the stack anyway.
Expand Down Expand Up @@ -3793,7 +3799,7 @@ bool ByteCodeParser::parseBlock(unsigned limit)
locker, m_inlineStackTop->m_profiledBlock,
byValInfo->stubInfo, currentCodeOrigin(), uid);

handleGetById(currentInstruction[1].u.operand, prediction, base, identifierNumber, getByIdStatus);
handleGetById(currentInstruction[1].u.operand, prediction, base, identifierNumber, getByIdStatus, AccessType::Get);
}
}

Expand Down Expand Up @@ -3847,7 +3853,22 @@ bool ByteCodeParser::parseBlock(unsigned limit)

NEXT_OPCODE(op_put_by_val);
}


case op_try_get_by_id: {
Node* base = get(VirtualRegister(currentInstruction[2].u.operand));
unsigned identifierNumber = m_inlineStackTop->m_identifierRemap[currentInstruction[3].u.operand];
UniquedStringImpl* uid = m_graph.identifiers()[identifierNumber];

GetByIdStatus getByIdStatus = GetByIdStatus::computeFor(
m_inlineStackTop->m_profiledBlock, m_dfgCodeBlock,
m_inlineStackTop->m_stubInfos, m_dfgStubInfos,
currentCodeOrigin(), uid);

handleGetById(currentInstruction[1].u.operand, SpecHeapTop, base, identifierNumber, getByIdStatus, AccessType::GetPure);

NEXT_OPCODE(op_try_get_by_id);
}

case op_get_by_id:
case op_get_array_length: {
SpeculatedType prediction = getPrediction();
Expand All @@ -3862,7 +3883,7 @@ bool ByteCodeParser::parseBlock(unsigned limit)
currentCodeOrigin(), uid);

handleGetById(
currentInstruction[1].u.operand, prediction, base, identifierNumber, getByIdStatus);
currentInstruction[1].u.operand, prediction, base, identifierNumber, getByIdStatus, AccessType::Get);

NEXT_OPCODE(op_get_by_id);
}
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/dfg/DFGCapabilities.cpp
Expand Up @@ -152,6 +152,7 @@ CapabilityLevel capabilityLevel(OpcodeID opcodeID, CodeBlock* codeBlock, Instruc
case op_get_by_val:
case op_put_by_val:
case op_put_by_val_direct:
case op_try_get_by_id:
case op_get_by_id:
case op_get_array_length:
case op_put_by_id:
Expand Down
7 changes: 6 additions & 1 deletion Source/JavaScriptCore/dfg/DFGClobberize.h
Expand Up @@ -848,7 +848,12 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
def(HeapLocation(NamedPropertyLoc, heap, node->child2()), LazyNode(node));
return;
}


case TryGetById: {
read(Heap);
return;
}

case MultiGetByOffset: {
read(JSCell_structureID);
read(JSObject_butterfly);
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/dfg/DFGDoesGC.cpp
Expand Up @@ -96,6 +96,7 @@ bool doesGC(Graph& graph, Node* node)
case ArithCos:
case ArithLog:
case ValueAdd:
case TryGetById:
case GetById:
case GetByIdFlush:
case PutById:
Expand Down
6 changes: 6 additions & 0 deletions Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Expand Up @@ -1069,6 +1069,12 @@ class FixupPhase : public Phase {
break;
}

case TryGetById: {
if (node->child1()->shouldSpeculateCell())
fixEdge<CellUse>(node->child1());
break;
}

case GetById:
case GetByIdFlush: {
// FIXME: This should be done in the ByteCodeParser based on reading the
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/dfg/DFGNode.h
Expand Up @@ -866,6 +866,7 @@ struct Node {
bool hasIdentifier()
{
switch (op()) {
case TryGetById:
case GetById:
case GetByIdFlush:
case PutById:
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/dfg/DFGNodeType.h
Expand Up @@ -182,6 +182,7 @@ namespace JSC { namespace DFG {
macro(PutByValDirect, NodeMustGenerate | NodeHasVarArgs) \
macro(PutByVal, NodeMustGenerate | NodeHasVarArgs) \
macro(PutByValAlias, NodeMustGenerate | NodeHasVarArgs) \
macro(TryGetById, NodeResultJS) \
macro(GetById, NodeResultJS | NodeMustGenerate) \
macro(GetByIdFlush, NodeResultJS | NodeMustGenerate) \
macro(PutById, NodeMustGenerate) \
Expand Down
7 changes: 6 additions & 1 deletion Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
Expand Up @@ -172,7 +172,12 @@ class PredictionPropagationPhase : public Phase {
changed |= setPrediction(SpecInt32);
break;
}


case TryGetById: {
changed |= setPrediction(SpecBytecodeTop);
break;
}

case ArrayPop:
case ArrayPush:
case RegExpExec:
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/dfg/DFGSafeToExecute.h
Expand Up @@ -191,6 +191,7 @@ bool safeToExecute(AbstractStateType& state, Graph& graph, Node* node)
case ArithCos:
case ArithLog:
case ValueAdd:
case TryGetById:
case GetById:
case GetByIdFlush:
case PutById:
Expand Down
79 changes: 79 additions & 0 deletions Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Expand Up @@ -964,6 +964,47 @@ void SpeculativeJIT::useChildren(Node* node)
}
}

void SpeculativeJIT::compileTryGetById(Node* node)
{
switch (node->child1().useKind()) {
case CellUse: {
SpeculateCellOperand base(this, node->child1());
JSValueRegsTemporary result(this, Reuse, base);

JSValueRegs baseRegs = JSValueRegs::payloadOnly(base.gpr());
JSValueRegs resultRegs = result.regs();

base.use();

cachedGetById(node->origin.semantic, baseRegs, resultRegs, node->identifierNumber(), JITCompiler::Jump(), DontSpill, AccessType::GetPure);

jsValueResult(resultRegs, node, DataFormatJS, UseChildrenCalledExplicitly);
break;
}

case UntypedUse: {
JSValueOperand base(this, node->child1());
JSValueRegsTemporary result(this, Reuse, base);

JSValueRegs baseRegs = base.jsValueRegs();
JSValueRegs resultRegs = result.regs();

base.use();

JITCompiler::Jump notCell = m_jit.branchIfNotCell(baseRegs);

cachedGetById(node->origin.semantic, baseRegs, resultRegs, node->identifierNumber(), notCell, DontSpill, AccessType::GetPure);

jsValueResult(resultRegs, node, DataFormatJS, UseChildrenCalledExplicitly);
break;
}

default:
DFG_CRASH(m_jit.graph(), node, "Bad use kind");
break;
}
}

void SpeculativeJIT::compileIn(Node* node)
{
SpeculateCellOperand base(this, node->child2());
Expand Down Expand Up @@ -1168,6 +1209,44 @@ JSValueRegsTemporary::JSValueRegsTemporary(SpeculativeJIT* jit)
{
}

#if USE(JSVALUE64)
template<typename T>
JSValueRegsTemporary::JSValueRegsTemporary(SpeculativeJIT* jit, ReuseTag, T& operand, WhichValueWord)
: m_gpr(jit, Reuse, operand)
{
}
#else
template<typename T>
JSValueRegsTemporary::JSValueRegsTemporary(SpeculativeJIT* jit, ReuseTag, T& operand, WhichValueWord resultWord)
{
if (resultWord == PayloadWord) {
m_payloadGPR = GPRTemporary(jit, Reuse, operand);
m_tagGPR = GPRTemporary(jit);
} else {
m_payloadGPR = GPRTemporary(jit);
m_tagGPR = GPRTemporary(jit, Reuse, operand);
}
}
#endif

#if USE(JSVALUE64)
JSValueRegsTemporary::JSValueRegsTemporary(SpeculativeJIT* jit, ReuseTag, JSValueOperand& operand)
{
m_gpr = GPRTemporary(jit, Reuse, operand);
}
#else
JSValueRegsTemporary::JSValueRegsTemporary(SpeculativeJIT* jit, ReuseTag, JSValueOperand& operand)
{
if (jit->canReuse(operand.node())) {
m_payloadGPR = GPRTemporary(jit, Reuse, operand, PayloadWord);
m_tagGPR = GPRTemporary(jit, Reuse, operand, TagWord);
} else {
m_payloadGPR = GPRTemporary(jit);
m_tagGPR = GPRTemporary(jit);
}
}
#endif

JSValueRegsTemporary::~JSValueRegsTemporary() { }

JSValueRegs JSValueRegsTemporary::regs()
Expand Down

0 comments on commit 28643a9

Please sign in to comment.