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

Wed, 24 Jul 2013 21:05:08 -0700

Title: [153291] trunk/Source/_javascript_Core
Revision
153291
Author
[email protected]
Date
2013-07-24 21:05:22 -0700 (Wed, 24 Jul 2013)

Log Message

fourthTier: It should be possible for a DFG::Node to claim to exit to one CodeOrigin, but then claim that it belongs to a different CodeOrigin for all other purposes
https://bugs.webkit.org/show_bug.cgi?id=118946

Reviewed by Geoffrey Garen.

We want to decouple the exit target code origin of a node from the code origin
for all other purposes. The purposes of code origins are:

- Where the node will exit, if it exits. The exit target should be consistent with
  the surrounding nodes, in that if you just looked at the code origins of nodes in
  the graph, they would be consistent with the code origins in bytecode. This is
  necessary for live-at-bytecode analyses to work, and to preserve the original
  bytecode semantics when exiting.

- What kind of code the node came from, for semantics thingies. For example, we
  might use the code origin to find the node's global object for doing an original
  array check. Or we might use it to determine if the code is in strict mode. Or
  other similar things. When we use the code origin in this way, we're basically
  using it as a way of describing the node's meta-data without putting it into the
  node directly, to save space. In the absurd extreme you could imagine nodes not
  even having NodeTypes or NodeFlags, and just using the CodeOrigin to determine
  what bytecode the node originated from. We won't do that, but you can think of
  this use of code origins as just a way of compressing meta-data.

- What code origin we should supply profiling to, if we exit. This is closely
  related to the semantics thingies, in that the exit profiling is a persistent
  kind of semantic meta-data that survives between recompiles, and the only way to
  do that is to ascribe it to the original bytecode via the code origin.

If we hoist a node, we need to change the exit target code origin, but we must not
change the code origin for other purposes. The best way to do this is to decouple
the two kinds of code origin.

OSR exit data structures already do this, because they may edit the exit target
code origin while keeping the code origin for profiling intact. This happens for
forward exits. So, we just need to thread separation all the way back to DFG::Node.
That's what this patch does.

* dfg/DFGNode.h:
(JSC::DFG::Node::Node):
(Node):
* dfg/DFGOSRExit.cpp:
(JSC::DFG::OSRExit::OSRExit):
* dfg/DFGOSRExitBase.h:
(JSC::DFG::OSRExitBase::OSRExitBase):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileCurrentBlock):
(JSC::DFG::SpeculativeJIT::checkArgumentTypes):
* dfg/DFGSpeculativeJIT.h:
(SpeculativeJIT):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::compileNode):
(JSC::FTL::LowerDFGToLLVM::appendOSRExit):
(LowerDFGToLLVM):
* ftl/FTLOSRExit.cpp:
(JSC::FTL::OSRExit::OSRExit):
* ftl/FTLOSRExit.h:
(OSRExit):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (153290 => 153291)


