Merge "Small refactor of MIN/MAX compiler code."

3 files changed, 33 insertions, 103 deletions

diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc
index 5f0533cbe9..8aa790db34 100644
--- a/compiler/optimizing/code_generator_arm64.cc
+++ b/compiler/optimizing/code_generator_arm64.cc
@@ -2459,6 +2459,9 @@ void InstructionCodeGeneratorARM64::HandleBinaryOp(HBinaryOperation* instr) {
           // all & reg_bits - 1.
           __ Ror(dst, lhs, RegisterFrom(instr->GetLocations()->InAt(1), type));
         }
+      } else if (instr->IsMin() || instr->IsMax()) {
+          __ Cmp(lhs, rhs);
+          __ Csel(dst, lhs, rhs, instr->IsMin() ? lt : gt);
       } else {
         DCHECK(instr->IsXor());
         __ Eor(dst, lhs, rhs);
@@ -2474,6 +2477,10 @@ void InstructionCodeGeneratorARM64::HandleBinaryOp(HBinaryOperation* instr) {
         __ Fadd(dst, lhs, rhs);
       } else if (instr->IsSub()) {
         __ Fsub(dst, lhs, rhs);
+      } else if (instr->IsMin()) {
+        __ Fmin(dst, lhs, rhs);
+      } else if (instr->IsMax()) {
+        __ Fmax(dst, lhs, rhs);
       } else {
         LOG(FATAL) << "Unexpected floating-point binary operation";
       }
@@ -5671,111 +5678,20 @@ void InstructionCodeGeneratorARM64::VisitRem(HRem* rem) {
   }
 }
 
