Title: [180703] trunk
Revision
180703
Author
fpi...@apple.com
Date
2015-02-26 14:44:45 -0800 (Thu, 26 Feb 2015)

Log Message

Various array access corner cases should take OSR exit feedback
https://bugs.webkit.org/show_bug.cgi?id=142056

Reviewed by Geoffrey Garen.
Source/_javascript_Core:

        
Two major changes here:
        
- Don't keep converting GetById into GetArrayLength if we exited due to any kind of array
  type check.
        
- Use a generic form of GetByVal/PutByVal if we exited due to any kind of exotic checks,
  like the Arguments safety checks. We use the "ExoticObjectMode" for out-of-bounds on
  arguments for now, since it's a convenient way of forcing out-of-bounds to be handled by
  the Generic array mode.

* bytecode/ExitKind.cpp:
(JSC::exitKindToString):
* bytecode/ExitKind.h:
* dfg/DFGArrayMode.cpp:
(JSC::DFG::ArrayMode::refine):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileGetByValOnArguments):
(JSC::DFG::SpeculativeJIT::compileGetArgumentsLength):
* tests/stress/array-length-array-storage-plain-object.js: Added.
(foo):
* tests/stress/array-length-plain-object.js: Added.
(foo):

LayoutTests:


* js/regress/arguments-out-of-bounds-expected.txt: Added.
* js/regress/arguments-out-of-bounds.html: Added.
* js/regress/exit-length-on-plain-object-expected.txt: Added.
* js/regress/exit-length-on-plain-object.html: Added.
* js/regress/script-tests/arguments-out-of-bounds.js: Added.
(foo):
(bar):
* js/regress/script-tests/exit-length-on-plain-object.js: Added.
(foo):
* js/regress/script-tests/string-out-of-bounds.js: Added.
(bar):
* js/regress/string-out-of-bounds-expected.txt: Added.
* js/regress/string-out-of-bounds.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (180702 => 180703)


--- trunk/LayoutTests/ChangeLog	2015-02-26 22:25:31 UTC (rev 180702)
+++ trunk/LayoutTests/ChangeLog	2015-02-26 22:44:45 UTC (rev 180703)
@@ -1,3 +1,24 @@
+2015-02-26  Filip Pizlo  <fpi...@apple.com>
+
+        Various array access corner cases should take OSR exit feedback
+        https://bugs.webkit.org/show_bug.cgi?id=142056
+
+        Reviewed by Geoffrey Garen.
+
+        * js/regress/arguments-out-of-bounds-expected.txt: Added.
+        * js/regress/arguments-out-of-bounds.html: Added.
+        * js/regress/exit-length-on-plain-object-expected.txt: Added.
+        * js/regress/exit-length-on-plain-object.html: Added.
+        * js/regress/script-tests/arguments-out-of-bounds.js: Added.
+        (foo):
+        (bar):
+        * js/regress/script-tests/exit-length-on-plain-object.js: Added.
+        (foo):
+        * js/regress/script-tests/string-out-of-bounds.js: Added.
+        (bar):
+        * js/regress/string-out-of-bounds-expected.txt: Added.
+        * js/regress/string-out-of-bounds.html: Added.
+
 2015-02-26  Mark Lam  <mark....@apple.com>
 
         Rolling out r180602, r180608, r180613, r180617, r180671.

Added: trunk/LayoutTests/js/regress/arguments-out-of-bounds-expected.txt (0 => 180703)


--- trunk/LayoutTests/js/regress/arguments-out-of-bounds-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/js/regress/arguments-out-of-bounds-expected.txt	2015-02-26 22:44:45 UTC (rev 180703)
@@ -0,0 +1,10 @@
+JSRegress/arguments-out-of-bounds
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS no exception thrown
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/js/regress/arguments-out-of-bounds.html (0 => 180703)


--- trunk/LayoutTests/js/regress/arguments-out-of-bounds.html	                        (rev 0)
+++ trunk/LayoutTests/js/regress/arguments-out-of-bounds.html	2015-02-26 22:44:45 UTC (rev 180703)
@@ -0,0 +1,12 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+<script src=""
+<script src=""
+</body>
+</html>

Added: trunk/LayoutTests/js/regress/exit-length-on-plain-object-expected.txt (0 => 180703)