--- trunk/Source/_javascript_Core/ChangeLog	2013-07-25 04:05:20 UTC (rev 153290)
+++ trunk/Source/_javascript_Core/ChangeLog	2013-07-25 04:05:22 UTC (rev 153291)
@@ -1,3 +1,64 @@
+2013-07-21  Filip Pizlo  <[email protected]>
+
+        fourthTier: It should be possible for a DFG::Node to claim to exit to one CodeOrigin, but then claim that it belongs to a different CodeOrigin for all other purposes
+        https://bugs.webkit.org/show_bug.cgi?id=118946
+
+        Reviewed by Geoffrey Garen.
+        
+        We want to decouple the exit target code origin of a node from the code origin
+        for all other purposes. The purposes of code origins are:
+        
+        - Where the node will exit, if it exits. The exit target should be consistent with
+          the surrounding nodes, in that if you just looked at the code origins of nodes in
+          the graph, they would be consistent with the code origins in bytecode. This is
+          necessary for live-at-bytecode analyses to work, and to preserve the original
+          bytecode semantics when exiting.
+        
+        - What kind of code the node came from, for semantics thingies. For example, we
+          might use the code origin to find the node's global object for doing an original
+          array check. Or we might use it to determine if the code is in strict mode. Or
+          other similar things. When we use the code origin in this way, we're basically
+          using it as a way of describing the node's meta-data without putting it into the
+          node directly, to save space. In the absurd extreme you could imagine nodes not
+          even having NodeTypes or NodeFlags, and just using the CodeOrigin to determine
+          what bytecode the node originated from. We won't do that, but you can think of
+          this use of code origins as just a way of compressing meta-data.
+        
+        - What code origin we should supply profiling to, if we exit. This is closely
+          related to the semantics thingies, in that the exit profiling is a persistent
+          kind of semantic meta-data that survives between recompiles, and the only way to
+          do that is to ascribe it to the original bytecode via the code origin.
+        
+        If we hoist a node, we need to change the exit target code origin, but we must not
+        change the code origin for other purposes. The best way to do this is to decouple
+        the two kinds of code origin.
+        
+        OSR exit data structures already do this, because they may edit the exit target
+        code origin while keeping the code origin for profiling intact. This happens for
+        forward exits. So, we just need to thread separation all the way back to DFG::Node.
+        That's what this patch does.
+
+        * dfg/DFGNode.h:
+        (JSC::DFG::Node::Node):
+        (Node):
+        * dfg/DFGOSRExit.cpp:
+        (JSC::DFG::OSRExit::OSRExit):
+        * dfg/DFGOSRExitBase.h:
+        (JSC::DFG::OSRExitBase::OSRExitBase):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileCurrentBlock):
+        (JSC::DFG::SpeculativeJIT::checkArgumentTypes):
+        * dfg/DFGSpeculativeJIT.h:
+        (SpeculativeJIT):
+        * ftl/FTLLowerDFGToLLVM.cpp:
+        (JSC::FTL::LowerDFGToLLVM::compileNode):
+        (JSC::FTL::LowerDFGToLLVM::appendOSRExit):
+        (LowerDFGToLLVM):
+        * ftl/FTLOSRExit.cpp:
+        (JSC::FTL::OSRExit::OSRExit):
+        * ftl/FTLOSRExit.h:
+        (OSRExit):
+
 2013-07-20  Filip Pizlo  <[email protected]>
 
         fourthTier: each DFG node that relies on other nodes to do their type checks should be able to tell you if those type checks happened

Modified: trunk/Source/_javascript_Core/dfg/DFGNode.h (153290 => 153291)


--- trunk/Source/_javascript_Core/dfg/DFGNode.h	2013-07-25 04:05:20 UTC (rev 153290)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.h	2013-07-25 04:05:22 UTC (rev 153291)
@@ -162,6 +162,7 @@
     
     Node(NodeType op, CodeOrigin codeOrigin, const AdjacencyList& children)
         : codeOrigin(codeOrigin)
+        , codeOriginForExitTarget(codeOrigin)
         , children(children)
         , m_virtualRegister(InvalidVirtualRegister)
         , m_refCount(1)
@@ -174,6 +175,7 @@
     // Construct a node with up to 3 children, no immediate value.
     Node(NodeType op, CodeOrigin codeOrigin, Edge child1 = Edge(), Edge child2 = Edge(), Edge child3 = Edge())
         : codeOrigin(codeOrigin)
+        , codeOriginForExitTarget(codeOrigin)
         , children(AdjacencyList::Fixed, child1, child2, child3)
         , m_virtualRegister(InvalidVirtualRegister)
         , m_refCount(1)
@@ -187,6 +189,7 @@
     // Construct a node with up to 3 children and an immediate value.
     Node(NodeType op, CodeOrigin codeOrigin, OpInfo imm, Edge child1 = Edge(), Edge child2 = Edge(), Edge child3 = Edge())
         : codeOrigin(codeOrigin)
