- Revision
- 149941
- Author
- [email protected]
- Date
- 2013-05-11 14:07:51 -0700 (Sat, 11 May 2013)
Log Message
fourthTier: FTL should support Jump and ForceOSRExit
https://bugs.webkit.org/show_bug.cgi?id=115942
Reviewed by Oliver Hunt.
Added two obvious nodes: Jump and ForceOSRExit. We already had everything we needed
to support them.
Adding these increases our coverage a fair bit, and revealed a bug: LLVM's full
instruction selector currently appears to mishandle doubles in constant pools (or
just constant pools in general) with the small code model in the MCJIT. But switching
to FastISel "fixes" it. That's what this patch does, for now. This will probably
actually be permanent; the FastISel does pretty much everything we would ever want,
at least in the foreseeable future.
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
(FTL):
* ftl/FTLCompile.cpp:
(JSC::FTL::compile):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::compileBlock):
(JSC::FTL::LowerDFGToLLVM::compileNode):
(JSC::FTL::LowerDFGToLLVM::compileJSConstant):
(LowerDFGToLLVM):
(JSC::FTL::LowerDFGToLLVM::compileJump):
(JSC::FTL::LowerDFGToLLVM::compileReturn):
(JSC::FTL::LowerDFGToLLVM::compileForceOSRExit):
* runtime/Options.h:
(JSC):
Modified Paths
Diff
Modified: branches/dfgFourthTier/Source/_javascript_Core/ChangeLog (149940 => 149941)
--- branches/dfgFourthTier/Source/_javascript_Core/ChangeLog 2013-05-11 20:57:29 UTC (rev 149940)
+++ branches/dfgFourthTier/Source/_javascript_Core/ChangeLog 2013-05-11 21:07:51 UTC (rev 149941)
@@ -1,3 +1,36 @@
+2013-05-11 Filip Pizlo <[email protected]>
+
+ fourthTier: FTL should support Jump and ForceOSRExit
+ https://bugs.webkit.org/show_bug.cgi?id=115942
+
+ Reviewed by Oliver Hunt.
+
+ Added two obvious nodes: Jump and ForceOSRExit. We already had everything we needed
+ to support them.
+
+ Adding these increases our coverage a fair bit, and revealed a bug: LLVM's full
+ instruction selector currently appears to mishandle doubles in constant pools (or
+ just constant pools in general) with the small code model in the MCJIT. But switching
+ to FastISel "fixes" it. That's what this patch does, for now. This will probably
+ actually be permanent; the FastISel does pretty much everything we would ever want,
+ at least in the foreseeable future.
+
+ * ftl/FTLCapabilities.cpp:
+ (JSC::FTL::canCompile):
+ (FTL):
+ * ftl/FTLCompile.cpp:
+ (JSC::FTL::compile):
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::LowerDFGToLLVM::compileBlock):
+ (JSC::FTL::LowerDFGToLLVM::compileNode):
+ (JSC::FTL::LowerDFGToLLVM::compileJSConstant):
+ (LowerDFGToLLVM):
+ (JSC::FTL::LowerDFGToLLVM::compileJump):
+ (JSC::FTL::LowerDFGToLLVM::compileReturn):
+ (JSC::FTL::LowerDFGToLLVM::compileForceOSRExit):
+ * runtime/Options.h:
+ (JSC):
+
2013-05-10 Filip Pizlo <[email protected]>
fourthTier: FTL should support CompareStrictEqConstant
Modified: branches/dfgFourthTier/Source/_javascript_Core/ftl/FTLCapabilities.cpp (149940 => 149941)
--- branches/dfgFourthTier/Source/_javascript_Core/ftl/FTLCapabilities.cpp 2013-05-11 20:57:29 UTC (rev 149940)
+++ branches/dfgFourthTier/Source/_javascript_Core/ftl/FTLCapabilities.cpp 2013-05-11 21:07:51 UTC (rev 149941)
@@ -32,6 +32,108 @@
using namespace DFG;
+inline bool canCompile(Node* node)
+{
+ switch (node->op()) {
+ case JSConstant:
+ case WeakJSConstant:
+ case GetLocal:
+ case SetLocal:
+ case MovHintAndCheck:
+ case MovHint:
+ case ZombieHint:
+ case Phantom:
+ case Flush:
+ case PhantomLocal:
+ case SetArgument:
+ case Return:
+ case BitAnd:
+ case BitOr:
+ case BitXor:
+ case BitRShift:
+ case BitLShift:
+ case BitURShift:
+ case CheckStructure:
+ case StructureTransitionWatchpoint:
+ case PutStructure:
+ case PhantomPutStructure:
+ case GetButterfly:
+ case GetByOffset:
+ case PutByOffset:
+ case GetGlobalVar:
+ case PutGlobalVar:
+ case ValueAdd:
+ case ArithAdd:
+ case ArithSub:
+ case ArithMul:
+ case ArithNegate:
+ case UInt32ToNumber:
+ case Int32ToDouble:
+ case CompareEqConstant:
+ case CompareStrictEqConstant:
+ case Jump:
+ case ForceOSRExit:
+ // These are OK.
+ break;
+ case GetArrayLength:
+ switch (node->arrayMode().type()) {
+ case Array::Int32:
+ case Array::Double:
+ case Array::Contiguous:
+ break;
+ default:
+ return false;
+ }
+ break;
+ case GetByVal:
+ switch (node->arrayMode().type()) {
+ case Array::ForceExit:
+ return true;
+ case Array::Int32:
+ case Array::Double:
+ case Array::Contiguous:
+ break;
+ default:
+ return false;
+ }
+ switch (node->arrayMode().speculation()) {
+ case Array::SaneChain:
+ case Array::InBounds:
+ break;
+ default:
+ return false;
+ }
+ break;
+ case CompareEq:
+ case CompareStrictEq:
+ if (node->isBinaryUseKind(Int32Use))
+ break;
+ if (node->isBinaryUseKind(NumberUse))
+ break;
+ if (node->isBinaryUseKind(ObjectUse))
+ break;
+ return false;
+ case CompareLess:
+ case CompareLessEq:
+ case CompareGreater:
+ case CompareGreaterEq:
+ if (node->isBinaryUseKind(Int32Use))
+ break;
+ if (node->isBinaryUseKind(NumberUse))
+ break;
+ return false;
+ case Branch:
+ case LogicalNot:
+ if (node->child1().useKind() == BooleanUse)
+ break;
+ return false;
+ default:
+ // Don't know how to handle anything else.
+ return false;
+ }
+ return true;
+}
+
bool canCompile(Graph& graph)
{
for (BlockIndex blockIndex = graph.m_blocks.size(); blockIndex--;) {
@@ -65,99 +167,8 @@
}
}
- switch (node->op()) {
- case JSConstant:
- case WeakJSConstant:
- case GetLocal:
- case SetLocal:
- case MovHintAndCheck:
- case MovHint:
- case ZombieHint:
- case Phantom:
- case Flush:
- case PhantomLocal:
- case SetArgument:
- case Return:
- case BitAnd:
- case BitOr:
- case BitXor:
- case BitRShift:
- case BitLShift:
- case BitURShift:
- case CheckStructure:
- case StructureTransitionWatchpoint:
- case PutStructure:
- case PhantomPutStructure:
- case GetButterfly:
- case GetByOffset:
- case PutByOffset:
- case GetGlobalVar:
- case PutGlobalVar:
- case ValueAdd:
- case ArithAdd:
- case ArithSub:
- case ArithMul:
- case ArithNegate:
- case UInt32ToNumber:
- case Int32ToDouble:
- case CompareEqConstant:
- case CompareStrictEqConstant:
- // These are OK.
- break;
- case GetArrayLength:
- switch (node->arrayMode().type()) {
- case Array::Int32:
- case Array::Double:
- case Array::Contiguous:
- break;
- default:
- return false;
- }
- break;
- case GetByVal:
- switch (node->arrayMode().type()) {
- case Array::Int32:
- case Array::Double:
- case Array::Contiguous:
- break;
- default:
- return false;
- }
- switch (node->arrayMode().speculation()) {
- case Array::SaneChain:
- case Array::InBounds:
- break;
- default:
- return false;
- }
- break;
- case CompareEq:
- case CompareStrictEq:
- if (node->isBinaryUseKind(Int32Use))
- break;
- if (node->isBinaryUseKind(NumberUse))
- break;
- if (node->isBinaryUseKind(ObjectUse))
- break;
+ if (!canCompile(node))
return false;
- case CompareLess:
- case CompareLessEq:
- case CompareGreater:
- case CompareGreaterEq:
- if (node->isBinaryUseKind(Int32Use))
- break;
- if (node->isBinaryUseKind(NumberUse))
- break;
- return false;
- case Branch:
- case LogicalNot:
- if (node->child1().useKind() == BooleanUse)
- break;
- return false;
- default:
- // Don't know how to handle anything else.
- return false;
- }
}
}
Modified: branches/dfgFourthTier/Source/_javascript_Core/ftl/FTLCompile.cpp (149940 => 149941)
--- branches/dfgFourthTier/Source/_javascript_Core/ftl/FTLCompile.cpp 2013-05-11 20:57:29 UTC (rev 149940)
+++ branches/dfgFourthTier/Source/_javascript_Core/ftl/FTLCompile.cpp 2013-05-11 21:07:51 UTC (rev 149941)
@@ -48,6 +48,7 @@
options.OptLevel = Options::llvmOptimizationLevel();
options.NoFramePointerElim = true;
options.CodeModel = LLVMCodeModelSmall;
+ options.EnableFastISel = Options::enableLLVMFastISel();
if (LLVMCreateMCJITCompilerForModule(&state.engine, state.module, &options, sizeof(options), &error)) {
dataLog("FATAL: Could not create LLVM execution engine: ", error, "\n");
Modified: branches/dfgFourthTier/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp (149940 => 149941)
--- branches/dfgFourthTier/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2013-05-11 20:57:29 UTC (rev 149940)
+++ branches/dfgFourthTier/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2013-05-11 21:07:51 UTC (rev 149941)
@@ -243,12 +243,19 @@
m_state.reset();
m_state.beginBasicBlock(m_highBlock);
- for (m_nodeIndex = 0; m_nodeIndex < m_highBlock->size(); ++m_nodeIndex)
- compileNode(m_nodeIndex);
+ for (m_nodeIndex = 0; m_nodeIndex < m_highBlock->size(); ++m_nodeIndex) {
+ if (!compileNode(m_nodeIndex))
+ break;
+ }
}
- void compileNode(unsigned nodeIndex)
+ bool compileNode(unsigned nodeIndex)
{
+ if (!m_state.isValid()) {
+ m_out.unreachable();
+ return false;
+ }
+
m_node = m_highBlock->at(nodeIndex);
m_codeOrigin = m_node->codeOrigin;
@@ -288,9 +295,6 @@
case PhantomLocal:
case SetArgument:
break;
- case Return:
- compileReturn();
- break;
case ArithAdd:
case ValueAdd:
compileAdd();
@@ -388,9 +392,18 @@
case LogicalNot:
compileLogicalNot();
break;
+ case Jump:
+ compileJump();
+ break;
case Branch:
compileBranch();
break;
+ case Return:
+ compileReturn();
+ break;
+ case ForceOSRExit:
+ compileForceOSRExit();
+ break;
default:
RELEASE_ASSERT_NOT_REACHED();
break;
@@ -404,11 +417,17 @@
if (shouldExecuteEffects)
m_state.executeEffects(nodeIndex);
+
+ return true;
}
void compileJSConstant()
{
- m_jsValueValues.add(m_node, m_out.constInt64(JSValue::encode(m_graph.valueOfJSConstant(m_node))));
+ JSValue value = m_graph.valueOfJSConstant(m_node);
+ if (value.isDouble())
+ m_doubleValues.add(m_node, m_out.constDouble(value.asDouble()));
+ else
+ m_jsValueValues.add(m_node, m_out.constInt64(JSValue::encode(value)));
}
void compileWeakJSConstant()
@@ -555,13 +574,6 @@
DFG_NODE_DO_TO_CHILDREN(m_graph, m_node, speculate);
}
- void compileReturn()
- {
- // FIXME: have a real epilogue when we switch to using our calling convention.
- // https://bugs.webkit.org/show_bug.cgi?id=113621
- m_out.ret(lowJSValue(m_node->child1()));
- }
-
void compileAdd()
{
switch (m_node->binaryUseKind()) {
@@ -1123,6 +1135,11 @@
}
}
+ void compileJump()
+ {
+ m_out.jump(m_blocks.get(m_graph.m_blocks[m_node->takenBlockIndex()].get()));
+ }
+
void compileBranch()
{
switch (m_node->child1().useKind()) {
@@ -1140,6 +1157,18 @@
}
}
+ void compileReturn()
+ {
+ // FIXME: have a real epilogue when we switch to using our calling convention.
+ // https://bugs.webkit.org/show_bug.cgi?id=113621
+ m_out.ret(lowJSValue(m_node->child1()));
+ }
+
+ void compileForceOSRExit()
+ {
+ terminate(InadequateCoverage);
+ }
+
enum EqualNullOrUndefinedMode { EqualNull, EqualUndefined, EqualNullOrUndefined };
void equalNullOrUndefined(Edge edge, EqualNullOrUndefinedMode mode)
{
Modified: branches/dfgFourthTier/Source/_javascript_Core/runtime/Options.h (149940 => 149941)
--- branches/dfgFourthTier/Source/_javascript_Core/runtime/Options.h 2013-05-11 20:57:29 UTC (rev 149940)
+++ branches/dfgFourthTier/Source/_javascript_Core/runtime/Options.h 2013-05-11 21:07:51 UTC (rev 149941)
@@ -85,6 +85,7 @@
v(bool, useExperimentalFTL, false) \
v(bool, useFTLTBAA, true) \
v(bool, enableLLVMLICM, true) \
+ v(bool, enableLLVMFastISel, true) \
v(unsigned, llvmOptimizationLevel, 2) \
\
v(bool, enableProfiler, false) \
