Title: [187579] trunk/Source/_javascript_Core
Revision
187579
Author
fpi...@apple.com
Date
2015-07-29 23:26:52 -0700 (Wed, 29 Jul 2015)

Log Message

DFG::ArgumentsEliminationPhase should emit a PutStack for all of the GetStacks that the ByteCodeParser emitted
https://bugs.webkit.org/show_bug.cgi?id=147433
rdar://problem/21668986

Reviewed by Mark Lam.

Ideally, the ByteCodeParser would only emit SetArgument nodes for named arguments.  But
currently that's not what it does - it emits a SetArgument for every argument that a varargs
call may pass.  Each SetArgument gets turned into a GetStack.  This means that if
ArgumentsEliminationPhase optimizes away PutStacks for those varargs arguments that didn't
get passed or used, we get degenerate IR where we have a GetStack of something that didn't
have a PutStack.

This fixes the bug by removing the code to optimize away PutStacks in
ArgumentsEliminationPhase.

* dfg/DFGArgumentsEliminationPhase.cpp:
* tests/stress/varargs-inlining-underflow.js: Added.
(baz):
(bar):
(foo):

Modified Paths

Added Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (187578 => 187579)


--- trunk/Source/_javascript_Core/ChangeLog	2015-07-30 05:47:26 UTC (rev 187578)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-07-30 06:26:52 UTC (rev 187579)
@@ -1,3 +1,27 @@
+2015-07-29  Filip Pizlo  <fpi...@apple.com>
+
+        DFG::ArgumentsEliminationPhase should emit a PutStack for all of the GetStacks that the ByteCodeParser emitted
+        https://bugs.webkit.org/show_bug.cgi?id=147433
+        rdar://problem/21668986
+
+        Reviewed by Mark Lam.
+
+        Ideally, the ByteCodeParser would only emit SetArgument nodes for named arguments.  But
+        currently that's not what it does - it emits a SetArgument for every argument that a varargs
+        call may pass.  Each SetArgument gets turned into a GetStack.  This means that if
+        ArgumentsEliminationPhase optimizes away PutStacks for those varargs arguments that didn't
+        get passed or used, we get degenerate IR where we have a GetStack of something that didn't
+        have a PutStack.
+
+        This fixes the bug by removing the code to optimize away PutStacks in
+        ArgumentsEliminationPhase.
+
+        * dfg/DFGArgumentsEliminationPhase.cpp:
+        * tests/stress/varargs-inlining-underflow.js: Added.
+        (baz):
+        (bar):
+        (foo):
+
 2015-07-29  Andy VanWagoner  <thetalecraf...@gmail.com>
 
         Implement basic types for ECMAScript Internationalization API

Modified: trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp (187578 => 187579)


--- trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp	2015-07-30 05:47:26 UTC (rev 187578)
+++ trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp	2015-07-30 06:26:52 UTC (rev 187579)
@@ -516,11 +516,12 @@
                                 value = insertionSet.insertNode(
                                     nodeIndex, SpecNone, GetStack, node->origin, OpInfo(data));
                             } else {
-                                // Check if this an element that we must initialize.
-                                if (storeIndex >= varargsData->mandatoryMinimum) {
-                                    // It's not. We're done.
-                                    break;
-                                }
+                                // FIXME: We shouldn't have to store anything if
+                                // storeIndex >= varargsData->mandatoryMinimum, but we will still
+                                // have GetStacks in that range. So if we don't do the stores, we'll
+                                // have degenerate IR: we'll have GetStacks of something that didn't
+                                // have PutStacks.
+                                // https://bugs.webkit.org/show_bug.cgi?id=147434
                                 
                                 if (!undefined) {
                                     undefined = insertionSet.insertConstant(

Added: trunk/Source/_javascript_Core/tests/stress/varargs-inlining-underflow.js (0 => 187579)


--- trunk/Source/_javascript_Core/tests/stress/varargs-inlining-underflow.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/varargs-inlining-underflow.js	2015-07-30 06:26:52 UTC (rev 187579)
@@ -0,0 +1,18 @@
+function baz() {
+}
+
+function bar() {
+    baz.apply(this, arguments);
+}
+
+for (var i = 0; i < 1000; ++i)
+    bar(1, 2, 3, 4, 5, 6, 7);
+
+function foo() {
+    bar();
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i)
+    foo();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to