Changeset 234066 in webkit

Timestamp:

Jul 20, 2018 2:25:19 PM (6 years ago)

Author:

Yusuke Suzuki

Message:

[DFG] Fold GetByVal if Array is CoW
​https://bugs.webkit.org/show_bug.cgi?id=186459

Reviewed by Saam Barati.

JSTests:

• stress/folding-get-by-val-with-immutable-butterfly-out-of-bounds-foldable.js: Added.

(shouldBe):
(test0):
(test1):
(test2):
(test3):
(test4):
(test5):

• stress/folding-get-by-val-with-immutable-butterfly-out-of-bounds.js: Added.

(shouldBe):
(test0):
(test1):
(test2):
(test3):
(test4):
(test5):

• stress/folding-get-by-val-with-immutable-butterfly-with-types.js: Added.

(shouldBe):
(test0):
(test1):
(test2):
(test3):
(test4):
(test5):

• stress/folding-get-by-val-with-immutable-butterfly.js: Added.

(shouldBe):
(checking):
(test):

Source/JavaScriptCore:

CoW indexing type means that we now tracks the changes in CoW Array by structure. So DFG has a chance to
fold GetByVal if the given array is CoW. This patch folds GetByVal onto the CoW Array. If the structure
is watched and the butterfly is JSImmutableButterfly, we can load the value from this butterfly.

This can be useful since these CoW arrays are used for a storage for constants. Constant-indexed access
to these constant arrays can be folded into an actual constant by this patch.

baseline patched

template_string.es6 4993.9853+-147.5308 ^{824.1685+-44.1839} definitely 6.0594x faster
template_string_tag.es5 67.0822+-2.0100 ^{9.3540+-0.5376} definitely 7.1715x faster

• dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/folding-get-by-val-with-immutable-butterfly-out-of-bounds-foldable.js (added)
• JSTests/stress/folding-get-by-val-with-immutable-butterfly-out-of-bounds.js (added)
• JSTests/stress/folding-get-by-val-with-immutable-butterfly-with-types.js (added)
• JSTests/stress/folding-get-by-val-with-immutable-butterfly.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h (modified) (3 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2018-07-20  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DFG] Fold GetByVal if Array is CoW
+        https://bugs.webkit.org/show_bug.cgi?id=186459
+
+        Reviewed by Saam Barati.
+
+        * stress/folding-get-by-val-with-immutable-butterfly-out-of-bounds-foldable.js: Added.
+        (shouldBe):
+        (test0):
+        (test1):
+        (test2):
+        (test3):
+        (test4):
+        (test5):
+        * stress/folding-get-by-val-with-immutable-butterfly-out-of-bounds.js: Added.
+        (shouldBe):
+        (test0):
+        (test1):
+        (test2):
+        (test3):
+        (test4):
+        (test5):
+        * stress/folding-get-by-val-with-immutable-butterfly-with-types.js: Added.
+        (shouldBe):
+        (test0):
+        (test1):
+        (test2):
+        (test3):
+        (test4):
+        (test5):
+        * stress/folding-get-by-val-with-immutable-butterfly.js: Added.
+        (shouldBe):
+        (checking):
+        (test):
+
 2018-07-20  Saam Barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2018-07-20  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DFG] Fold GetByVal if Array is CoW
+        https://bugs.webkit.org/show_bug.cgi?id=186459
+
+        Reviewed by Saam Barati.
+
+        CoW indexing type means that we now tracks the changes in CoW Array by structure. So DFG has a chance to
+        fold GetByVal if the given array is CoW. This patch folds GetByVal onto the CoW Array. If the structure
+        is watched and the butterfly is JSImmutableButterfly, we can load the value from this butterfly.
+
+        This can be useful since these CoW arrays are used for a storage for constants. Constant-indexed access
+        to these constant arrays can be folded into an actual constant by this patch.
+
+                                           baseline                  patched
+
+        template_string.es6          4993.9853+-147.5308   ^    824.1685+-44.1839       ^ definitely 6.0594x faster
+        template_string_tag.es5        67.0822+-2.0100     ^      9.3540+-0.5376        ^ definitely 7.1715x faster
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+
 2018-07-20  Yusuke Suzuki  <utatane.tea@gmail.com>
 

trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h

@@ -29 +29 @@
 
 #include "ArrayConstructor.h"
+#include "ArrayPrototype.h"
 #include "DFGAbstractInterpreter.h"
 #include "DFGAbstractInterpreterClobberState.h"
@@ -37 +38 @@
 #include "HashMapImpl.h"
 #include "JITOperations.h"
+#include "JSImmutableButterfly.h"
 #include "MathCommon.h"
 #include "NumberConstructor.h"
@@ -1816 +1818 @@
     case AtomicsSub:
     case AtomicsXor: {
+        if (node->op() == GetByVal) {
+            auto foldGetByValOnCoWArray = [&] (Edge& arrayEdge, Edge& indexEdge) {
+                // FIXME: We can expand this for non x86 environments.
+                // https://bugs.webkit.org/show_bug.cgi?id=134641
+                if (!isX86())
+                    return false;
+
+                AbstractValue& arrayValue = forNode(arrayEdge);
+                if (node->arrayMode().arrayClass() != Array::OriginalCopyOnWriteArray)
+                    return false;
+
+                // Check the structure set is finite. This means that this constant's structure is watched and guaranteed the one of this set.
+                // When the structure is changed, this code should be invalidated. This is important since the following code relies on the
+                // constant object's is not changed.
+                if (!arrayValue.m_structure.isFinite())
+                    return false;
+
+                JSValue arrayConstant = arrayValue.value();
+                if (!arrayConstant)
+                    return false;
+
+                JSObject* array = jsDynamicCast<JSObject*>(m_vm, arrayConstant);
+                if (!array)
+                    return false;
+
+                JSValue indexConstant = forNode(indexEdge).value();
+                if (!indexConstant || !indexConstant.isInt32() || indexConstant.asInt32() < 0)
+                    return false;
+                uint32_t index = indexConstant.asUInt32();
+
+                // Check that the early StructureID is not nuked, get the butterfly, and check the late StructureID again.
+                // And we check the indexing mode of the structure. If the indexing mode is CoW, the butterfly is
+                // definitely JSImmutableButterfly.
+                StructureID structureIDEarly = array->structureID();
+                if (isNuked(structureIDEarly))
+                    return false;
+
+                WTF::loadLoadFence();
+                Butterfly* butterfly = array->butterfly();
+
+                WTF::loadLoadFence();
+                StructureID structureIDLate = array->structureID();
+
+                if (structureIDEarly != structureIDLate)
+                    return false;
+
+                if (!isCopyOnWrite(m_vm.getStructure(structureIDLate)->indexingMode()))
+                    return false;
+
+                JSImmutableButterfly* immutableButterfly = JSImmutableButterfly::fromButterfly(butterfly);
+                if (index < immutableButterfly->length()) {
+                    JSValue value = immutableButterfly->get(index);
+                    ASSERT(value);
+                    if (value.isCell())
+                        setConstant(node, *m_graph.freeze(value.asCell()));
+                    else
+                        setConstant(node, value);
+                    return true;
+                }
+
+                if (node->arrayMode().isOutOfBounds()) {
+                    JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
+                    Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure(m_vm);
+                    Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure(m_vm);
+                    if (arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
+                        && objectPrototypeStructure->transitionWatchpointSetIsStillValid()
+                        && globalObject->arrayPrototypeChainIsSane()) {
+                        m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
+                        m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
+                        didFoldClobberWorld();
+                        // Note that Array::Double and Array::Int32 return JSValue if array mode is OutOfBounds.
+                        setConstant(node, jsUndefined());
+                        return true;
+                    }
+                }
+                return false;
+            };
+
+            if (foldGetByValOnCoWArray(m_graph.child(node, 0), m_graph.child(node, 1)))
+                break;
+        }
+
         if (node->op() != GetByVal) {
             unsigned numExtraArgs = numExtraAtomicsArgs(node->op());