[DFG][FTL] Support Array::DirectArguments with OutOfBounds
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Nov 2017 17:35:33 +0000 (17:35 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Nov 2017 17:35:33 +0000 (17:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179594

Reviewed by Saam Barati.

JSTests:

* stress/direct-arguments-in-bounds-to-out-of-bounds.js: Added.
(shouldBe):
(args):
* stress/direct-arguments-out-of-bounds-watchpoint.js: Added.
(shouldBe):
(args):

Source/JavaScriptCore:

Currently we handle OOB access to DirectArguments as GetByVal(Array::Generic).
If we can handle it as GetByVal(Array::DirectArguments+OutOfBounds), we can (1) optimize
`arguments[i]` accesses if i is in bound, and (2) encourage arguments elimination phase
to convert CreateDirectArguments and GetByVal(Array::DirectArguments+OutOfBounds) to
PhantomDirectArguments and GetMyArgumentOutOfBounds respectively.

This patch introduces Array::DirectArguments+OutOfBounds array mode. GetByVal can
accept this type, and emit optimized code compared to Array::Generic case.

We make OOB check failures in GetByVal(Array::DirectArguments+InBounds) as OutOfBounds
exit instead of ExoticObjectMode.

This change significantly improves SixSpeed rest.es5 since it uses OOB access.
Our arguments elimination phase can change CreateDirectArguments to PhantomDirectArguments.

    rest.es5                       59.6719+-2.2440     ^      3.1634+-0.5507        ^ definitely 18.8635x faster

* dfg/DFGArgumentsEliminationPhase.cpp:
* dfg/DFGArrayMode.cpp:
(JSC::DFG::ArrayMode::refine const):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileGetByValOnDirectArguments):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
(JSC::FTL::DFG::LowerDFGToB3::compileGetMyArgumentByVal):

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

JSTests/ChangeLog
JSTests/stress/direct-arguments-in-bounds-to-out-of-bounds.js [new file with mode: 0644]
JSTests/stress/direct-arguments-out-of-bounds-watchpoint.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp
Source/JavaScriptCore/dfg/DFGArrayMode.cpp
Source/JavaScriptCore/dfg/DFGClobberize.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

index 2469a45..a6cef43 100644 (file)
@@ -1,3 +1,17 @@
+2017-11-14  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DFG][FTL] Support Array::DirectArguments with OutOfBounds
+        https://bugs.webkit.org/show_bug.cgi?id=179594
+
+        Reviewed by Saam Barati.
+
+        * stress/direct-arguments-in-bounds-to-out-of-bounds.js: Added.
+        (shouldBe):
+        (args):
+        * stress/direct-arguments-out-of-bounds-watchpoint.js: Added.
+        (shouldBe):
+        (args):
+
 2017-11-14  Saam Barati  <sbarati@apple.com>
 
         We need to set topCallFrame when calling Wasm::Memory::grow from the JIT
