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

Get rid of InlineStart so that I don't have to implement it in FTL

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

Reviewed by Geoffrey Garen.
        
InlineStart was a special instruction that we would insert at the top of inlined code,
so that the backend could capture the OSR state of arguments to an inlined call. It used
to be that only the backend had this information, so this instruction was sort of an ugly
callback from the backend for filling in some data structures.
        
But in the time since when that code was written (two years ago?), we rationalized how
variables work. It's now the case that variables that the runtime must know about are
treated specially in IR (they are "flushed") and we know how we will represent them even
before we get to the backend. The last place that makes changes to their representation
is the StackLayoutPhase.
        
So, this patch gets rid of InlineStart, but keeps around the special meta-data that the
instruction had. Instead of handling the bookkeeping in the backend, we handle it in
StackLayoutPhase. This means that the DFG and FTL can share code for handling this
bookkeeping. This also means that now the FTL can compile code blocks that had inlining.
        
Of course, giving the FTL the ability to handle code blocks that had inlining means that
we're going to have new bugs. Sure enough, the FTL's linker didn't handle inline call
frames. This patch also fixes that.

* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::::executeEffects):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleInlining):
(JSC::DFG::ByteCodeParser::InlineStackEntry::InlineStackEntry):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGGraph.h:
* dfg/DFGNode.h:
* dfg/DFGNodeType.h:
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::propagate):
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):
* dfg/DFGSpeculativeJIT.cpp:
* dfg/DFGSpeculativeJIT.h:
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGStackLayoutPhase.cpp:
(JSC::DFG::StackLayoutPhase::run):
* ftl/FTLLink.cpp:
(JSC::FTL::link):



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@158116 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent e3280cb3
2013-10-24 Filip Pizlo <fpizlo@apple.com>
Get rid of InlineStart so that I don't have to implement it in FTL
https://bugs.webkit.org/show_bug.cgi?id=123302
Reviewed by Geoffrey Garen.
InlineStart was a special instruction that we would insert at the top of inlined code,
so that the backend could capture the OSR state of arguments to an inlined call. It used
to be that only the backend had this information, so this instruction was sort of an ugly
callback from the backend for filling in some data structures.
But in the time since when that code was written (two years ago?), we rationalized how
variables work. It's now the case that variables that the runtime must know about are
treated specially in IR (they are "flushed") and we know how we will represent them even
before we get to the backend. The last place that makes changes to their representation
is the StackLayoutPhase.
So, this patch gets rid of InlineStart, but keeps around the special meta-data that the
instruction had. Instead of handling the bookkeeping in the backend, we handle it in
StackLayoutPhase. This means that the DFG and FTL can share code for handling this
bookkeeping. This also means that now the FTL can compile code blocks that had inlining.
Of course, giving the FTL the ability to handle code blocks that had inlining means that
we're going to have new bugs. Sure enough, the FTL's linker didn't handle inline call
frames. This patch also fixes that.
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::::executeEffects):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleInlining):
(JSC::DFG::ByteCodeParser::InlineStackEntry::InlineStackEntry):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGGraph.h:
* dfg/DFGNode.h:
* dfg/DFGNodeType.h:
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::propagate):
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):
* dfg/DFGSpeculativeJIT.cpp:
* dfg/DFGSpeculativeJIT.h:
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGStackLayoutPhase.cpp:
(JSC::DFG::StackLayoutPhase::run):
* ftl/FTLLink.cpp:
(JSC::FTL::link):
2013-10-24 Filip Pizlo <fpizlo@apple.com>
The GetById->GetByOffset AI-based optimization should actually do things
......
......@@ -1541,7 +1541,6 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
break;
case Phantom:
case InlineStart:
case CountExecution:
case CheckTierUpInLoop:
case CheckTierUpAtReturn:
......
......@@ -1304,11 +1304,11 @@ bool ByteCodeParser::handleInlining(Node* callTargetNode, int resultOperand, con
unsigned oldIndex = m_currentIndex;
m_currentIndex = 0;
InlineStartData* inlineStartData = &m_graph.m_inlineStartData.alloc();
inlineStartData->argumentPositionStart = argumentPositionStart;
inlineStartData->calleeVariable = 0;
InlineVariableData inlineVariableData;
inlineVariableData.inlineCallFrame = m_inlineStackTop->m_inlineCallFrame;
inlineVariableData.argumentPositionStart = argumentPositionStart;
inlineVariableData.calleeVariable = 0;
addToGraph(InlineStart, OpInfo(inlineStartData));
RELEASE_ASSERT(
m_inlineStackTop->m_inlineCallFrame->isClosureCall
== callLinkStatus.isClosureCall());
......@@ -1321,9 +1321,11 @@ bool ByteCodeParser::handleInlining(Node* callTargetNode, int resultOperand, con
calleeVariable->mergeShouldNeverUnbox(true);
scopeVariable->mergeShouldNeverUnbox(true);
inlineStartData->calleeVariable = calleeVariable;
inlineVariableData.calleeVariable = calleeVariable;
}
m_graph.m_inlineVariableData.append(inlineVariableData);
parseCodeBlock();
m_currentIndex = oldIndex;
......@@ -3370,7 +3372,7 @@ ByteCodeParser::InlineStackEntry::InlineStackEntry(
m_inlineCallFrame->executable,
byteCodeParser->m_codeBlock,
m_inlineCallFrame,
byteCodeParser->m_codeBlock->ownerExecutable(),
byteCodeParser->m_codeBlock->ownerExecutable(),
codeBlock->ownerExecutable());
m_inlineCallFrame->stackOffset = inlineCallFrameStart.offset() - JSStack::CallFrameHeaderSize;
if (callee) {
......
......@@ -124,7 +124,6 @@ void clobberize(Graph& graph, Node* node, ReadFunctor& read, WriteFunctor& write
case Flush:
case PhantomLocal:
case SetArgument:
case InlineStart:
case Breakpoint:
case PhantomArguments:
case Jump:
......
......@@ -907,7 +907,6 @@ private:
case Flush:
case PhantomLocal:
case GetLocalUnlinked:
case InlineStart:
case GetMyScope:
case GetClosureVar:
case GetGlobalVar:
......
......@@ -61,6 +61,12 @@ struct StorageAccessData {
unsigned identifierNumber;
};
struct InlineVariableData {
InlineCallFrame* inlineCallFrame;
unsigned argumentPositionStart;
VariableAccessData* calleeVariable;
};
enum AddSpeculationMode {
DontSpeculateInt32,
SpeculateInt32AndTruncateConstants,
......@@ -789,7 +795,7 @@ public:
SegmentedVector<StructureTransitionData, 8> m_structureTransitionData;
SegmentedVector<NewArrayBufferData, 4> m_newArrayBufferData;
SegmentedVector<SwitchData, 4> m_switchData;
SegmentedVector<InlineStartData, 4> m_inlineStartData;
Vector<InlineVariableData, 4> m_inlineVariableData;
OwnPtr<InlineCallFrameSet> m_inlineCallFrames;
bool m_hasArguments;
HashSet<ExecutableBase*> m_executablesWhoseArgumentsEscaped;
......
......@@ -139,11 +139,6 @@ struct SwitchData {
bool didUseJumpTable;
};
struct InlineStartData {
unsigned argumentPositionStart;
VariableAccessData* calleeVariable;
};
// This type used in passing an immediate argument to Node constructor;
// distinguishes an immediate value (typically an index into a CodeBlock data structure -
// a constant index, argument, or identifier) from a Node*.
......@@ -1124,17 +1119,6 @@ struct Node {
m_virtualRegister = virtualRegister;
}
bool hasInlineStartData()
{
return op() == InlineStart;
}
InlineStartData* inlineStartData()
{
ASSERT(hasInlineStartData());
return bitwise_cast<InlineStartData*>(m_opInfo);
}
bool hasExecutionCounter()
{
return op() == CountExecution;
......
......@@ -91,11 +91,6 @@ namespace JSC { namespace DFG {
/* Marker for an argument being set at the prologue of a function. */\
macro(SetArgument, 0 | NodeDoesNotExit) \
\
/* Hint that inlining begins here. No code is generated for this node. It's only */\
/* used for copying OSR data into inline frame data, to support reification of */\
/* call frames of inlined functions. */\
macro(InlineStart, NodeMustGenerate | NodeDoesNotExit) \
\
/* Nodes for bitwise operations. */\
macro(BitAnd, NodeResultInt32 | NodeMustGenerate) \
macro(BitOr, NodeResultInt32 | NodeMustGenerate) \
......
......@@ -586,7 +586,6 @@ private:
break;
// These gets ignored because it doesn't do anything.
case InlineStart:
case CountExecution:
case PhantomLocal:
case Flush:
......
......@@ -129,7 +129,6 @@ bool safeToExecute(AbstractStateType& state, Graph& graph, Node* node)
case PhantomLocal:
case GetLocalUnlinked:
case SetArgument:
case InlineStart:
case BitAnd:
case BitOr:
case BitXor:
......
......@@ -1557,37 +1557,6 @@ void SpeculativeJIT::compileMovHintAndCheck(Node* node)
noResult(node);
}
void SpeculativeJIT::compileInlineStart(Node* node)
{
InlineCallFrame* inlineCallFrame = node->codeOrigin.inlineCallFrame;
InlineStartData* data = node->inlineStartData();
int argumentCountIncludingThis = inlineCallFrame->arguments.size();
for (int i = 0; i < argumentCountIncludingThis; ++i) {
ArgumentPosition& position = m_jit.graph().m_argumentPositions[
data->argumentPositionStart + i];
VariableAccessData* variable = position.someVariable();
ValueSource source;
if (!variable)
source = ValueSource(SourceIsDead);
else {
source = ValueSource::forFlushFormat(
variable->machineLocal(),
m_jit.graph().m_argumentPositions[data->argumentPositionStart + i].flushFormat());
}
inlineCallFrame->arguments[i] = source.valueRecovery();
}
RELEASE_ASSERT(inlineCallFrame->isClosureCall == !!data->calleeVariable);
if (inlineCallFrame->isClosureCall) {
ValueSource source = ValueSource::forFlushFormat(
data->calleeVariable->machineLocal(),
data->calleeVariable->flushFormat());
inlineCallFrame->calleeRecovery = source.valueRecovery();
} else
RELEASE_ASSERT(inlineCallFrame->calleeRecovery.isConstant());
}
void SpeculativeJIT::bail()
{
m_compileOkay = true;
......
......@@ -707,7 +707,6 @@ public:
void compileMovHint(Node*);
void compileMovHintAndCheck(Node*);
void compileInlineStart(Node*);
void nonSpeculativeUInt32ToNumber(Node*);
......
......@@ -1958,11 +1958,6 @@ void SpeculativeJIT::compile(Node* node)
break;
}
case InlineStart: {
compileInlineStart(node);
break;
}
case MovHint:
case ZombieHint: {
RELEASE_ASSERT_NOT_REACHED();
......
......@@ -2269,11 +2269,6 @@ void SpeculativeJIT::compile(Node* node)
break;
}
case InlineStart: {
compileInlineStart(node);
break;
}
case MovHint:
case ZombieHint: {
RELEASE_ASSERT_NOT_REACHED();
......
......@@ -30,6 +30,7 @@
#include "DFGGraph.h"
#include "DFGPhase.h"
#include "DFGValueSource.h"
#include "Operations.h"
namespace JSC { namespace DFG {
......@@ -166,35 +167,42 @@ public:
virtualRegisterForLocal(allocation[codeBlock()->activationRegister().toLocal()]));
}
for (InlineCallFrameSet::iterator iter = m_graph.m_inlineCallFrames->begin(); !!iter; ++iter) {
InlineCallFrame* inlineCallFrame = *iter;
if (!inlineCallFrame->executable->usesArguments())
continue;
inlineCallFrame->argumentsRegister = virtualRegisterForLocal(
allocation[m_graph.argumentsRegisterFor(inlineCallFrame).toLocal()]);
for (unsigned i = m_graph.m_inlineVariableData.size(); i--;) {
InlineVariableData data = m_graph.m_inlineVariableData[i];
InlineCallFrame* inlineCallFrame = data.inlineCallFrame;
if (inlineCallFrame->executable->usesArguments()) {
inlineCallFrame->argumentsRegister = virtualRegisterForLocal(
allocation[m_graph.argumentsRegisterFor(inlineCallFrame).toLocal()]);
RELEASE_ASSERT(
virtualRegisterForLocal(allocation[unmodifiedArgumentsRegister(
m_graph.argumentsRegisterFor(inlineCallFrame)).toLocal()])
== unmodifiedArgumentsRegister(inlineCallFrame->argumentsRegister));
}
// SpeculativeJIT::compileInlineStart() will do the same thing that this does,
// for cases where usesArguments() is true. That's OK. compileInlineStart() is
// designed for cases where the arguments end up having interesting data
// representations, but that only happens if they're not captured. And if they
// are captured, then we want to make sure everyone agrees on where they landed
// in the stack before we start generating any code - since SpeculativeJIT does
// not have to generate code in any particular order.
for (unsigned argument = inlineCallFrame->arguments.size(); argument-- > 1;) {
VirtualRegister originalRegister = VirtualRegister(
virtualRegisterForArgument(argument).offset() +
inlineCallFrame->stackOffset);
VirtualRegister newRegister =
virtualRegisterForLocal(allocation[originalRegister.toLocal()]);
inlineCallFrame->arguments[argument] =
ValueRecovery::displacedInJSStack(newRegister, DataFormatJS);
ArgumentPosition& position = m_graph.m_argumentPositions[
data.argumentPositionStart + argument];
VariableAccessData* variable = position.someVariable();
ValueSource source;
if (!variable)
source = ValueSource(SourceIsDead);
else {
source = ValueSource::forFlushFormat(
variable->machineLocal(), variable->flushFormat());
}
inlineCallFrame->arguments[argument] = source.valueRecovery();
}
RELEASE_ASSERT(
virtualRegisterForLocal(allocation[
unmodifiedArgumentsRegister(
m_graph.argumentsRegisterFor(inlineCallFrame)).toLocal()])
== unmodifiedArgumentsRegister(inlineCallFrame->argumentsRegister));
RELEASE_ASSERT(inlineCallFrame->isClosureCall == !!data.calleeVariable);
if (inlineCallFrame->isClosureCall) {
ValueSource source = ValueSource::forFlushFormat(
data.calleeVariable->machineLocal(),
data.calleeVariable->flushFormat());
inlineCallFrame->calleeRecovery = source.valueRecovery();
} else
RELEASE_ASSERT(inlineCallFrame->calleeRecovery.isConstant());
}
if (symbolTable) {
......@@ -223,7 +231,7 @@ public:
}
}
// Finally, fix GetLocalUnlinked's.
// Fix GetLocalUnlinked's variable references.
if (hasGetLocalUnlinked) {
for (BlockIndex blockIndex = m_graph.numBlocks(); blockIndex--;) {
BasicBlock* block = m_graph.block(blockIndex);
......
......@@ -57,6 +57,9 @@ void link(State& state)
// LLVM will create its own jump tables as needed.
codeBlock->clearSwitchJumpTables();
if (!state.graph.m_inlineCallFrames->isEmpty())
state.jitCode->common.inlineCallFrames = std::move(state.graph.m_inlineCallFrames);
// Create the entrypoint. Note that we use this entrypoint totally differently
// depending on whether we're doing OSR entry or not.
// FIXME: Except for OSR entry, this is a total kludge - LLVM should just use our
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment