[DFG] Drop unknown use of CheckCell's child2 to work ObjectAllocationSinking for...

https://bugs.webkit.org/show_bug.cgi?id=170940

Reviewed by Filip Pizlo.

JSTests:

* microbenchmarks/for-of-array.js: Added.
(fn):

Source/JavaScriptCore:

The second argument of CheckCell is not used in meaningful way. It is just *use* the node.
The problem is that it effectively *use* the child2 in ObjectAllocationSinking phase, and
prevent us from eliminating object allocations. Actually, it materializes Array iterator
when inlining `next()`. Instead, we should use Phantom in such a case.

It improves destructuring.es6 in SixSpeed 2.5x.

destructuring.es6      308.5184+-25.3490    ^    119.5680+-15.0520       ^ definitely 2.5803x faster

Note that SixSpeed tested in arewefastyet executes all the tests in one process while our SixSpeed
tests each one in isolated way.

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::emitFunctionChecks):
(JSC::DFG::ByteCodeParser::handleGetById):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@215459 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
index 867170f..a65310d 100644 (file)
--- a/JSTests/ChangeLog
+++ b/JSTests/ChangeLog
@@ -1,3 +1,13 @@
+2017-04-18  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DFG] Drop unknown use of CheckCell's child2 to work ObjectAllocationSinking for Array iterator object
+        https://bugs.webkit.org/show_bug.cgi?id=170940
+
+        Reviewed by Filip Pizlo.
+
+        * microbenchmarks/for-of-array.js: Added.
+        (fn):
+
 2017-04-17  Saam Barati  <sbarati@apple.com>
 
         BytecodeGenerator ".call" and ".apply" is exponential in nesting depth

diff --git a/JSTests/microbenchmarks/for-of-array.js b/JSTests/microbenchmarks/for-of-array.js
new file mode 100644 (file)
index 0000000..b378bf1
--- /dev/null
+++ b/JSTests/microbenchmarks/for-of-array.js
@@ -0,0 +1,12 @@
+var data = [1,2,3,4,5,6,7,8,9,10];
+
+function fn() {
+  var ret = '';
+  for (var value of data)
+    ret += value;
+  return ret;
+}
+noInline(fn);
+
+for (var i = 0; i < 1e5; ++i)
+    fn();

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 10e6b61..927ef77 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,5 +1,28 @@
 2017-04-18  Yusuke Suzuki  <utatane.tea@gmail.com>
 
+        [DFG] Drop unknown use of CheckCell's child2 to work ObjectAllocationSinking for Array iterator object
+        https://bugs.webkit.org/show_bug.cgi?id=170940
+
+        Reviewed by Filip Pizlo.
+
+        The second argument of CheckCell is not used in meaningful way. It is just *use* the node.
+        The problem is that it effectively *use* the child2 in ObjectAllocationSinking phase, and
+        prevent us from eliminating object allocations. Actually, it materializes Array iterator
+        when inlining `next()`. Instead, we should use Phantom in such a case.
+
+        It improves destructuring.es6 in SixSpeed 2.5x.
+
+        destructuring.es6      308.5184+-25.3490    ^    119.5680+-15.0520       ^ definitely 2.5803x faster
+
+        Note that SixSpeed tested in arewefastyet executes all the tests in one process while our SixSpeed
+        tests each one in isolated way.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::emitFunctionChecks):
+        (JSC::DFG::ByteCodeParser::handleGetById):
+
+2017-04-18  Yusuke Suzuki  <utatane.tea@gmail.com>
+
         [JSC][GTK] glib RunLoop does not accept negative start interval
         https://bugs.webkit.org/show_bug.cgi?id=170775
 

diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
index b8e199a..b2c9b1b 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
@@ -1370,7 +1370,7 @@ void ByteCodeParser::emitFunctionChecks(CallVariant callee, Node* callTarget, Vi
     if (thisArgumentReg.isValid())
         thisArgument = get(thisArgumentReg);
     else
-        thisArgument = 0;
+        thisArgument = nullptr;
 
     JSCell* calleeCell;
     Node* callTargetForCheck;
@@ -1383,7 +1383,9 @@ void ByteCodeParser::emitFunctionChecks(CallVariant callee, Node* callTarget, Vi
     }
     
     ASSERT(calleeCell);
-    addToGraph(CheckCell, OpInfo(m_graph.freeze(calleeCell)), callTargetForCheck, thisArgument);
+    if (thisArgument)
+        addToGraph(Phantom, thisArgument);
+    addToGraph(CheckCell, OpInfo(m_graph.freeze(calleeCell)), callTargetForCheck);
 }
 
 Node* ByteCodeParser::getArgumentCount()
@@ -3612,7 +3614,8 @@ void ByteCodeParser::handleGetById(
 
     if (handleIntrinsicGetter(destinationOperand, variant, base,
             [&] () {
-                addToGraph(CheckCell, OpInfo(m_graph.freeze(variant.intrinsicFunction())), getter, base);
+                addToGraph(Phantom, base);
+                addToGraph(CheckCell, OpInfo(m_graph.freeze(variant.intrinsicFunction())), getter);
             })) {
         addToGraph(Phantom, getter);
         return;