OSDN Git Service

Do not use register pair in a parallel move.
authorNicolas Geoffray <ngeoffray@google.com>
Fri, 16 Jan 2015 09:14:18 +0000 (09:14 +0000)
committerNicolas Geoffray <ngeoffray@google.com>
Fri, 16 Jan 2015 11:22:08 +0000 (11:22 +0000)
The ParallelMoveResolver does not work with pairs. Instead,
decompose the pair into two individual moves.

Change-Id: Ie9d3f0b078cef8dc20640c98b20bb20cc4971a7f

compiler/optimizing/code_generator.cc
compiler/optimizing/code_generator_arm.cc
compiler/optimizing/code_generator_x86.cc
compiler/optimizing/intrinsics_x86_64.cc
compiler/optimizing/locations.h
compiler/optimizing/nodes.h
compiler/optimizing/parallel_move_resolver.cc
compiler/optimizing/parallel_move_test.cc
compiler/optimizing/register_allocator.cc

index 9e89070..85724fa 100644 (file)
@@ -696,11 +696,9 @@ void CodeGenerator::ClearSpillSlotsFromLoopPhisInStackMap(HSuspendCheck* suspend
 }
 
 void CodeGenerator::EmitParallelMoves(Location from1, Location to1, Location from2, Location to2) {
-  MoveOperands move1(from1, to1, nullptr);
-  MoveOperands move2(from2, to2, nullptr);
   HParallelMove parallel_move(GetGraph()->GetArena());
-  parallel_move.AddMove(&move1);
-  parallel_move.AddMove(&move2);
+  parallel_move.AddMove(from1, to1, nullptr);
+  parallel_move.AddMove(from2, to2, nullptr);
   GetMoveResolver()->EmitNativeCode(&parallel_move);
 }
 
index c4ba0fd..5b2db13 100644 (file)
@@ -3302,15 +3302,6 @@ void ParallelMoveResolverARM::EmitMove(size_t index) {
       DCHECK(destination.IsStackSlot());
       __ StoreSToOffset(source.AsFpuRegister<SRegister>(), SP, destination.GetStackIndex());
     }
