Title: [230928] trunk/Source/_javascript_Core
Revision
230928
Author
fpi...@apple.com
Date
2018-04-23 15:25:29 -0700 (Mon, 23 Apr 2018)

Log Message

Roll out r226655 because it broke OSR entry when the pre-header is inadequately profiled.

Rubber stamped by Saam Barati.
        
This is a >2x speed-up in SunSpider/bitops-bitwise-and. We don't really care about SunSpider
anymore, but r226655 didn't result in any benchmark wins and just regressed this test by a lot.
Seems sensible to just roll it out.

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::addToGraph):
(JSC::DFG::ByteCodeParser::parse):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (230927 => 230928)


--- trunk/Source/_javascript_Core/ChangeLog	2018-04-23 21:59:46 UTC (rev 230927)
+++ trunk/Source/_javascript_Core/ChangeLog	2018-04-23 22:25:29 UTC (rev 230928)
@@ -1,3 +1,17 @@
+2018-04-23  Filip Pizlo  <fpi...@apple.com>
+
+        Roll out r226655 because it broke OSR entry when the pre-header is inadequately profiled.
+
+        Rubber stamped by Saam Barati.
+        
+        This is a >2x speed-up in SunSpider/bitops-bitwise-and. We don't really care about SunSpider
+        anymore, but r226655 didn't result in any benchmark wins and just regressed this test by a lot.
+        Seems sensible to just roll it out.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::addToGraph):
+        (JSC::DFG::ByteCodeParser::parse):
+
 2018-04-22  Yusuke Suzuki  <utatane....@gmail.com>
 
         [JSC] Remove ModuleLoaderPrototype

Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (230927 => 230928)


--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2018-04-23 21:59:46 UTC (rev 230927)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2018-04-23 22:25:29 UTC (rev 230928)
@@ -71,7 +71,7 @@
 
 namespace DFGByteCodeParserInternal {
 #ifdef NDEBUG
-static const bool verbose = false;
+static const bool verbose = true;
 #else
 static const bool verbose = true;
 #endif
@@ -692,10 +692,7 @@
     
     Node* addToGraph(Node* node)
     {
-        m_hasAnyForceOSRExits |= (node->op() == ForceOSRExit);
-
         VERBOSE_LOG("        appended ", node, " ", Graph::opName(node->op()), "\n");
-
         m_currentBlock->append(node);
         if (clobbersExitState(m_graph, node))
             m_exitOK = false;
@@ -1152,7 +1149,6 @@
     
     Instruction* m_currentInstruction;
     bool m_hasDebuggerEnabled;
-    bool m_hasAnyForceOSRExits { false };
 };
 
 BasicBlock* ByteCodeParser::allocateTargetableBlock(unsigned bytecodeIndex)
@@ -6694,78 +6690,6 @@
     parseCodeBlock();
     linkBlocks(inlineStackEntry.m_unlinkedBlocks, inlineStackEntry.m_blockLinkingTargets);
 
-    if (m_hasAnyForceOSRExits) {
-        InsertionSet insertionSet(m_graph);
-        Operands<VariableAccessData*> mapping(OperandsLike, m_graph.block(0)->variablesAtHead);
-
-        for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
-            mapping.fill(nullptr);
-            if (validationEnabled()) {
-                // Verify that it's correct to fill mapping with nullptr.
-                for (unsigned i = 0; i < block->variablesAtHead.size(); ++i) {
-                    Node* node = block->variablesAtHead.at(i);
-                    RELEASE_ASSERT(!node);
-                }
-            }
-
-            for (unsigned nodeIndex = 0; nodeIndex < block->size(); ++nodeIndex) {
-                Node* node = block->at(nodeIndex);
-
-                if (node->hasVariableAccessData(m_graph))
-                    mapping.operand(node->local()) = node->variableAccessData();
-
-                if (node->op() == ForceOSRExit) {
-                    NodeOrigin endOrigin = node->origin.withExitOK(true);
-
-                    if (validationEnabled()) {
-                        // This verifies that we don't need to change any of the successors's predecessor
-                        // list after planting the Unreachable below. At this point in the bytecode
-                        // parser, we haven't linked up the predecessor lists yet.
-                        for (BasicBlock* successor : block->successors())
-                            RELEASE_ASSERT(successor->predecessors.isEmpty());
-                    }
-
-                    block->resize(nodeIndex + 1);
-
-                    insertionSet.insertNode(block->size(), SpecNone, ExitOK, endOrigin);
-
-                    auto insertLivenessPreservingOp = [&] (InlineCallFrame* inlineCallFrame, NodeType op, VirtualRegister operand) {
-                        VariableAccessData* variable = mapping.operand(operand);
-                        if (!variable) {
-                            variable = newVariableAccessData(operand);
-                            mapping.operand(operand) = variable;
-                        }
-
-                        VirtualRegister argument = operand - (inlineCallFrame ? inlineCallFrame->stackOffset : 0);
-                        if (argument.isArgument() && !argument.isHeader()) {
-                            const Vector<ArgumentPosition*>& arguments = m_inlineCallFrameToArgumentPositions.get(inlineCallFrame);
-                            arguments[argument.toArgument()]->addVariable(variable);
-                        }
-
-                        insertionSet.insertNode(block->size(), SpecNone, op, endOrigin, OpInfo(variable));
-                    };
-                    auto addFlushDirect = [&] (InlineCallFrame* inlineCallFrame, VirtualRegister operand) {
-                        insertLivenessPreservingOp(inlineCallFrame, Flush, operand);
-                    };
-                    auto addPhantomLocalDirect = [&] (InlineCallFrame* inlineCallFrame, VirtualRegister operand) {
-                        insertLivenessPreservingOp(inlineCallFrame, PhantomLocal, operand);
-                    };
-                    flushForTerminalImpl(endOrigin.semantic, addFlushDirect, addPhantomLocalDirect);
-
-                    insertionSet.insertNode(block->size(), SpecNone, Unreachable, endOrigin);
-                    insertionSet.execute(block);
-                    break;
-                }
-            }
-        }
-    } else if (validationEnabled()) {
-        // Ensure our bookkeeping for ForceOSRExit nodes is working.
-        for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
-            for (Node* node : *block)
-                RELEASE_ASSERT(node->op() != ForceOSRExit);
-        }
-    }
-
     m_graph.determineReachability();
     m_graph.killUnreachableBlocks();
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to