[turbofan]: delay ssa deconstruction in register allocator

BUG=

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

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

diff --git a/src/compiler/instruction-selector.cc b/src/compiler/instruction-selector.cc
index 567b1c9..da8dafd 100644 (file)
--- a/src/compiler/instruction-selector.cc
+++ b/src/compiler/instruction-selector.cc
@@ -925,16 +925,15 @@ void InstructionSelector::VisitParameter(Node* node) {
 
 
 void InstructionSelector::VisitPhi(Node* node) {
-  // TODO(bmeurer): Emit a PhiInstruction here.
+  const int input_count = node->op()->ValueInputCount();
   PhiInstruction* phi = new (instruction_zone())
-      PhiInstruction(instruction_zone(), GetVirtualRegister(node));
+      PhiInstruction(instruction_zone(), GetVirtualRegister(node),
+                     static_cast<size_t>(input_count));
   sequence()->InstructionBlockAt(current_block_->GetRpoNumber())->AddPhi(phi);
-  const int input_count = node->op()->ValueInputCount();
-  phi->operands().reserve(static_cast<size_t>(input_count));
   for (int i = 0; i < input_count; ++i) {
     Node* const input = node->InputAt(i);
     MarkAsUsed(input);
-    phi->operands().push_back(GetVirtualRegister(input));
+    phi->Extend(instruction_zone(), GetVirtualRegister(input));
   }
 }
 

diff --git a/src/compiler/instruction.cc b/src/compiler/instruction.cc
index a713471..3c61732 100644 (file)
--- a/src/compiler/instruction.cc
+++ b/src/compiler/instruction.cc
@@ -644,9 +644,12 @@ std::ostream& operator<<(std::ostream& os,
     os << "\n";
 
     for (auto phi : block->phis()) {
-      os << "     phi: v" << phi->virtual_register() << " =";
-      for (auto op_vreg : phi->operands()) {
-        os << " v" << op_vreg;
+      PrintableInstructionOperand printable_op = {
+          printable.register_configuration_, phi->output()};
+      os << "     phi: " << printable_op << " =";
+      for (auto input : phi->inputs()) {
+        printable_op.op_ = input;
+        os << " " << printable_op;
       }
       os << "\n";
     }

diff --git a/src/compiler/instruction.h b/src/compiler/instruction.h
index d6245d5..1740dfb 100644 (file)
--- a/src/compiler/instruction.h
+++ b/src/compiler/instruction.h
@@ -787,19 +787,45 @@ class FrameStateDescriptor : public ZoneObject {
 std::ostream& operator<<(std::ostream& os, const Constant& constant);
 
 
-// TODO(dcarney): this is a temporary hack.  turn into an actual instruction.
 class PhiInstruction FINAL : public ZoneObject {
  public:
-  PhiInstruction(Zone* zone, int virtual_register)
-      : virtual_register_(virtual_register), operands_(zone) {}
+  typedef ZoneVector<InstructionOperand*> Inputs;
+
+  PhiInstruction(Zone* zone, int virtual_register, size_t reserved_input_count)
+      : virtual_register_(virtual_register),
+        operands_(zone),
+        output_(nullptr),
+        inputs_(zone) {
+    UnallocatedOperand* output =
+        new (zone) UnallocatedOperand(UnallocatedOperand::NONE);
+    output->set_virtual_register(virtual_register);
+    output_ = output;
+    inputs_.reserve(reserved_input_count);
+    operands_.reserve(reserved_input_count);
+  }
 
   int virtual_register() const { return virtual_register_; }
   const IntVector& operands() const { return operands_; }
-  IntVector& operands() { return operands_; }
+
+  void Extend(Zone* zone, int virtual_register) {
+    UnallocatedOperand* input =
+        new (zone) UnallocatedOperand(UnallocatedOperand::ANY);
+    input->set_virtual_register(virtual_register);
+    operands_.push_back(virtual_register);
+    inputs_.push_back(input);
+  }
+
+  InstructionOperand* output() const { return output_; }
+  const Inputs& inputs() const { return inputs_; }
+  Inputs& inputs() { return inputs_; }
 
  private:
+  // TODO(dcarney): some of these fields are only for verification, move them to
+  // verifier.
   const int virtual_register_;
   IntVector operands_;
+  InstructionOperand* output_;
+  Inputs inputs_;
 };
 
 

diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc
index d237c4a..236e7f6 100644 (file)
--- a/src/compiler/pipeline.cc
+++ b/src/compiler/pipeline.cc
@@ -506,15 +506,6 @@ struct MeetRegisterConstraintsPhase {
 };
 
 
-struct ResolvePhisPhase {
-  static const char* phase_name() { return "resolve phis"; }
-
-  void Run(PipelineData* data, Zone* temp_zone) {
-    data->register_allocator()->ResolvePhis();
-  }
-};
-
-
 struct BuildLiveRangesPhase {
   static const char* phase_name() { return "build live ranges"; }
 
@@ -926,7 +917,6 @@ void Pipeline::AllocateRegisters(const RegisterConfiguration* config,
                                     debug_name.get());
 
   Run<MeetRegisterConstraintsPhase>();
-  Run<ResolvePhisPhase>();
   Run<BuildLiveRangesPhase>();
   if (FLAG_trace_turbo) {
     OFStream os(stdout);

diff --git a/src/compiler/register-allocator-verifier.cc b/src/compiler/register-allocator-verifier.cc
index c4482a5..ac29fc8 100644 (file)
--- a/src/compiler/register-allocator-verifier.cc
+++ b/src/compiler/register-allocator-verifier.cc
@@ -244,21 +244,17 @@ class RegisterAllocatorVerifier::OutgoingMapping : public ZoneObject {
     size_t predecessor_index = block->predecessors()[phi_index].ToSize();
     CHECK(sequence->instruction_blocks()[predecessor_index]->SuccessorCount() ==
           1);
-    const auto* gap = sequence->GetBlockStart(block->rpo_number());
-    // The first moves in the BlockStartInstruction are the phi moves inserted
-    // by ResolvePhis.
-    const ParallelMove* move = gap->GetParallelMove(GapInstruction::START);
-    CHECK_NE(nullptr, move);
-    const auto* move_ops = move->move_operands();
-    CHECK(block->phis().size() <= static_cast<size_t>(move_ops->length()));
-    auto move_it = move_ops->begin();
     for (const auto* phi : block->phis()) {
-      const auto* op = move_it->source();
-      auto it = locations()->find(op);
+      auto input = phi->inputs()[phi_index];
+      CHECK(locations()->find(input) != locations()->end());
+      auto it = locations()->find(phi->output());
       CHECK(it != locations()->end());
-      CHECK_EQ(it->second, phi->operands()[phi_index]);
+      if (input->IsConstant()) {
+        CHECK_EQ(it->second, input->index());
+      } else {
+        CHECK_EQ(it->second, phi->operands()[phi_index]);
+      }
       it->second = phi->virtual_register();
-      ++move_it;
     }
   }
 

diff --git a/src/compiler/register-allocator.cc b/src/compiler/register-allocator.cc
index 66e250c..a9cdf51 100644 (file)
--- a/src/compiler/register-allocator.cc
+++ b/src/compiler/register-allocator.cc
@@ -100,8 +100,6 @@ bool LiveRange::HasOverlap(UseInterval* target) const {
 LiveRange::LiveRange(int id, Zone* zone)
     : id_(id),
       spilled_(false),
-      is_phi_(false),
-      is_non_loop_phi_(false),
       kind_(UNALLOCATED_REGISTERS),
       assigned_register_(kInvalidAssignment),
       last_interval_(NULL),
@@ -990,19 +988,12 @@ void RegisterAllocator::ProcessInstructions(const InstructionBlock* block,
         InstructionOperand* hint = to;
         if (to->IsUnallocated()) {
           int to_vreg = UnallocatedOperand::cast(to)->virtual_register();
-          LiveRange* to_range = LiveRangeFor(to_vreg);
-          if (to_range->is_phi()) {
-            if (to_range->is_non_loop_phi()) {
-              hint = to_range->current_hint_operand();
-            }
+          if (live->Contains(to_vreg)) {
+            Define(curr_position, to, from);
+            live->Remove(to_vreg);
           } else {
-            if (live->Contains(to_vreg)) {
-              Define(curr_position, to, from);
-              live->Remove(to_vreg);
-            } else {
-              cur->Eliminate();
-              continue;
-            }
+            cur->Eliminate();
+            continue;
           }
         } else {
           Define(curr_position, to, from);
@@ -1082,58 +1073,24 @@ void RegisterAllocator::ProcessInstructions(const InstructionBlock* block,
 }
 
 
-void RegisterAllocator::ResolvePhis(const InstructionBlock* block) {
+void RegisterAllocator::ProcessPhis(const InstructionBlock* block) {
   for (auto phi : block->phis()) {
-    UnallocatedOperand* phi_operand =
-        new (code_zone()) UnallocatedOperand(UnallocatedOperand::NONE);
+    auto output = phi->output();
     int phi_vreg = phi->virtual_register();
-    phi_operand->set_virtual_register(phi_vreg);
-
-    for (size_t i = 0; i < phi->operands().size(); ++i) {
-      UnallocatedOperand* operand =
-          new (code_zone()) UnallocatedOperand(UnallocatedOperand::ANY);
-      operand->set_virtual_register(phi->operands()[i]);
-      InstructionBlock* cur_block =
-          code()->InstructionBlockAt(block->predecessors()[i]);
-      // The gap move must be added without any special processing as in
-      // the AddConstraintsGapMove.
-      code()->AddGapMove(cur_block->last_instruction_index() - 1, operand,
-                         phi_operand);
-
-      Instruction* branch = InstructionAt(cur_block->last_instruction_index());
-      DCHECK(!branch->HasPointerMap());
-      USE(branch);
-    }
-
     LiveRange* live_range = LiveRangeFor(phi_vreg);
     BlockStartInstruction* block_start =
         code()->GetBlockStart(block->rpo_number());
-    block_start->GetOrCreateParallelMove(GapInstruction::START, code_zone())
-        ->AddMove(phi_operand, live_range->GetSpillOperand(), code_zone());
+    block_start->GetOrCreateParallelMove(GapInstruction::BEFORE, code_zone())
+        ->AddMove(output, live_range->GetSpillOperand(), code_zone());
     live_range->SetSpillStartIndex(block->first_instruction_index());
-
-    // We use the phi-ness of some nodes in some later heuristics.
-    live_range->set_is_phi(true);
-    if (!block->IsLoopHeader()) {
-      live_range->set_is_non_loop_phi(true);
-    }
   }
 }
 
 
 void RegisterAllocator::MeetRegisterConstraints() {
   for (auto block : code()->instruction_blocks()) {
+    ProcessPhis(block);
     MeetRegisterConstraints(block);
-    if (!AllocationOk()) return;
-  }
-}
-
-
-void RegisterAllocator::ResolvePhis() {
-  // Process the blocks in reverse order.
-  for (auto i = code()->instruction_blocks().rbegin();
-       i != code()->instruction_blocks().rend(); ++i) {
-    ResolvePhis(*i);
   }
 }
 
@@ -1266,6 +1223,18 @@ class LiveRangeBoundArray {
     }
   }
 
+  LiveRangeBound* FindPred(const InstructionBlock* pred) {
+    const LifetimePosition pred_end =
+        LifetimePosition::FromInstructionIndex(pred->last_instruction_index());
+    return Find(pred_end);
+  }
+
+  LiveRangeBound* FindSucc(const InstructionBlock* succ) {
+    const LifetimePosition succ_start =
+        LifetimePosition::FromInstructionIndex(succ->first_instruction_index());
+    return Find(succ_start);
+  }
+
   void Find(const InstructionBlock* block, const InstructionBlock* pred,
             FindResult* result) const {
     const LifetimePosition pred_end =
@@ -1330,19 +1299,39 @@ void RegisterAllocator::ResolveControlFlow() {
   LiveRangeFinder finder(*this);
   for (auto block : code()->instruction_blocks()) {
     if (CanEagerlyResolveControlFlow(block)) continue;
+    // resolve phis
+    for (auto phi : block->phis()) {
+      auto* block_bound =
+          finder.ArrayFor(phi->virtual_register())->FindSucc(block);
+      auto phi_output = block_bound->range_->CreateAssignedOperand(code_zone());
+      phi->output()->ConvertTo(phi_output->kind(), phi_output->index());
+      size_t pred_index = 0;
+      for (auto pred : block->predecessors()) {
+        const InstructionBlock* pred_block = code()->InstructionBlockAt(pred);
+        auto* pred_bound =
+            finder.ArrayFor(phi->operands()[pred_index])->FindPred(pred_block);
+        auto pred_op = pred_bound->range_->CreateAssignedOperand(code_zone());
+        phi->inputs()[pred_index] = pred_op;
+        ResolveControlFlow(block, phi_output, pred_block, pred_op);
+        pred_index++;
+      }
+    }
     BitVector* live = live_in_sets_[block->rpo_number().ToInt()];
     BitVector::Iterator iterator(live);
     while (!iterator.Done()) {
-      LiveRangeBoundArray* array = finder.ArrayFor(iterator.Current());
+      auto* array = finder.ArrayFor(iterator.Current());
       for (auto pred : block->predecessors()) {
         FindResult result;
-        const InstructionBlock* pred_block = code()->InstructionBlockAt(pred);
+        const auto* pred_block = code()->InstructionBlockAt(pred);
         array->Find(block, pred_block, &result);
         if (result.cur_cover_ == result.pred_cover_ ||
             result.cur_cover_->IsSpilled())
           continue;
-        ResolveControlFlow(block, result.cur_cover_, pred_block,
-                           result.pred_cover_);
+        InstructionOperand* pred_op =
+            result.pred_cover_->CreateAssignedOperand(code_zone());
+        InstructionOperand* cur_op =
+            result.cur_cover_->CreateAssignedOperand(code_zone());
+        ResolveControlFlow(block, cur_op, pred_block, pred_op);
       }
       iterator.Advance();
     }
@@ -1351,26 +1340,23 @@ void RegisterAllocator::ResolveControlFlow() {
 
 
 void RegisterAllocator::ResolveControlFlow(const InstructionBlock* block,
-                                           const LiveRange* cur_cover,
+                                           InstructionOperand* cur_op,
                                            const InstructionBlock* pred,
-                                           const LiveRange* pred_cover) {
-  InstructionOperand* pred_op = pred_cover->CreateAssignedOperand(code_zone());
-  InstructionOperand* cur_op = cur_cover->CreateAssignedOperand(code_zone());
-  if (!pred_op->Equals(cur_op)) {
-    GapInstruction* gap = NULL;
-    if (block->PredecessorCount() == 1) {
-      gap = code()->GapAt(block->first_instruction_index());
-    } else {
-      DCHECK(pred->SuccessorCount() == 1);
-      gap = GetLastGap(pred);
+                                           InstructionOperand* pred_op) {
+  if (pred_op->Equals(cur_op)) return;
+  GapInstruction* gap = nullptr;
+  if (block->PredecessorCount() == 1) {
+    gap = code()->GapAt(block->first_instruction_index());
+  } else {
+    DCHECK(pred->SuccessorCount() == 1);
+    gap = GetLastGap(pred);
 
-      Instruction* branch = InstructionAt(pred->last_instruction_index());
-      DCHECK(!branch->HasPointerMap());
-      USE(branch);
-    }
-    gap->GetOrCreateParallelMove(GapInstruction::START, code_zone())
-        ->AddMove(pred_op, cur_op, code_zone());
+    Instruction* branch = InstructionAt(pred->last_instruction_index());
+    DCHECK(!branch->HasPointerMap());
+    USE(branch);
   }
+  gap->GetOrCreateParallelMove(GapInstruction::START, code_zone())
+      ->AddMove(pred_op, cur_op, code_zone());
 }
 
 
@@ -1395,29 +1381,6 @@ void RegisterAllocator::BuildLiveRanges() {
       // block.
       int phi_vreg = phi->virtual_register();
       live->Remove(phi_vreg);
-
-      InstructionOperand* hint = NULL;
-      InstructionOperand* phi_operand = NULL;
-      GapInstruction* gap =
-          GetLastGap(code()->InstructionBlockAt(block->predecessors()[0]));
-
-      // TODO(titzer): no need to create the parallel move if it doesn't exit.
-      ParallelMove* move =
-          gap->GetOrCreateParallelMove(GapInstruction::START, code_zone());
-      for (int j = 0; j < move->move_operands()->length(); ++j) {
-        InstructionOperand* to = move->move_operands()->at(j).destination();
-        if (to->IsUnallocated() &&
-            UnallocatedOperand::cast(to)->virtual_register() == phi_vreg) {
-          hint = move->move_operands()->at(j).source();
-          phi_operand = to;
-          break;
-        }
-      }
-      DCHECK(hint != NULL);
-
-      LifetimePosition block_start = LifetimePosition::FromInstructionIndex(
-          block->first_instruction_index());
-      Define(block_start, phi_operand, hint);
     }
 
     // Now live is live_in for this block except not including values live
@@ -1462,13 +1425,7 @@ void RegisterAllocator::BuildLiveRanges() {
         for (UsePosition* pos = range->first_pos(); pos != NULL;
              pos = pos->next_) {
           pos->register_beneficial_ = true;
-          // TODO(dcarney): should the else case assert requires_reg_ == false?
-          // Can't mark phis as needing a register.
-          if (!code()
-                   ->InstructionAt(pos->pos().InstructionIndex())
-                   ->IsGapMoves()) {
-            pos->requires_reg_ = true;
-          }
+          pos->requires_reg_ = true;
         }
       }
     }

diff --git a/src/compiler/register-allocator.h b/src/compiler/register-allocator.h
index 45c3371..7ffb379 100644 (file)
--- a/src/compiler/register-allocator.h
+++ b/src/compiler/register-allocator.h
@@ -197,12 +197,6 @@ class LiveRange FINAL : public ZoneObject {
   int spill_start_index() const { return spill_start_index_; }
   void set_assigned_register(int reg, Zone* zone);
   void MakeSpilled(Zone* zone);
-  bool is_phi() const { return is_phi_; }
-  void set_is_phi(bool is_phi) { is_phi_ = is_phi; }
-  bool is_non_loop_phi() const { return is_non_loop_phi_; }
-  void set_is_non_loop_phi(bool is_non_loop_phi) {
-    is_non_loop_phi_ = is_non_loop_phi;
-  }
 
   // Returns use position in this live range that follows both start
   // and last processed use position.
@@ -295,8 +289,6 @@ class LiveRange FINAL : public ZoneObject {
 
   int id_;
   bool spilled_;
-  bool is_phi_;
-  bool is_non_loop_phi_;
   RegisterKind kind_;
   int assigned_register_;
   UseInterval* last_interval_;
@@ -341,25 +333,21 @@ class RegisterAllocator FINAL : public ZoneObject {
   // Phase 1 : insert moves to account for fixed register operands.
   void MeetRegisterConstraints();
 
-  // Phase 2: deconstruct SSA by inserting moves in successors and the headers
-  // of blocks containing phis.
-  void ResolvePhis();
-
-  // Phase 3: compute liveness of all virtual register.
+  // Phase 2: compute liveness of all virtual register.
   void BuildLiveRanges();
   bool ExistsUseWithoutDefinition();
 
-  // Phase 4: compute register assignments.
+  // Phase 3: compute register assignments.
   void AllocateGeneralRegisters();
   void AllocateDoubleRegisters();
 
-  // Phase 5: compute values for pointer maps.
+  // Phase 4: compute values for pointer maps.
   void PopulatePointerMaps();  // TODO(titzer): rename to PopulateReferenceMaps.
 
-  // Phase 6: reconnect split ranges with moves.
+  // Phase 5: reconnect split ranges with moves.
   void ConnectRanges();
 
-  // Phase 7: insert moves to connect ranges across basic blocks.
+  // Phase 6: insert moves to connect ranges across basic blocks.
   void ResolveControlFlow();
 
  private:
@@ -407,7 +395,7 @@ class RegisterAllocator FINAL : public ZoneObject {
                               int gap_index);
   void MeetRegisterConstraintsForLastInstructionInBlock(
       const InstructionBlock* block);
-  void ResolvePhis(const InstructionBlock* block);
+  void ProcessPhis(const InstructionBlock* block);
 
   // Helper methods for building intervals.
   InstructionOperand* AllocateFixed(UnallocatedOperand* operand, int pos,
@@ -482,9 +470,9 @@ class RegisterAllocator FINAL : public ZoneObject {
 
   // Helper methods for resolving control flow.
   void ResolveControlFlow(const InstructionBlock* block,
-                          const LiveRange* cur_cover,
+                          InstructionOperand* cur_op,
                           const InstructionBlock* pred,
-                          const LiveRange* pred_cover);
+                          InstructionOperand* pred_op);
 
   void SetLiveRangeAssignedRegister(LiveRange* range, int reg);
 

diff --git a/test/unittests/compiler/register-allocator-unittest.cc b/test/unittests/compiler/register-allocator-unittest.cc
index 290c285..d4dd49f 100644 (file)
--- a/test/unittests/compiler/register-allocator-unittest.cc
+++ b/test/unittests/compiler/register-allocator-unittest.cc
@@ -243,8 +243,9 @@ class RegisterAllocatorTest : public TestWithZone {
   int Return(VReg vreg) { return Return(Reg(vreg, 0)); }
 
   PhiInstruction* Phi(VReg incoming_vreg) {
-    PhiInstruction* phi = new (zone()) PhiInstruction(zone(), NewReg().value_);
-    phi->operands().push_back(incoming_vreg.value_);
+    PhiInstruction* phi =
+        new (zone()) PhiInstruction(zone(), NewReg().value_, 10);
+    phi->Extend(zone(), incoming_vreg.value_);
     current_block_->AddPhi(phi);
     return phi;
   }
@@ -255,8 +256,8 @@ class RegisterAllocatorTest : public TestWithZone {
     return phi;
   }
 
-  static void Extend(PhiInstruction* phi, VReg vreg) {
-    phi->operands().push_back(vreg.value_);
+  void Extend(PhiInstruction* phi, VReg vreg) {
+    phi->Extend(zone(), vreg.value_);
   }
 
   VReg DefineConstant(int32_t imm = 0) {