Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (158115 => 158116)
--- trunk/Source/_javascript_Core/ChangeLog 2013-10-28 17:35:52 UTC (rev 158115)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-10-28 18:03:38 UTC (rev 158116)
@@ -1,5 +1,59 @@
2013-10-24 Filip Pizlo <[email protected]>
+ 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 <[email protected]>
+
The GetById->GetByOffset AI-based optimization should actually do things
https://bugs.webkit.org/show_bug.cgi?id=123299
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (158115 => 158116)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2013-10-28 17:35:52 UTC (rev 158115)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2013-10-28 18:03:38 UTC (rev 158116)
@@ -1541,7 +1541,6 @@
break;
case Phantom:
- case InlineStart:
case CountExecution:
case CheckTierUpInLoop:
case CheckTierUpAtReturn:
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (158115 => 158116)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2013-10-28 17:35:52 UTC (rev 158115)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2013-10-28 18:03:38 UTC (rev 158116)
@@ -1304,11 +1304,11 @@
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 @@
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 @@
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) {
Modified: trunk/Source/_javascript_Core/dfg/DFGClobberize.h (158115 => 158116)
--- trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2013-10-28 17:35:52 UTC (rev 158115)
+++ trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2013-10-28 18:03:38 UTC (rev 158116)
@@ -124,7 +124,6 @@
case Flush:
case PhantomLocal:
case SetArgument:
- case InlineStart:
case Breakpoint:
case PhantomArguments:
case Jump:
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (158115 => 158116)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2013-10-28 17:35:52 UTC (rev 158115)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2013-10-28 18:03:38 UTC (rev 158116)
@@ -907,7 +907,6 @@
case Flush:
case PhantomLocal:
case GetLocalUnlinked:
- case InlineStart:
case GetMyScope:
case GetClosureVar:
case GetGlobalVar:
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.h (158115 => 158116)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.h 2013-10-28 17:35:52 UTC (rev 158115)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.h 2013-10-28 18:03:38 UTC (rev 158116)
@@ -61,6 +61,12 @@
unsigned identifierNumber;
};
+struct InlineVariableData {
+ InlineCallFrame* inlineCallFrame;
+ unsigned argumentPositionStart;
+ VariableAccessData* calleeVariable;
+};
+
enum AddSpeculationMode {
DontSpeculateInt32,
SpeculateInt32AndTruncateConstants,
@@ -789,7 +795,7 @@
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;
Modified: trunk/Source/_javascript_Core/dfg/DFGNode.h (158115 => 158116)
--- trunk/Source/_javascript_Core/dfg/DFGNode.h 2013-10-28 17:35:52 UTC (rev 158115)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.h 2013-10-28 18:03:38 UTC (rev 158116)
@@ -139,11 +139,6 @@
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 @@
m_virtualRegister = virtualRegister;
}
- bool hasInlineStartData()
- {
- return op() == InlineStart;
- }
-
- InlineStartData* inlineStartData()
- {
- ASSERT(hasInlineStartData());
- return bitwise_cast<InlineStartData*>(m_opInfo);
- }
-
bool hasExecutionCounter()
{
return op() == CountExecution;
Modified: trunk/Source/_javascript_Core/dfg/DFGNodeType.h (158115 => 158116)
--- trunk/Source/_javascript_Core/dfg/DFGNodeType.h 2013-10-28 17:35:52 UTC (rev 158115)
+++ trunk/Source/_javascript_Core/dfg/DFGNodeType.h 2013-10-28 18:03:38 UTC (rev 158116)
@@ -91,11 +91,6 @@
/* 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) \
Modified: trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (158115 => 158116)
--- trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2013-10-28 17:35:52 UTC (rev 158115)
+++ trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2013-10-28 18:03:38 UTC (rev 158116)
@@ -586,7 +586,6 @@
break;
// These gets ignored because it doesn't do anything.
- case InlineStart:
case CountExecution:
case PhantomLocal:
case Flush:
Modified: trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h (158115 => 158116)
--- trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h 2013-10-28 17:35:52 UTC (rev 158115)
+++ trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h 2013-10-28 18:03:38 UTC (rev 158116)
@@ -129,7 +129,6 @@
case PhantomLocal:
case GetLocalUnlinked:
case SetArgument:
- case InlineStart:
case BitAnd:
case BitOr:
case BitXor:
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (158115 => 158116)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2013-10-28 17:35:52 UTC (rev 158115)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2013-10-28 18:03:38 UTC (rev 158116)
@@ -1557,37 +1557,6 @@
noResult(node);
}
-void SpeculativeJIT::compileInlineStart(Node* node)
-{
- InlineCallFrame* inlineCallFrame = node->codeOrigin.inlineCallFrame;
- InlineStartData* data = ""
- 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;
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (158115 => 158116)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2013-10-28 17:35:52 UTC (rev 158115)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2013-10-28 18:03:38 UTC (rev 158116)
@@ -707,7 +707,6 @@
void compileMovHint(Node*);
void compileMovHintAndCheck(Node*);
- void compileInlineStart(Node*);
void nonSpeculativeUInt32ToNumber(Node*);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (158115 => 158116)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2013-10-28 17:35:52 UTC (rev 158115)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2013-10-28 18:03:38 UTC (rev 158116)
@@ -1958,11 +1958,6 @@
break;
}
- case InlineStart: {
- compileInlineStart(node);
- break;
- }
-
case MovHint:
case ZombieHint: {
RELEASE_ASSERT_NOT_REACHED();
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (158115 => 158116)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2013-10-28 17:35:52 UTC (rev 158115)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2013-10-28 18:03:38 UTC (rev 158116)
@@ -2269,11 +2269,6 @@
break;
}
- case InlineStart: {
- compileInlineStart(node);
- break;
- }
-
case MovHint:
case ZombieHint: {
RELEASE_ASSERT_NOT_REACHED();
Modified: trunk/Source/_javascript_Core/dfg/DFGStackLayoutPhase.cpp (158115 => 158116)
--- trunk/Source/_javascript_Core/dfg/DFGStackLayoutPhase.cpp 2013-10-28 17:35:52 UTC (rev 158115)
+++ trunk/Source/_javascript_Core/dfg/DFGStackLayoutPhase.cpp 2013-10-28 18:03:38 UTC (rev 158116)
@@ -30,6 +30,7 @@
#include "DFGGraph.h"
#include "DFGPhase.h"
+#include "DFGValueSource.h"
#include "Operations.h"
namespace JSC { namespace DFG {
@@ -166,35 +167,42 @@
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 = ""
+ InlineCallFrame* inlineCallFrame = data.inlineCallFrame;
- // 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.
+ 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));
+ }
+
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 @@
}
}
- // Finally, fix GetLocalUnlinked's.
+ // Fix GetLocalUnlinked's variable references.
if (hasGetLocalUnlinked) {
for (BlockIndex blockIndex = m_graph.numBlocks(); blockIndex--;) {
BasicBlock* block = m_graph.block(blockIndex);
Modified: trunk/Source/_javascript_Core/ftl/FTLLink.cpp (158115 => 158116)
--- trunk/Source/_javascript_Core/ftl/FTLLink.cpp 2013-10-28 17:35:52 UTC (rev 158115)
+++ trunk/Source/_javascript_Core/ftl/FTLLink.cpp 2013-10-28 18:03:38 UTC (rev 158116)
@@ -57,6 +57,9 @@
// 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
