Small refactor of MIN/MAX compiler code.
Integrate instruction code generation and location creation with
HandleBinaryOp. Code generation has been improved for constant
inputs 0, 1 and -1.
Test: 679-checker-minmax
Test: test-art-host, test-art-target.
Change-Id: Ib34eb8a4b29d22a2491d21656e1f64011ef9f986
diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc
index 5f0533c..8aa790db 100644
--- a/compiler/optimizing/code_generator_arm64.cc
+++ b/compiler/optimizing/code_generator_arm64.cc
@@ -2459,6 +2459,9 @@
// 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 @@
__ 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 @@
}
}
-// 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 e7fe5b7..5afb712 100644
--- a/compiler/optimizing/code_generator_arm64.h
+++ b/compiler/optimizing/code_generator_arm64.h
@@ -280,10 +280,6 @@
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 ed2f8e9..5191ee2 100644
--- a/compiler/optimizing/common_arm64.h
+++ b/compiler/optimizing/common_arm64.h
@@ -234,6 +234,13 @@
}
}
+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 @@
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 @@
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);
}
}
diff --git a/test/679-checker-minmax/src/Main.java b/test/679-checker-minmax/src/Main.java
index abf8c27..4b72656 100644
--- a/test/679-checker-minmax/src/Main.java
+++ b/test/679-checker-minmax/src/Main.java
@@ -37,6 +37,13 @@
//
/// CHECK-START: int Main.minI(int) instruction_simplifier (after)
/// CHECK-NOT: InvokeStaticOrDirect
+ //
+ /// CHECK-START-ARM64: int Main.minI(int) disassembly (after)
+ /// CHECK-NOT: mov {{w\d+}}, #0x14
+ /// CHECK: cmp {{w\d+}}, #0x14
+ // Check that the constant generation was handled by VIXL.
+ /// CHECK: mov w16, #0x14
+ /// CHECK: csel {{w\d+}}, {{w\d+}}, w16, lt
public static int minI(int a) {
return Math.min(a, 20);
}
@@ -55,6 +62,13 @@
//
/// CHECK-START: long Main.minL(long) instruction_simplifier (after)
/// CHECK-NOT: InvokeStaticOrDirect
+ //
+ /// CHECK-START-ARM64: long Main.minL(long) disassembly (after)
+ /// CHECK-NOT: mov {{x\d+}}, #0x14
+ /// CHECK: cmp {{x\d+}}, #0x14
+ // Check that the constant generation was handled by VIXL.
+ /// CHECK: mov x16, #0x14
+ /// CHECK: csel {{x\d+}}, {{x\d+}}, x16, lt
public static long minL(long a) {
return Math.min(a, 20L);
}
@@ -73,6 +87,13 @@
//
/// CHECK-START: int Main.maxI(int) instruction_simplifier (after)
/// CHECK-NOT: InvokeStaticOrDirect
+ //
+ /// CHECK-START-ARM64: int Main.maxI(int) disassembly (after)
+ /// CHECK-NOT: mov {{w\d+}}, #0x14
+ /// CHECK: cmp {{w\d+}}, #0x14
+ // Check that the constant generation was handled by VIXL.
+ /// CHECK: mov w16, #0x14
+ /// CHECK: csel {{w\d+}}, {{w\d+}}, w16, gt
public static int maxI(int a) {
return Math.max(a, 20);
}
@@ -91,11 +112,166 @@
//
/// CHECK-START: long Main.maxL(long) instruction_simplifier (after)
/// CHECK-NOT: InvokeStaticOrDirect
+ //
+ /// CHECK-START-ARM64: long Main.maxL(long) disassembly (after)
+ /// CHECK-NOT: mov {{x\d+}}, #0x14
+ /// CHECK: cmp {{x\d+}}, #0x14
+ // Check that the constant generation was handled by VIXL.
+ /// CHECK: mov x16, #0x14
+ /// CHECK: csel {{x\d+}}, {{x\d+}}, x16, gt
public static long maxL(long a) {
return Math.max(a, 20L);
}
//
+ // Special Cases
+ //
+
+ /// CHECK-START-ARM64: int Main.minIntConstantZero(int) disassembly (after)
+ /// CHECK-NOT: InvokeStaticOrDirect
+ /// CHECK-NOT: mov {{w\d+}}, #0x0
+ /// CHECK: cmp {{w\d+}}, #0x0 (0)
+ /// CHECK: csel {{w\d+}}, {{w\d+}}, wzr, lt
+ /// CHECK: ret
+ public static int minIntConstantZero(int a) {
+ return Math.min(a, 0);
+ }
+
+ /// CHECK-START-ARM64: int Main.minIntConstantOne(int) disassembly (after)
+ /// CHECK-NOT: InvokeStaticOrDirect
+ /// CHECK-NOT: mov {{w\d+}}, #0x1
+ /// CHECK: cmp {{w\d+}}, #0x1 (1)
+ /// CHECK: csinc {{w\d+}}, {{w\d+}}, wzr, lt
+ /// CHECK: ret
+ public static int minIntConstantOne(int a) {
+ return Math.min(a, 1);
+ }
+
+ /// CHECK-START-ARM64: int Main.minIntConstantMinusOne(int) disassembly (after)
+ /// CHECK-NOT: InvokeStaticOrDirect
+ /// CHECK-NOT: mov {{w\d+}}, #0xffffffff
+ /// CHECK: cmn {{w\d+}}, #0x1 (1)
+ /// CHECK: csinv {{w\d+}}, {{w\d+}}, wzr, lt
+ /// CHECK: ret
+ public static int minIntConstantMinusOne(int a) {
+ return Math.min(a, -1);
+ }
+
+ /// CHECK-START-ARM64: long Main.minLongConstantZero(long) disassembly (after)
+ /// CHECK-NOT: InvokeStaticOrDirect
+ /// CHECK-NOT: mov {{x\d+}}, #0x0
+ /// CHECK: cmp {{x\d+}}, #0x0 (0)
+ /// CHECK: csel {{x\d+}}, {{x\d+}}, xzr, lt
+ /// CHECK: ret
+ public static long minLongConstantZero(long a) {
+ return Math.min(a, 0L);
+ }
+
+ /// CHECK-START-ARM64: long Main.minLongConstantOne(long) disassembly (after)
+ /// CHECK-NOT: InvokeStaticOrDirect
+ /// CHECK-NOT: mov {{x\d+}}, #0x1
+ /// CHECK: cmp {{x\d+}}, #0x1 (1)
+ /// CHECK: csinc {{x\d+}}, {{x\d+}}, xzr, lt
+ /// CHECK: ret
+ public static long minLongConstantOne(long a) {
+ return Math.min(a, 1L);
+ }
+
+ /// CHECK-START-ARM64: long Main.minLongConstantMinusOne(long) disassembly (after)
+ /// CHECK-NOT: InvokeStaticOrDirect
+ /// CHECK-NOT: mov {{x\d+}}, #0xffffffffffffffff
+ /// CHECK: cmn {{x\d+}}, #0x1 (1)
+ /// CHECK: csinv {{x\d+}}, {{x\d+}}, xzr, lt
+ /// CHECK: ret
+ public static long minLongConstantMinusOne(long a) {
+ return Math.min(a, -1L);
+ }
+
+ /// CHECK-START-ARM64: int Main.maxIntConstantZero(int) disassembly (after)
+ /// CHECK-NOT: InvokeStaticOrDirect
+ /// CHECK-NOT: mov {{w\d+}}, #0x0
+ /// CHECK: cmp {{w\d+}}, #0x0 (0)
+ /// CHECK: csel {{w\d+}}, {{w\d+}}, wzr, gt
+ /// CHECK: ret
+ public static int maxIntConstantZero(int a) {
+ return Math.max(a, 0);
+ }
+
+ /// CHECK-START-ARM64: int Main.maxIntConstantOne(int) disassembly (after)
+ /// CHECK-NOT: InvokeStaticOrDirect
+ /// CHECK-NOT: mov {{w\d+}}, #0x1
+ /// CHECK: cmp {{w\d+}}, #0x1 (1)
+ /// CHECK: csinc {{w\d+}}, {{w\d+}}, wzr, gt
+ /// CHECK: ret
+ public static int maxIntConstantOne(int a) {
+ return Math.max(a, 1);
+ }
+
+ /// CHECK-START-ARM64: int Main.maxIntConstantMinusOne(int) disassembly (after)
+ /// CHECK-NOT: InvokeStaticOrDirect
+ /// CHECK-NOT: mov {{w\d+}}, #0xffffffff
+ /// CHECK: cmn {{w\d+}}, #0x1 (1)
+ /// CHECK: csinv {{w\d+}}, {{w\d+}}, wzr, gt
+ /// CHECK: ret
+ public static int maxIntConstantMinusOne(int a) {
+ return Math.max(a, -1);
+ }
+
+ /// CHECK-START-ARM64: int Main.maxIntLargeConstant(int) disassembly (after)
+ /// CHECK-NOT: InvokeStaticOrDirect
+ /// CHECK: mov {{w\d+}}, #0x2001
+ /// CHECK: cmp {{w\d+}}, {{w\d+}}
+ // Check that constant generation was not handled by VIXL.
+ /// CHECK-NOT: mov {{w\d+}}, #0x2001
+ /// CHECK: csel {{w\d+}}, {{w\d+}}, {{w\d+}}, gt
+ /// CHECK: ret
+ public static int maxIntLargeConstant(int a) {
+ return Math.max(a, 8193);
+ }
+
+ /// CHECK-START-ARM64: long Main.maxLongConstantZero(long) disassembly (after)
+ /// CHECK-NOT: InvokeStaticOrDirect
+ /// CHECK-NOT: mov {{x\d+}}, #0x0
+ /// CHECK: cmp {{x\d+}}, #0x0 (0)
+ /// CHECK: csel {{x\d+}}, {{x\d+}}, xzr, gt
+ /// CHECK: ret
+ public static long maxLongConstantZero(long a) {
+ return Math.max(a, 0L);
+ }
+
+ /// CHECK-START-ARM64: long Main.maxLongConstantOne(long) disassembly (after)
+ /// CHECK-NOT: InvokeStaticOrDirect
+ /// CHECK-NOT: mov {{x\d+}}, #0x1
+ /// CHECK: cmp {{x\d+}}, #0x1 (1)
+ /// CHECK: csinc {{x\d+}}, {{x\d+}}, xzr, gt
+ /// CHECK: ret
+ public static long maxLongConstantOne(long a) {
+ return Math.max(a, 1L);
+ }
+
+ /// CHECK-START-ARM64: long Main.maxLongConstantMinusOne(long) disassembly (after)
+ /// CHECK-NOT: InvokeStaticOrDirect
+ /// CHECK-NOT: mov {{x\d+}}, #0xffffffffffffffff
+ /// CHECK: cmn {{x\d+}}, #0x1 (1)
+ /// CHECK: csinv {{x\d+}}, {{x\d+}}, xzr, gt
+ /// CHECK: ret
+ public static long maxLongConstantMinusOne(long a) {
+ return Math.max(a, -1L);
+ }
+
+ /// CHECK-START-ARM64: long Main.maxLongLargeConstant(long) disassembly (after)
+ /// CHECK-NOT: InvokeStaticOrDirect
+ /// CHECK: mov {{x\d+}}, #0x2001
+ /// CHECK: cmp {{x\d+}}, {{x\d+}}
+ // Check that constant generation was not handled by VIXL.
+ /// CHECK-NOT: mov {{x\d+}}, #0x2001
+ /// CHECK: csel {{x\d+}}, {{x\d+}}, {{x\d+}}, gt
+ /// CHECK: ret
+ public static long maxLongLargeConstant(long a) {
+ return Math.max(a, 8193L);
+ }
+
+ //
// Different types.
//
@@ -538,12 +714,40 @@
// Intrinsics.
expectEquals(10, minI(10));
expectEquals(20, minI(25));
+ expectEquals(-1, minIntConstantZero(-1));
+ expectEquals(0, minIntConstantZero(1));
+ expectEquals(0, minIntConstantOne(0));
+ expectEquals(1, minIntConstantOne(2));
+ expectEquals(-2, minIntConstantMinusOne(-2));
+ expectEquals(-1, minIntConstantMinusOne(0));
expectEquals(10L, minL(10L));
expectEquals(20L, minL(25L));
+ expectEquals(-1L, minLongConstantZero(-1L));
+ expectEquals(0L, minLongConstantZero(1L));
+ expectEquals(0L, minLongConstantOne(0L));
+ expectEquals(1L, minLongConstantOne(2L));
+ expectEquals(-2L, minLongConstantMinusOne(-2L));
+ expectEquals(-1L, minLongConstantMinusOne(0L));
expectEquals(20, maxI(10));
expectEquals(25, maxI(25));
+ expectEquals(0, maxIntConstantZero(-1));
+ expectEquals(1, maxIntConstantZero(1));
+ expectEquals(1, maxIntConstantOne(0));
+ expectEquals(2, maxIntConstantOne(2));
+ expectEquals(-1, maxIntConstantMinusOne(-2));
+ expectEquals(0, maxIntConstantMinusOne(0));
+ expectEquals(8193, maxIntLargeConstant(8192));
+ expectEquals(9000, maxIntLargeConstant(9000));
expectEquals(20L, maxL(10L));
expectEquals(25L, maxL(25L));
+ expectEquals(0L, maxLongConstantZero(-1L));
+ expectEquals(1L, maxLongConstantZero(1L));
+ expectEquals(1L, maxLongConstantOne(0L));
+ expectEquals(2L, maxLongConstantOne(2L));
+ expectEquals(-1L, maxLongConstantMinusOne(-2L));
+ expectEquals(0L, maxLongConstantMinusOne(0L));
+ expectEquals(8193L, maxLongLargeConstant(8192L));
+ expectEquals(9000L, maxLongLargeConstant(9000L));
// Types.
expectEquals(10, min1(10, 20));
expectEquals(10, min2(10, 20));