+        , codeOriginForExitTarget(codeOrigin)
         , children(AdjacencyList::Fixed, child1, child2, child3)
         , m_virtualRegister(InvalidVirtualRegister)
         , m_refCount(1)
@@ -201,6 +204,7 @@
     // Construct a node with up to 3 children and two immediate values.
     Node(NodeType op, CodeOrigin codeOrigin, OpInfo imm1, OpInfo imm2, Edge child1 = Edge(), Edge child2 = Edge(), Edge child3 = Edge())
         : codeOrigin(codeOrigin)
+        , codeOriginForExitTarget(codeOrigin)
         , children(AdjacencyList::Fixed, child1, child2, child3)
         , m_virtualRegister(InvalidVirtualRegister)
         , m_refCount(1)
@@ -216,6 +220,7 @@
     // Construct a node with a variable number of children and two immediate values.
     Node(VarArgTag, NodeType op, CodeOrigin codeOrigin, OpInfo imm1, OpInfo imm2, unsigned firstChild, unsigned numChildren)
         : codeOrigin(codeOrigin)
+        , codeOriginForExitTarget(codeOrigin)
         , children(AdjacencyList::Variable, firstChild, numChildren)
         , m_virtualRegister(InvalidVirtualRegister)
         , m_refCount(1)
@@ -1383,8 +1388,11 @@
     
     // NB. This class must have a trivial destructor.
     
-    // Used to look up exception handling information (currently implemented as a bytecode index).
+    // Used for determining what bytecode this came from. This is important for
+    // debugging, exceptions, and even basic execution semantics.
     CodeOrigin codeOrigin;
+    // Code origin for where the node exits to.
+    CodeOrigin codeOriginForExitTarget;
     // References to up to 3 children, or links to a variable length set of children.
     AdjacencyList children;

Modified: trunk/Source/_javascript_Core/dfg/DFGOSRExit.cpp (153290 => 153291)


--- trunk/Source/_javascript_Core/dfg/DFGOSRExit.cpp	2013-07-25 04:05:20 UTC (rev 153290)
+++ trunk/Source/_javascript_Core/dfg/DFGOSRExit.cpp	2013-07-25 04:05:22 UTC (rev 153291)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011 Apple Inc. All rights reserved.
+ * Copyright (C) 2011, 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -36,7 +36,7 @@
 namespace JSC { namespace DFG {
 
 OSRExit::OSRExit(ExitKind kind, JSValueSource jsValueSource, MethodOfGettingAValueProfile valueProfile, SpeculativeJIT* jit, unsigned streamIndex, unsigned recoveryIndex)
-    : OSRExitBase(kind, jit->m_codeOriginForOSR)
+    : OSRExitBase(kind, jit->m_codeOriginForExitTarget, jit->m_codeOriginForExitProfile)
     , m_jsValueSource(jsValueSource)
     , m_valueProfile(valueProfile)
     , m_patchableCodeOffset(0)

Modified: trunk/Source/_javascript_Core/dfg/DFGOSRExitBase.cpp (153290 => 153291)


--- trunk/Source/_javascript_Core/dfg/DFGOSRExitBase.cpp	2013-07-25 04:05:20 UTC (rev 153290)
+++ trunk/Source/_javascript_Core/dfg/DFGOSRExitBase.cpp	2013-07-25 04:05:22 UTC (rev 153291)
@@ -53,7 +53,7 @@
         && !currentNode->containsMovHint()) {
         Node* setLocal = block->at(nodeIndex - 1);
         ASSERT_UNUSED(setLocal, setLocal->containsMovHint());
-        ASSERT_UNUSED(setLocal, setLocal->codeOrigin == currentNode->codeOrigin);
+        ASSERT_UNUSED(setLocal, setLocal->codeOriginForExitTarget == currentNode->codeOriginForExitTarget);
     }
     
     // Find the first node for the next bytecode instruction. Also track the last mov hint
@@ -72,12 +72,12 @@
         node = block->at(indexInBlock);
         if (node->containsMovHint() && node->child1() == currentNode)
             lastMovHint = node;
-        if (node->codeOrigin != currentNode->codeOrigin)
+        if (node->codeOriginForExitTarget != currentNode->codeOriginForExitTarget)
             break;
         indexInBlock++;
     }
     