diff --git a/JSTests/stress/direct-arguments-in-bounds-to-out-of-bounds.js b/JSTests/stress/direct-arguments-in-bounds-to-out-of-bounds.js
new file mode 100644 (file)
index 0000000..2dac7d2
--- /dev/null
@@ -0,0 +1,17 @@
+function shouldBe(actual, expected)
+{
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function args()
+{
+    return arguments[1];
+}
+noInline(args);
+
+for (var i = 0; i < 1e6; ++i)
+    shouldBe(args(0, 1, 2), 1);
+
+for (var i = 0; i < 1e6; ++i)
+    shouldBe(args(0), undefined);
diff --git a/JSTests/stress/direct-arguments-out-of-bounds-watchpoint.js b/JSTests/stress/direct-arguments-out-of-bounds-watchpoint.js
new file mode 100644 (file)
index 0000000..7169141
--- /dev/null
@@ -0,0 +1,19 @@
+function shouldBe(actual, expected)
+{
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function args()
+{
+    return arguments[1];
+}
+noInline(args);
+
+shouldBe(args(), undefined);
+shouldBe(args(0), undefined);
+shouldBe(args(0, 1), 1);
+for (var i = 0; i < 1e6; ++i)
+    shouldBe(args(), undefined);
+Object.prototype[1] = 42;
+shouldBe(args(), 42);
index bcaafdd..e9e69d1 100644 (file)
@@ -1,3 +1,38 @@
+2017-11-14  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DFG][FTL] Support Array::DirectArguments with OutOfBounds
+        https://bugs.webkit.org/show_bug.cgi?id=179594
+
+        Reviewed by Saam Barati.
+
+        Currently we handle OOB access to DirectArguments as GetByVal(Array::Generic).
+        If we can handle it as GetByVal(Array::DirectArguments+OutOfBounds), we can (1) optimize
+        `arguments[i]` accesses if i is in bound, and (2) encourage arguments elimination phase
+        to convert CreateDirectArguments and GetByVal(Array::DirectArguments+OutOfBounds) to
+        PhantomDirectArguments and GetMyArgumentOutOfBounds respectively.
+
+        This patch introduces Array::DirectArguments+OutOfBounds array mode. GetByVal can
+        accept this type, and emit optimized code compared to Array::Generic case.
+
+        We make OOB check failures in GetByVal(Array::DirectArguments+InBounds) as OutOfBounds
+        exit instead of ExoticObjectMode.
+
+        This change significantly improves SixSpeed rest.es5 since it uses OOB access.
+        Our arguments elimination phase can change CreateDirectArguments to PhantomDirectArguments.
+
+            rest.es5                       59.6719+-2.2440     ^      3.1634+-0.5507        ^ definitely 18.8635x faster
+
+        * dfg/DFGArgumentsEliminationPhase.cpp:
+        * dfg/DFGArrayMode.cpp:
+        (JSC::DFG::ArrayMode::refine const):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileGetByValOnDirectArguments):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetMyArgumentByVal):
+
 2017-11-14  Saam Barati  <sbarati@apple.com>
 
         We need to set topCallFrame when calling Wasm::Memory::grow from the JIT
