[webkit-changes] [166276] trunk/Source/JavaScriptCore

Tue, 25 Mar 2014 19:15:00 -0700

Title: [166276] trunk/Source/_javascript_Core
Revision
166276
Author
fpi...@apple.com
Date
2014-03-25 19:14:40 -0700 (Tue, 25 Mar 2014)

Log Message

DFG::ByteCodeParser::SetMode should distinguish between setting immediately without a flush and setting immediately with a flush
https://bugs.webkit.org/show_bug.cgi?id=130760

Reviewed by Mark Hahnenberg.

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::setLocal):
(JSC::DFG::ByteCodeParser::setArgument):
(JSC::DFG::ByteCodeParser::handleInlining):
(JSC::DFG::ByteCodeParser::parseBlock):
* tests/stress/assign-argument-in-inlined-call.js: Added.
(f1):
(getF2Arguments):
(f2):
(f3):
* tests/stress/assign-captured-argument-in-inlined-call.js: Added.
(f1):
(f2):
(f3):

Modified Paths

Added Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (166275 => 166276)


--- trunk/Source/_javascript_Core/ChangeLog	2014-03-26 01:47:26 UTC (rev 166275)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-03-26 02:14:40 UTC (rev 166276)
@@ -1,5 +1,27 @@
 2014-03-25  Filip Pizlo  <fpi...@apple.com>
 
+        DFG::ByteCodeParser::SetMode should distinguish between setting immediately without a flush and setting immediately with a flush
+        https://bugs.webkit.org/show_bug.cgi?id=130760
+
+        Reviewed by Mark Hahnenberg.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::setLocal):
+        (JSC::DFG::ByteCodeParser::setArgument):
+        (JSC::DFG::ByteCodeParser::handleInlining):
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        * tests/stress/assign-argument-in-inlined-call.js: Added.
+        (f1):
+        (getF2Arguments):
+        (f2):
+        (f3):
+        * tests/stress/assign-captured-argument-in-inlined-call.js: Added.
+        (f1):
+        (f2):
+        (f3):
+
+2014-03-25  Filip Pizlo  <fpi...@apple.com>
+
         Fix 32-bit getter call alignment.
 
         Reviewed by Mark Hahnenberg.

Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (166275 => 166276)


--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2014-03-26 01:47:26 UTC (rev 166275)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2014-03-26 02:14:40 UTC (rev 166276)
@@ -249,7 +249,25 @@
         return getDirect(m_inlineStackTop->remapOperand(operand));
     }
     
