Title: [202101] trunk/Source/_javascript_Core
Revision
202101
Author
keith_mil...@apple.com
Date
2016-06-15 12:40:43 -0700 (Wed, 15 Jun 2016)

Log Message

DFGByteCodeParser should be able to infer the value of unset properties in MultiGetByOffset
https://bugs.webkit.org/show_bug.cgi?id=158802

Reviewed by Filip Pizlo.

This patch adds support for unset properties in MultiGetByOffset. Since MultiGetByOffset
already supports constant values this patch just adds a constant case where the fetched
value is undefined. Fortunately (or unfortunately) we don't support object allocation
sinking for constant cases of MultiGetByOffset, which means we don't need to adjust any
in that phase.

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::planLoad):
(JSC::DFG::ByteCodeParser::handleGetById):
* dfg/DFGMultiGetByOffsetData.h:
* tests/stress/multi-get-by-offset-proto-or-unset.js: Added.
(foo):
* tests/stress/multi-get-by-offset-proto-self-or-unset.js: Added.
(foo):
* tests/stress/multi-get-by-offset-self-or-unset.js: Added.
(foo):

Modified Paths

Added Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (202100 => 202101)


--- trunk/Source/_javascript_Core/ChangeLog	2016-06-15 19:32:50 UTC (rev 202100)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-06-15 19:40:43 UTC (rev 202101)
@@ -1,3 +1,27 @@
+2016-06-15  Keith Miller  <keith_mil...@apple.com>
+
+        DFGByteCodeParser should be able to infer the value of unset properties in MultiGetByOffset
+        https://bugs.webkit.org/show_bug.cgi?id=158802
+
+        Reviewed by Filip Pizlo.
+
+        This patch adds support for unset properties in MultiGetByOffset. Since MultiGetByOffset
+        already supports constant values this patch just adds a constant case where the fetched
+        value is undefined. Fortunately (or unfortunately) we don't support object allocation
+        sinking for constant cases of MultiGetByOffset, which means we don't need to adjust any
+        in that phase.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::planLoad):
+        (JSC::DFG::ByteCodeParser::handleGetById):
+        * dfg/DFGMultiGetByOffsetData.h:
+        * tests/stress/multi-get-by-offset-proto-or-unset.js: Added.
+        (foo):
+        * tests/stress/multi-get-by-offset-proto-self-or-unset.js: Added.
+        (foo):
+        * tests/stress/multi-get-by-offset-self-or-unset.js: Added.
+        (foo):
+
 2016-06-15  Chris Dumez  <cdu...@apple.com>
 
         Unreviewed GCC build fix after r202098.

Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (202100 => 202101)


--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2016-06-15 19:32:50 UTC (rev 202100)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2016-06-15 19:40:43 UTC (rev 202101)
@@ -2942,7 +2942,11 @@
             break;
         }
     }
-    RELEASE_ASSERT(!!result);
+    if (!result) {
+        // We have a unset property.
+        ASSERT(!conditionSet.numberOfConditionsWithKind(PropertyCondition::Presence));
+        return GetByOffsetMethod::constant(m_constantUndefined);
+    }
     return result;
 }
 
@@ -3150,7 +3154,7 @@
         //    optimal, if there is some rarely executed case in the chain that requires a lot
         //    of checks and those checks are not watchpointable.
         for (const GetByIdVariant& variant : getByIdStatus.variants()) {
-            if (variant.intrinsic() != NoIntrinsic || variant.isPropertyUnset()) {
+            if (variant.intrinsic() != NoIntrinsic) {
                 set(VirtualRegister(destinationOperand),
                     addToGraph(getById, OpInfo(identifierNumber), OpInfo(prediction), base));
                 return;
@@ -3163,7 +3167,7 @@
                         GetByOffsetMethod::load(variant.offset())));
                 continue;
             }
