Use constexpr instead of const in symbol definitions that are obviously constexpr.
[WebKit-https.git] / Source / JavaScriptCore / dfg / DFGPutStackSinkingPhase.cpp
index 8889c0e..8727760 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014, 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -42,7 +42,9 @@ namespace JSC { namespace DFG {
 
 namespace {
 
-bool verbose = false;
+namespace DFGPutStackSinkingPhaseInternal {
+static constexpr bool verbose = false;
+}
 
 class PutStackSinkingPhase : public Phase {
 public:
@@ -57,7 +59,7 @@ public:
         // for sunken PutStacks in the presence of interesting control flow merges, and where the
         // value being PutStack'd is also otherwise live in the DFG code. We could work around this
         // by doing the sinking over CPS, or maybe just by doing really smart hoisting. It's also
-        // possible that the duplicate Phi graph can be deduplicated by LLVM. It would be best if we
+        // possible that the duplicate Phi graph can be deduplicated by B3. It would be best if we
         // could observe that there is already a Phi graph in place that does what we want. In
         // principle if we have a request to place a Phi at a particular place, we could just check
         // if there is already a Phi that does what we want. Because PutStackSinkingPhase runs just
@@ -71,12 +73,12 @@ public:
         // the stack. It's not clear to me if this is important or not.
         // https://bugs.webkit.org/show_bug.cgi?id=145296
         
-        if (verbose) {
+        if (DFGPutStackSinkingPhaseInternal::verbose) {
             dataLog("Graph before PutStack sinking:\n");
             m_graph.dump();
         }
 
-        m_graph.m_dominators.computeIfNecessary(m_graph);
+        m_graph.ensureSSADominators();
         
         SSACalculator ssaCalculator(m_graph);
         InsertionSet insertionSet(m_graph);
@@ -105,34 +107,34 @@ public:
                 Operands<bool> live = liveAtTail[block];
                 for (unsigned nodeIndex = block->size(); nodeIndex--;) {
                     Node* node = block->at(nodeIndex);
-                    if (verbose)
+                    if (DFGPutStackSinkingPhaseInternal::verbose)
                         dataLog("Live at ", node, ": ", live, "\n");
                     
+                    Vector<VirtualRegister, 4> reads;
+                    Vector<VirtualRegister, 4> writes;
                     auto escapeHandler = [&] (VirtualRegister operand) {
                         if (operand.isHeader())
                             return;
-                        if (verbose)
+                        if (DFGPutStackSinkingPhaseInternal::verbose)
                             dataLog("    ", operand, " is live at ", node, "\n");
-                        live.operand(operand) = true;
+                        reads.append(operand);
                     };
-                    
-                    // FIXME: This might mishandle LoadVarargs and ForwardVarargs. It might make us
-                    // think that the locals being written are stack-live here. They aren't. This
-                    // should be harmless since we overwrite them anyway, but still, it's sloppy.
-                    // https://bugs.webkit.org/show_bug.cgi?id=145295
+
+                    auto writeHandler = [&] (VirtualRegister operand) {
+                        if (operand.isHeader())
+                            return;
+                        RELEASE_ASSERT(node->op() == PutStack || node->op() == LoadVarargs || node->op() == ForwardVarargs || node->op() == KillStack);
+                        writes.append(operand);
+                    };
+
                     preciseLocalClobberize(
-                        m_graph, node, escapeHandler, escapeHandler,
-                        [&] (VirtualRegister operand, LazyNode source) {
-                            RELEASE_ASSERT(source.isNode());
+                        m_graph, node, escapeHandler, writeHandler,
+                        [&] (VirtualRegister, LazyNode) { });
 
-                            if (source.asNode() == node) {
-                                // This is a load. Ignore it.
-                                return;
-                            }
-                            
-                            RELEASE_ASSERT(node->op() == PutStack);
-                            live.operand(operand) = false;
-                        });
+                    for (VirtualRegister operand : writes)
+                        live.operand(operand) = false;
+                    for (VirtualRegister operand : reads)
+                        live.operand(operand) = true;
                 }
                 
                 if (live == liveAtHead[block])
@@ -205,10 +207,13 @@ public:
         //     Represents the fact that the original code would have done a PutStack but we haven't
         //     identified an operation that would have observed that PutStack.
         //
-        // This code has some interesting quirks because of the fact that neither liveness nor
-        // deferrals are very precise. They are only precise enough to be able to correctly tell us
-        // when we may [sic] need to execute PutStacks. This means that they may report the need to
-        // execute a PutStack in cases where we actually don't really need it, and that's totally OK.
+        // We need to be precise about liveness in this phase because not doing so
+        // could cause us to insert a PutStack before a node we thought may escape a 
+        // value that it doesn't really escape. Sinking this PutStack above such a node may
+        // cause us to insert a GetStack that we forward to the Phi we're feeding into the
+        // sunken PutStack. Inserting such a GetStack could cause us to load garbage and
+        // can confuse the AI to claim untrue things (like that the program will exit when
+        // it really won't).
         BlockMap<Operands<FlushFormat>> deferredAtHead(m_graph);
         BlockMap<Operands<FlushFormat>> deferredAtTail(m_graph);
         
@@ -229,7 +234,7 @@ public:
                 Operands<FlushFormat> deferred = deferredAtHead[block];
                 
                 for (Node* node : *block) {
-                    if (verbose)
+                    if (DFGPutStackSinkingPhaseInternal::verbose)
                         dataLog("Deferred at ", node, ":", deferred, "\n");
                     
                     if (node->op() == GetStack) {
@@ -269,29 +274,35 @@ public:
                         // A GetStack doesn't affect anything, since we know which local we are reading
                         // from.
                         continue;
+                    } else if (node->op() == PutStack) {
+                        VirtualRegister operand = node->stackAccessData()->local;
+                        deferred.operand(operand) = node->stackAccessData()->format;
+                        continue;
+                    } else if (node->op() == KillStack) {
+                        // We don't want to sink a PutStack past a KillStack.
+                        deferred.operand(node->unlinkedLocal()) = ConflictingFlush;
+                        continue;
                     }
                     
                     auto escapeHandler = [&] (VirtualRegister operand) {
-                        if (verbose)
+                        if (DFGPutStackSinkingPhaseInternal::verbose)
                             dataLog("For ", node, " escaping ", operand, "\n");
                         if (operand.isHeader())
                             return;
                         // We will materialize just before any reads.
                         deferred.operand(operand) = DeadFlush;
                     };
+
+                    auto writeHandler = [&] (VirtualRegister operand) {
+                        if (operand.isHeader())
+                            return;
+                        RELEASE_ASSERT(node->op() == LoadVarargs || node->op() == ForwardVarargs);
+                        deferred.operand(operand) = DeadFlush;
+                    };
                     
                     preciseLocalClobberize(
-                        m_graph, node, escapeHandler, escapeHandler,
-                        [&] (VirtualRegister operand, LazyNode source) {
-                            RELEASE_ASSERT(source.isNode());
-
-                            if (source.asNode() == node) {
-                                // This is a load. Ignore it.
-                                return;
-                            }
-                            
-                            deferred.operand(operand) = node->stackAccessData()->format;
-                        });
+                        m_graph, node, escapeHandler, writeHandler,
+                        [&] (VirtualRegister, LazyNode) { });
                 }
                 
                 if (deferred == deferredAtTail[block])
@@ -302,13 +313,13 @@ public:
                 
                 for (BasicBlock* successor : block->successors()) {
                     for (size_t i = deferred.size(); i--;) {
-                        if (verbose)
+                        if (DFGPutStackSinkingPhaseInternal::verbose)
                             dataLog("Considering ", VirtualRegister(deferred.operandForIndex(i)), " at ", pointerDump(block), "->", pointerDump(successor), ": ", deferred[i], " and ", deferredAtHead[successor][i], " merges to ");
 
                         deferredAtHead[successor][i] =
                             merge(deferredAtHead[successor][i], deferred[i]);
                         
-                        if (verbose)
+                        if (DFGPutStackSinkingPhaseInternal::verbose)
                             dataLog(deferredAtHead[successor][i], "\n");
                     }
                 }
@@ -351,13 +362,13 @@ public:
             indexToOperand.append(operand);
         }
         
-        HashSet<Node*> putLocalsToSink;
+        HashSet<Node*> putStacksToSink;
         
         for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
             for (Node* node : *block) {
                 switch (node->op()) {
                 case PutStack:
-                    putLocalsToSink.add(node);
+                    putStacksToSink.add(node);
                     ssaCalculator.newDef(
                         operandToVariable.operand(node->stackAccessData()->local),
                         block, node->child1().node());
@@ -386,7 +397,7 @@ public:
                 if (!isConcrete(format))
                     return nullptr;
 
-                if (verbose)
+                if (DFGPutStackSinkingPhaseInternal::verbose)
                     dataLog("Adding Phi for ", operand, " at ", pointerDump(block), "\n");
                 
                 Node* phiNode = m_graph.addNode(SpecHeapTop, Phi, block->at(0)->origin.withInvalidExit());
@@ -410,7 +421,7 @@ public:
                 mapping.operand(operand) = def->value();
             }
             
-            if (verbose)
+            if (DFGPutStackSinkingPhaseInternal::verbose)
                 dataLog("Mapping at top of ", pointerDump(block), ": ", mapping, "\n");
             
             for (SSACalculator::Def* phiDef : ssaCalculator.phisForBlock(block)) {
@@ -418,7 +429,7 @@ public:
                 
                 insertionSet.insert(0, phiDef->value());
                 
-                if (verbose)
+                if (DFGPutStackSinkingPhaseInternal::verbose)
                     dataLog("   Mapping ", operand, " to ", phiDef->value(), "\n");
                 mapping.operand(operand) = phiDef->value();
             }
@@ -426,7 +437,7 @@ public:
             deferred = deferredAtHead[block];
             for (unsigned nodeIndex = 0; nodeIndex < block->size(); ++nodeIndex) {
                 Node* node = block->at(nodeIndex);
-                if (verbose)
+                if (DFGPutStackSinkingPhaseInternal::verbose)
                     dataLog("Deferred at ", node, ":", deferred, "\n");
                 
                 switch (node->op()) {
@@ -434,7 +445,7 @@ public:
                     StackAccessData* data = node->stackAccessData();
                     VirtualRegister operand = data->local;
                     deferred.operand(operand) = data->format;
-                    if (verbose)
+                    if (DFGPutStackSinkingPhaseInternal::verbose)
                         dataLog("   Mapping ", operand, " to ", node->child1().node(), " at ", node, "\n");
                     mapping.operand(operand) = node->child1().node();
                     break;
@@ -446,7 +457,7 @@ public:
                     if (!isConcrete(format)) {
                         DFG_ASSERT(
                             m_graph, node,
-                            deferred.operand(data->local) != ConflictingFlush);
+                            deferred.operand(data->local) != ConflictingFlush, deferred.operand(data->local));
                         
                         // This means there is no deferral. No deferral means that the most
                         // authoritative value for this stack slot is what is stored in the stack. So,
@@ -459,17 +470,22 @@ public:
                     // would have stored a value with a certain format. That format must match our
                     // format. But more importantly, we can simply use the value that the PutStack would
                     // have stored and get rid of the GetStack.
-                    DFG_ASSERT(m_graph, node, format == data->format);
+                    DFG_ASSERT(m_graph, node, format == data->format, format, data->format);
                     
                     Node* incoming = mapping.operand(data->local);
                     node->child1() = incoming->defaultEdge();
                     node->convertToIdentity();
                     break;
                 }
+
+                case KillStack: {
+                    deferred.operand(node->unlinkedLocal()) = ConflictingFlush;
+                    break;
+                }
                 
                 default: {
                     auto escapeHandler = [&] (VirtualRegister operand) {
-                        if (verbose)
+                        if (DFGPutStackSinkingPhaseInternal::verbose)
                             dataLog("For ", node, " escaping ", operand, "\n");
 
                         if (operand.isHeader())
@@ -483,7 +499,7 @@ public:
                         }
                     
                         // Gotta insert a PutStack.
-                        if (verbose)
+                        if (DFGPutStackSinkingPhaseInternal::verbose)
                             dataLog("Inserting a PutStack for ", operand, " at ", node, "\n");
 
                         Node* incoming = mapping.operand(operand);
@@ -496,9 +512,21 @@ public:
                     
                         deferred.operand(operand) = DeadFlush;
                     };
-                
+
+                    auto writeHandler = [&] (VirtualRegister operand) {
+                        if (operand.isHeader())
+                            return;
+                        // LoadVarargs and ForwardVarargs are unconditional writes to the stack
+                        // locations they claim to write to. They do not read from the stack 
+                        // locations they write to. This makes those stack locations dead right 
+                        // before a LoadVarargs/ForwardVarargs. This means we should never sink
+                        // PutStacks right to this point.
+                        RELEASE_ASSERT(node->op() == LoadVarargs || node->op() == ForwardVarargs);
+                        deferred.operand(operand) = DeadFlush;
+                    };
+
                     preciseLocalClobberize(
-                        m_graph, node, escapeHandler, escapeHandler,
+                        m_graph, node, escapeHandler, writeHandler,
                         [&] (VirtualRegister, LazyNode) { });
                     break;
                 } }
@@ -512,11 +540,11 @@ public:
                     Node* phiNode = phiDef->value();
                     SSACalculator::Variable* variable = phiDef->variable();
                     VirtualRegister operand = indexToOperand[variable->index()];
-                    if (verbose)
+                    if (DFGPutStackSinkingPhaseInternal::verbose)
                         dataLog("Creating Upsilon for ", operand, " at ", pointerDump(block), "->", pointerDump(successorBlock), "\n");
                     FlushFormat format = deferredAtHead[successorBlock].operand(operand);
-                    DFG_ASSERT(m_graph, nullptr, isConcrete(format));
-                    UseKind useKind = useKindFor(format);
+                    DFG_ASSERT(m_graph, nullptr, isConcrete(format), format);
+                    UseKind useKind = uncheckedUseKindFor(format);
                     
                     // We need to get a value for the stack slot. This phase doesn't really have a
                     // good way of determining if a stack location got clobbered. It just knows if
@@ -549,19 +577,15 @@ public:
         // Finally eliminate the sunken PutStacks by turning them into Checks. This keeps whatever
         // type check they were doing.
         for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
-            for (unsigned nodeIndex = 0; nodeIndex < block->size(); ++nodeIndex) {
-                Node* node = block->at(nodeIndex);
-                
-                if (!putLocalsToSink.contains(node))
+            for (auto* node : *block) {
+                if (!putStacksToSink.contains(node))
                     continue;
                 
-                node->remove();
+                node->remove(m_graph);
             }
-            
-            insertionSet.execute(block);
         }
         
-        if (verbose) {
+        if (DFGPutStackSinkingPhaseInternal::verbose) {
             dataLog("Graph after PutStack sinking:\n");
             m_graph.dump();
         }
@@ -574,7 +598,6 @@ public:
     
 bool performPutStackSinking(Graph& graph)
 {
-    SamplingRegion samplingRegion("DFG PutStack Sinking Phase");
     return runPhase<PutStackSinkingPhase>(graph);
 }