[turbofan] Wire in floating control during effect linearization phase.

src/compiler/effect-control-linearizer.cc

Index: src/compiler/effect-control-linearizer.cc
diff --git a/src/compiler/effect-control-linearizer.cc b/src/compiler/effect-control-linearizer.cc
index ec21247bfc8c30d5b8a6efb5c794c5cd41e2c51f..4165efd586a9d5f5e94863940d2e02c1eb10a5a5 100644
--- a/src/compiler/effect-control-linearizer.cc
+++ b/src/compiler/effect-control-linearizer.cc
@@ -34,8 +34,9 @@ MachineOperatorBuilder* EffectControlLinearizer::machine() const {
 
 namespace {
 
-struct BlockEffectData {
+struct BlockEffectControlData {
   Node* current_effect = nullptr;  // New effect.
+  Node* current_control = nullptr;  // New control.
 };
 
 // Effect phis that need to be updated after the first pass.
@@ -48,7 +49,7 @@ struct PendingEffectPhi {
 };
 
 void UpdateEffectPhi(Node* node, BasicBlock* block,
-                     ZoneVector<BlockEffectData>* block_effects) {
+                     ZoneVector<BlockEffectControlData>* block_effects) {
   // Update all inputs to an effect phi with the effects from the given
   // block->effect map.
   DCHECK_EQ(IrOpcode::kEffectPhi, node->opcode());
@@ -64,6 +65,27 @@ void UpdateEffectPhi(Node* node, BasicBlock* block,
   }
 }
 
+void UpdateBlockControl(BasicBlock* block,
+                        ZoneVector<BlockEffectControlData>* block_effects) {
+  Node* control = block->NodeAt(0);
+  DCHECK(NodeProperties::IsControl(control));
+
+  // Do not rewire the end node.
+  if (control->opcode() == IrOpcode::kEnd) return;
+
+  // Update all inputs to the given control node with the correct control.
+  DCHECK_EQ(control->op()->ControlInputCount(), block->PredecessorCount());
+  for (int i = 0; i < control->op()->ControlInputCount(); i++) {
+    Node* input = NodeProperties::GetControlInput(control, i);
+    BasicBlock* predecessor = block->PredecessorAt(static_cast<size_t>(i));
+    Node* input_control =
+        (*block_effects)[predecessor->rpo_number()].current_control;
+    if (input != input_control) {
+      NodeProperties::ReplaceControlInput(control, input_control, i);
+    }
+  }
+}
+
 bool HasIncomingBackEdges(BasicBlock* block) {
   for (BasicBlock* pred : block->predecessors()) {
     if (pred->rpo_number() >= block->rpo_number()) {
@@ -94,8 +116,9 @@ void RemoveRegionNode(Node* node) {
 }  // namespace
 
 void EffectControlLinearizer::Run() {
-  ZoneVector<BlockEffectData> block_effects(temp_zone());
+  ZoneVector<BlockEffectControlData> block_effects(temp_zone());
   ZoneVector<PendingEffectPhi> pending_effect_phis(temp_zone());
+  ZoneVector<BasicBlock*> pending_block_controls(temp_zone());
   block_effects.resize(schedule()->RpoBlockCount());
   NodeVector inputs_buffer(temp_zone());
 
@@ -105,6 +128,16 @@ void EffectControlLinearizer::Run() {
     // The control node should be the first.
     Node* control = block->NodeAt(instr);
     DCHECK(NodeProperties::IsControl(control));
+    // Update the control inputs.
+    if (HasIncomingBackEdges(block)) {
+      // If there are back edges, we need to update later because we have not
+      // computed the control yet. This should only happen for loops.
+      DCHECK_EQ(IrOpcode::kLoop, control->opcode());
+      pending_block_controls.push_back(block);
+    } else {
+      // If there are no back edges, we can update now.
+      UpdateBlockControl(block, &block_effects);
+    }
     instr++;
 
     // Iterate over the phis and update the effect phis.
@@ -177,8 +210,9 @@ void EffectControlLinearizer::Run() {
           pending_effect_phis.push_back(PendingEffectPhi(effect, block));
         } else if (control->opcode() == IrOpcode::kIfException) {
           // The IfException is connected into the effect chain, so we need
-          // to process it here.
-          ProcessNode(control, &effect, &control);
+          // to update the effect here.
+          NodeProperties::ReplaceEffectInput(control, effect);
+          effect = control;
         }
       }
     }
@@ -212,6 +246,7 @@ void EffectControlLinearizer::Run() {
 
     // Store the effect for later use.
     block_effects[block->rpo_number()].current_effect = effect;
+    block_effects[block->rpo_number()].current_control = control;
   }
 
   // Update the incoming edges of the effect phis that could not be processed
@@ -220,6 +255,9 @@ void EffectControlLinearizer::Run() {
     UpdateEffectPhi(pending_effect_phi.effect_phi, pending_effect_phi.block,
                     &block_effects);
   }
+  for (BasicBlock* pending_block_control : pending_block_controls) {
+    UpdateBlockControl(pending_block_control, &block_effects);
+  }
 }
 
 void EffectControlLinearizer::ProcessNode(Node* node, Node** effect,
@@ -244,6 +282,25 @@ void EffectControlLinearizer::ProcessNode(Node* node, Node** effect,
     }
   }
 
+  if (node->opcode() == IrOpcode::kIfSuccess) {
+    Node* success_input = NodeProperties::GetControlInput(node, 0);
+    if (*control == success_input) {
+      // The IfSuccess node refers to the last control node in the schedule,
+      // so we just update the current control.
+      *control = node;
+    } else {
+      // Some other intervening control has been scheduled after call,
+      // we need to insert the IfSuccess node just after the call.
+      for (Edge edge : success_input->use_edges()) {

[Benedikt Meurer, 2016/04/24 18:12:49]

Not sure I really get this one: This tries to update all "other" control uses of
the call and thread the IfSuccess in instead? Can we somehow document this this
with some ascii art? And maybe we could even put this whole thing into a
dedicated ProcessIfSuccess() method to highlight the fact that this is tricky
stuff.

[Jarin, 2016/04/24 19:21:23]

I have changed this to keep the wiring of IfSuccess while we connect the call
node. That seems more understandable (and less brittle) than wedging-in the
IfSuccess node.

+        if (NodeProperties::IsControlEdge(edge) && edge.from() != node &&
+            edge.from()->op()->ControlOutputCount() > 0) {
+          edge.UpdateTo(node);
+        }
+      }
+    }
+    return;
+  }
+
   // If the node takes an effect, replace with the current one.
   if (node->op()->EffectInputCount() > 0) {
     DCHECK_EQ(1, node->op()->EffectInputCount());
@@ -264,6 +321,13 @@ void EffectControlLinearizer::ProcessNode(Node* node, Node** effect,
     DCHECK(node->op()->EffectOutputCount() == 0 ||
            node->opcode() == IrOpcode::kStart);
   }
+  // Rewire control inputs of control nodes, and update the current control
+  // input.
+  if (node->op()->ControlOutputCount() > 0) {
+    DCHECK_EQ(1, node->op()->ControlInputCount());
+    NodeProperties::ReplaceControlInput(node, *control);
+    *control = node;
+  }
 }
 
 bool EffectControlLinearizer::TryWireInStateEffect(Node* node, Node** effect,