--- trunk/LayoutTests/js/regress/exit-length-on-plain-object-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/js/regress/exit-length-on-plain-object-expected.txt	2015-02-26 22:44:45 UTC (rev 180703)
@@ -0,0 +1,10 @@
+JSRegress/exit-length-on-plain-object
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS no exception thrown
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/js/regress/exit-length-on-plain-object.html (0 => 180703)


--- trunk/LayoutTests/js/regress/exit-length-on-plain-object.html	                        (rev 0)
+++ trunk/LayoutTests/js/regress/exit-length-on-plain-object.html	2015-02-26 22:44:45 UTC (rev 180703)
@@ -0,0 +1,12 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+<script src=""
+<script src=""
+</body>
+</html>

Added: trunk/LayoutTests/js/regress/script-tests/arguments-out-of-bounds.js (0 => 180703)


--- trunk/LayoutTests/js/regress/script-tests/arguments-out-of-bounds.js	                        (rev 0)
+++ trunk/LayoutTests/js/regress/script-tests/arguments-out-of-bounds.js	2015-02-26 22:44:45 UTC (rev 180703)
@@ -0,0 +1,21 @@
+function foo() { return arguments; }
+noInline(foo);
+
+function bar(o) {
+    var tmp = o[0];
+    var result = 0;
+    for (var i = 0; i < 1000; ++i) {
+        if (tmp)
+            result += tmp * i;
+    }
+    return result;
+}
+noInline(bar);
+
+var result = 0;
+var o = foo();
+for (var i = 0; i < 10000; ++i)
+    result += bar(o);
+
+if (result !== 0)
+    throw "Error: bad result: " + result;

Added: trunk/LayoutTests/js/regress/script-tests/exit-length-on-plain-object.js (0 => 180703)


--- trunk/LayoutTests/js/regress/script-tests/exit-length-on-plain-object.js	                        (rev 0)
+++ trunk/LayoutTests/js/regress/script-tests/exit-length-on-plain-object.js	2015-02-26 22:44:45 UTC (rev 180703)
@@ -0,0 +1,31 @@
+function foo(o) {
+    var result;
+    for (var i = 0; i < 2; ++i) {
+        var subResult = o.length;
+        if (i == 0) {
+            result = subResult;
+            if (subResult !== void 0)
+                break;
+        }
+        o = [1];
+    }
+    if (result === void 0) {
+        for (var i = 0; i < 10000; ++i) { }
+    }
+    return result;
+}
+
+noInline(foo);
+
+for (var j = 0; j < 10; ++j) {
+    for (var i = 0; i < 10000; ++i) {
+        var a = [1];
+        a.length = 99999999;
+        a.f = 42;
+        foo(a);
+    }
+    
+    var result = foo({});
+    if (result !== void 0)
+        throw "Error: bad result: " + result;
+}

Added: trunk/LayoutTests/js/regress/script-tests/string-out-of-bounds.js (0 => 180703)


--- trunk/LayoutTests/js/regress/script-tests/string-out-of-bounds.js	                        (rev 0)
+++ trunk/LayoutTests/js/regress/script-tests/string-out-of-bounds.js	2015-02-26 22:44:45 UTC (rev 180703)
@@ -0,0 +1,17 @@
+function bar(o) {
+    var tmp = o[0];
+    var result = 0;
+    for (var i = 0; i < 1000; ++i) {
+        if (tmp)
+            result += tmp * i;
+    }
+    return result;
+}
+noInline(bar);
+
+var result = 0;
+for (var i = 0; i < 10000; ++i)
+    result += bar("");
+
+if (result !== 0)
+    throw "Error: bad result: " + result;

Added: trunk/LayoutTests/js/regress/string-out-of-bounds-expected.txt (0 => 180703)


--- trunk/LayoutTests/js/regress/string-out-of-bounds-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/js/regress/string-out-of-bounds-expected.txt	2015-02-26 22:44:45 UTC (rev 180703)
@@ -0,0 +1,10 @@
+JSRegress/string-out-of-bounds
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS no exception thrown
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/js/regress/string-out-of-bounds.html (0 => 180703)


--- trunk/LayoutTests/js/regress/string-out-of-bounds.html	                        (rev 0)
+++ trunk/LayoutTests/js/regress/string-out-of-bounds.html	2015-02-26 22:44:45 UTC (rev 180703)
@@ -0,0 +1,12 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+<script src=""
+<script src=""
+</body>
+</html>