-    enum SetMode { NormalSet, ImmediateSet };
+    enum SetMode {
+        // A normal set which follows a two-phase commit that spans code origins. During
+        // the current code origin it issues a MovHint, and at the start of the next
+        // code origin there will be a SetLocal. If the local needs flushing, the second
+        // SetLocal will be preceded with a Flush.
+        NormalSet,
+        
+        // A set where the SetLocal happens immediately and there is still a Flush. This
+        // is relevant when assigning to a local in tricky situations for the delayed
+        // SetLocal logic but where we know that we have not performed any side effects
+        // within this code origin. This is a safe replacement for NormalSet anytime we
+        // know that we have not yet performed side effects in this code origin.
+        ImmediateSetWithFlush,
+        
+        // A set where the SetLocal happens immediately and we do not Flush it even if
+        // this is a local that is marked as needing it. This is relevant when
+        // initializing locals at the top of a function.
+        ImmediateNakedSet
+    };
     Node* setDirect(VirtualRegister operand, Node* value, SetMode setMode = NormalSet)
     {
         addToGraph(MovHint, OpInfo(operand.offset()), value);
@@ -340,7 +358,7 @@
         unsigned local = operand.toLocal();
         bool isCaptured = m_codeBlock->isCaptured(operand, inlineCallFrame());
         
-        if (setMode == NormalSet) {
+        if (setMode != ImmediateNakedSet) {
             ArgumentPosition* argumentPosition = findArgumentPositionForLocal(operand);
             if (isCaptured || argumentPosition)
                 flushDirect(operand, argumentPosition);
@@ -399,7 +417,7 @@
         // Always flush arguments, except for 'this'. If 'this' is created by us,
         // then make sure that it's never unboxed.
         if (argument) {
-            if (setMode == NormalSet)
+            if (setMode != ImmediateNakedSet)
                 flushDirect(operand);
         } else if (m_codeBlock->specializationKind() == CodeForConstruct)
             variableAccessData->mergeShouldNeverUnbox(true);
@@ -1399,9 +1417,9 @@
         == callLinkStatus.isClosureCall());
     if (callLinkStatus.isClosureCall()) {
         VariableAccessData* calleeVariable =
-            set(VirtualRegister(JSStack::Callee), callTargetNode, ImmediateSet)->variableAccessData();
+            set(VirtualRegister(JSStack::Callee), callTargetNode, ImmediateNakedSet)->variableAccessData();
         VariableAccessData* scopeVariable =
-            set(VirtualRegister(JSStack::ScopeChain), addToGraph(GetScope, callTargetNode), ImmediateSet)->variableAccessData();
+            set(VirtualRegister(JSStack::ScopeChain), addToGraph(GetScope, callTargetNode), ImmediateNakedSet)->variableAccessData();
         
         calleeVariable->mergeShouldNeverUnbox(true);
         scopeVariable->mergeShouldNeverUnbox(true);
@@ -2147,9 +2165,9 @@
         case op_enter:
             // Initialize all locals to undefined.
             for (int i = 0; i < m_inlineStackTop->m_codeBlock->m_numVars; ++i)
-                set(virtualRegisterForLocal(i), constantUndefined(), ImmediateSet);
+                set(virtualRegisterForLocal(i), constantUndefined(), ImmediateNakedSet);
             if (m_inlineStackTop->m_codeBlock->specializationKind() == CodeForConstruct)
-                set(virtualRegisterForArgument(0), constantUndefined(), ImmediateSet);
+                set(virtualRegisterForArgument(0), constantUndefined(), ImmediateNakedSet);
             NEXT_OPCODE(op_enter);
             
         case op_touch_entry:
@@ -2845,7 +2863,7 @@
             flushForReturn();
             if (inlineCallFrame()) {
                 ASSERT(m_inlineStackTop->m_returnValue.isValid());
-                setDirect(m_inlineStackTop->m_returnValue, get(VirtualRegister(currentInstruction[1].u.operand)), ImmediateSet);
+                setDirect(m_inlineStackTop->m_returnValue, get(VirtualRegister(currentInstruction[1].u.operand)), ImmediateSetWithFlush);
                 m_inlineStackTop->m_didReturn = true;
                 if (m_inlineStackTop->m_unlinkedBlocks.isEmpty()) {
                     // If we're returning from the first block, then we're done parsing.
@@ -2928,9 +2946,9 @@
             // The bytecode wouldn't have set up the arguments. But we'll do it and make it
             // look like the bytecode had done it.
             int nextRegister = registerOffset + JSStack::CallFrameHeaderSize;
-            set(VirtualRegister(nextRegister++), get(VirtualRegister(thisReg)), ImmediateSet);
+            set(VirtualRegister(nextRegister++), get(VirtualRegister(thisReg)), ImmediateNakedSet);
             for (unsigned argument = 1; argument < argCount; ++argument)
-                set(VirtualRegister(nextRegister++), get(virtualRegisterForArgument(argument)), ImmediateSet);
+                set(VirtualRegister(nextRegister++), get(virtualRegisterForArgument(argument)), ImmediateNakedSet);
             
             handleCall(
                 result, Call, CodeForCall, OPCODE_LENGTH(op_call_varargs),

Added: trunk/Source/_javascript_Core/tests/stress/assign-argument-in-inlined-call.js (0 => 166276)


--- trunk/Source/_javascript_Core/tests/stress/assign-argument-in-inlined-call.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/assign-argument-in-inlined-call.js	2014-03-26 02:14:40 UTC (rev 166276)
@@ -0,0 +1,27 @@
+function f1(a) {
+    return a[0];
+}
+
+function getF2Arguments() {
+    return f2.arguments;
+}
+
+noInline(getF2Arguments);
+
+function f2(a) {
+    a = f1(getF2Arguments());
+    return a;
+}
+
+function f3(a) {
+    return f2(a);
+}
+
+noInline(f3);
+
+for (var i = 0; i < 10000; ++i) {
+    var result = f3(42);
+    if (result != 42)
+        throw "Error: bad result: " + result;
+}
+

Added: trunk/Source/_javascript_Core/tests/stress/assign-captured-argument-in-inlined-call.js (0 => 166276)


--- trunk/Source/_javascript_Core/tests/stress/assign-captured-argument-in-inlined-call.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/assign-captured-argument-in-inlined-call.js	2014-03-26 02:14:40 UTC (rev 166276)
@@ -0,0 +1,21 @@
+function f1(a) {
+    return a[0];
+}
+
+function f2(a) {
+    a = f1(arguments);
+    return a;
+}
+
+function f3(a) {
+    return f2(a);
+}
+
+noInline(f3);
+
+for (var i = 0; i < 10000; ++i) {
+    var result = f3(42);
+    if (result != 42)
+        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