index 50aca23..8b8bd20 100644 (file)
@@ -230,15 +230,33 @@ private:
         
         auto escapeBasedOnArrayMode = [&] (ArrayMode mode, Edge edge, Node* source) {
             switch (mode.type()) {
-            case Array::DirectArguments:
-                if (edge->op() != CreateDirectArguments)
+            case Array::DirectArguments: {
+                if (edge->op() != CreateDirectArguments) {
                     escape(edge, source);
+                    break;
+                }
+
+                // Everything is fine if we're doing an in-bounds access.
+                if (mode.isInBounds())
+                    break;
+
+                // If we're out-of-bounds then we proceed only if the prototype chain
+                // for the allocation is sane (i.e. doesn't have indexed properties).
+                JSGlobalObject* globalObject = m_graph.globalObjectFor(edge->origin.semantic);
+                Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure();
+                if (objectPrototypeStructure->transitionWatchpointSetIsStillValid()
+                    && globalObject->objectPrototypeIsSane()) {
+                    m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
+                    break;
+                }
+                escape(edge, source);
                 break;
+            }
             
             case Array::Contiguous: {
                 if (edge->op() != CreateClonedArguments && edge->op() != CreateRest) {
                     escape(edge, source);
-                    return;
+                    break;
                 }
             
                 // Everything is fine if we're doing an in-bounds access.
index 319f419..19da3e4 100644 (file)
@@ -259,12 +259,15 @@ ArrayMode ArrayMode::refine(
         
         if (isDirectArgumentsSpeculation(base) || isScopedArgumentsSpeculation(base)) {
             // Handle out-of-bounds accesses as generic accesses.
-            if (graph.hasExitSite(node->origin.semantic, OutOfBounds) || !isInBounds())
+            Array::Type type = isDirectArgumentsSpeculation(base) ? Array::DirectArguments : Array::ScopedArguments;
+            if (graph.hasExitSite(node->origin.semantic, OutOfBounds) || !isInBounds()) {
+                // FIXME: Support OOB access for ScopedArguments.
+                // https://bugs.webkit.org/show_bug.cgi?id=179596
+                if (type == Array::DirectArguments)
+                    return ArrayMode(type, Array::NonArray, Array::OutOfBounds, Array::AsIs);
                 return ArrayMode(Array::Generic);
-            
-            if (isDirectArgumentsSpeculation(base))
-                return withType(Array::DirectArguments);
-            return withType(Array::ScopedArguments);
+            }
+            return withType(type);
         }
         
         ArrayMode result;
index ba848e5..2f77d45 100644 (file)
@@ -783,8 +783,13 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
             return;
             
         case Array::DirectArguments:
-            read(DirectArgumentsProperties);
-            def(HeapLocation(indexedPropertyLoc, DirectArgumentsProperties, node->child1(), node->child2()), LazyNode(node));
+            if (mode.isInBounds()) {
+                read(DirectArgumentsProperties);
+                def(HeapLocation(indexedPropertyLoc, DirectArgumentsProperties, node->child1(), node->child2()), LazyNode(node));
+                return;
+            }
+            read(World);
+            write(Heap);
             return;
             
         case Array::ScopedArguments:
index 9f0b256..618bac1 100644 (file)
@@ -6490,16 +6490,22 @@ void SpeculativeJIT::compileGetByValOnDirectArguments(Node* node)
         m_jit.branchTestPtr(
             MacroAssembler::NonZero,
             MacroAssembler::Address(baseReg, DirectArguments::offsetOfMappedArguments())));
-    speculationCheck(
-        ExoticObjectMode, JSValueSource(), 0,
-        m_jit.branch32(
-            MacroAssembler::AboveOrEqual, propertyReg,
-            MacroAssembler::Address(baseReg, DirectArguments::offsetOfLength())));
+
+    auto isOutOfBounds = m_jit.branch32(CCallHelpers::AboveOrEqual, propertyReg, CCallHelpers::Address(baseReg, DirectArguments::offsetOfLength()));
+    if (node->arrayMode().isInBounds())
+        speculationCheck(OutOfBounds, JSValueSource(), 0, isOutOfBounds);
     
     m_jit.loadValue(
         MacroAssembler::BaseIndex(
             baseReg, propertyReg, MacroAssembler::TimesEight, DirectArguments::storageOffset()),
         resultRegs);
+
+    if (!node->arrayMode().isInBounds()) {
+        addSlowPathGenerator(
+            slowPathCall(
+                isOutOfBounds, this, operationGetByValObjectInt,
+                extractResult(resultRegs), baseReg, propertyReg));
+    }
     
     jsValueResult(resultRegs, node);
 }
index d3cde2a..dedda1c 100644 (file)
@@ -3711,15 +3711,35 @@ private:
             speculate(
                 ExoticObjectMode, noValue(), nullptr,
                 m_out.notNull(m_out.loadPtr(base, m_heaps.DirectArguments_mappedArguments)));
-            speculate(
-                ExoticObjectMode, noValue(), nullptr,
-                m_out.aboveOrEqual(
-                    index,
-                    m_out.load32NonNegative(base, m_heaps.DirectArguments_length)));
 
+            auto isOutOfBounds = m_out.aboveOrEqual(index, m_out.load32NonNegative(base, m_heaps.DirectArguments_length));
+            if (m_node->arrayMode().isInBounds()) {
+                speculate(OutOfBounds, noValue(), nullptr, isOutOfBounds);
+                TypedPointer address = m_out.baseIndex(
+                    m_heaps.DirectArguments_storage, base, m_out.zeroExtPtr(index));
+                setJSValue(m_out.load64(address));
+                return;
+            }
+
+            LBasicBlock inBounds = m_out.newBlock();
+            LBasicBlock slowCase = m_out.newBlock();
+            LBasicBlock continuation = m_out.newBlock();
+
+            m_out.branch(isOutOfBounds, rarely(slowCase), usually(inBounds));
+
+            LBasicBlock lastNext = m_out.appendTo(inBounds, slowCase);
             TypedPointer address = m_out.baseIndex(
                 m_heaps.DirectArguments_storage, base, m_out.zeroExtPtr(index));
-            setJSValue(m_out.load64(address));
+            ValueFromBlock fastResult = m_out.anchor(m_out.load64(address));
+            m_out.jump(continuation);
+
+            m_out.appendTo(slowCase, continuation);
+            ValueFromBlock slowResult = m_out.anchor(
+                vmCall(Int64, m_out.operation(operationGetByValObjectInt), m_callFrame, base, index));
+            m_out.jump(continuation);
+
+            m_out.appendTo(continuation, lastNext);
+            setJSValue(m_out.phi(Int64, fastResult, slowResult));
             return;
         }
             
@@ -3912,7 +3932,7 @@ private:
             
             lastNext = m_out.appendTo(normalCase, continuation);
         } else
-            speculate(ExoticObjectMode, noValue(), 0, isOutOfBounds);
+            speculate(OutOfBounds, noValue(), 0, isOutOfBounds);
         
         TypedPointer base;
         if (inlineCallFrame) {