Attachment 342315 Details for Bug 181409 – WIP

[patch] WIP

a-backup.diff (text/plain), 6.76 KB, created by Saam Barati on 2018-06-08 13:31:52 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2018-06-08 13:31:52 PDT

Size: 6.76 KB

patch

obsolete

>Index: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp	(revision 232627)
>+++ Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp	(working copy)
>@@ -697,6 +697,9 @@ private:
>     Node* addToGraph(Node* node)
>     {
>         VERBOSE_LOG("        appended ", node, " ", Graph::opName(node->op()), "\n");
>+
>+        m_hasAnyForceOSRExits |= (node->op() == ForceOSRExit);
>+
>         m_currentBlock->append(node);
>         if (clobbersExitState(m_graph, node))
>             m_exitOK = false;
>@@ -1150,6 +1153,7 @@ private:
>     
>     Instruction* m_currentInstruction;
>     bool m_hasDebuggerEnabled;
>+    bool m_hasAnyForceOSRExits { false };
> };
> 
> BasicBlock* ByteCodeParser::allocateTargetableBlock(unsigned bytecodeIndex)
>@@ -1669,7 +1673,7 @@ void ByteCodeParser::inlineCall(Node* ca
>     m_exitOK = false;
> 
>     linkBlocks(inlineStackEntry.m_unlinkedBlocks, inlineStackEntry.m_blockLinkingTargets);
>-    
>+
>     // Most functions have at least one op_ret and thus set up the continuation block.
>     // In some rare cases, a function ends in op_unreachable, forcing us to allocate a new continuationBlock here.
>     if (inlineStackEntry.m_continuationBlock)
>@@ -6832,6 +6836,103 @@ void ByteCodeParser::parse()
>     parseCodeBlock();
>     linkBlocks(inlineStackEntry.m_unlinkedBlocks, inlineStackEntry.m_blockLinkingTargets);
> 
>+    if (m_hasAnyForceOSRExits) {
>+        InsertionSet insertionSet(m_graph);
>+        Operands<VariableAccessData*> mapping(OperandsLike, m_graph.block(0)->variablesAtHead);
>+
>+        BlockSet blocksToIgnore;
>+        for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
>+            if (block->isOSRTarget)
>+                blocksToIgnore.add(block);
>+        }
>+
>+        {
>+            bool changed;
>+            auto postOrder = m_graph.blocksInPostOrder(); // This algorithm doesn't rely on the predecessors list, which is not yet built.
>+            do {
>+                changed = false;
>+                for (BasicBlock* block : postOrder) {
>+                    for (BasicBlock* successor : block->successors()) {
>+                        if (blocksToIgnore.contains(successor))
>+                            changed |= blocksToIgnore.add(block);
>+                    }
>+
>+                }
>+            } while (changed);
>+        }
>+
>+        for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
>+            if (blocksToIgnore.contains(block)) {
>+                continue;
>+            }
>+
>+            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();
> 
>Index: Source/JavaScriptCore/dfg/DFGGraph.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGGraph.cpp	(revision 232627)
>+++ Source/JavaScriptCore/dfg/DFGGraph.cpp	(working copy)
>@@ -930,6 +930,7 @@ BlockList Graph::blocksInPostOrder()
>         }
>     }
> 
>+    /* OOPS: This relies on dominators which we can't compute w/out predecessors. Need to find a way to exclude
>     if (validationEnabled()) {
>         auto validateResults = [&] (auto& dominators) {
>             // When iterating over reverse post order, we should see dominators
>@@ -953,6 +954,7 @@ BlockList Graph::blocksInPostOrder()
>         else
>             validateResults(ensureCPSDominators());
>     }
>+    */
> 
>     return result;
> }

Flags:

ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 181409: 330759 | 330760 | 330772 | 330785 | 342315 | 342352 | 342499