-            
+
             GetByOffsetMethod method = planLoad(variant.conditionSet());
             if (!method) {
                 set(VirtualRegister(destinationOperand),

Modified: trunk/Source/_javascript_Core/dfg/DFGMultiGetByOffsetData.h (202100 => 202101)


--- trunk/Source/_javascript_Core/dfg/DFGMultiGetByOffsetData.h	2016-06-15 19:32:50 UTC (rev 202100)
+++ trunk/Source/_javascript_Core/dfg/DFGMultiGetByOffsetData.h	2016-06-15 19:40:43 UTC (rev 202101)
@@ -40,6 +40,9 @@
 public:
     enum Kind {
         Invalid,
+        // Constant might mean either that we have some fixed property or that the
+        // property is unset and we know the result is undefined. We don't distingish
+        // between these cases because no one cares about this distintion yet.
         Constant,
         Load,
         LoadFromPrototype

Added: trunk/Source/_javascript_Core/tests/stress/multi-get-by-offset-proto-or-unset.js (0 => 202101)


--- trunk/Source/_javascript_Core/tests/stress/multi-get-by-offset-proto-or-unset.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/multi-get-by-offset-proto-or-unset.js	2016-06-15 19:40:43 UTC (rev 202101)
@@ -0,0 +1,41 @@
+function foo(o, start) {
+    var result = 0;
+    for (var i = 0; i < 100; ++i)
+        result += o.f;
+    return result;
+}
+
+noInline(foo);
+
+
+var p = {};
+p.f = 42;
+var o = Object.create(p);
+
+var q = {}
+q.f = 42;
+
+var f = {};
+
+for (var i = 0; i < 10000; ++i)
+    o.f = i;
+o.f = 42;
+
+for (var i = 0; i < 10000; ++i) {
+    if (i % 100 === 0) {
+        if (foo(q) !== 42000)
+            throw new Error("bad result: " + result);
+    }
+
+    if (foo(o) !== 4200)
+        throw new Error("bad result: " + result);
+    var result = foo(f);
+    if (!Number.isNaN(result))
+        throw "Error: bad result: " + result;
+}
+
+var q = {};
+q.f = 43;
+var result = foo(q);
+if (result != 100 * 43)
+    throw "Error: bad result at end: " + result;

Added: trunk/Source/_javascript_Core/tests/stress/multi-get-by-offset-proto-self-or-unset.js (0 => 202101)


--- trunk/Source/_javascript_Core/tests/stress/multi-get-by-offset-proto-self-or-unset.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/multi-get-by-offset-proto-self-or-unset.js	2016-06-15 19:40:43 UTC (rev 202101)
@@ -0,0 +1,30 @@
+function foo(o, start) {
+    var result = 0;
+    for (var i = 0; i < 100; ++i)
+        result += o.f;
+    return result;
+}
+
+noInline(foo);
+
+var o = {};
+o.f = 42;
+var f = {};
+
+for (var i = 0; i < 10000; ++i)
+    o.f = i;
+o.f = 42;
+
+for (var i = 0; i < 10000; ++i) {
+    if (foo(o) !== 4200)
+        throw new Error("bad result: " + result);
+    var result = foo(f);
+    if (!Number.isNaN(result))
+        throw "Error: bad result: " + result;
+}
+
+var q = {};
+q.f = 43;
+var result = foo(q);
+if (result != 100 * 43)
+    throw "Error: bad result at end: " + result;

Added: trunk/Source/_javascript_Core/tests/stress/multi-get-by-offset-self-or-unset.js (0 => 202101)


--- trunk/Source/_javascript_Core/tests/stress/multi-get-by-offset-self-or-unset.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/multi-get-by-offset-self-or-unset.js	2016-06-15 19:40:43 UTC (rev 202101)
@@ -0,0 +1,30 @@
+function foo(o, start) {
+    var result = 0;
+    for (var i = 0; i < 100; ++i)
+        result += o.f;
+    return result;
+}
+
+noInline(foo);
+
+var o = {};
+o.f = 42;
+var f = {};
+
+for (var i = 0; i < 10000; ++i)
+    o.f = i;
+o.f = 42;
+
+for (var i = 0; i < 10000; ++i) {
+    if (foo(o) !== 4200)
+        throw new Error("bad result: " + result);
+    var result = foo(f);
+    if (!Number.isNaN(result))
+        throw "Error: bad result: " + result;
+}
+
+var q = {};
+q.f = 43;
+var result = foo(q);
+if (result != 100 * 43)
+    throw "Error: bad result at end: " + result;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to