Title: [261842] trunk/Source/_javascript_Core
Revision
261842
Author
sbar...@apple.com
Date
2020-05-18 18:17:48 -0700 (Mon, 18 May 2020)

Log Message

Do more speculation that a GetByVal/PutByVal will have an int32 index based on data from ArrayProfile
https://bugs.webkit.org/show_bug.cgi?id=211877

Reviewed by Yusuke Suzuki.

Before this patch, when a GetByVal or PutByVal had a non int32 prediction for
their incoming index, they'd fall completely off the fast path. However, there
are programs where an int32 is boxed inside a double, but our notion of
predicted types don't fully capture this fact. For example, if we have a double Add
to produce an array index, that double Add will predict a full double result,
not a SpecAnyIntAsDouble. However, for GetByVal and PutByVal, there is information
from ArrayProfile we can use to determine if the incoming value is expected to
be in int32 range. The heuristic this patch introduces is:

isFullNumberSpeculation(indexSpeculation)
&& node->arrayMode().isSpecific()
&& node->arrayMode().isInBounds()
&& !m_graph.hasExitSite(node->origin.semantic, Overflow) // DoubleAsInt32 will exit with Overflow on failure

If these conditions are met, we'll now emit a DoubleAsInt32 conversion node
for the index. This puts along the fast path for GetByVal and PutByVal on
array accesses where the incoming index is an int32 boxed in a double.

To make the above isFullNumberSpeculation check more robust, this patch also
makes it so non index double accesses result in marking the array profile as
out of bounds. So this means indices greater than max safe index, and also,
fractional doubles.

This is a 3.75x speedup on microbenchmarks/get-and-put-by-val-double-index-dont-fall-off-a-cliff.js

* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* jit/JITOperations.cpp:
(JSC::getByVal):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (261841 => 261842)


--- trunk/Source/_javascript_Core/ChangeLog	2020-05-19 00:52:23 UTC (rev 261841)
+++ trunk/Source/_javascript_Core/ChangeLog	2020-05-19 01:17:48 UTC (rev 261842)
@@ -1,3 +1,41 @@
+2020-05-18  Saam Barati  <sbar...@apple.com>
+
+        Do more speculation that a GetByVal/PutByVal will have an int32 index based on data from ArrayProfile
+        https://bugs.webkit.org/show_bug.cgi?id=211877
+
+        Reviewed by Yusuke Suzuki.
+
+        Before this patch, when a GetByVal or PutByVal had a non int32 prediction for
+        their incoming index, they'd fall completely off the fast path. However, there
+        are programs where an int32 is boxed inside a double, but our notion of
+        predicted types don't fully capture this fact. For example, if we have a double Add
+        to produce an array index, that double Add will predict a full double result,
+        not a SpecAnyIntAsDouble. However, for GetByVal and PutByVal, there is information
+        from ArrayProfile we can use to determine if the incoming value is expected to
+        be in int32 range. The heuristic this patch introduces is:
+        
+        isFullNumberSpeculation(indexSpeculation)
+        && node->arrayMode().isSpecific()
+        && node->arrayMode().isInBounds()
+        && !m_graph.hasExitSite(node->origin.semantic, Overflow) // DoubleAsInt32 will exit with Overflow on failure
+        
+        If these conditions are met, we'll now emit a DoubleAsInt32 conversion node
+        for the index. This puts along the fast path for GetByVal and PutByVal on
+        array accesses where the incoming index is an int32 boxed in a double.
+        
+        To make the above isFullNumberSpeculation check more robust, this patch also
+        makes it so non index double accesses result in marking the array profile as
+        out of bounds. So this means indices greater than max safe index, and also,
+        fractional doubles.
+        
+        
+        This is a 3.75x speedup on microbenchmarks/get-and-put-by-val-double-index-dont-fall-off-a-cliff.js
+
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * jit/JITOperations.cpp:
+        (JSC::getByVal):
+
 2020-05-18  Yusuke Suzuki  <ysuz...@apple.com>
 
         [JSC] BigInt peephole compare should speculate appropriately

Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (261841 => 261842)


--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2020-05-19 00:52:23 UTC (rev 261841)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2020-05-19 01:17:48 UTC (rev 261842)
@@ -1028,7 +1028,24 @@
                 m_insertionSet.insertNode(
                     m_indexInBlock, SpecNone, ForceOSRExit, node->origin);
             }
+
+            {
+                auto indexSpeculation = m_graph.varArgChild(node, 1)->prediction();
+                if (!isInt32Speculation(indexSpeculation) 
+                    && isFullNumberSpeculation(indexSpeculation)
+                    && node->arrayMode().isSpecific()
+                    && node->arrayMode().isInBounds()
+                    && !m_graph.hasExitSite(node->origin.semantic, Overflow)) {
+
+                    Node* newIndex = m_insertionSet.insertNode(
+                        m_indexInBlock, SpecInt32Only, DoubleAsInt32, node->origin,
+                        Edge(m_graph.varArgChild(node, 1).node(), DoubleRepUse));
+                    newIndex->setArithMode(Arith::CheckOverflow);
+                    m_graph.varArgChild(node, 1).setNode(newIndex);
+                }
+            }
             
+            
             node->setArrayMode(
                 node->arrayMode().refine(
                     m_graph, node,
@@ -1170,6 +1187,22 @@
             Edge& child2 = m_graph.varArgChild(node, 1);
             Edge& child3 = m_graph.varArgChild(node, 2);
 
+            {
+                auto indexSpeculation = child2->prediction();
+                if (!isInt32Speculation(indexSpeculation) 
+                    && isFullNumberSpeculation(indexSpeculation)
+                    && node->arrayMode().isSpecific()
+                    && node->arrayMode().isInBounds()
+                    && !m_graph.hasExitSite(node->origin.semantic, Overflow)) {
+
+                    Node* newIndex = m_insertionSet.insertNode(
+                        m_indexInBlock, SpecInt32Only, DoubleAsInt32, node->origin,
+                        Edge(child2.node(), DoubleRepUse));
+                    newIndex->setArithMode(Arith::CheckOverflow);
+                    child2.setNode(newIndex);
+                }
+            }
+
             node->setArrayMode(
                 node->arrayMode().refine(
                     m_graph, node,

Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (261841 => 261842)


--- trunk/Source/_javascript_Core/jit/JITOperations.cpp	2020-05-19 00:52:23 UTC (rev 261841)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp	2020-05-19 01:17:48 UTC (rev 261842)
@@ -721,7 +721,7 @@
         baseValue.putByIndex(globalObject, i, value, ecmaMode.isStrict());
         return;
     } 
-    if (subscript.isInt32()) {
+    if (subscript.isNumber()) {
         byValInfo->tookSlowPath = true;
         if (baseValue.isObject())
             byValInfo->arrayProfile->setOutOfBounds();
@@ -1976,7 +1976,8 @@
 
         if (i >= 0)
             RELEASE_AND_RETURN(scope, baseValue.get(globalObject, static_cast<uint32_t>(i)));
-    }
+    } else if (subscript.isNumber() && baseValue.isCell() && arrayProfile)
+        arrayProfile->setOutOfBounds();
 
     baseValue.requireObjectCoercible(globalObject);
     RETURN_IF_EXCEPTION(scope, JSValue());
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to