Fix a bug in the linear scan register allocator.

Triggered by:
org.apache.harmony.tests.java.util.jar.JarFileTest#testGetJarEntry.

By miscompling:
okhttp.CacheControl#parse.

A move occuring just before the first instruction of a block
should not be handled by ConnectSplitSiblings, but by ConnectSiblings
instead.

Change-Id: I8ad409734809e6787bb7321563e1331e7a6906c0

1 files changed, 26 insertions, 10 deletions

diff --git a/compiler/optimizing/register_allocator.cc b/compiler/optimizing/register_allocator.cc
index 2948496e15..6d03ad827f 100644
--- a/compiler/optimizing/register_allocator.cc
+++ b/compiler/optimizing/register_allocator.cc
@@ -887,6 +887,14 @@ void RegisterAllocator::AddInputMoveFor(HInstruction* user,
   move->AddMove(new (allocator_) MoveOperands(source, destination, nullptr));
 }
 
+static bool IsInstructionStart(size_t position) {
+  return (position & 1) == 0;
+}
+
+static bool IsInstructionEnd(size_t position) {
+  return (position & 1) == 1;
+}
+
 void RegisterAllocator::InsertParallelMoveAt(size_t position,
                                              HInstruction* instruction,
                                              Location source,
@@ -895,12 +903,22 @@ void RegisterAllocator::InsertParallelMoveAt(size_t position,
   if (source.Equals(destination)) return;
 
   HInstruction* at = liveness_.GetInstructionFromPosition(position / 2);
-  if (at == nullptr) {
-    // Block boundary, don't do anything the connection of split siblings will handle it.
-    return;
-  }
   HParallelMove* move;
-  if ((position & 1) == 1) {
+  if (at == nullptr) {
+    if (IsInstructionStart(position)) {
+      // Block boundary, don't do anything the connection of split siblings will handle it.
+      return;
+    } else {
+      // Move must happen before the first instruction of the block.
+      at = liveness_.GetInstructionFromPosition((position + 1) / 2);
+      move = at->AsParallelMove();
+      if (move == nullptr || move->GetLifetimePosition() < position) {
+        move = new (allocator_) HParallelMove(allocator_);
+        move->SetLifetimePosition(position);
+        at->GetBlock()->InsertInstructionBefore(move, at);
+      }
+    }
+  } else if (IsInstructionEnd(position)) {
     // Move must happen after the instruction.
     DCHECK(!at->IsControlFlow());
     move = at->GetNext()->AsParallelMove();
@@ -1107,12 +1125,10 @@ void RegisterAllocator::ConnectSplitSiblings(LiveInterval* interval,
     return;
   }
 
+  // Intervals end at the lifetime end of a block. The decrement by one
+  // ensures the `Cover` call will return true.
   size_t from_position = from->GetLifetimeEnd() - 1;
-  // When an instruction dies at entry of another, and the latter is the beginning
-  // of a block, the register allocator ensures the former has a register
-  // at block->GetLifetimeStart() + 1. Since this is at a block boundary, it must
-  // must be handled in this method.
-  size_t to_position = to->GetLifetimeStart() + 1;
+  size_t to_position = to->GetLifetimeStart();
 
   LiveInterval* destination = nullptr;
   LiveInterval* source = nullptr;