Merge "Opt compiler: Correctly require register or FPU register."

5 files changed, 167 insertions, 32 deletions

diff --git a/compiler/optimizing/code_generator.cc b/compiler/optimizing/code_generator.cc
index f7fa5db8d5..8ab759d393 100644
--- a/compiler/optimizing/code_generator.cc
+++ b/compiler/optimizing/code_generator.cc
@@ -36,6 +36,80 @@
 
 namespace art {
 
+// Return whether a location is consistent with a type.
+static bool CheckType(Primitive::Type type, Location location) {
+  if (location.IsFpuRegister()
+      || (location.IsUnallocated() && (location.GetPolicy() == Location::kRequiresFpuRegister))) {
+    return (type == Primitive::kPrimFloat) || (type == Primitive::kPrimDouble);
+  } else if (location.IsRegister() ||
+             (location.IsUnallocated() && (location.GetPolicy() == Location::kRequiresRegister))) {
+    return Primitive::IsIntegralType(type) || (type == Primitive::kPrimNot);
+  } else if (location.IsRegisterPair()) {
+    return type == Primitive::kPrimLong;
+  } else if (location.IsFpuRegisterPair()) {
+    return type == Primitive::kPrimDouble;
+  } else if (location.IsStackSlot()) {
+    return (Primitive::IsIntegralType(type) && type != Primitive::kPrimLong)
+           || (type == Primitive::kPrimFloat)
+           || (type == Primitive::kPrimNot);
+  } else if (location.IsDoubleStackSlot()) {
+    return (type == Primitive::kPrimLong) || (type == Primitive::kPrimDouble);
+  } else if (location.IsConstant()) {
+    if (location.GetConstant()->IsIntConstant()) {
+      return Primitive::IsIntegralType(type) && (type != Primitive::kPrimLong);
+    } else if (location.GetConstant()->IsNullConstant()) {
+      return type == Primitive::kPrimNot;
+    } else if (location.GetConstant()->IsLongConstant()) {
+      return type == Primitive::kPrimLong;
+    } else if (location.GetConstant()->IsFloatConstant()) {
+      return type == Primitive::kPrimFloat;
+    } else {
+      return location.GetConstant()->IsDoubleConstant()
+          && (type == Primitive::kPrimDouble);
+    }
+  } else {
+    return location.IsInvalid() || (location.GetPolicy() == Location::kAny);
+  }
+}
+
+// Check that a location summary is consistent with an instruction.
+static bool CheckTypeConsistency(HInstruction* instruction) {
+  LocationSummary* locations = instruction->GetLocations();
+  if (locations == nullptr) {
+    return true;
+  }
+
+  if (locations->Out().IsUnallocated()
+      && (locations->Out().GetPolicy() == Location::kSameAsFirstInput)) {
+    DCHECK(CheckType(instruction->GetType(), locations->InAt(0)))
+        << instruction->GetType()
+        << " " << locations->InAt(0);
+  } else {
+    DCHECK(CheckType(instruction->GetType(), locations->Out()))
+        << instruction->GetType()
+        << " " << locations->Out();
+  }
+
+  for (size_t i = 0, e = instruction->InputCount(); i < e; ++i) {
+    DCHECK(CheckType(instruction->InputAt(i)->GetType(), locations->InAt(i)))
+      << instruction->InputAt(i)->GetType()
+      << " " << locations->InAt(i);
+  }
+
+  HEnvironment* environment = instruction->GetEnvironment();
+  for (size_t i = 0; i < instruction->EnvironmentSize(); ++i) {
+    if (environment->GetInstructionAt(i) != nullptr) {
+      Primitive::Type type = environment->GetInstructionAt(i)->GetType();
+      DCHECK(CheckType(type, locations->GetEnvironmentAt(i)))
+        << type << " " << locations->GetEnvironmentAt(i);
+    } else {
+      DCHECK(locations->GetEnvironmentAt(i).IsInvalid())
+        << locations->GetEnvironmentAt(i);
+    }
+  }
+  return true;
+}
+
 size_t CodeGenerator::GetCacheOffset(uint32_t index) {
   return mirror::ObjectArray<mirror::Object>::OffsetOfElement(index).SizeValue();
 }
@@ -95,6 +169,7 @@ void CodeGenerator::CompileInternal(CodeAllocator* allocator, bool is_baseline)
       if (is_baseline) {
         InitLocationsBaseline(current);
       }
+      DCHECK(CheckTypeConsistency(current));
       current->Accept(instruction_visitor);
     }
   }
