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

Review URL: https://codereview.chromium.org/1921483002

Cr-Commit-Position: refs/heads/master@{#35747}

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,8 +255,27 @@ 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);
+  }
+}
+
+namespace {
+
+void TryScheduleCallIfSuccess(Node* node, Node** control) {
+  // Schedule the call's IfSuccess node if there is no exception use.
+  if (!NodeProperties::IsExceptionalCall(node)) {
+    for (Edge edge : node->use_edges()) {
+      if (NodeProperties::IsControlEdge(edge) &&
+          edge.from()->opcode() == IrOpcode::kIfSuccess) {
+        *control = edge.from();
+      }
+    }
+  }
 }
 
+}  // namespace
+
 void EffectControlLinearizer::ProcessNode(Node* node, Node** effect,
                                           Node** control) {
   // If the node needs to be wired into the effect/control chain, do this
@@ -244,6 +298,16 @@ void EffectControlLinearizer::ProcessNode(Node* node, Node** effect,
     }
   }
 
+  if (node->opcode() == IrOpcode::kIfSuccess) {
+    // We always schedule IfSuccess with its call, so skip it here.
+    DCHECK_EQ(IrOpcode::kCall, node->InputAt(0)->opcode());
+    // The IfSuccess node should not belong to an exceptional call node
+    // because such IfSuccess nodes should only start a basic block (and
+    // basic block start nodes are not handled in the ProcessNode method).
+    DCHECK(!NodeProperties::IsExceptionalCall(node->InputAt(0)));
+    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 +328,18 @@ 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;
+
+    if (node->opcode() == IrOpcode::kCall) {
+      // Schedule the call's IfSuccess node (if there is no exception use).
+      TryScheduleCallIfSuccess(node, control);
+    }
+  }
 }
 
 bool EffectControlLinearizer::TryWireInStateEffect(Node* node, Node** effect,

src/compiler/node-properties.cc

@@ -158,8 +158,9 @@ void NodeProperties::ReplaceContextInput(Node* node, Node* context) {
 
 
 // static
-void NodeProperties::ReplaceControlInput(Node* node, Node* control) {
-  node->ReplaceInput(FirstControlIndex(node), control);
+void NodeProperties::ReplaceControlInput(Node* node, Node* control, int index) {
+  DCHECK(index < node->op()->ControlInputCount());
+  node->ReplaceInput(FirstControlIndex(node) + index, control);
 }
 
 

src/compiler/node-properties.h

@@ -81,7 +81,7 @@ class NodeProperties final {
 
   static void ReplaceValueInput(Node* node, Node* value, int index);
   static void ReplaceContextInput(Node* node, Node* context);
-  static void ReplaceControlInput(Node* node, Node* control);
+  static void ReplaceControlInput(Node* node, Node* control, int index = 0);
   static void ReplaceEffectInput(Node* node, Node* effect, int index = 0);
   static void ReplaceFrameStateInput(Node* node, int index, Node* frame_state);
   static void RemoveFrameStateInput(Node* node, int index);

src/compiler/pipeline.cc

@@ -836,6 +836,7 @@ struct EffectControlLinearizationPhase {
     Schedule* schedule = Scheduler::ComputeSchedule(temp_zone, data->graph(),
                                                     Scheduler::kNoFlags);
     if (FLAG_turbo_verify) ScheduleVerifier::Run(schedule);
+    TraceSchedule(data->info(), schedule);
 
     // Post-pass for wiring the control/effects
     // - connect allocating representation changes into the control&effect

test/unittests/compiler/effect-control-linearizer-unittest.cc

@@ -5,6 +5,7 @@
 #include "src/compiler/effect-control-linearizer.h"
 #include "src/compiler/access-builder.h"
 #include "src/compiler/js-graph.h"
+#include "src/compiler/linkage.h"
 #include "src/compiler/node-properties.h"
 #include "src/compiler/schedule.h"
 #include "src/compiler/simplified-operator.h"
@@ -140,6 +141,127 @@ TEST_F(EffectControlLinearizerTest, DiamondLoad) {
       ret, IsReturn(phi, IsEffectPhi(vtrue, graph()->start(), merge), merge));
 }
 
+TEST_F(EffectControlLinearizerTest, FloatingDiamondsControlWiring) {
+  Schedule schedule(zone());
+
+  // Create the graph and schedule. Roughly (omitting effects and unimportant
+  // nodes):
+  //
+  //            BLOCK 0:
+  //             r1: Start
+  //             c1: Call
+  //             b1: Branch(const0, s1)
+  //                |
+  //        +-------+------+
+  //        |              |
+  //   BLOCK 1:           BLOCK 2:
+  //    t1: IfTrue(b1)     f1: IfFalse(b1)
+  //        |              |
+  //        +-------+------+
+  //                |
+  //            BLOCK 3:
+  //             m1: Merge(t1, f1)
+  //             c2: IfSuccess(c1)
+  //             b2: Branch(const0 , s1)
+  //                |
+  //        +-------+------+
+  //        |              |
+  //   BLOCK 4:           BLOCK 5:
+  //    t2: IfTrue(b2)     f2:IfFalse(b2)
+  //        |              |
+  //        +-------+------+
+  //                |
+  //            BLOCK 6:
+  //             m2: Merge(t2, f2)
+  //             r1: Return(c1, c2)
+  MachineType kMachineSignature[] = {MachineType::AnyTagged(),
+                                     MachineType::AnyTagged()};
+  LinkageLocation kLocationSignature[] = {LinkageLocation::ForRegister(0),
+                                          LinkageLocation::ForRegister(1)};
+  const CallDescriptor* kCallDescriptor = new (zone()) CallDescriptor(
+      CallDescriptor::kCallCodeObject, MachineType::AnyTagged(),
+      LinkageLocation::ForRegister(0),
+      new (zone()) MachineSignature(1, 1, kMachineSignature),
+      new (zone()) LocationSignature(1, 1, kLocationSignature), 0,
+      Operator::kNoProperties, 0, 0, CallDescriptor::kNoFlags);
+  Node* p0 = Parameter(0);
+  Node* p1 = Parameter(1);
+  Node* const0 = Int32Constant(0);
+  Node* call = graph()->NewNode(common()->Call(kCallDescriptor), p0, p1,
+                                graph()->start(), graph()->start());
+  Node* if_success = graph()->NewNode(common()->IfSuccess(), call);
+
+  // First Floating diamond.
+  Node* branch1 =
+      graph()->NewNode(common()->Branch(), const0, graph()->start());
+  Node* if_true1 = graph()->NewNode(common()->IfTrue(), branch1);
+  Node* if_false1 = graph()->NewNode(common()->IfFalse(), branch1);
+  Node* merge1 = graph()->NewNode(common()->Merge(2), if_true1, if_false1);
+
+  // Second floating diamond.
+  Node* branch2 =
+      graph()->NewNode(common()->Branch(), const0, graph()->start());
+  Node* if_true2 = graph()->NewNode(common()->IfTrue(), branch2);
+  Node* if_false2 = graph()->NewNode(common()->IfFalse(), branch2);
+  Node* merge2 = graph()->NewNode(common()->Merge(2), if_true2, if_false2);
+
+  Node* ret =
+      graph()->NewNode(common()->Return(), call, graph()->start(), if_success);
+
+  // Build the basic block structure.
+  BasicBlock* start = schedule.start();
+  schedule.rpo_order()->push_back(start);
+  start->set_rpo_number(0);
+
+  BasicBlock* t1block = AddBlockToSchedule(&schedule);
+  BasicBlock* f1block = AddBlockToSchedule(&schedule);
+  BasicBlock* m1block = AddBlockToSchedule(&schedule);
+
+  BasicBlock* t2block = AddBlockToSchedule(&schedule);
+  BasicBlock* f2block = AddBlockToSchedule(&schedule);
+  BasicBlock* m2block = AddBlockToSchedule(&schedule);
+
+  // Populate the basic blocks with nodes.
+  schedule.AddNode(start, graph()->start());
+  schedule.AddNode(start, p0);
+  schedule.AddNode(start, p1);
+  schedule.AddNode(start, const0);
+  schedule.AddNode(start, call);
+  schedule.AddBranch(start, branch1, t1block, f1block);
+
+  schedule.AddNode(t1block, if_true1);
+  schedule.AddGoto(t1block, m1block);
+
+  schedule.AddNode(f1block, if_false1);
+  schedule.AddGoto(f1block, m1block);
+
+  schedule.AddNode(m1block, merge1);
+  // The scheduler does not always put the IfSuccess node to the corresponding
+  // call's block, simulate that here.
+  schedule.AddNode(m1block, if_success);
+  schedule.AddBranch(m1block, branch2, t2block, f2block);
+
+  schedule.AddNode(t2block, if_true2);
+  schedule.AddGoto(t2block, m2block);
+
+  schedule.AddNode(f2block, if_false2);
+  schedule.AddGoto(f2block, m2block);
+
+  schedule.AddNode(m2block, merge2);
+  schedule.AddReturn(m2block, ret);
+
+  // Run the state effect introducer.
+  EffectControlLinearizer introducer(jsgraph(), &schedule, zone());
+  introducer.Run();
+
+  // The effect input to the return should be an effect phi with the
+  // newly introduced effectful change operators.
+  ASSERT_THAT(ret, IsReturn(call, call, merge2));
+  ASSERT_THAT(branch2, IsBranch(const0, merge1));
+  ASSERT_THAT(branch1, IsBranch(const0, if_success));
+  ASSERT_THAT(if_success, IsIfSuccess(call));
+}
+
 TEST_F(EffectControlLinearizerTest, LoopLoad) {
   Schedule schedule(zone());