Changeset 239882 in webkit for trunk/Source/JavaScriptCore/dfg/DFGCombinedLiveness.cpp

Timestamp:

Jan 11, 2019, 4:26:06 PM (7 years ago)

Author:

[email protected]

Message:

DFG combined liveness can be wrong for terminal basic blocks
​https://bugs.webkit.org/show_bug.cgi?id=193304
<rdar://problem/45268632>

Reviewed by Yusuke Suzuki.

JSTests:

• stress/dfg-combined-liveness-consider-terminal-blocks-bytecode-liveness.js: Added.

Source/JavaScriptCore:

If a block doesn't have any successors, it can't rely on the typical
backwards liveness propagation that CombinedLiveness was doing. The phase
first got what was live in bytecode and IR at the heads of each block. Then
for each block, it made the live at tail the union of the live at head for
each successor. For a terminal block though, this could be wrong. We could
end up saying nothing is live even though many things may be live in bytecode.
We must account for what's bytecode live at the end of the block. Consider a
block that ends with:
`
ForceOSRExit
Unreachable
`

Things may definitely be live in bytecode at the tail. However, we'll
report nothing as being alive. This probably subtly breaks many analyses,
but we have a test case of it breaking the interference analysis that
the ArgumentsEliminationPhase performs.

• dfg/DFGBasicBlock.h:

(JSC::DFG::BasicBlock::last const):

• dfg/DFGCombinedLiveness.cpp:

(JSC::DFG::addBytecodeLiveness):
(JSC::DFG::liveNodesAtHead):
(JSC::DFG::CombinedLiveness::CombinedLiveness):

• dfg/DFGCombinedLiveness.h:

File:

• trunk/Source/JavaScriptCore/dfg/DFGCombinedLiveness.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/dfg/DFGCombinedLiveness.cpp

@@ -36 +36 @@
 namespace JSC { namespace DFG {
 
-NodeSet liveNodesAtHead(Graph& graph, BasicBlock* block)
+static void addBytecodeLiveness(Graph& graph, AvailabilityMap& availabilityMap, NodeSet& seen, Node* node)
 {
-    NodeSet seen;
-    for (NodeFlowProjection node : block->ssa->liveAtHead) {
-        if (node.kind() == NodeFlowProjection::Primary)
-            seen.addVoid(node.node());
-    }
-    
-    AvailabilityMap& availabilityMap = block->ssa->availabilityAtHead;
     graph.forAllLiveInBytecode(
-        block->at(0)->origin.forExit,
+        node->origin.forExit,
         [&] (VirtualRegister reg) {
             availabilityMap.closeStartingWithLocal(
@@ -57 +50 @@
                 });
         });
-    
+}
+
+NodeSet liveNodesAtHead(Graph& graph, BasicBlock* block)
+{
+    NodeSet seen;
+    for (NodeFlowProjection node : block->ssa->liveAtHead) {
+        if (node.kind() == NodeFlowProjection::Primary)
+            seen.addVoid(node.node());
+    }
+
+    addBytecodeLiveness(graph, block->ssa->availabilityAtHead, seen, block->at(0));
     return seen;
 }
@@ -65 +68 @@
     , liveAtTail(graph)
 {
-    // First compute the liveAtHead for each block.
-    for (BasicBlock* block : graph.blocksInNaturalOrder())
+    // First compute 
+    // - The liveAtHead for each block.
+    // - The liveAtTail for blocks that won't properly propagate
+    //   the information based on their empty successor list.
+    for (BasicBlock* block : graph.blocksInNaturalOrder()) {
         liveAtHead[block] = liveNodesAtHead(graph, block);
+
+        // If we don't have successors, we can't rely on the propagation below. This doesn't usually
+        // do anything for terminal blocks, since the last node is usually a return, so nothing is live
+        // after it. However, we may also have the end of the basic block be:
+        //
+        // ForceOSRExit
+        // Unreachable
+        //
+        // And things may definitely be live in bytecode at that point in the program.
+        if (!block->numSuccessors()) {
+            NodeSet seen;
+            addBytecodeLiveness(graph, block->ssa->availabilityAtTail, seen, block->last());
+            liveAtTail[block] = seen;
+        }
+    }
     
     // Now compute the liveAtTail by unifying the liveAtHead of the successors.