@@ -347,6 +422,7 @@ void CodeGenerator::InitLocationsBaseline(HInstruction* instruction) {
 
 void CodeGenerator::AllocateLocations(HInstruction* instruction) {
   instruction->Accept(GetLocationBuilder());
+  DCHECK(CheckTypeConsistency(instruction));
   LocationSummary* locations = instruction->GetLocations();
   if (!instruction->IsSuspendCheckEntry()) {
     if (locations != nullptr && locations->CanCall()) {
diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc
index 2ea920310a..4fcf5b397d 100644
--- a/compiler/optimizing/code_generator_arm.cc
+++ b/compiler/optimizing/code_generator_arm.cc
@@ -2826,10 +2826,14 @@ void LocationsBuilderARM::HandleFieldSet(HInstruction* instruction, const FieldI
   LocationSummary* locations =
       new (GetGraph()->GetArena()) LocationSummary(instruction, LocationSummary::kNoCall);
   locations->SetInAt(0, Location::RequiresRegister());
-  locations->SetInAt(1, Location::RequiresRegister());
-
 
   Primitive::Type field_type = field_info.GetFieldType();
+  if (Primitive::IsFloatingPointType(field_type)) {
+    locations->SetInAt(1, Location::RequiresFpuRegister());
+  } else {
+    locations->SetInAt(1, Location::RequiresRegister());
+  }
+
   bool is_wide = field_type == Primitive::kPrimLong || field_type == Primitive::kPrimDouble;
   bool generate_volatile = field_info.IsVolatile()
       && is_wide
@@ -2965,8 +2969,13 @@ void LocationsBuilderARM::HandleFieldGet(HInstruction* instruction, const FieldI
       && (field_info.GetFieldType() == Primitive::kPrimDouble)
       && !codegen_->GetInstructionSetFeatures().HasAtomicLdrdAndStrd();
   bool overlap = field_info.IsVolatile() && (field_info.GetFieldType() == Primitive::kPrimLong);
-  locations->SetOut(Location::RequiresRegister(),
-                    (overlap ? Location::kOutputOverlap : Location::kNoOutputOverlap));
+ 
+  if (Primitive::IsFloatingPointType(instruction->GetType())) {
+    locations->SetOut(Location::RequiresFpuRegister());
+  } else {
+    locations->SetOut(Location::RequiresRegister(),
+                      (overlap ? Location::kOutputOverlap : Location::kNoOutputOverlap));
+  }
   if (volatile_for_double) {
     // Arm encoding have some additional constraints for ldrexd/strexd:
     // - registers need to be consecutive
@@ -3139,7 +3148,11 @@ void LocationsBuilderARM::VisitArrayGet(HArrayGet* instruction) {
       new (GetGraph()->GetArena()) LocationSummary(instruction, LocationSummary::kNoCall);
   locations->SetInAt(0, Location::RequiresRegister());
   locations->SetInAt(1, Location::RegisterOrConstant(instruction->InputAt(1)));
-  locations->SetOut(Location::RequiresRegister(), Location::kNoOutputOverlap);
+  if (Primitive::IsFloatingPointType(instruction->GetType())) {
+    locations->SetOut(Location::RequiresFpuRegister(), Location::kNoOutputOverlap);
+  } else {
+    locations->SetOut(Location::RequiresRegister(), Location::kNoOutputOverlap);
+  }
 }
 
 void InstructionCodeGeneratorARM::VisitArrayGet(HArrayGet* instruction) {
@@ -3286,7 +3299,11 @@ void LocationsBuilderARM::VisitArraySet(HArraySet* instruction) {
   } else {
     locations->SetInAt(0, Location::RequiresRegister());
     locations->SetInAt(1, Location::RegisterOrConstant(instruction->InputAt(1)));
-    locations->SetInAt(2, Location::RequiresRegister());
+    if (Primitive::IsFloatingPointType(value_type)) {
+      locations->SetInAt(2, Location::RequiresFpuRegister());
+    } else {
+      locations->SetInAt(2, Location::RequiresRegister());
+    }
 
     if (needs_write_barrier) {
       // Temporary registers for the write barrier.
diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc
index efc41e7c06..4be46126e9 100644
--- a/compiler/optimizing/code_generator_arm64.cc
+++ b/compiler/optimizing/code_generator_arm64.cc
@@ -1250,7 +1250,11 @@ void LocationsBuilderARM64::VisitArrayGet(HArrayGet* instruction) {
       new (GetGraph()->GetArena()) LocationSummary(instruction, LocationSummary::kNoCall);
   locations->SetInAt(0, Location::RequiresRegister());
   locations->SetInAt(1, Location::RegisterOrConstant(instruction->InputAt(1)));
-  locations->SetOut(Location::RequiresRegister());
+  if (Primitive::IsFloatingPointType(instruction->GetType())) {
+    locations->SetOut(Location::RequiresFpuRegister(), Location::kNoOutputOverlap);
+  } else {
+    locations->SetOut(Location::RequiresRegister(), Location::kNoOutputOverlap);
+  }
 }
 
 void InstructionCodeGeneratorARM64::VisitArrayGet(HArrayGet* instruction) {
@@ -1301,7 +1305,11 @@ void LocationsBuilderARM64::VisitArraySet(HArraySet* instruction) {
   } else {
     locations->SetInAt(0, Location::RequiresRegister());
     locations->SetInAt(1, Location::RegisterOrConstant(instruction->InputAt(1)));
-    locations->SetInAt(2, Location::RequiresRegister());
+    if (Primitive::IsFloatingPointType(instruction->InputAt(2)->GetType())) {
+      locations->SetInAt(2, Location::RequiresFpuRegister());
+    } else {
+      locations->SetInAt(2, Location::RequiresRegister());
+    }
   }
 }
 
@@ -1745,7 +1753,11 @@ void LocationsBuilderARM64::VisitInstanceFieldGet(HInstanceFieldGet* instruction
   LocationSummary* locations =
       new (GetGraph()->GetArena()) LocationSummary(instruction, LocationSummary::kNoCall);
   locations->SetInAt(0, Location::RequiresRegister());
-  locations->SetOut(Location::RequiresRegister(), Location::kNoOutputOverlap);
+  if (Primitive::IsFloatingPointType(instruction->GetType())) {
+    locations->SetOut(Location::RequiresFpuRegister());
+  } else {
+    locations->SetOut(Location::RequiresRegister(), Location::kNoOutputOverlap);
+  }
 }
 
 void InstructionCodeGeneratorARM64::VisitInstanceFieldGet(HInstanceFieldGet* instruction) {
@@ -1772,7 +1784,11 @@ void LocationsBuilderARM64::VisitInstanceFieldSet(HInstanceFieldSet* instruction
   LocationSummary* locations =
       new (GetGraph()->GetArena()) LocationSummary(instruction, LocationSummary::kNoCall);
   locations->SetInAt(0, Location::RequiresRegister());
-  locations->SetInAt(1, Location::RequiresRegister());
+  if (Primitive::IsFloatingPointType(instruction->InputAt(1)->GetType())) {
+    locations->SetInAt(1, Location::RequiresFpuRegister());
+  } else {
+    locations->SetInAt(1, Location::RequiresRegister());
+  }
 }
 
 void InstructionCodeGeneratorARM64::VisitInstanceFieldSet(HInstanceFieldSet* instruction) {
@@ -2506,7 +2522,11 @@ void LocationsBuilderARM64::VisitStaticFieldGet(HStaticFieldGet* instruction) {
   LocationSummary* locations =
       new (GetGraph()->GetArena()) LocationSummary(instruction, LocationSummary::kNoCall);
   locations->SetInAt(0, Location::RequiresRegister());
-  locations->SetOut(Location::RequiresRegister(), Location::kNoOutputOverlap);
+  if (Primitive::IsFloatingPointType(instruction->GetType())) {
+    locations->SetOut(Location::RequiresFpuRegister());
+  } else {
+    locations->SetOut(Location::RequiresRegister(), Location::kNoOutputOverlap);
+  }
 }
 
 void InstructionCodeGeneratorARM64::VisitStaticFieldGet(HStaticFieldGet* instruction) {
@@ -2531,7 +2551,11 @@ void LocationsBuilderARM64::VisitStaticFieldSet(HStaticFieldSet* instruction) {
   LocationSummary* locations =
       new (GetGraph()->GetArena()) LocationSummary(instruction, LocationSummary::kNoCall);
   locations->SetInAt(0, Location::RequiresRegister());
-  locations->SetInAt(1, Location::RequiresRegister());
+  if (Primitive::IsFloatingPointType(instruction->InputAt(1)->GetType())) {
+    locations->SetInAt(1, Location::RequiresFpuRegister());
+  } else {
+    locations->SetInAt(1, Location::RequiresRegister());
+  }
 }
 
 void InstructionCodeGeneratorARM64::VisitStaticFieldSet(HStaticFieldSet* instruction) {
diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc
index 879216d59b..0f1175563e 100644
--- a/compiler/optimizing/code_generator_x86.cc
+++ b/compiler/optimizing/code_generator_x86.cc
@@ -3119,11 +3119,15 @@ void LocationsBuilderX86::HandleFieldGet(HInstruction* instruction, const FieldI
       new (GetGraph()->GetArena()) LocationSummary(instruction, LocationSummary::kNoCall);
   locations->SetInAt(0, Location::RequiresRegister());
 
-  // The output overlaps in case of long: we don't want the low move to overwrite
-  // the object's location.
-  locations->SetOut(Location::RequiresRegister(),
-      (instruction->GetType() == Primitive::kPrimLong) ? Location::kOutputOverlap
-                                                       : Location::kNoOutputOverlap);
+  if (Primitive::IsFloatingPointType(instruction->GetType())) {
+    locations->SetOut(Location::RequiresFpuRegister());
+  } else {
+    // The output overlaps in case of long: we don't want the low move to overwrite
+    // the object's location.
+    locations->SetOut(Location::RequiresRegister(),
+        (instruction->GetType() == Primitive::kPrimLong) ? Location::kOutputOverlap
+                                                         : Location::kNoOutputOverlap);
+  }
 
   if (field_info.IsVolatile() && (field_info.GetFieldType() == Primitive::kPrimLong)) {
     // Long values can be loaded atomically into an XMM using movsd.
@@ -3229,6 +3233,8 @@ void LocationsBuilderX86::HandleFieldSet(HInstruction* instruction, const FieldI
   if (is_byte_type) {
     // Ensure the value is in a byte register.
     locations->SetInAt(1, Location::RegisterLocation(EAX));
+  } else if (Primitive::IsFloatingPointType(field_type)) {
+    locations->SetInAt(1, Location::RequiresFpuRegister());
   } else {
     locations->SetInAt(1, Location::RequiresRegister());
   }
@@ -3418,11 +3424,15 @@ void LocationsBuilderX86::VisitArrayGet(HArrayGet* instruction) {
       new (GetGraph()->GetArena()) LocationSummary(instruction, LocationSummary::kNoCall);
   locations->SetInAt(0, Location::RequiresRegister());
   locations->SetInAt(1, Location::RegisterOrConstant(instruction->InputAt(1)));
-  // The output overlaps in case of long: we don't want the low move to overwrite
-  // the array's location.
-  locations->SetOut(Location::RequiresRegister(),
-      (instruction->GetType() == Primitive::kPrimLong) ? Location::kOutputOverlap
-                                                       : Location::kNoOutputOverlap);
+  if (Primitive::IsFloatingPointType(instruction->GetType())) {
+    locations->SetOut(Location::RequiresFpuRegister(), Location::kNoOutputOverlap);
+  } else {
+    // The output overlaps in case of long: we don't want the low move to overwrite
+    // the array's location.
+    locations->SetOut(Location::RequiresRegister(),
+        (instruction->GetType() == Primitive::kPrimLong) ? Location::kOutputOverlap
+                                                         : Location::kNoOutputOverlap);
+  }
 }
 
 void InstructionCodeGeneratorX86::VisitArrayGet(HArrayGet* instruction) {
@@ -3578,14 +3588,10 @@ void LocationsBuilderX86::VisitArraySet(HArraySet* instruction) {
     if (is_byte_type) {
       // Ensure the value is in a byte register.
       locations->SetInAt(2, Location::ByteRegisterOrConstant(EAX, instruction->InputAt(2)));
+    } else if (Primitive::IsFloatingPointType(value_type)) {
+      locations->SetInAt(2, Location::RequiresFpuRegister());
     } else {
-      bool is_fp_type = (value_type == Primitive::kPrimFloat)
-          || (value_type == Primitive::kPrimDouble);
-      if (is_fp_type) {
-        locations->SetInAt(2, Location::RequiresFpuRegister());
-      } else {
-        locations->SetInAt(2, Location::RegisterOrConstant(instruction->InputAt(2)));
-      }
+      locations->SetInAt(2, Location::RegisterOrConstant(instruction->InputAt(2)));
     }
     // Temporary registers for the write barrier.
     if (needs_write_barrier) {
diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc
index a3d3490357..5b681fa62b 100644
--- a/compiler/optimizing/code_generator_x86_64.cc
+++ b/compiler/optimizing/code_generator_x86_64.cc
@@ -3035,7 +3035,11 @@ void LocationsBuilderX86_64::HandleFieldGet(HInstruction* instruction) {
   LocationSummary* locations =
       new (GetGraph()->GetArena()) LocationSummary(instruction, LocationSummary::kNoCall);
   locations->SetInAt(0, Location::RequiresRegister());
-  locations->SetOut(Location::RequiresRegister(), Location::kNoOutputOverlap);
+  if (Primitive::IsFloatingPointType(instruction->GetType())) {
+    locations->SetOut(Location::RequiresFpuRegister());
+  } else {
+    locations->SetOut(Location::RequiresRegister(), Location::kNoOutputOverlap);
+  }
 }
 
 void InstructionCodeGeneratorX86_64::HandleFieldGet(HInstruction* instruction,
@@ -3113,7 +3117,11 @@ void LocationsBuilderX86_64::HandleFieldSet(HInstruction* instruction,
       CodeGenerator::StoreNeedsWriteBarrier(field_info.GetFieldType(), instruction->InputAt(1));
 
   locations->SetInAt(0, Location::RequiresRegister());
-  locations->SetInAt(1, Location::RequiresRegister());
+  if (Primitive::IsFloatingPointType(instruction->InputAt(1)->GetType())) {
+    locations->SetInAt(1, Location::RequiresFpuRegister());
+  } else {
+    locations->SetInAt(1, Location::RequiresRegister());
+  }
   if (needs_write_barrier) {
     // Temporary registers for the write barrier.
     locations->AddTemp(Location::RequiresRegister());
@@ -3277,7 +3285,11 @@ void LocationsBuilderX86_64::VisitArrayGet(HArrayGet* instruction) {
   locations->SetInAt(0, Location::RequiresRegister());
   locations->SetInAt(
       1, Location::RegisterOrConstant(instruction->InputAt(1)));
-  locations->SetOut(Location::RequiresRegister(), Location::kNoOutputOverlap);
+  if (Primitive::IsFloatingPointType(instruction->GetType())) {
+    locations->SetOut(Location::RequiresFpuRegister(), Location::kNoOutputOverlap);
+  } else {
+    locations->SetOut(Location::RequiresRegister(), Location::kNoOutputOverlap);
+  }
 }
 
 void InstructionCodeGeneratorX86_64::VisitArrayGet(HArrayGet* instruction) {