Source/JavaScriptCore: DFG inlining breaks function.arguments[something] if the argument being
retrieved was subjected to DFG's unboxing optimizations
https://bugs.webkit.org/show_bug.cgi?id=71436
Reviewed by Oliver Hunt.
This makes inlined arguments retrieval use some of the same machinery as
OSR to determine where from, and how, to retrieve a value that the DFG
might have somehow squirreled away while the old JIT would put it in its
obvious location, using an obvious format.
To that end, previously DFG-internal notions such as DataFormat,
VirtualRegister, and ValueRecovery are now in bytecode/ since they are
stored as part of InlineCallFrames.
* bytecode/CodeOrigin.h:
* dfg/DFGAbstractState.cpp:
(JSC::DFG::AbstractState::execute):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleInlining):
(JSC::DFG::ByteCodeParser::InlineStackEntry::InlineStackEntry):
* dfg/DFGJITCompiler.cpp:
(JSC::DFG::JITCompiler::exitSpeculativeWithOSR):
* dfg/DFGJITCompiler32_64.cpp:
(JSC::DFG::JITCompiler::exitSpeculativeWithOSR):
* dfg/DFGNode.h:
* dfg/DFGPropagator.cpp:
(JSC::DFG::Propagator::propagateNodePredictions):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* interpreter/CallFrame.cpp:
(JSC::CallFrame::trueCallerFrame):
* interpreter/CallFrame.h:
(JSC::ExecState::inlineCallFrame):
* interpreter/Register.h:
(JSC::Register::asInlineCallFrame):
(JSC::Register::unboxedInt32):
(JSC::Register::unboxedBoolean):
(JSC::Register::unboxedCell):
* runtime/Arguments.h:
(JSC::Arguments::finishCreationAndCopyRegisters):
LayoutTests: DFG inlining breaks function.arguments[something] if the argument being
retrieved was subjected to DFG's unboxing optimizations
https://bugs.webkit.org/show_bug.cgi?id=71436
Reviewed by Oliver Hunt.
* fast/js/dfg-inline-arguments-int32-expected.txt: Added.
* fast/js/dfg-inline-arguments-int32.html: Added.
* fast/js/script-tests/dfg-inline-arguments-int32.js: Added.
(foo):
(bar):
(baz):
(argsToStr):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@99148 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index d6dd1e5..988f750 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,5 +1,51 @@
2011-11-02 Filip Pizlo <fpizlo@apple.com>
+ DFG inlining breaks function.arguments[something] if the argument being
+ retrieved was subjected to DFG's unboxing optimizations
+ https://bugs.webkit.org/show_bug.cgi?id=71436
+
+ Reviewed by Oliver Hunt.
+
+ This makes inlined arguments retrieval use some of the same machinery as
+ OSR to determine where from, and how, to retrieve a value that the DFG
+ might have somehow squirreled away while the old JIT would put it in its
+ obvious location, using an obvious format.
+
+ To that end, previously DFG-internal notions such as DataFormat,
+ VirtualRegister, and ValueRecovery are now in bytecode/ since they are
+ stored as part of InlineCallFrames.
+
+ * bytecode/CodeOrigin.h:
+ * dfg/DFGAbstractState.cpp:
+ (JSC::DFG::AbstractState::execute):
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::handleInlining):
+ (JSC::DFG::ByteCodeParser::InlineStackEntry::InlineStackEntry):
+ * dfg/DFGJITCompiler.cpp:
+ (JSC::DFG::JITCompiler::exitSpeculativeWithOSR):
+ * dfg/DFGJITCompiler32_64.cpp:
+ (JSC::DFG::JITCompiler::exitSpeculativeWithOSR):
+ * dfg/DFGNode.h:
+ * dfg/DFGPropagator.cpp:
+ (JSC::DFG::Propagator::propagateNodePredictions):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * interpreter/CallFrame.cpp:
+ (JSC::CallFrame::trueCallerFrame):
+ * interpreter/CallFrame.h:
+ (JSC::ExecState::inlineCallFrame):
+ * interpreter/Register.h:
+ (JSC::Register::asInlineCallFrame):
+ (JSC::Register::unboxedInt32):
+ (JSC::Register::unboxedBoolean):
+ (JSC::Register::unboxedCell):
+ * runtime/Arguments.h:
+ (JSC::Arguments::finishCreationAndCopyRegisters):
+
+2011-11-02 Filip Pizlo <fpizlo@apple.com>
+
ValueRecovery should be moved out of the DFG JIT
https://bugs.webkit.org/show_bug.cgi?id=71439
diff --git a/Source/JavaScriptCore/bytecode/CodeOrigin.h b/Source/JavaScriptCore/bytecode/CodeOrigin.h
index 8a2d65e..045e645 100644
--- a/Source/JavaScriptCore/bytecode/CodeOrigin.h
+++ b/Source/JavaScriptCore/bytecode/CodeOrigin.h
@@ -26,6 +26,7 @@
#ifndef CodeOrigin_h
#define CodeOrigin_h
+#include "ValueRecovery.h"
#include "WriteBarrier.h"
#include <wtf/StdLibExtras.h>
#include <wtf/Vector.h>
@@ -75,11 +76,11 @@
};
struct InlineCallFrame {
+ Vector<ValueRecovery> arguments;
WriteBarrier<ExecutableBase> executable;
WriteBarrier<JSFunction> callee;
CodeOrigin caller;
- unsigned stackOffset;
- unsigned numArgumentsIncludingThis : 31;
+ unsigned stackOffset : 31;
bool isCall : 1;
};
diff --git a/Source/JavaScriptCore/dfg/DFGAbstractState.cpp b/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
index 329db61..00ad7ce 100644
--- a/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
+++ b/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
@@ -659,6 +659,7 @@
break;
case Phantom:
+ case InlineStart:
break;
}
diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
index 0c99438..2c8368f 100644
--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
@@ -1003,7 +1003,11 @@
// This is where the actual inlining really happens.
unsigned oldIndex = m_currentIndex;
m_currentIndex = 0;
+
+ addToGraph(InlineStart);
+
parseCodeBlock();
+
m_currentIndex = oldIndex;
// If the inlined code created some new basic blocks, then we have linking to do.
@@ -2326,7 +2330,7 @@
inlineCallFrame.stackOffset = inlineCallFrameStart + RegisterFile::CallFrameHeaderSize;
inlineCallFrame.callee.set(*byteCodeParser->m_globalData, byteCodeParser->m_codeBlock->ownerExecutable(), callee);
inlineCallFrame.caller = byteCodeParser->currentCodeOrigin();
- inlineCallFrame.numArgumentsIncludingThis = codeBlock->m_numParameters;
+ inlineCallFrame.arguments.resize(codeBlock->m_numParameters); // Set the number of arguments including this, but don't configure the value recoveries, yet.
inlineCallFrame.isCall = isCall(kind);
byteCodeParser->m_codeBlock->inlineCallFrames().append(inlineCallFrame);
m_inlineCallFrame = &byteCodeParser->m_codeBlock->inlineCallFrames().last();
diff --git a/Source/JavaScriptCore/dfg/DFGJITCompiler.cpp b/Source/JavaScriptCore/dfg/DFGJITCompiler.cpp
index 87537f5..466b5aa 100644
--- a/Source/JavaScriptCore/dfg/DFGJITCompiler.cpp
+++ b/Source/JavaScriptCore/dfg/DFGJITCompiler.cpp
@@ -783,7 +783,7 @@
storePtr(TrustedImmPtr(inlineCallFrame->callee->scope()), addressFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::ScopeChain)));
storePtr(callerFrameGPR, addressFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::CallerFrame)));
storePtr(TrustedImmPtr(jumpTarget), addressFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::ReturnPC)));
- storePtr(TrustedImmPtr(JSValue::encode(jsNumber(inlineCallFrame->numArgumentsIncludingThis))), addressFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::ArgumentCount)));
+ storePtr(TrustedImmPtr(JSValue::encode(jsNumber(inlineCallFrame->arguments.size()))), addressFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::ArgumentCount)));
storePtr(TrustedImmPtr(inlineCallFrame->callee.get()), addressFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::Callee)));
}
diff --git a/Source/JavaScriptCore/dfg/DFGJITCompiler32_64.cpp b/Source/JavaScriptCore/dfg/DFGJITCompiler32_64.cpp
index fce5ddb3..a6868f4 100644
--- a/Source/JavaScriptCore/dfg/DFGJITCompiler32_64.cpp
+++ b/Source/JavaScriptCore/dfg/DFGJITCompiler32_64.cpp
@@ -528,7 +528,7 @@
storePtr(callerFrameGPR, payloadFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::CallerFrame)));
storePtr(TrustedImmPtr(jumpTarget), payloadFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::ReturnPC)));
store32(Imm32(JSValue::Int32Tag), tagFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::ArgumentCount)));
- store32(Imm32(inlineCallFrame->numArgumentsIncludingThis), payloadFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::ArgumentCount)));
+ store32(Imm32(inlineCallFrame->arguments.size()), payloadFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::ArgumentCount)));
store32(Imm32(JSValue::CellTag), tagFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::Callee)));
storePtr(TrustedImmPtr(inlineCallFrame->callee.get()), payloadFor((VirtualRegister)(inlineCallFrame->stackOffset + RegisterFile::Callee)));
}
diff --git a/Source/JavaScriptCore/dfg/DFGNode.h b/Source/JavaScriptCore/dfg/DFGNode.h
index d4726d0..f261a63 100644
--- a/Source/JavaScriptCore/dfg/DFGNode.h
+++ b/Source/JavaScriptCore/dfg/DFGNode.h
@@ -220,6 +220,11 @@
/* Marker for arguments being set. */\
macro(SetArgument, 0) \
\
+ /* 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, 0) \
+ \
/* Nodes for bitwise operations. */\
macro(BitAnd, NodeResultInt32) \
macro(BitOr, NodeResultInt32) \
diff --git a/Source/JavaScriptCore/dfg/DFGPropagator.cpp b/Source/JavaScriptCore/dfg/DFGPropagator.cpp
index 17737e2..c3901b8 100644
--- a/Source/JavaScriptCore/dfg/DFGPropagator.cpp
+++ b/Source/JavaScriptCore/dfg/DFGPropagator.cpp
@@ -607,8 +607,9 @@
case PutByOffset:
break;
- // This gets ignored because it doesn't do anything.
+ // These gets ignored because it doesn't do anything.
case Phantom:
+ case InlineStart:
break;
#else
default:
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
index d58aa45..19f28fc 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -263,8 +263,28 @@
#if DFG_ENABLE(DEBUG_VERBOSE)
fprintf(stderr, "SpeculativeJIT skipping Node @%d (bc#%u) at JIT offset 0x%x ", (int)m_compileIndex, node.codeOrigin.bytecodeIndex, m_jit.debugOffset());
#endif
- if (node.op == SetLocal)
+ switch (node.op) {
+ case SetLocal:
compileMovHint(node);
+ break;
+
+ case InlineStart: {
+ InlineCallFrame* inlineCallFrame = node.codeOrigin.inlineCallFrame;
+ unsigned argumentsStart = inlineCallFrame->stackOffset - RegisterFile::CallFrameHeaderSize - inlineCallFrame->arguments.size();
+ for (unsigned i = 0; i < inlineCallFrame->arguments.size(); ++i) {
+ ValueRecovery recovery = computeValueRecoveryFor(m_variables[argumentsStart + i]);
+ // The recovery cannot point to registers, since the call frame reification isn't
+ // as smart as OSR, so it can't handle that. The exception is the this argument,
+ // which we don't really need to be able to recover.
+ ASSERT(!i || !recovery.isInRegisters());
+ inlineCallFrame->arguments[i] = recovery;
+ }
+ break;
+ }
+
+ default:
+ break;
+ }
} else {
#if DFG_ENABLE(DEBUG_VERBOSE)
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
index d343d46..c384ab0 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
@@ -2491,6 +2491,10 @@
// This is a no-op.
noResult(m_compileIndex);
break;
+
+ case InlineStart:
+ ASSERT_NOT_REACHED();
+ break;
}
if (node.hasResult() && node.mustGenerate())
diff --git a/Source/JavaScriptCore/interpreter/CallFrame.cpp b/Source/JavaScriptCore/interpreter/CallFrame.cpp
index 5fa4599..6dba778 100644
--- a/Source/JavaScriptCore/interpreter/CallFrame.cpp
+++ b/Source/JavaScriptCore/interpreter/CallFrame.cpp
@@ -110,7 +110,7 @@
inlinedCaller->setCallerFrame(machineCaller);
inlinedCaller->setInlineCallFrame(inlineCallFrame);
- inlinedCaller->setArgumentCountIncludingThis(inlineCallFrame->numArgumentsIncludingThis);
+ inlinedCaller->setArgumentCountIncludingThis(inlineCallFrame->arguments.size());
inlinedCaller->setCallee(calleeAsFunction);
inlineCallFrame = nextInlineCallFrame;
diff --git a/Source/JavaScriptCore/interpreter/CallFrame.h b/Source/JavaScriptCore/interpreter/CallFrame.h
index 61c7320..123edac 100644
--- a/Source/JavaScriptCore/interpreter/CallFrame.h
+++ b/Source/JavaScriptCore/interpreter/CallFrame.h
@@ -105,7 +105,16 @@
ReturnAddressPtr returnPC() const { return ReturnAddressPtr(this[RegisterFile::ReturnPC].vPC()); }
#endif
#if ENABLE(DFG_JIT)
- InlineCallFrame* inlineCallFrame() const { return this[RegisterFile::ReturnPC].inlineCallFrame(); }
+ InlineCallFrame* inlineCallFrame() const { return this[RegisterFile::ReturnPC].asInlineCallFrame(); }
+#else
+ // This will never be called if !ENABLE(DFG_JIT) since all calls should be guarded by
+ // isInlineCallFrame(). But to make it easier to write code without having a bunch of
+ // #if's, we make a dummy implementation available anyway.
+ InlineCallFrame* inlineCallFrame() const
+ {
+ ASSERT_NOT_REACHED();
+ return 0;
+ }
#endif
#if ENABLE(INTERPRETER)
Instruction* returnVPC() const { return this[RegisterFile::ReturnPC].vPC(); }
diff --git a/Source/JavaScriptCore/interpreter/Register.h b/Source/JavaScriptCore/interpreter/Register.h
index adad1d7..deaa68d 100644
--- a/Source/JavaScriptCore/interpreter/Register.h
+++ b/Source/JavaScriptCore/interpreter/Register.h
@@ -71,7 +71,10 @@
JSPropertyNameIterator* propertyNameIterator() const;
ScopeChainNode* scopeChain() const;
Instruction* vPC() const;
- InlineCallFrame* inlineCallFrame() const;
+ InlineCallFrame* asInlineCallFrame() const;
+ int32_t unboxedInt32() const;
+ bool unboxedBoolean() const;
+ JSCell* unboxedCell() const;
static Register withInt(int32_t i)
{
@@ -88,6 +91,7 @@
CodeBlock* codeBlock;
Instruction* vPC;
InlineCallFrame* inlineCallFrame;
+ EncodedValueDescriptor encodedValue;
} u;
};
@@ -165,10 +169,29 @@
return u.vPC;
}
- ALWAYS_INLINE InlineCallFrame* Register::inlineCallFrame() const
+ ALWAYS_INLINE InlineCallFrame* Register::asInlineCallFrame() const
{
return u.inlineCallFrame;
}
+
+ ALWAYS_INLINE int32_t Register::unboxedInt32() const
+ {
+ return u.encodedValue.asBits.payload;
+ }
+
+ ALWAYS_INLINE bool Register::unboxedBoolean() const
+ {
+ return !!u.encodedValue.asBits.payload;
+ }
+
+ ALWAYS_INLINE JSCell* Register::unboxedCell() const
+ {
+#if USE(JSVALUE64)
+ return u.encodedValue.ptr;
+#else
+ return bitwise_cast<JSCell*>(u.encodedValue.asBits.payload);
+#endif
+ }
} // namespace JSC
diff --git a/Source/JavaScriptCore/runtime/Arguments.h b/Source/JavaScriptCore/runtime/Arguments.h
index 0175e39..bfb4d80 100644
--- a/Source/JavaScriptCore/runtime/Arguments.h
+++ b/Source/JavaScriptCore/runtime/Arguments.h
@@ -259,8 +259,43 @@
size_t registerArraySize = d->numParameters;
OwnArrayPtr<WriteBarrier<Unknown> > registerArray = adoptArrayPtr(new WriteBarrier<Unknown>[registerArraySize]);
- for (size_t i = 0; i < registerArraySize; ++i)
- registerArray[i].set(callFrame->globalData(), this, callFrame->registers()[i - registerOffset].jsValue());
+ if (callFrame->isInlineCallFrame()) {
+ InlineCallFrame* inlineCallFrame = callFrame->inlineCallFrame();
+ for (size_t i = 0; i < registerArraySize; ++i) {
+ ValueRecovery& recovery = inlineCallFrame->arguments[i + 1];
+ // In the future we'll support displaced recoveries (indicating that the
+ // argument was flushed to a different location), but for now we don't do
+ // that so this code will fail if that were to happen. On the other hand,
+ // it's much less likely that we'll support in-register recoveries since
+ // this code does not (easily) have access to registers.
+ JSValue value;
+ Register* location = callFrame->registers() + i - registerOffset;
+ switch (recovery.technique()) {
+ case AlreadyInRegisterFile:
+ value = location->jsValue();
+ break;
+ case AlreadyInRegisterFileAsUnboxedInt32:
+ value = jsNumber(location->unboxedInt32());
+ break;
+ case AlreadyInRegisterFileAsUnboxedCell:
+ value = location->unboxedCell();
+ break;
+ case AlreadyInRegisterFileAsUnboxedBoolean:
+ value = jsBoolean(location->unboxedBoolean());
+ break;
+ case Constant:
+ value = recovery.constant();
+ break;
+ default:
+ ASSERT_NOT_REACHED();
+ break;
+ }
+ registerArray[i].set(callFrame->globalData(), this, value);
+ }
+ } else {
+ for (size_t i = 0; i < registerArraySize; ++i)
+ registerArray[i].set(callFrame->globalData(), this, callFrame->registers()[i - registerOffset].jsValue());
+ }
d->registers = registerArray.get() + d->numParameters + RegisterFile::CallFrameHeaderSize;
d->registerArray = registerArray.release();
}
