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

Mon, 23 Feb 2015 19:33:54 -0800

Title: [180546] trunk/Source/_javascript_Core
Revision
180546
Author
[email protected]
Date
2015-02-23 19:33:01 -0800 (Mon, 23 Feb 2015)

Log Message

Set the semantic origin of delayed SetLocal to the Bytecode that originated it
https://bugs.webkit.org/show_bug.cgi?id=141727

Patch by Benjamin Poulain <[email protected]> on 2015-02-23
Reviewed by Filip Pizlo.

Previously, delayed SetLocals would have the NodeOrigin of the next
bytecode. This was because delayed SetLocal are...delayed... and
currentCodeOrigin() is the one where the node is emitted.

This made debugging a little awkward since the OSR exits on SetLocal
were reported for the next bytecode. This patch changes the semantic
origin to keep the original bytecode.

>From benchmarks, this looks like it could be a tiny bit faster
but it likely just noise.

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::setDirect):
(JSC::DFG::ByteCodeParser::setLocal):
(JSC::DFG::ByteCodeParser::setArgument):
(JSC::DFG::ByteCodeParser::currentNodeOrigin):
(JSC::DFG::ByteCodeParser::addToGraph):
(JSC::DFG::ByteCodeParser::DelayedSetLocal::DelayedSetLocal):
(JSC::DFG::ByteCodeParser::DelayedSetLocal::execute):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (180545 => 180546)


--- trunk/Source/_javascript_Core/ChangeLog	2015-02-24 03:32:03 UTC (rev 180545)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-02-24 03:33:01 UTC (rev 180546)
@@ -1,5 +1,32 @@
 2015-02-23  Benjamin Poulain  <[email protected]>
 
+        Set the semantic origin of delayed SetLocal to the Bytecode that originated it
+        https://bugs.webkit.org/show_bug.cgi?id=141727
+
+        Reviewed by Filip Pizlo.
+
+        Previously, delayed SetLocals would have the NodeOrigin of the next
+        bytecode. This was because delayed SetLocal are...delayed... and
+        currentCodeOrigin() is the one where the node is emitted.
+
+        This made debugging a little awkward since the OSR exits on SetLocal
+        were reported for the next bytecode. This patch changes the semantic
+        origin to keep the original bytecode.
+
+        From benchmarks, this looks like it could be a tiny bit faster
+        but it likely just noise.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::setDirect):
+        (JSC::DFG::ByteCodeParser::setLocal):
+        (JSC::DFG::ByteCodeParser::setArgument):
+        (JSC::DFG::ByteCodeParser::currentNodeOrigin):
+        (JSC::DFG::ByteCodeParser::addToGraph):
+        (JSC::DFG::ByteCodeParser::DelayedSetLocal::DelayedSetLocal):
+        (JSC::DFG::ByteCodeParser::DelayedSetLocal::execute):
+
+2015-02-23  Benjamin Poulain  <[email protected]>
+
         Remove DFGNode::predictHeap()
         https://bugs.webkit.org/show_bug.cgi?id=141864

Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (180545 => 180546)


--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2015-02-24 03:32:03 UTC (rev 180545)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2015-02-24 03:33:01 UTC (rev 180546)
@@ -301,9 +301,9 @@
     Node* setDirect(VirtualRegister operand, Node* value, SetMode setMode = NormalSet)
     {
         addToGraph(MovHint, OpInfo(operand.offset()), value);
+
+        DelayedSetLocal delayed(currentCodeOrigin(), operand, value);
         
-        DelayedSetLocal delayed = DelayedSetLocal(operand, value);
-        
         if (setMode == NormalSet) {
             m_setLocalQueue.append(delayed);
             return 0;
@@ -383,8 +383,11 @@
         return node;
     }
 
-    Node* setLocal(VirtualRegister operand, Node* value, SetMode setMode = NormalSet)
+    Node* setLocal(const CodeOrigin& semanticOrigin, VirtualRegister operand, Node* value, SetMode setMode = NormalSet)
     {
+        CodeOrigin oldSemanticOrigin = m_currentSemanticOrigin;
+        m_currentSemanticOrigin = semanticOrigin;
+
         unsigned local = operand.toLocal();
         bool isCaptured = m_codeBlock->isCaptured(operand, inlineCallFrame());
         
@@ -396,11 +399,13 @@
 
         VariableAccessData* variableAccessData = newVariableAccessData(operand, isCaptured);
         variableAccessData->mergeStructureCheckHoistingFailed(
-            m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadCache));
+            m_inlineStackTop->m_exitProfile.hasExitSite(semanticOrigin.bytecodeIndex, BadCache));
         variableAccessData->mergeCheckArrayHoistingFailed(
-            m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadIndexingType));
+            m_inlineStackTop->m_exitProfile.hasExitSite(semanticOrigin.bytecodeIndex, BadIndexingType));
         Node* node = addToGraph(SetLocal, OpInfo(variableAccessData), value);
         m_currentBlock->variablesAtTail.local(local) = node;
+
+        m_currentSemanticOrigin = oldSemanticOrigin;
         return node;
     }
 
@@ -436,8 +441,11 @@
         m_currentBlock->variablesAtTail.argument(argument) = node;
         return node;
     }