-    ASSERT(node->codeOrigin != currentNode->codeOrigin);
+    ASSERT(node->codeOriginForExitTarget != currentNode->codeOriginForExitTarget);
     return true;
 }

Modified: trunk/Source/_javascript_Core/dfg/DFGOSRExitBase.h (153290 => 153291)


--- trunk/Source/_javascript_Core/dfg/DFGOSRExitBase.h	2013-07-25 04:05:20 UTC (rev 153290)
+++ trunk/Source/_javascript_Core/dfg/DFGOSRExitBase.h	2013-07-25 04:05:22 UTC (rev 153291)
@@ -42,11 +42,11 @@
 // and the FTL.
 
 struct OSRExitBase {
-    OSRExitBase(ExitKind kind, CodeOrigin origin)
+    OSRExitBase(ExitKind kind, CodeOrigin origin, CodeOrigin originForProfile)
         : m_kind(kind)
         , m_count(0)
         , m_codeOrigin(origin)
-        , m_codeOriginForExitProfile(origin)
+        , m_codeOriginForExitProfile(originForProfile)
     {
     }

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (153290 => 153291)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2013-07-25 04:05:20 UTC (rev 153290)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2013-07-25 04:05:22 UTC (rev 153291)
@@ -1722,7 +1722,8 @@
     }
     
     m_lastSetOperand = std::numeric_limits<int>::max();
-    m_codeOriginForOSR = CodeOrigin();
+    m_codeOriginForExitTarget = CodeOrigin();
+    m_codeOriginForExitProfile = CodeOrigin();
     
 #if DFG_ENABLE(DEBUG_VERBOSE)
     dataLogF("\n");
@@ -1741,7 +1742,8 @@
         m_canExit = m_currentNode->canExit();
         bool shouldExecuteEffects = m_interpreter.startExecuting(m_currentNode);
         m_jit.setForNode(m_currentNode);
-        m_codeOriginForOSR = m_currentNode->codeOrigin;
+        m_codeOriginForExitTarget = m_currentNode->codeOriginForExitTarget;
+        m_codeOriginForExitProfile = m_currentNode->codeOrigin;
         if (!m_currentNode->shouldGenerate()) {
 #if DFG_ENABLE(DEBUG_VERBOSE)
             dataLogF("SpeculativeJIT skipping Node @%d (bc#%u) at JIT offset 0x%x     ", m_currentNode->index(), m_currentNode->codeOrigin.bytecodeIndex, m_jit.debugOffset());
@@ -1858,7 +1860,8 @@
     ASSERT(!m_currentNode);
     m_isCheckingArgumentTypes = true;
     m_speculationDirection = BackwardSpeculation;
-    m_codeOriginForOSR = CodeOrigin(0);
+    m_codeOriginForExitTarget = CodeOrigin(0);
+    m_codeOriginForExitProfile = CodeOrigin(0);
 
     for (size_t i = 0; i < m_arguments.size(); ++i)
         m_arguments[i] = ValueSource(ValueInJSStack);

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (153290 => 153291)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h	2013-07-25 04:05:20 UTC (rev 153290)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h	2013-07-25 04:05:22 UTC (rev 153291)
@@ -2193,7 +2193,8 @@
     Vector<ValueSource, 0> m_arguments;
     Vector<ValueSource, 0> m_variables;
     int m_lastSetOperand;
-    CodeOrigin m_codeOriginForOSR;
+    CodeOrigin m_codeOriginForExitTarget;
+    CodeOrigin m_codeOriginForExitProfile;
     
     InPlaceAbstractState m_state;
     AbstractInterpreter<InPlaceAbstractState> m_interpreter;

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp (153290 => 153291)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp	2013-07-25 04:05:20 UTC (rev 153290)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp	2013-07-25 04:05:22 UTC (rev 153291)
@@ -216,7 +216,8 @@
         }
         
         m_node = m_highBlock->at(nodeIndex);