Modified: trunk/Source/_javascript_Core/ChangeLog (180702 => 180703)


--- trunk/Source/_javascript_Core/ChangeLog	2015-02-26 22:25:31 UTC (rev 180702)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-02-26 22:44:45 UTC (rev 180703)
@@ -1,3 +1,35 @@
+2015-02-26  Filip Pizlo  <fpi...@apple.com>
+
+        Various array access corner cases should take OSR exit feedback
+        https://bugs.webkit.org/show_bug.cgi?id=142056
+
+        Reviewed by Geoffrey Garen.
+        
+        Two major changes here:
+        
+        - Don't keep converting GetById into GetArrayLength if we exited due to any kind of array
+          type check.
+        
+        - Use a generic form of GetByVal/PutByVal if we exited due to any kind of exotic checks,
+          like the Arguments safety checks. We use the "ExoticObjectMode" for out-of-bounds on
+          arguments for now, since it's a convenient way of forcing out-of-bounds to be handled by
+          the Generic array mode.
+
+        * bytecode/ExitKind.cpp:
+        (JSC::exitKindToString):
+        * bytecode/ExitKind.h:
+        * dfg/DFGArrayMode.cpp:
+        (JSC::DFG::ArrayMode::refine):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileGetByValOnArguments):
+        (JSC::DFG::SpeculativeJIT::compileGetArgumentsLength):
+        * tests/stress/array-length-array-storage-plain-object.js: Added.
+        (foo):
+        * tests/stress/array-length-plain-object.js: Added.
+        (foo):
+
 2015-02-25  Filip Pizlo  <fpi...@apple.com>
 
         DFG SSA stack accesses shouldn't speak of VariableAccessDatas

Modified: trunk/Source/_javascript_Core/bytecode/ExitKind.cpp (180702 => 180703)


--- trunk/Source/_javascript_Core/bytecode/ExitKind.cpp	2015-02-26 22:25:31 UTC (rev 180702)
+++ trunk/Source/_javascript_Core/bytecode/ExitKind.cpp	2015-02-26 22:44:45 UTC (rev 180703)
@@ -64,6 +64,8 @@
         return "InadequateCoverage";
     case ArgumentsEscaped:
         return "ArgumentsEscaped";
+    case ExoticObjectMode:
+        return "ExoticObjectMode";
     case NotStringObject:
         return "NotStringObject";
     case VarargsOverflow:

Modified: trunk/Source/_javascript_Core/bytecode/ExitKind.h (180702 => 180703)


--- trunk/Source/_javascript_Core/bytecode/ExitKind.h	2015-02-26 22:25:31 UTC (rev 180702)
+++ trunk/Source/_javascript_Core/bytecode/ExitKind.h	2015-02-26 22:44:45 UTC (rev 180703)
@@ -44,6 +44,7 @@
     OutOfBounds, // We had an out-of-bounds access to an array.
     InadequateCoverage, // We exited because we ended up in code that didn't have profiling coverage.
     ArgumentsEscaped, // We exited because arguments escaped but we didn't expect them to.
+    ExoticObjectMode, // We exited because some exotic object that we were accessing was in an exotic mode (like Arguments with slow arguments).
     NotStringObject, // We exited because we shouldn't have attempted to optimize string object access.
     VarargsOverflow, // We exited because a varargs call passed more arguments than we expected.
     Uncountable, // We exited for none of the above reasons, and we should not count it. Most uses of this should be viewed as a FIXME.

Modified: trunk/Source/_javascript_Core/dfg/DFGArrayMode.cpp (180702 => 180703)


--- trunk/Source/_javascript_Core/dfg/DFGArrayMode.cpp	2015-02-26 22:25:31 UTC (rev 180702)
+++ trunk/Source/_javascript_Core/dfg/DFGArrayMode.cpp	2015-02-26 22:44:45 UTC (rev 180703)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012, 2013, 2014 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -147,6 +147,10 @@
     if (!isInt32Speculation(index))
         return ArrayMode(Array::Generic);
     
+    // If we had exited because of an exotic object behavior, then don't try to specialize.
+    if (graph.hasExitSite(node->origin.semantic, ExoticObjectMode))
+        return ArrayMode(Array::Generic);
+    
     // Note: our profiling currently doesn't give us good information in case we have
     // an unlikely control flow path that sets the base to a non-cell value. Value
     // profiling and prediction propagation will probably tell us that the value is

Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (180702 => 180703)


