From 47c6c8686e3d9b5c8161cf467020aeec10f338a6 Mon Sep 17 00:00:00 2001 From: "fpizlo@apple.com" Date: Mon, 29 Feb 2016 18:05:17 +0000 Subject: [PATCH] FTL should be able to run everything in Octane/regexp https://bugs.webkit.org/show_bug.cgi?id=154266 Reviewed by Saam Barati. Adds FTL support for NewRegexp, RegExpTest, and RegExpExec. I couldn't figure out how to make the RegExpExec peephole optimization work in FTL. This optimizations shouldn't be a DFG backend optimization anyway - if we need this optimization then it should be a strength reduction rule over IR. That way, it can be shared by all backends. I measured whether removing that optimization had any effect on performance separately from measuring the performance of this patch. Removing that optimization did not change our score on any benchmarks. This patch does have an overall negative effect on the Octane/regexp score. This is presumably because tiering up to the FTL has no value to the code in the regexp test. Or maybe it's something else. No matter - the overall effect on the Octane score is not statistically significant and we don't want this kind of coverage blocked by the fact that adding coverage hurts a benchmark. * dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::parseBlock): * dfg/DFGNode.h: (JSC::DFG::Node::setIndexingType): (JSC::DFG::Node::hasRegexpIndex): * dfg/DFGSpeculativeJIT.cpp: (JSC::DFG::SpeculativeJIT::compileNotifyWrite): (JSC::DFG::SpeculativeJIT::compileIsObjectOrNull): (JSC::DFG::SpeculativeJIT::compileRegExpExec): Deleted. * dfg/DFGSpeculativeJIT32_64.cpp: (JSC::DFG::SpeculativeJIT::compile): * dfg/DFGSpeculativeJIT64.cpp: (JSC::DFG::SpeculativeJIT::compile): * ftl/FTLCapabilities.cpp: (JSC::FTL::canCompile): * ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::compileNode): (JSC::FTL::DFG::LowerDFGToB3::compileCheckWatchdogTimer): (JSC::FTL::DFG::LowerDFGToB3::compileRegExpExec): (JSC::FTL::DFG::LowerDFGToB3::compileRegExpTest): (JSC::FTL::DFG::LowerDFGToB3::compileNewRegexp): (JSC::FTL::DFG::LowerDFGToB3::didOverflowStack): * tests/stress/ftl-regexp-exec.js: Added. * tests/stress/ftl-regexp-test.js: Added. git-svn-id: https://svn.webkit.org/repository/webkit/trunk@197357 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/JavaScriptCore/ChangeLog | 47 ++++++++++++++++++++++ Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp | 3 ++ Source/JavaScriptCore/dfg/DFGNode.h | 3 ++ Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp | 40 ------------------ .../JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp | 20 --------- Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp | 22 ++-------- Source/JavaScriptCore/ftl/FTLCapabilities.cpp | 3 ++ Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp | 39 ++++++++++++++++++ .../JavaScriptCore/tests/stress/ftl-regexp-exec.js | 17 ++++++++ .../JavaScriptCore/tests/stress/ftl-regexp-test.js | 12 ++++++ 10 files changed, 127 insertions(+), 79 deletions(-) create mode 100644 Source/JavaScriptCore/tests/stress/ftl-regexp-exec.js create mode 100644 Source/JavaScriptCore/tests/stress/ftl-regexp-test.js diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog index 5cece2f..7af29476 100644 --- a/Source/JavaScriptCore/ChangeLog +++ b/Source/JavaScriptCore/ChangeLog @@ -1,3 +1,50 @@ +2016-02-28 Filip Pizlo + + FTL should be able to run everything in Octane/regexp + https://bugs.webkit.org/show_bug.cgi?id=154266 + + Reviewed by Saam Barati. + + Adds FTL support for NewRegexp, RegExpTest, and RegExpExec. I couldn't figure out how to + make the RegExpExec peephole optimization work in FTL. This optimizations shouldn't be a + DFG backend optimization anyway - if we need this optimization then it should be a + strength reduction rule over IR. That way, it can be shared by all backends. + + I measured whether removing that optimization had any effect on performance separately + from measuring the performance of this patch. Removing that optimization did not change + our score on any benchmarks. + + This patch does have an overall negative effect on the Octane/regexp score. This is + presumably because tiering up to the FTL has no value to the code in the regexp test. Or + maybe it's something else. No matter - the overall effect on the Octane score is not + statistically significant and we don't want this kind of coverage blocked by the fact + that adding coverage hurts a benchmark. + + * dfg/DFGByteCodeParser.cpp: + (JSC::DFG::ByteCodeParser::parseBlock): + * dfg/DFGNode.h: + (JSC::DFG::Node::setIndexingType): + (JSC::DFG::Node::hasRegexpIndex): + * dfg/DFGSpeculativeJIT.cpp: + (JSC::DFG::SpeculativeJIT::compileNotifyWrite): + (JSC::DFG::SpeculativeJIT::compileIsObjectOrNull): + (JSC::DFG::SpeculativeJIT::compileRegExpExec): Deleted. + * dfg/DFGSpeculativeJIT32_64.cpp: + (JSC::DFG::SpeculativeJIT::compile): + * dfg/DFGSpeculativeJIT64.cpp: + (JSC::DFG::SpeculativeJIT::compile): + * ftl/FTLCapabilities.cpp: + (JSC::FTL::canCompile): + * ftl/FTLLowerDFGToB3.cpp: + (JSC::FTL::DFG::LowerDFGToB3::compileNode): + (JSC::FTL::DFG::LowerDFGToB3::compileCheckWatchdogTimer): + (JSC::FTL::DFG::LowerDFGToB3::compileRegExpExec): + (JSC::FTL::DFG::LowerDFGToB3::compileRegExpTest): + (JSC::FTL::DFG::LowerDFGToB3::compileNewRegexp): + (JSC::FTL::DFG::LowerDFGToB3::didOverflowStack): + * tests/stress/ftl-regexp-exec.js: Added. + * tests/stress/ftl-regexp-test.js: Added. + 2016-02-28 Andreas Kling Make JSFunction.name allocation fully lazy. diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp index 8eda439..f098b64 100644 --- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp +++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp @@ -3340,6 +3340,9 @@ bool ByteCodeParser::parseBlock(unsigned limit) } case op_new_regexp: { + // FIXME: We really should be able to inline code that uses NewRegexp. That means + // using something other than the index into the CodeBlock here. + // https://bugs.webkit.org/show_bug.cgi?id=154808 set(VirtualRegister(currentInstruction[1].u.operand), addToGraph(NewRegexp, OpInfo(currentInstruction[2].u.operand))); NEXT_OPCODE(op_new_regexp); } diff --git a/Source/JavaScriptCore/dfg/DFGNode.h b/Source/JavaScriptCore/dfg/DFGNode.h index 7612896..eeab257 100644 --- a/Source/JavaScriptCore/dfg/DFGNode.h +++ b/Source/JavaScriptCore/dfg/DFGNode.h @@ -1022,6 +1022,9 @@ struct Node { m_opInfo = indexingType; } + // FIXME: We really should be able to inline code that uses NewRegexp. That means + // using something other than the index into the CodeBlock here. + // https://bugs.webkit.org/show_bug.cgi?id=154808 bool hasRegexpIndex() { return op() == NewRegexp; diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp index 5062f1e..6174603 100644 --- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp +++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp @@ -5910,46 +5910,6 @@ void SpeculativeJIT::compileNotifyWrite(Node* node) noResult(node); } -bool SpeculativeJIT::compileRegExpExec(Node* node) -{ - unsigned branchIndexInBlock = detectPeepHoleBranch(); - if (branchIndexInBlock == UINT_MAX) - return false; - Node* branchNode = m_block->at(branchIndexInBlock); - ASSERT(node->adjustedRefCount() == 1); - - BasicBlock* taken = branchNode->branchData()->taken.block; - BasicBlock* notTaken = branchNode->branchData()->notTaken.block; - - bool invert = false; - if (taken == nextBlock()) { - invert = true; - BasicBlock* tmp = taken; - taken = notTaken; - notTaken = tmp; - } - - SpeculateCellOperand base(this, node->child1()); - SpeculateCellOperand argument(this, node->child2()); - GPRReg baseGPR = base.gpr(); - GPRReg argumentGPR = argument.gpr(); - - flushRegisters(); - GPRFlushedCallResult result(this); - callOperation(operationRegExpTest, result.gpr(), baseGPR, argumentGPR); - m_jit.exceptionCheck(); - - branchTest32(invert ? JITCompiler::Zero : JITCompiler::NonZero, result.gpr(), taken); - jump(notTaken); - - use(node->child1()); - use(node->child2()); - m_indexInBlock = branchIndexInBlock; - m_currentNode = branchNode; - - return true; -} - void SpeculativeJIT::compileIsObjectOrNull(Node* node) { JSGlobalObject* globalObject = m_jit.graph().globalObjectFor(node->origin.semantic); diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp index 18a7885..8472b8c 100644 --- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp +++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp @@ -2834,26 +2834,6 @@ void SpeculativeJIT::compile(Node* node) } case RegExpExec: { - if (compileRegExpExec(node)) - return; - - if (!node->adjustedRefCount()) { - SpeculateCellOperand base(this, node->child1()); - SpeculateCellOperand argument(this, node->child2()); - GPRReg baseGPR = base.gpr(); - GPRReg argumentGPR = argument.gpr(); - - flushRegisters(); - GPRFlushedCallResult result(this); - callOperation(operationRegExpTest, result.gpr(), baseGPR, argumentGPR); - m_jit.exceptionCheck(); - - // Must use jsValueResult because otherwise we screw up register - // allocation, which thinks that this node has a result. - booleanResult(result.gpr(), node); - break; - } - SpeculateCellOperand base(this, node->child1()); SpeculateCellOperand argument(this, node->child2()); GPRReg baseGPR = base.gpr(); diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp index 8f5688a..83b6f5b 100644 --- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp +++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp @@ -2959,25 +2959,6 @@ void SpeculativeJIT::compile(Node* node) } case RegExpExec: { - if (compileRegExpExec(node)) - return; - if (!node->adjustedRefCount()) { - SpeculateCellOperand base(this, node->child1()); - SpeculateCellOperand argument(this, node->child2()); - GPRReg baseGPR = base.gpr(); - GPRReg argumentGPR = argument.gpr(); - - flushRegisters(); - GPRFlushedCallResult result(this); - callOperation(operationRegExpTest, result.gpr(), baseGPR, argumentGPR); - m_jit.exceptionCheck(); - - // Must use jsValueResult because otherwise we screw up register - // allocation, which thinks that this node has a result. - jsValueResult(result.gpr(), node); - break; - } - SpeculateCellOperand base(this, node->child1()); SpeculateCellOperand argument(this, node->child2()); GPRReg baseGPR = base.gpr(); @@ -3683,6 +3664,9 @@ void SpeculativeJIT::compile(Node* node) flushRegisters(); GPRFlushedCallResult result(this); + // FIXME: We really should be able to inline code that uses NewRegexp. That means not + // reaching into the CodeBlock here. + // https://bugs.webkit.org/show_bug.cgi?id=154808 callOperation(operationNewRegexp, result.gpr(), m_jit.codeBlock()->regexp(node->regexpIndex())); m_jit.exceptionCheck(); diff --git a/Source/JavaScriptCore/ftl/FTLCapabilities.cpp b/Source/JavaScriptCore/ftl/FTLCapabilities.cpp index 8348dcc..01c8e87 100644 --- a/Source/JavaScriptCore/ftl/FTLCapabilities.cpp +++ b/Source/JavaScriptCore/ftl/FTLCapabilities.cpp @@ -218,6 +218,9 @@ inline CapabilityLevel canCompile(Node* node) case PutSetterByVal: case CopyRest: case GetRestLength: + case RegExpExec: + case RegExpTest: + case NewRegexp: // These are OK. break; diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp index 13113c6..53e926e 100644 --- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp +++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp @@ -908,6 +908,15 @@ private: case GetRestLength: compileGetRestLength(); break; + case RegExpExec: + compileRegExpExec(); + break; + case RegExpTest: + compileRegExpTest(); + break; + case NewRegexp: + compileNewRegexp(); + break; case PhantomLocal: case LoopHint: @@ -6387,6 +6396,36 @@ private: m_out.appendTo(continuation, lastNext); } + void compileRegExpExec() + { + LValue base = lowCell(m_node->child1()); + LValue argument = lowCell(m_node->child2()); + setJSValue( + vmCall(Int64, m_out.operation(operationRegExpExec), m_callFrame, base, argument)); + } + + void compileRegExpTest() + { + LValue base = lowCell(m_node->child1()); + LValue argument = lowCell(m_node->child2()); + setBoolean( + vmCall(Int32, m_out.operation(operationRegExpTest), m_callFrame, base, argument)); + } + + void compileNewRegexp() + { + // FIXME: We really should be able to inline code that uses NewRegexp. That means not + // reaching into the CodeBlock here. + // https://bugs.webkit.org/show_bug.cgi?id=154808 + + LValue result = vmCall( + pointerType(), + m_out.operation(operationNewRegexp), m_callFrame, + m_out.constIntPtr(codeBlock()->regexp(m_node->regexpIndex()))); + + setJSValue(result); + } + LValue didOverflowStack() { // This does a very simple leaf function analysis. The invariant of FTL call diff --git a/Source/JavaScriptCore/tests/stress/ftl-regexp-exec.js b/Source/JavaScriptCore/tests/stress/ftl-regexp-exec.js new file mode 100644 index 0000000..03149a9 --- /dev/null +++ b/Source/JavaScriptCore/tests/stress/ftl-regexp-exec.js @@ -0,0 +1,17 @@ +function foo(s) { + return /foo/.exec(s); +} + +noInline(foo); + +for (var i = 0; i < 100000; ++i) { + var result = foo("foo"); + if (!result) + throw "Error: bad result for foo"; + if (result.length != 1) + throw "Error: bad result for foo: " + result; + if (result[0] != "foo") + throw "Error: bad result for foo: " + result; + if (foo("bar")) + throw "Error: bad result for bar"; +} diff --git a/Source/JavaScriptCore/tests/stress/ftl-regexp-test.js b/Source/JavaScriptCore/tests/stress/ftl-regexp-test.js new file mode 100644 index 0000000..da377a1 --- /dev/null +++ b/Source/JavaScriptCore/tests/stress/ftl-regexp-test.js @@ -0,0 +1,12 @@ +function foo(s) { + return /foo/.test(s); +} + +noInline(foo); + +for (var i = 0; i < 100000; ++i) { + if (!foo("foo")) + throw "Error: bad result for foo"; + if (foo("bar")) + throw "Error: bad result for bar"; +} -- 1.8.3.1