Changeset 179477 in webkit for trunk/Source/JavaScriptCore/dfg/DFGCPSRethreadingPhase.cpp

Timestamp:

Feb 2, 2015, 10:15:44 AM (10 years ago)

Author:

[email protected]

Message:

Converting Flushes and PhantomLocals to Phantoms requires an OSR availability analysis rather than just using the SetLocal's child
​https://bugs.webkit.org/show_bug.cgi?id=141107

Reviewed by Michael Saboff.

See the bugzilla for a discussion of the problem. This addresses the problem by ensuring
that Flushes are always strength-reduced to PhantomLocals, and CPS rethreading does a mini
OSR availability analysis to determine the right MovHint value to use for the Phantom.

• dfg/DFGCPSRethreadingPhase.cpp:

(JSC::DFG::CPSRethreadingPhase::CPSRethreadingPhase):
(JSC::DFG::CPSRethreadingPhase::freeUnnecessaryNodes):
(JSC::DFG::CPSRethreadingPhase::clearVariables):
(JSC::DFG::CPSRethreadingPhase::canonicalizeFlushOrPhantomLocalFor):
(JSC::DFG::CPSRethreadingPhase::canonicalizeLocalsInBlock):
(JSC::DFG::CPSRethreadingPhase::clearVariablesAtHeadAndTail): Deleted.

• dfg/DFGNode.h:

(JSC::DFG::Node::convertPhantomToPhantomLocal):
(JSC::DFG::Node::convertFlushToPhantomLocal):
(JSC::DFG::Node::convertToPhantomLocal): Deleted.

• dfg/DFGStrengthReductionPhase.cpp:

(JSC::DFG::StrengthReductionPhase::handleNode):

• tests/stress/inline-call-that-doesnt-use-all-args.js: Added.

(foo):
(bar):
(baz):

File:

• trunk/Source/JavaScriptCore/dfg/DFGCPSRethreadingPhase.cpp (modified) (8 diffs)

trunk/Source/JavaScriptCore/dfg/DFGCPSRethreadingPhase.cpp

@@ -1 +1 @@
 /*
- * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -40 +40 @@
     CPSRethreadingPhase(Graph& graph)
         : Phase(graph, "CPS rethreading")
+        , m_availableForOSR(OperandsLike, graph.block(0)->variablesAtHead)
     {
     }
@@ -99 +100 @@
                     case SetArgument:
                     case SetLocal:
-                        node->convertToPhantomLocal();
+                        node->convertPhantomToPhantomLocal();
                         break;
                     default:
@@ -120 +121 @@
     
     template<OperandKind operandKind>
-    void clearVariablesAtHeadAndTail()
+    void clearVariables()
     {
         ASSERT(
             m_block->variablesAtHead.sizeFor<operandKind>()
             == m_block->variablesAtTail.sizeFor<operandKind>());
+        ASSERT(
+            m_block->variablesAtHead.sizeFor<operandKind>()
+            == m_availableForOSR.sizeFor<operandKind>());
         
         for (unsigned i = m_block->variablesAtHead.sizeFor<operandKind>(); i--;) {
-            m_block->variablesAtHead.atFor<operandKind>(i) = 0;
-            m_block->variablesAtTail.atFor<operandKind>(i) = 0;
+            m_block->variablesAtHead.atFor<operandKind>(i) = nullptr;
+            m_block->variablesAtTail.atFor<operandKind>(i) = nullptr;
+            m_availableForOSR.atFor<operandKind>(i) = Edge();
         }
     }
@@ -260 +265 @@
                 // know that I would have read the value written by that SetLocal. This is
                 // redundant and inefficient, since really it just means that we want to
-                // be keeping the operand to the SetLocal alive. The SetLocal may die, and
-                // we'll be fine because OSR tracks dead SetLocals.
-                
-                // So we turn this into a Phantom on the child of the SetLocal.
-                
+                // keep the last MovHinted value of that local alive.
+                
+                node->children.setChild1(m_availableForOSR.atFor<operandKind>(idx));
                 node->convertToPhantom();
-                node->children.setChild1(otherNode->child1());
                 return;
             }
@@ -312 +314 @@
         ASSERT(m_block->isReachable);
         
-        clearVariablesAtHeadAndTail<ArgumentOperand>();
-        clearVariablesAtHeadAndTail<LocalOperand>();
+        clearVariables<ArgumentOperand>();
+        clearVariables<LocalOperand>();
         
         // Assumes that all phi references have been removed. Assumes that things that
@@ -383 +385 @@
                 break;
                 
+            case MovHint:
+                m_availableForOSR.operand(node->unlinkedLocal()) = node->child1();
+                break;
+                
             default:
                 break;
@@ -523 +529 @@
     Vector<PhiStackEntry, 128> m_localPhiStack;
     Vector<Node*, 128> m_flushedLocalOpWorklist;
+    Operands<Edge> m_availableForOSR;
 };