Changeset 166276 in webkit

Timestamp:

Mar 25, 2014 7:14:40 PM (10 years ago)

Author:

fpizlo@apple.com

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):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• dfg/DFGByteCodeParser.cpp (modified) (7 diffs)
• tests/stress/assign-argument-in-inlined-call.js (added)
• tests/stress/assign-captured-argument-in-inlined-call.js (added)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2014-03-25  Filip Pizlo  <fpizlo@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  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

@@ -250 +250 @@
     }
     
-    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)
     {
@@ -341 +359 @@
         bool isCaptured = m_codeBlock->isCaptured(operand, inlineCallFrame());
         
-        if (setMode == NormalSet) {
+        if (setMode != ImmediateNakedSet) {
             ArgumentPosition* argumentPosition = findArgumentPositionForLocal(operand);
             if (isCaptured || argumentPosition)
@@ -400 +418 @@
         // then make sure that it's never unboxed.
         if (argument) {
-            if (setMode == NormalSet)
+            if (setMode != ImmediateNakedSet)
                 flushDirect(operand);
         } else if (m_codeBlock->specializationKind() == CodeForConstruct)
@@ -1400 +1418 @@
     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);
@@ -2148 +2166 @@
             // 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);
             
@@ -2846 +2864 @@
             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()) {
@@ -2929 +2947 @@
             // 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(