Title: [197663] releases/WebKitGTK/webkit-2.12/Source/_javascript_Core
Revision
197663
Author
[email protected]
Date
2016-03-07 01:36:47 -0800 (Mon, 07 Mar 2016)

Log Message

Merge r197357 - 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.

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/ChangeLog (197662 => 197663)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/ChangeLog	2016-03-07 09:25:46 UTC (rev 197662)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/ChangeLog	2016-03-07 09:36:47 UTC (rev 197663)
@@ -1,3 +1,50 @@
+2016-02-28  Filip Pizlo  <[email protected]>
+
+        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-27  Filip Pizlo  <[email protected]>
 
         FTL should lower its abstract heaps to B3 heap ranges

Modified: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (197662 => 197663)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2016-03-07 09:25:46 UTC (rev 197662)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2016-03-07 09:36:47 UTC (rev 197663)
@@ -3340,6 +3340,9 @@
         }
             
         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);
         }

Modified: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGNode.h (197662 => 197663)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGNode.h	2016-03-07 09:25:46 UTC (rev 197662)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGNode.h	2016-03-07 09:36:47 UTC (rev 197663)
@@ -1022,6 +1022,9 @@
         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;

Modified: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (197662 => 197663)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2016-03-07 09:25:46 UTC (rev 197662)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2016-03-07 09:36:47 UTC (rev 197663)
@@ -5910,46 +5910,6 @@
     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);

Modified: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (197662 => 197663)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp	2016-03-07 09:25:46 UTC (rev 197662)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp	2016-03-07 09:36:47 UTC (rev 197663)
@@ -2834,26 +2834,6 @@
     }
 
     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();

Modified: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (197662 => 197663)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2016-03-07 09:25:46 UTC (rev 197662)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2016-03-07 09:36:47 UTC (rev 197663)
@@ -2959,25 +2959,6 @@
     }
 
     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 @@
         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();
         

Modified: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/ftl/FTLCapabilities.cpp (197662 => 197663)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/ftl/FTLCapabilities.cpp	2016-03-07 09:25:46 UTC (rev 197662)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/ftl/FTLCapabilities.cpp	2016-03-07 09:36:47 UTC (rev 197663)
@@ -218,6 +218,9 @@
     case PutSetterByVal:
     case CopyRest:
     case GetRestLength:
+    case RegExpExec:
+    case RegExpTest:
+    case NewRegexp:
         // These are OK.
         break;
 

Modified: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (197662 => 197663)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2016-03-07 09:25:46 UTC (rev 197662)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2016-03-07 09:36:47 UTC (rev 197663)
@@ -908,6 +908,15 @@
         case GetRestLength:
             compileGetRestLength();
             break;
+        case RegExpExec:
+            compileRegExpExec();
+            break;
+        case RegExpTest:
+            compileRegExpTest();
+            break;
+        case NewRegexp:
+            compileNewRegexp();
+            break;
 
         case PhantomLocal:
         case LoopHint:
@@ -6387,6 +6396,36 @@
         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

Added: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/tests/stress/ftl-regexp-exec.js (0 => 197663)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/tests/stress/ftl-regexp-exec.js	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/tests/stress/ftl-regexp-exec.js	2016-03-07 09:36:47 UTC (rev 197663)
@@ -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";
+}

Added: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/tests/stress/ftl-regexp-test.js (0 => 197663)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/tests/stress/ftl-regexp-test.js	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/tests/stress/ftl-regexp-test.js	2016-03-07 09:36:47 UTC (rev 197663)
@@ -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";
+}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to