[turbofan] Spill rsi and rdi in their existing locations.

src/compiler/register-allocator.cc

 // Copyright 2014 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/base/adapters.h"
 #include "src/compiler/linkage.h"
 #include "src/compiler/register-allocator.h"
 #include "src/string-stream.h"
 
 namespace v8 {
@@ skipping 674 matching lines @@
     : LiveRange(0, machine_type, this),
       vreg_(vreg),
       last_child_id_(0),
       splintered_from_(nullptr),
       spill_operand_(nullptr),
       spills_at_definition_(nullptr),
       spilled_in_deferred_blocks_(false),
       spill_start_index_(kMaxInt),
       last_child_(this),
       last_pos_(nullptr),
-      splinter_(nullptr) {
+      splinter_(nullptr),
+      elidable_def_(nullptr) {
   bits_ |= SpillTypeField::encode(SpillType::kNoSpillType);
 }
 
 
 #if DEBUG
 int TopLevelLiveRange::debug_virt_reg() const {
   return IsSplinter() ? splintered_from()->vreg() : vreg();
 }
 #endif
 
@@ skipping 912 matching lines @@
 
 
 void ConstraintBuilder::MeetConstraintsAfter(int instr_index) {
   auto first = code()->InstructionAt(instr_index);
   // Handle fixed temporaries.
   for (size_t i = 0; i < first->TempCount(); i++) {
     auto temp = UnallocatedOperand::cast(first->TempAt(i));
     if (temp->HasFixedPolicy()) AllocateFixed(temp, instr_index, false);
   }
   // Handle constant/fixed output operands.
+  bool is_prespilled_parameter =
+      first->arch_opcode() == ArchOpcode::kArchNop && first->OutputCount() == 2;

[Jarin, 2015/11/12 12:08:33]

This looks really brittle because we do not even check that both outputs define
the same vreg. Even then I would not like it because there might be legitimate
reasons for having two operands to ArchNop that have nothing to do with
pre-spilling (and we do not really want to special-case for specific opcodes
here).

[Mircea Trofin, 2015/11/12 18:01:35]

I see. I thought we used ArchNop just for introducing parameters. If that's not
the case, and we don't want to add another opcode (I'd also be weary of doing
that), then we're back to needing the tying info within the operand, which
basically removes most of the reasons for doing this design here, and favors the
previous one.

+
   for (size_t i = 0; i < first->OutputCount(); i++) {
     InstructionOperand* output = first->OutputAt(i);
     if (output->IsConstant()) {
       int output_vreg = ConstantOperand::cast(output)->virtual_register();
       auto range = data()->GetOrCreateLiveRangeFor(output_vreg);
       range->SetSpillStartIndex(instr_index + 1);
       range->SetSpillOperand(output);
       continue;
     }
     auto first_output = UnallocatedOperand::cast(output);
     auto range =
         data()->GetOrCreateLiveRangeFor(first_output->virtual_register());
     bool assigned = false;
     if (first_output->HasFixedPolicy()) {
       int output_vreg = first_output->virtual_register();
       UnallocatedOperand output_copy(UnallocatedOperand::ANY, output_vreg);
       bool is_tagged = code()->IsReference(output_vreg);
       AllocateFixed(first_output, instr_index, is_tagged);
+      MoveOperands* def = data()->AddGapMove(
+          instr_index + 1, Instruction::START, *first_output, output_copy);
 
       // This value is produced on the stack, we never need to spill it.
       if (first_output->IsStackSlot()) {
         DCHECK(LocationOperand::cast(first_output)->index() <
                data()->frame()->GetTotalFrameSlotCount());
         range->SetSpillOperand(LocationOperand::cast(first_output));
         range->SetSpillStartIndex(instr_index + 1);
         assigned = true;
+      } else if (is_prespilled_parameter) {
+        // we need the def so that the live range has the shape expected by
+        // the rest of the pipeline. Upon assigning operands, if this move
+        // ends up assigning to the stack slot the register value, we may
+        // delete it.

[Jarin, 2015/11/12 12:08:33]

To be honest, this MarkElidableDef business seems to be quite hacky.

I do not really understand why this is not a problem when we have a normal
fixed-register policy. Would not we then move to the spill slot twice? (Once
with the move on line 1650 and once on line 1677.)

[Mircea Trofin, 2015/11/12 18:01:35]

Here's what happens with a vanilla fixed register: the first move says initially
"v_x = [register]" (line 1650). Line 1677 only schedules a move.
OperandAssigner::CommitAssignment will take care of that. See the call to
TopLevelLiveRange::CommitSpillsAtDefinition from CommitAssignment. First, if
there is no spill operand, there won't be a move to a stack slot, so the move
inserted at line 1650 will either be to the same register, so that will be
elided later (==redundant), or to some other register, so not redundant.

If there are spills, and if the first segment of the live range chain (so the
top level leftover) is spilled, then CommitSpillsAtDefinition won't insert
another move (see the logic around "might_be_duplicated" in
CommitSpillsAtDefinition), and the move resulting from operand assignment
becomes the spill. If the top level isn't spilled, then the move at line 1650 is
to a register (see the paragraph above, same reasoning applies here). So we need
to spill at definition, which will happen in CommitSpillsAtDefinition because,
even if might_be_duplicated, we won't find the matching move.

In our case, we still need the move at line 1650 to define the virtual register.
After that, though, we need to somehow not spill at definition no matter what. 

Now that I think about it, maybe a better concept for the live range would be
"IsPreSpilled". Then, we could still schedule the spill at definition, but then
the logic would be to elide the move in CommitSpillsAtDefinition, instead of
adding a move (so "commit" means in this case "elided").

+        range->MarkElidableDef(def);
+        UnallocatedOperand* spill_op =
+            UnallocatedOperand::cast(first->OutputAt(1));
+        AllocateFixed(spill_op, instr_index, is_tagged);
+        range->SetSpillOperand(spill_op);
+        range->SetSpillStartIndex(instr_index + 1);
+        break;
       }
-      data()->AddGapMove(instr_index + 1, Instruction::START, *first_output,
-                         output_copy);
     }
     // Make sure we add a gap move for spilling (if we have not done
     // so already).
     if (!assigned) {
       range->SpillAtDefinition(allocation_zone(), instr_index + 1,
                                first_output);
       range->SetSpillStartIndex(instr_index + 1);
     }
   }
 }
@@ skipping 1342 matching lines @@
     if (top_range->is_phi()) {
       data()->GetPhiMapValueFor(top_range)->CommitAssignment(
           top_range->GetAssignedOperand());
     }
     for (LiveRange* range = top_range; range != nullptr;
          range = range->next()) {
       auto assigned = range->GetAssignedOperand();
       range->ConvertUsesToOperand(assigned, spill_operand);
     }
 
+    MoveOperands* move = top_range->elidable_def();
+    if (move != nullptr && move->destination().Equals(spill_operand)) {
+      move->Eliminate();
+    }
+
     if (!spill_operand.IsInvalid()) {
       // If this top level range has a child spilled in a deferred block, we use
       // the range and control flow connection mechanism instead of spilling at
       // definition. Refer to the ConnectLiveRanges and ResolveControlFlow
       // phases. Normally, when we spill at definition, we do not insert a
       // connecting move when a successor child range is spilled - because the
       // spilled range picks up its value from the slot which was assigned at
       // definition. For ranges that are determined to spill only in deferred
       // blocks, we let ConnectLiveRanges and ResolveControlFlow insert such
       // moves between ranges. Because of how the ranges are split around
@@ skipping 37 matching lines @@
   // for all spilled live ranges at this point.
   int last_range_start = 0;
   auto reference_maps = data()->code()->reference_maps();
   ReferenceMapDeque::const_iterator first_it = reference_maps->begin();
   for (TopLevelLiveRange* range : data()->live_ranges()) {
     if (range == nullptr) continue;
     // Skip non-reference values.
     if (!data()->IsReference(range)) continue;
     // Skip empty live ranges.
     if (range->IsEmpty()) continue;
+    if (range->IsReferenceAccountedByCaller()) continue;
 
     // Find the extent of the range and its children.
     int start = range->Start().ToInstructionIndex();
     int end = 0;
     for (LiveRange* cur = range; cur != nullptr; cur = cur->next()) {
       auto this_end = cur->End();
       if (this_end.ToInstructionIndex() > end)
         end = this_end.ToInstructionIndex();
       DCHECK(cur->Start().ToInstructionIndex() >= start);
     }
@@ skipping 355 matching lines @@
     auto eliminate = moves->PrepareInsertAfter(move);
     to_insert.push_back(move);
     if (eliminate != nullptr) to_eliminate.push_back(eliminate);
   }
 }
 
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8