-  } else if (source.IsFpuRegisterPair()) {
-    if (destination.IsFpuRegisterPair()) {
-      __ vmovd(FromLowSToD(destination.AsFpuRegisterPairLow<SRegister>()),
-               FromLowSToD(source.AsFpuRegisterPairLow<SRegister>()));
-    } else {
-      DCHECK(destination.IsDoubleStackSlot()) << destination;
-      __ StoreDToOffset(FromLowSToD(source.AsFpuRegisterPairLow<SRegister>()),
-                        SP, destination.GetStackIndex());
-    }
   } else if (source.IsDoubleStackSlot()) {
     if (destination.IsFpuRegisterPair()) {
       __ LoadDFromOffset(FromLowSToD(destination.AsFpuRegisterPairLow<SRegister>()),
@@ -3396,22 +3387,6 @@ void ParallelMoveResolverARM::EmitSwap(size_t index) {
     __ vmovs(STMP, reg);
     __ LoadSFromOffset(reg, SP, mem);
     __ StoreSToOffset(STMP, SP, mem);
-  } else if (source.IsFpuRegisterPair() && destination.IsFpuRegisterPair()) {
-    __ vmovd(DTMP, FromLowSToD(source.AsFpuRegisterPairLow<SRegister>()));
-    __ vmovd(FromLowSToD(source.AsFpuRegisterPairLow<SRegister>()),
-             FromLowSToD(destination.AsFpuRegisterPairLow<SRegister>()));
-    __ vmovd(FromLowSToD(destination.AsFpuRegisterPairLow<SRegister>()), DTMP);
-  } else if (source.IsFpuRegisterPair() || destination.IsFpuRegisterPair()) {
-    DRegister reg = source.IsFpuRegisterPair()
-        ? FromLowSToD(source.AsFpuRegisterPairLow<SRegister>())
-        : FromLowSToD(destination.AsFpuRegisterPairLow<SRegister>());
-    int mem = source.IsFpuRegisterPair()
-        ? destination.GetStackIndex()
-        : source.GetStackIndex();
-
-    __ vmovd(DTMP, reg);
-    __ LoadDFromOffset(reg, SP, mem);
-    __ StoreDToOffset(DTMP, SP, mem);
   } else if (source.IsDoubleStackSlot() && destination.IsDoubleStackSlot()) {
     // TODO: We could use DTMP and ask for a pair scratch register (float or core).
     // This would save four instructions if two scratch registers are available, and
index 1a0df44..bdd0979 100644 (file)
@@ -3363,7 +3363,7 @@ void ParallelMoveResolverX86::EmitMove(size_t index) {
       __ movl(Address(ESP, destination.GetStackIndex()), imm);
     }
   } else {
-    LOG(FATAL) << "Unimplemented";
+    LOG(FATAL) << "Unimplemented move: " << destination << " <- " << source;
   }
 }
 
index c1f4c94..2c23945 100644 (file)
@@ -116,7 +116,7 @@ static void MoveArguments(HInvoke* invoke, ArenaAllocator* arena, CodeGeneratorX
     Location cc_loc = calling_convention_visitor.GetNextLocation(input->GetType());
     Location actual_loc = locations->InAt(i);
 
-    parallel_move.AddMove(new (arena) MoveOperands(actual_loc, cc_loc, nullptr));
+    parallel_move.AddMove(actual_loc, cc_loc, nullptr);
   }
 
   codegen->GetMoveResolver()->EmitNativeCode(&parallel_move);
index d41b3ae..68d6059 100644 (file)
@@ -290,18 +290,6 @@ class Location : public ValueObject {
     return value_ == other.value_;
   }
 
-  // Returns whether this location contains `other`.
-  bool Contains(Location other) const {
-    if (Equals(other)) return true;
-    if (IsRegisterPair() && other.IsRegister()) {
-      return low() == other.reg() || high() == other.reg();
-    }
-    if (IsFpuRegisterPair() && other.IsFpuRegister()) {
-      return low() == other.reg() || high() == other.reg();
-    }
-    return false;
-  }
-
   const char* DebugString() const {
     switch (GetKind()) {
       case kInvalid: return "I";
index f2aa043..fa51f27 100644 (file)
@@ -2755,7 +2755,7 @@ class MoveOperands : public ArenaObject<kArenaAllocMisc> {
 
   // True if this blocks a move from the given location.
   bool Blocks(Location loc) const {
-    return !IsEliminated() && source_.Contains(loc);
+    return !IsEliminated() && source_.Equals(loc);
   }
 
   // A move is redundant if it's been eliminated, if its source and
@@ -2784,8 +2784,6 @@ class MoveOperands : public ArenaObject<kArenaAllocMisc> {
   // This is only used in debug mode, to ensure we do not connect interval siblings
   // in the same parallel move.
   HInstruction* instruction_;
-
-  DISALLOW_COPY_AND_ASSIGN(MoveOperands);
 };
 
 static constexpr size_t kDefaultNumberOfMoves = 4;
@@ -2795,24 +2793,46 @@ class HParallelMove : public HTemplateInstruction<0> {
   explicit HParallelMove(ArenaAllocator* arena)
       : HTemplateInstruction(SideEffects::None()), moves_(arena, kDefaultNumberOfMoves) {}
 
-  void AddMove(MoveOperands* move) {
-    if (kIsDebugBuild) {
-      if (move->GetInstruction() != nullptr) {
+  void AddMove(Location source, Location destination, HInstruction* instruction) {
+    DCHECK(source.IsValid());
+    DCHECK(destination.IsValid());
+    // The parallel move resolver does not handle pairs. So we decompose the
+    // pair locations into two moves.
+    if (source.IsPair() && destination.IsPair()) {
+      AddMove(source.ToLow(), destination.ToLow(), instruction);
+      AddMove(source.ToHigh(), destination.ToHigh(), nullptr);
+    } else if (source.IsPair()) {
+      DCHECK(destination.IsDoubleStackSlot());
+      AddMove(source.ToLow(), Location::StackSlot(destination.GetStackIndex()), instruction);
+      AddMove(source.ToHigh(), Location::StackSlot(destination.GetHighStackIndex(4)), nullptr);
+    } else if (destination.IsPair()) {
+      DCHECK(source.IsDoubleStackSlot());
+      AddMove(Location::StackSlot(source.GetStackIndex()), destination.ToLow(), instruction);
+      // TODO: rewrite GetHighStackIndex to not require a word size. It's supposed to
+      // always be 4.
+      static constexpr int kHighOffset = 4;
+      AddMove(Location::StackSlot(source.GetHighStackIndex(kHighOffset)),
+              destination.ToHigh(),
+              nullptr);
+    } else {
+      if (kIsDebugBuild) {
+        if (instruction != nullptr) {
+          for (size_t i = 0, e = moves_.Size(); i < e; ++i) {
+            DCHECK_NE(moves_.Get(i).GetInstruction(), instruction)
+              << "Doing parallel moves for the same instruction.";
+          }
+        }
         for (size_t i = 0, e = moves_.Size(); i < e; ++i) {
-          DCHECK_NE(moves_.Get(i)->GetInstruction(), move->GetInstruction())
-            << "Doing parallel moves for the same instruction.";
+          DCHECK(!destination.Equals(moves_.Get(i).GetDestination()))
+              << "Same destination for two moves in a parallel move.";
         }
       }
-      for (size_t i = 0, e = moves_.Size(); i < e; ++i) {
-        DCHECK(!move->GetDestination().Contains(moves_.Get(i)->GetDestination()))
-            << "Same destination for two moves in a parallel move.";
-      }
+      moves_.Add(MoveOperands(source, destination, instruction));
     }
-    moves_.Add(move);
   }
 
   MoveOperands* MoveOperandsAt(size_t index) const {
-    return moves_.Get(index);
+    return moves_.GetRawStorage() + index;
   }
 
   size_t NumMoves() const { return moves_.Size(); }
@@ -2820,7 +2840,7 @@ class HParallelMove : public HTemplateInstruction<0> {
   DECLARE_INSTRUCTION(ParallelMove);
 
  private:
-  GrowableArray<MoveOperands*> moves_;
+  GrowableArray<MoveOperands> moves_;
 
   DISALLOW_COPY_AND_ASSIGN(HParallelMove);
 };
index b8f5070..debe466 100644 (file)
@@ -57,6 +57,9 @@ void ParallelMoveResolver::BuildInitialMoveList(HParallelMove* parallel_move) {
   // unallocated, or the move was already eliminated).
   for (size_t i = 0; i < parallel_move->NumMoves(); ++i) {
     MoveOperands* move = parallel_move->MoveOperandsAt(i);
+    // The parallel move resolver algorithm does not work with register pairs.
+    DCHECK(!move->GetSource().IsPair());
+    DCHECK(!move->GetDestination().IsPair());
     if (!move->IsRedundant()) {
       moves_.Add(move);
     }
index 7ab41b6..28b5697 100644 (file)
@@ -26,20 +26,26 @@ class TestParallelMoveResolver : public ParallelMoveResolver {
  public:
   explicit TestParallelMoveResolver(ArenaAllocator* allocator) : ParallelMoveResolver(allocator) {}
 
+  void Dump(Location location) {
+    if (location.IsConstant()) {
+      message_ << "C";
+    } else if (location.IsPair()) {
+      message_ << location.low() << "," << location.high();
+    } else {
+      message_ << location.reg();
+    }
+  }
+
   virtual void EmitMove(size_t index) {
     MoveOperands* move = moves_.Get(index);
     if (!message_.str().empty()) {
       message_ << " ";
     }
     message_ << "(";
-    if (move->GetSource().IsConstant()) {
-      message_ << "C";
-    } else {
-      message_ << move->GetSource().reg();
-    }
-    message_ << " -> "
-             << move->GetDestination().reg()
-             << ")";
+    Dump(move->GetSource());
+    message_ << " -> ";
+    Dump(move->GetDestination());
+    message_ << ")";
   }
 
   virtual void EmitSwap(size_t index) {
@@ -47,11 +53,11 @@ class TestParallelMoveResolver : public ParallelMoveResolver {
     if (!message_.str().empty()) {
       message_ << " ";
     }
-    message_ << "("
-             << move->GetSource().reg()
-             << " <-> "
-             << move->GetDestination().reg()
-             << ")";
+    message_ << "(";
+    Dump(move->GetSource());
+    message_ << " <-> ";
+    Dump(move->GetDestination());
+    message_ << ")";
   }
 
   virtual void SpillScratch(int reg ATTRIBUTE_UNUSED) {}
@@ -73,10 +79,10 @@ static HParallelMove* BuildParallelMove(ArenaAllocator* allocator,
                                         size_t number_of_moves) {
   HParallelMove* moves = new (allocator) HParallelMove(allocator);
   for (size_t i = 0; i < number_of_moves; ++i) {
-    moves->AddMove(new (allocator) MoveOperands(
+    moves->AddMove(
         Location::RegisterLocation(operands[i][0]),
         Location::RegisterLocation(operands[i][1]),
-        nullptr));
+        nullptr);
   }
   return moves;
 }
@@ -131,16 +137,66 @@ TEST(ParallelMoveTest, ConstantLast) {
   ArenaAllocator allocator(&pool);
   TestParallelMoveResolver resolver(&allocator);
   HParallelMove* moves = new (&allocator) HParallelMove(&allocator);
-  moves->AddMove(new (&allocator) MoveOperands(
+  moves->AddMove(
       Location::ConstantLocation(new (&allocator) HIntConstant(0)),
       Location::RegisterLocation(0),
-      nullptr));
-  moves->AddMove(new (&allocator) MoveOperands(
+      nullptr);
+  moves->AddMove(
       Location::RegisterLocation(1),
       Location::RegisterLocation(2),
-      nullptr));
+      nullptr);
   resolver.EmitNativeCode(moves);
   ASSERT_STREQ("(1 -> 2) (C -> 0)", resolver.GetMessage().c_str());
 }
 
+TEST(ParallelMoveTest, Pairs) {
+  ArenaPool pool;
+  ArenaAllocator allocator(&pool);
+
+  {
+    TestParallelMoveResolver resolver(&allocator);
+    HParallelMove* moves = new (&allocator) HParallelMove(&allocator);
+    moves->AddMove(
+        Location::RegisterLocation(2),
+        Location::RegisterLocation(4),
+        nullptr);
+    moves->AddMove(
+        Location::RegisterPairLocation(0, 1),
+        Location::RegisterPairLocation(2, 3),
+        nullptr);
+    resolver.EmitNativeCode(moves);
+    ASSERT_STREQ("(2 -> 4) (0 -> 2) (1 -> 3)", resolver.GetMessage().c_str());
+  }
+
+  {
+    TestParallelMoveResolver resolver(&allocator);
+    HParallelMove* moves = new (&allocator) HParallelMove(&allocator);
+    moves->AddMove(
+        Location::RegisterPairLocation(0, 1),
+        Location::RegisterPairLocation(2, 3),
+        nullptr);
+    moves->AddMove(
+        Location::RegisterLocation(2),
+        Location::RegisterLocation(4),
+        nullptr);
+    resolver.EmitNativeCode(moves);
+    ASSERT_STREQ("(2 -> 4) (0 -> 2) (1 -> 3)", resolver.GetMessage().c_str());
+  }
+
+  {
+    TestParallelMoveResolver resolver(&allocator);
+    HParallelMove* moves = new (&allocator) HParallelMove(&allocator);
+    moves->AddMove(
+        Location::RegisterPairLocation(0, 1),
+        Location::RegisterPairLocation(2, 3),
+        nullptr);
+    moves->AddMove(
+        Location::RegisterLocation(2),
+        Location::RegisterLocation(0),
+        nullptr);
+    resolver.EmitNativeCode(moves);
+    ASSERT_STREQ("(2 <-> 0) (1 -> 3)", resolver.GetMessage().c_str());
+  }
+}
+
 }  // namespace art
index 93ed44e..1b42e94 100644 (file)
@@ -1051,7 +1051,7 @@ void RegisterAllocator::AddInputMoveFor(HInstruction* user,
     move = previous->AsParallelMove();
   }
   DCHECK_EQ(move->GetLifetimePosition(), user->GetLifetimePosition());
-  move->AddMove(new (allocator_) MoveOperands(source, destination, nullptr));
+  move->AddMove(source, destination, nullptr);
 }
 
 static bool IsInstructionStart(size_t position) {
@@ -1123,7 +1123,7 @@ void RegisterAllocator::InsertParallelMoveAt(size_t position,
     }
   }
   DCHECK_EQ(move->GetLifetimePosition(), position);
-  move->AddMove(new (allocator_) MoveOperands(source, destination, instruction));
+  move->AddMove(source, destination, instruction);
 }
 
 void RegisterAllocator::InsertParallelMoveAtExitOf(HBasicBlock* block,
@@ -1153,7 +1153,7 @@ void RegisterAllocator::InsertParallelMoveAtExitOf(HBasicBlock* block,
   } else {
     move = previous->AsParallelMove();
   }
-  move->AddMove(new (allocator_) MoveOperands(source, destination, instruction));
+  move->AddMove(source, destination, instruction);
 }
 
 void RegisterAllocator::InsertParallelMoveAtEntryOf(HBasicBlock* block,
@@ -1172,7 +1172,7 @@ void RegisterAllocator::InsertParallelMoveAtEntryOf(HBasicBlock* block,
     move->SetLifetimePosition(block->GetLifetimeStart());
     block->InsertInstructionBefore(move, first);
   }
-  move->AddMove(new (allocator_) MoveOperands(source, destination, instruction));
+  move->AddMove(source, destination, instruction);
 }
 
 void RegisterAllocator::InsertMoveAfter(HInstruction* instruction,
@@ -1196,7 +1196,7 @@ void RegisterAllocator::InsertMoveAfter(HInstruction* instruction,
     move->SetLifetimePosition(position);
     instruction->GetBlock()->InsertInstructionBefore(move, instruction->GetNext());
   }
-  move->AddMove(new (allocator_) MoveOperands(source, destination, instruction));
+  move->AddMove(source, destination, instruction);
 }
 
 void RegisterAllocator::ConnectSiblings(LiveInterval* interval) {