-    Node* setArgument(VirtualRegister operand, Node* value, SetMode setMode = NormalSet)
+    Node* setArgument(const CodeOrigin& semanticOrigin, VirtualRegister operand, Node* value, SetMode setMode = NormalSet)
     {
+        CodeOrigin oldSemanticOrigin = m_currentSemanticOrigin;
+        m_currentSemanticOrigin = semanticOrigin;
+
         unsigned argument = operand.toArgument();
         ASSERT(argument < m_numArguments);
         
@@ -454,11 +462,13 @@
             variableAccessData->mergeShouldNeverUnbox(true);
         
         variableAccessData->mergeStructureCheckHoistingFailed(
-            m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadCache));
+            m_inlineStackTop->m_exitProfile.hasExitSite(semanticOrigin.bytecodeIndex, BadCache));
         variableAccessData->mergeCheckArrayHoistingFailed(
-            m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadIndexingType));
+            m_inlineStackTop->m_exitProfile.hasExitSite(semanticOrigin.bytecodeIndex, BadIndexingType));
         Node* node = addToGraph(SetLocal, OpInfo(variableAccessData), value);
         m_currentBlock->variablesAtTail.argument(argument) = node;
+
+        m_currentSemanticOrigin = oldSemanticOrigin;
         return node;
     }
     
@@ -602,6 +612,13 @@
     {
         return CodeOrigin(m_currentIndex, inlineCallFrame());
     }
+
+    NodeOrigin currentNodeOrigin()
+    {
+        if (m_currentSemanticOrigin.isSet())
+            return NodeOrigin(m_currentSemanticOrigin, currentCodeOrigin());
+        return NodeOrigin(currentCodeOrigin());
+    }
     
     BranchData* branchData(unsigned taken, unsigned notTaken)
     {
@@ -616,7 +633,7 @@
     Node* addToGraph(NodeType op, Node* child1 = 0, Node* child2 = 0, Node* child3 = 0)
     {
         Node* result = m_graph.addNode(
-            SpecNone, op, NodeOrigin(currentCodeOrigin()), Edge(child1), Edge(child2),
+            SpecNone, op, currentNodeOrigin(), Edge(child1), Edge(child2),
             Edge(child3));
         ASSERT(op != Phi);
         m_currentBlock->append(result);
@@ -625,7 +642,7 @@
     Node* addToGraph(NodeType op, Edge child1, Edge child2 = Edge(), Edge child3 = Edge())
     {
         Node* result = m_graph.addNode(
-            SpecNone, op, NodeOrigin(currentCodeOrigin()), child1, child2, child3);
+            SpecNone, op, currentNodeOrigin(), child1, child2, child3);
         ASSERT(op != Phi);
         m_currentBlock->append(result);
         return result;
@@ -633,7 +650,7 @@
     Node* addToGraph(NodeType op, OpInfo info, Node* child1 = 0, Node* child2 = 0, Node* child3 = 0)
     {
         Node* result = m_graph.addNode(
-            SpecNone, op, NodeOrigin(currentCodeOrigin()), info, Edge(child1), Edge(child2),
+            SpecNone, op, currentNodeOrigin(), info, Edge(child1), Edge(child2),
             Edge(child3));
         ASSERT(op != Phi);
         m_currentBlock->append(result);
@@ -642,7 +659,7 @@
     Node* addToGraph(NodeType op, OpInfo info1, OpInfo info2, Node* child1 = 0, Node* child2 = 0, Node* child3 = 0)
     {
         Node* result = m_graph.addNode(
-            SpecNone, op, NodeOrigin(currentCodeOrigin()), info1, info2,
+            SpecNone, op, currentNodeOrigin(), info1, info2,
             Edge(child1), Edge(child2), Edge(child3));
         ASSERT(op != Phi);
         m_currentBlock->append(result);
@@ -652,7 +669,7 @@
     Node* addToGraph(Node::VarArgTag, NodeType op, OpInfo info1, OpInfo info2)
     {
         Node* result = m_graph.addNode(
-            SpecNone, Node::VarArg, op, NodeOrigin(currentCodeOrigin()), info1, info2,
+            SpecNone, Node::VarArg, op, currentNodeOrigin(), info1, info2,
             m_graph.m_varArgChildren.size() - m_numPassedVarArgs, m_numPassedVarArgs);
         ASSERT(op != Phi);
         m_currentBlock->append(result);
@@ -844,6 +861,8 @@
     BasicBlock* m_currentBlock;
     // The bytecode index of the current instruction being generated.
     unsigned m_currentIndex;
+    // The semantic origin of the current node if different from the current Index.
+    CodeOrigin m_currentSemanticOrigin;
 
     FrozenValue* m_constantUndefined;
     FrozenValue* m_constantNull;
@@ -962,12 +981,14 @@
     InlineStackEntry* m_inlineStackTop;
     
     struct DelayedSetLocal {
+        CodeOrigin m_origin;
         VirtualRegister m_operand;
         Node* m_value;
         
         DelayedSetLocal() { }
-        DelayedSetLocal(VirtualRegister operand, Node* value)
-            : m_operand(operand)
+        DelayedSetLocal(const CodeOrigin& origin, VirtualRegister operand, Node* value)
+            : m_origin(origin)
+            , m_operand(operand)
             , m_value(value)
         {
         }
@@ -975,8 +996,8 @@
         Node* execute(ByteCodeParser* parser, SetMode setMode = NormalSet)
         {
             if (m_operand.isArgument())
-                return parser->setArgument(m_operand, m_value, setMode);
-            return parser->setLocal(m_operand, m_value, setMode);
+                return parser->setArgument(m_origin, m_operand, m_value, setMode);
+            return parser->setLocal(m_origin, m_operand, m_value, setMode);
         }
     };
     

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to