--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2015-02-26 22:25:31 UTC (rev 180702)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2015-02-26 22:44:45 UTC (rev 180703)
@@ -919,19 +919,26 @@
         case GetByIdFlush: {
             if (!node->child1()->shouldSpeculateCell())
                 break;
-            StringImpl* impl = m_graph.identifiers()[node->identifierNumber()];
-            if (impl == vm().propertyNames->length.impl()) {
-                attemptToMakeGetArrayLength(node);
-                break;
+
+            // If we hadn't exited because of BadCache, BadIndexingType, or ExoticObjectMode, then
+            // leave this as a GetById.
+            if (!m_graph.hasExitSite(node->origin.semantic, BadCache)
+                && !m_graph.hasExitSite(node->origin.semantic, BadIndexingType)
+                && !m_graph.hasExitSite(node->origin.semantic, ExoticObjectMode)) {
+                StringImpl* impl = m_graph.identifiers()[node->identifierNumber()];
+                if (impl == vm().propertyNames->length.impl()) {
+                    attemptToMakeGetArrayLength(node);
+                    break;
+                }
+                if (impl == vm().propertyNames->byteLength.impl()) {
+                    attemptToMakeGetTypedArrayByteLength(node);
+                    break;
+                }
+                if (impl == vm().propertyNames->byteOffset.impl()) {
+                    attemptToMakeGetTypedArrayByteOffset(node);
+                    break;
+                }
             }
-            if (impl == vm().propertyNames->byteLength.impl()) {
-                attemptToMakeGetTypedArrayByteLength(node);
-                break;
-            }
-            if (impl == vm().propertyNames->byteOffset.impl()) {
-                attemptToMakeGetTypedArrayByteOffset(node);
-                break;
-            }
             fixEdge<CellUse>(node->child1());
             break;
         }

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (180702 => 180703)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2015-02-26 22:25:31 UTC (rev 180702)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2015-02-26 22:44:45 UTC (rev 180703)
@@ -4113,12 +4113,12 @@
     
     // Two really lame checks.
     speculationCheck(
-        Uncountable, JSValueSource(), 0,
+        ExoticObjectMode, JSValueSource(), 0,
         m_jit.branch32(
             MacroAssembler::AboveOrEqual, propertyReg,
             MacroAssembler::Address(baseReg, Arguments::offsetOfNumArguments())));
     speculationCheck(
-        Uncountable, JSValueSource(), 0,
+        ExoticObjectMode, JSValueSource(), 0,
         m_jit.branchTestPtr(
             MacroAssembler::NonZero,
             MacroAssembler::Address(
@@ -4168,7 +4168,7 @@
     ASSERT(ArrayMode(Array::Arguments).alreadyChecked(m_jit.graph(), node, m_state.forNode(node->child1())));
     
     speculationCheck(
-        Uncountable, JSValueSource(), 0,
+        ExoticObjectMode, JSValueSource(), 0,
         m_jit.branchTest8(
             MacroAssembler::NonZero,
             MacroAssembler::Address(baseReg, Arguments::offsetOfOverrodeLength())));

Added: trunk/Source/_javascript_Core/tests/stress/array-length-array-storage-plain-object.js (0 => 180703)


--- trunk/Source/_javascript_Core/tests/stress/array-length-array-storage-plain-object.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/array-length-array-storage-plain-object.js	2015-02-26 22:44:45 UTC (rev 180703)
@@ -0,0 +1,16 @@
+function foo(o) {
+    return o.length;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+    var a = [1];
+    a.length = 99999999;
+    a.f = 42;
+    foo(a);
+}
+
+var result = foo({});
+if (result !== void 0)
+    throw "Error: bad result: " + result;

Added: trunk/Source/_javascript_Core/tests/stress/array-length-plain-object.js (0 => 180703)


--- trunk/Source/_javascript_Core/tests/stress/array-length-plain-object.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/array-length-plain-object.js	2015-02-26 22:44:45 UTC (rev 180703)
@@ -0,0 +1,15 @@
+function foo(o) {
+    return o.length;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+    var a = [1];
+    a.f = 42;
+    foo(a);
+}
+
+var result = foo({});
+if (result !== void 0)
+    throw "Error: bad result: " + result;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to