FTL should be able to run everything in Octane/regexp
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Feb 2016 18:05:17 +0000 (18:05 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Feb 2016 18:05:17 +0000 (18:05 +0000)
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
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGNode.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
Source/JavaScriptCore/ftl/FTLCapabilities.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/tests/stress/ftl-regexp-exec.js [new file with mode: 0644]
Source/JavaScriptCore/tests/stress/ftl-regexp-test.js [new file with mode: 0644]

index 5cece2f..7af2947 100644 (file)
@@ -1,3 +1,50 @@
+2016-02-28  Filip Pizlo  <fpizlo@apple.com>
+
+        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  <akling@apple.com>
 
         Make JSFunction.name allocation fully lazy.
 2016-02-28  Andreas Kling  <akling@apple.com>
 
         Make JSFunction.name allocation fully lazy.
index 8eda439..f098b64 100644 (file)
@@ -3340,6 +3340,9 @@ bool ByteCodeParser::parseBlock(unsigned limit)
         }
             
         case op_new_regexp: {
         }
             
         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);
         }
             set(VirtualRegister(currentInstruction[1].u.operand), addToGraph(NewRegexp, OpInfo(currentInstruction[2].u.operand)));
             NEXT_OPCODE(op_new_regexp);
         }
index 7612896..eeab257 100644 (file)
@@ -1022,6 +1022,9 @@ struct Node {
         m_opInfo = indexingType;
     }
     
         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;
     bool hasRegexpIndex()
     {
         return op() == NewRegexp;
index 5062f1e..6174603 100644 (file)
@@ -5910,46 +5910,6 @@ void SpeculativeJIT::compileNotifyWrite(Node* node)
     noResult(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);
 void SpeculativeJIT::compileIsObjectOrNull(Node* node)
 {
     JSGlobalObject* globalObject = m_jit.graph().globalObjectFor(node->origin.semantic);
index 18a7885..8472b8c 100644 (file)
@@ -2834,26 +2834,6 @@ void SpeculativeJIT::compile(Node* node)
     }
 
     case RegExpExec: {
     }
 
     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();
         SpeculateCellOperand base(this, node->child1());
         SpeculateCellOperand argument(this, node->child2());
         GPRReg baseGPR = base.gpr();
index 8f5688a..83b6f5b 100644 (file)
@@ -2959,25 +2959,6 @@ void SpeculativeJIT::compile(Node* node)
     }
 
     case RegExpExec: {
     }
 
     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();
         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);
         
         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();
         
         callOperation(operationNewRegexp, result.gpr(), m_jit.codeBlock()->regexp(node->regexpIndex()));
         m_jit.exceptionCheck();
         
index 8348dcc..01c8e87 100644 (file)
@@ -218,6 +218,9 @@ inline CapabilityLevel canCompile(Node* node)
     case PutSetterByVal:
     case CopyRest:
     case GetRestLength:
     case PutSetterByVal:
     case CopyRest:
     case GetRestLength:
+    case RegExpExec:
+    case RegExpTest:
+    case NewRegexp:
         // These are OK.
         break;
 
         // These are OK.
         break;
 
index 13113c6..53e926e 100644 (file)
@@ -908,6 +908,15 @@ private:
         case GetRestLength:
             compileGetRestLength();
             break;
         case GetRestLength:
             compileGetRestLength();
             break;
+        case RegExpExec:
+            compileRegExpExec();
+            break;
+        case RegExpTest:
+            compileRegExpTest();
+            break;
+        case NewRegexp:
+            compileNewRegexp();
+            break;
 
         case PhantomLocal:
         case LoopHint:
 
         case PhantomLocal:
         case LoopHint:
@@ -6387,6 +6396,36 @@ private:
         m_out.appendTo(continuation, lastNext);
     }
 
         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
     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 (file)
index 0000000..03149a9
--- /dev/null
@@ -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 (file)
index 0000000..da377a1
--- /dev/null
@@ -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";
+}