-        m_codeOrigin = m_node->codeOrigin;
+        m_codeOriginForExitProfile = m_node->codeOrigin;
+        m_codeOriginForExitTarget = m_node->codeOriginForExitTarget;
         
         if (verboseCompilationEnabled())
             dataLog("Lowering ", m_node, "\n");
@@ -2481,8 +2482,8 @@
         
         m_ftlState.jitCode->osrExit.append(OSRExit(
             kind, lowValue.format(), m_graph.methodOfGettingAValueProfileFor(highValue),
-            m_codeOrigin, m_lastSetOperand, m_valueSources.numberOfArguments(),
-            m_valueSources.numberOfLocals()));
+            m_codeOriginForExitTarget, m_codeOriginForExitProfile, m_lastSetOperand,
+            m_valueSources.numberOfArguments(), m_valueSources.numberOfLocals()));
         m_ftlState.osrExit.append(OSRExitCompilationInfo());
         
         OSRExit& exit = m_ftlState.jitCode->osrExit.last();
@@ -2889,7 +2890,8 @@
     BasicBlock* m_nextHighBlock;
     LBasicBlock m_nextLowBlock;
     
-    CodeOrigin m_codeOrigin;
+    CodeOrigin m_codeOriginForExitTarget;
+    CodeOrigin m_codeOriginForExitProfile;
     unsigned m_nodeIndex;
     Node* m_node;
     SpeculationDirection m_direction;

Modified: trunk/Source/_javascript_Core/ftl/FTLOSRExit.cpp (153290 => 153291)


--- trunk/Source/_javascript_Core/ftl/FTLOSRExit.cpp	2013-07-25 04:05:20 UTC (rev 153290)
+++ trunk/Source/_javascript_Core/ftl/FTLOSRExit.cpp	2013-07-25 04:05:22 UTC (rev 153291)
@@ -43,8 +43,9 @@
 OSRExit::OSRExit(
     ExitKind exitKind, ValueFormat profileValueFormat,
     MethodOfGettingAValueProfile valueProfile, CodeOrigin codeOrigin,
-    int lastSetOperand, unsigned numberOfArguments, unsigned numberOfLocals)
-    : OSRExitBase(exitKind, codeOrigin)
+    CodeOrigin originForProfile, int lastSetOperand, unsigned numberOfArguments,
+    unsigned numberOfLocals)
+    : OSRExitBase(exitKind, codeOrigin, originForProfile)
     , m_profileValueFormat(profileValueFormat)
     , m_valueProfile(valueProfile)
     , m_patchableCodeOffset(0)

Modified: trunk/Source/_javascript_Core/ftl/FTLOSRExit.h (153290 => 153291)


--- trunk/Source/_javascript_Core/ftl/FTLOSRExit.h	2013-07-25 04:05:20 UTC (rev 153290)
+++ trunk/Source/_javascript_Core/ftl/FTLOSRExit.h	2013-07-25 04:05:22 UTC (rev 153291)
@@ -144,8 +144,8 @@
 struct OSRExit : public DFG::OSRExitBase {
     OSRExit(
         ExitKind, ValueFormat profileValueFormat, MethodOfGettingAValueProfile,
-        CodeOrigin, int lastSetOperand, unsigned numberOfArguments,
-        unsigned numberOfLocals);
+        CodeOrigin, CodeOrigin originForProfile, int lastSetOperand,
+        unsigned numberOfArguments, unsigned numberOfLocals);
     
     MacroAssemblerCodeRef m_code;
     

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

Reply via email to