-// TODO: integrate with HandleBinaryOp?
-static void CreateMinMaxLocations(ArenaAllocator* allocator, HBinaryOperation* minmax) {
-  LocationSummary* locations = new (allocator) LocationSummary(minmax);
-  switch (minmax->GetResultType()) {
-    case DataType::Type::kInt32:
-    case DataType::Type::kInt64:
-      locations->SetInAt(0, Location::RequiresRegister());
-      locations->SetInAt(1, Location::RequiresRegister());
-      locations->SetOut(Location::RequiresRegister(), Location::kNoOutputOverlap);
-      break;
-    case DataType::Type::kFloat32:
-    case DataType::Type::kFloat64:
-      locations->SetInAt(0, Location::RequiresFpuRegister());
-      locations->SetInAt(1, Location::RequiresFpuRegister());
-      locations->SetOut(Location::RequiresFpuRegister(), Location::kNoOutputOverlap);
-      break;
-    default:
-      LOG(FATAL) << "Unexpected type for HMinMax " << minmax->GetResultType();
-  }
-}
-
-void InstructionCodeGeneratorARM64::GenerateMinMaxInt(LocationSummary* locations,
-                                                      bool is_min,
-                                                      DataType::Type type) {
-  Location op1 = locations->InAt(0);
-  Location op2 = locations->InAt(1);
-  Location out = locations->Out();
-
-  Register op1_reg;
-  Register op2_reg;
-  Register out_reg;
-  if (type == DataType::Type::kInt64) {
-    op1_reg = XRegisterFrom(op1);
-    op2_reg = XRegisterFrom(op2);
-    out_reg = XRegisterFrom(out);
-  } else {
-    DCHECK_EQ(type, DataType::Type::kInt32);
-    op1_reg = WRegisterFrom(op1);
-    op2_reg = WRegisterFrom(op2);
-    out_reg = WRegisterFrom(out);
-  }
-
-  __ Cmp(op1_reg, op2_reg);
-  __ Csel(out_reg, op1_reg, op2_reg, is_min ? lt : gt);
-}
-
-void InstructionCodeGeneratorARM64::GenerateMinMaxFP(LocationSummary* locations,
-                                                     bool is_min,
-                                                     DataType::Type type) {
-  Location op1 = locations->InAt(0);
-  Location op2 = locations->InAt(1);
-  Location out = locations->Out();
-
-  FPRegister op1_reg;
-  FPRegister op2_reg;
-  FPRegister out_reg;
-  if (type == DataType::Type::kFloat64) {
-    op1_reg = DRegisterFrom(op1);
-    op2_reg = DRegisterFrom(op2);
-    out_reg = DRegisterFrom(out);
-  } else {
-    DCHECK_EQ(type, DataType::Type::kFloat32);
-    op1_reg = SRegisterFrom(op1);
-    op2_reg = SRegisterFrom(op2);
-    out_reg = SRegisterFrom(out);
-  }
-
-  if (is_min) {
-    __ Fmin(out_reg, op1_reg, op2_reg);
-  } else {
-    __ Fmax(out_reg, op1_reg, op2_reg);
-  }
-}
-
-// TODO: integrate with HandleBinaryOp?
-void InstructionCodeGeneratorARM64::GenerateMinMax(HBinaryOperation* minmax, bool is_min) {
-  DataType::Type type = minmax->GetResultType();
-  switch (type) {
-    case DataType::Type::kInt32:
-    case DataType::Type::kInt64:
-      GenerateMinMaxInt(minmax->GetLocations(), is_min, type);
-      break;
-    case DataType::Type::kFloat32:
-    case DataType::Type::kFloat64:
-      GenerateMinMaxFP(minmax->GetLocations(), is_min, type);
-      break;
-    default:
-      LOG(FATAL) << "Unexpected type for HMinMax " << type;
-  }
-}
-
 void LocationsBuilderARM64::VisitMin(HMin* min) {
-  CreateMinMaxLocations(GetGraph()->GetAllocator(), min);
+  HandleBinaryOp(min);
 }
 
 void InstructionCodeGeneratorARM64::VisitMin(HMin* min) {
-  GenerateMinMax(min, /*is_min*/ true);
+  HandleBinaryOp(min);
 }
 
 void LocationsBuilderARM64::VisitMax(HMax* max) {
-  CreateMinMaxLocations(GetGraph()->GetAllocator(), max);
+  HandleBinaryOp(max);
 }
 
 void InstructionCodeGeneratorARM64::VisitMax(HMax* max) {
-  GenerateMinMax(max, /*is_min*/ false);
+  HandleBinaryOp(max);
 }
 
 void LocationsBuilderARM64::VisitAbs(HAbs* abs) {
diff --git a/compiler/optimizing/code_generator_arm64.h b/compiler/optimizing/code_generator_arm64.h
index e7fe5b71b7..5afb712d17 100644
--- a/compiler/optimizing/code_generator_arm64.h
+++ b/compiler/optimizing/code_generator_arm64.h
@@ -280,10 +280,6 @@ class InstructionCodeGeneratorARM64 : public InstructionCodeGenerator {
   void HandleFieldGet(HInstruction* instruction, const FieldInfo& field_info);
   void HandleCondition(HCondition* instruction);
 
-  void GenerateMinMaxInt(LocationSummary* locations, bool is_min, DataType::Type type);
-  void GenerateMinMaxFP(LocationSummary* locations, bool is_min, DataType::Type type);
-  void GenerateMinMax(HBinaryOperation* minmax, bool is_min);
-
   // Generate a heap reference load using one register `out`:
   //
   //   out <- *(out + offset)
diff --git a/compiler/optimizing/common_arm64.h b/compiler/optimizing/common_arm64.h
index ed2f8e995d..5191ee2b1e 100644
--- a/compiler/optimizing/common_arm64.h
+++ b/compiler/optimizing/common_arm64.h
@@ -234,6 +234,13 @@ inline vixl::aarch64::Operand OperandFromMemOperand(
   }
 }
 
+inline bool AddSubCanEncodeAsImmediate(int64_t value) {
+  // If `value` does not fit but `-value` does, VIXL will automatically use
+  // the 'opposite' instruction.
+  return vixl::aarch64::Assembler::IsImmAddSub(value)
+      || vixl::aarch64::Assembler::IsImmAddSub(-value);
+}
+
 inline bool Arm64CanEncodeConstantAsImmediate(HConstant* constant, HInstruction* instr) {
   int64_t value = CodeGenerator::GetInt64ValueOf(constant);
 
@@ -249,6 +256,20 @@ inline bool Arm64CanEncodeConstantAsImmediate(HConstant* constant, HInstruction*
     return IsUint<8>(value);
   }
 
+  // Code generation for Min/Max:
+  //    Cmp left_op, right_op
+  //    Csel dst, left_op, right_op, cond
+  if (instr->IsMin() || instr->IsMax()) {
+    if (constant->GetUses().HasExactlyOneElement()) {
+      // If value can be encoded as immediate for the Cmp, then let VIXL handle
+      // the constant generation for the Csel.
+      return AddSubCanEncodeAsImmediate(value);
+    }
+    // These values are encodable as immediates for Cmp and VIXL will use csinc and csinv
+    // with the zr register as right_op, hence no constant generation is required.
+    return constant->IsZeroBitPattern() || constant->IsOne() || constant->IsMinusOne();
+  }
+
   // For single uses we let VIXL handle the constant generation since it will
   // use registers that are not managed by the register allocator (wip0, wip1).
   if (constant->GetUses().HasExactlyOneElement()) {
@@ -275,10 +296,7 @@ inline bool Arm64CanEncodeConstantAsImmediate(HConstant* constant, HInstruction*
            instr->IsSub())
         << instr->DebugName();
     // Uses aliases of ADD/SUB instructions.
-    // If `value` does not fit but `-value` does, VIXL will automatically use
-    // the 'opposite' instruction.
-    return vixl::aarch64::Assembler::IsImmAddSub(value)
-        || vixl::aarch64::Assembler::IsImmAddSub(-value);
+    return AddSubCanEncodeAsImmediate(value);
   }
 }