Fix type conversion bug
Rationale:
Not simplifying double redundant AND
crashes a DCHECK due to order in which
rewrites are done.
Test: test-art-host test-art-target
Bug: 68142795
Change-Id: Ic14340ed336e6c163f9bd7aebd96f6cf41794c7e
diff --git a/compiler/optimizing/data_type.h b/compiler/optimizing/data_type.h
index 3b67efe..75a7fbe 100644
--- a/compiler/optimizing/data_type.h
+++ b/compiler/optimizing/data_type.h
@@ -123,7 +123,7 @@
}
static bool IsUnsignedType(Type type) {
- return type == Type::kUint8 || type == Type::kUint16;
+ return type == Type::kBool || type == Type::kUint8 || type == Type::kUint16;
}
// Return the general kind of `type`, fusing integer-like types as Type::kInt.
diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc
index d81a752..189d5ae 100644
--- a/compiler/optimizing/instruction_simplifier.cc
+++ b/compiler/optimizing/instruction_simplifier.cc
@@ -1044,12 +1044,14 @@
}
static bool IsTypeConversionLossless(DataType::Type input_type, DataType::Type result_type) {
+ // Make sure all implicit conversions have been simplified and no new ones have been introduced.
+ DCHECK(!DataType::IsTypeConversionImplicit(input_type, result_type))
+ << input_type << "," << result_type;
// The conversion to a larger type is loss-less with the exception of two cases,
// - conversion to the unsigned type Uint16, where we may lose some bits, and
// - conversion from float to long, the only FP to integral conversion with smaller FP type.
// For integral to FP conversions this holds because the FP mantissa is large enough.
// Note: The size check excludes Uint8 as the result type.
- DCHECK(!DataType::IsTypeConversionImplicit(input_type, result_type));
return DataType::Size(result_type) > DataType::Size(input_type) &&
result_type != DataType::Type::kUint16 &&
!(result_type == DataType::Type::kInt64 && input_type == DataType::Type::kFloat32);
@@ -1253,7 +1255,10 @@
if (input_cst != nullptr) {
int64_t value = Int64FromConstant(input_cst);
- if (value == -1) {
+ if (value == -1 ||
+ // Similar cases under zero extension.
+ (DataType::IsUnsignedType(input_other->GetType()) &&
+ ((DataType::MaxValueOfIntegralType(input_other->GetType()) & ~value) == 0))) {
// Replace code looking like
// AND dst, src, 0xFFF...FF
// with
@@ -1332,6 +1337,9 @@
TryReplaceFieldOrArrayGetType(input_other, new_type)) {
instruction->ReplaceWith(input_other);
instruction->GetBlock()->RemoveInstruction(instruction);
+ } else if (DataType::IsTypeConversionImplicit(input_other->GetType(), new_type)) {
+ instruction->ReplaceWith(input_other);
+ instruction->GetBlock()->RemoveInstruction(instruction);
} else {
HTypeConversion* type_conversion = new (GetGraph()->GetAllocator()) HTypeConversion(
new_type, input_other, instruction->GetDexPc());
diff --git a/test/458-checker-instruct-simplification/src/Main.java b/test/458-checker-instruct-simplification/src/Main.java
index 262d2c1..7797f31 100644
--- a/test/458-checker-instruct-simplification/src/Main.java
+++ b/test/458-checker-instruct-simplification/src/Main.java
@@ -2529,15 +2529,79 @@
/// CHECK-DAG: <<And:i\d+>> And [<<Get>>,<<Cst1ffff>>]
/// CHECK-DAG: Return [<<And>>]
- // TODO: Simplify this. The And is useless.
-
- // CHECK-START: int Main.$noinline$getInstanceCharFieldAnd0x1ffff(Main) instruction_simplifier (after)
- // CHECK-DAG: <<Get:c\d+>> InstanceFieldGet
- // CHECK-DAG: Return [<<Get>>]
+ /// CHECK-START: int Main.$noinline$getInstanceCharFieldAnd0x1ffff(Main) instruction_simplifier (after)
+ /// CHECK-DAG: <<Get:c\d+>> InstanceFieldGet
+ /// CHECK-DAG: Return [<<Get>>]
public static int $noinline$getInstanceCharFieldAnd0x1ffff(Main m) {
return m.instanceCharField & 0x1ffff;
}
+ /// CHECK-START: int Main.$noinline$bug68142795Byte(byte) instruction_simplifier (before)
+ /// CHECK-DAG: <<Arg:b\d+>> ParameterValue
+ /// CHECK-DAG: <<Const:i\d+>> IntConstant 255
+ /// CHECK-DAG: <<And1:i\d+>> And [<<Arg>>,<<Const>>]
+ /// CHECK-DAG: <<And2:i\d+>> And [<<And1>>,<<Const>>]
+ /// CHECK-DAG: <<Conv:b\d+>> TypeConversion [<<And2>>]
+ /// CHECK-DAG: Return [<<Conv>>]
+
+ /// CHECK-START: int Main.$noinline$bug68142795Byte(byte) instruction_simplifier (after)
+ /// CHECK-DAG: <<Arg:b\d+>> ParameterValue
+ /// CHECK-DAG: Return [<<Arg>>]
+ public static int $noinline$bug68142795Byte(byte b) {
+ return (byte)(0xff & (b & 0xff));
+ }
+
+ /// CHECK-START: int Main.$noinline$bug68142795Short(short) instruction_simplifier (before)
+ /// CHECK-DAG: <<Arg:s\d+>> ParameterValue
+ /// CHECK-DAG: <<Const:i\d+>> IntConstant 65535
+ /// CHECK-DAG: <<And1:i\d+>> And [<<Arg>>,<<Const>>]
+ /// CHECK-DAG: <<And2:i\d+>> And [<<And1>>,<<Const>>]
+ /// CHECK-DAG: <<Conv:s\d+>> TypeConversion [<<And2>>]
+ /// CHECK-DAG: Return [<<Conv>>]
+
+ /// CHECK-START: int Main.$noinline$bug68142795Short(short) instruction_simplifier (after)
+ /// CHECK-DAG: <<Arg:s\d+>> ParameterValue
+ /// CHECK-DAG: Return [<<Arg>>]
+ public static int $noinline$bug68142795Short(short s) {
+ return (short)(0xffff & (s & 0xffff));
+ }
+
+ /// CHECK-START: int Main.$noinline$bug68142795Boolean(boolean) instruction_simplifier$after_inlining (before)
+ /// CHECK-DAG: <<Arg:z\d+>> ParameterValue
+ /// CHECK-DAG: <<Const0:i\d+>> IntConstant 0
+ /// CHECK-DAG: <<Const1:i\d+>> IntConstant 1
+ /// CHECK-DAG: <<Const255:i\d+>> IntConstant 255
+ /// CHECK-DAG: <<Select:i\d+>> Select [<<Const0>>,<<Const1>>,<<Arg>>]
+ /// CHECK-DAG: <<And:i\d+>> And [<<Const255>>,<<Select>>]
+ /// CHECK-DAG: <<Conv:b\d+>> TypeConversion [<<And>>]
+ /// CHECK-DAG: Return [<<Conv>>]
+
+ /// CHECK-START: int Main.$noinline$bug68142795Boolean(boolean) instruction_simplifier$after_inlining (after)
+ /// CHECK-DAG: <<Arg:z\d+>> ParameterValue
+ /// CHECK-DAG: Return [<<Arg>>]
+ public static int $noinline$bug68142795Boolean(boolean b) {
+ int v = b ? 1 : 0; // Should be simplified to "b" after inlining.
+ return (byte)($inline$get255() & v);
+ }
+
+ /// CHECK-START: int Main.$noinline$bug68142795Elaborate(byte) instruction_simplifier (before)
+ /// CHECK-DAG: <<Arg:b\d+>> ParameterValue
+ /// CHECK-DAG: <<Int255:i\d+>> IntConstant 255
+ /// CHECK-DAG: <<Long255:j\d+>> LongConstant 255
+ /// CHECK-DAG: <<And1:i\d+>> And [<<Arg>>,<<Int255>>]
+ /// CHECK-DAG: <<Conv1:j\d+>> TypeConversion [<<And1>>]
+ /// CHECK-DAG: <<And2:j\d+>> And [<<Conv1>>,<<Long255>>]
+ /// CHECK-DAG: <<Conv2:i\d+>> TypeConversion [<<And2>>]
+ /// CHECK-DAG: <<Conv3:b\d+>> TypeConversion [<<Conv2>>]
+ /// CHECK-DAG: Return [<<Conv3>>]
+
+ /// CHECK-START: int Main.$noinline$bug68142795Elaborate(byte) instruction_simplifier (after)
+ /// CHECK-DAG: <<Arg:b\d+>> ParameterValue
+ /// CHECK-DAG: Return [<<Arg>>]
+ public static int $noinline$bug68142795Elaborate(byte b) {
+ return (byte)((int)(((long)(b & 0xff)) & 255L));
+ }
+
public static void main(String[] args) {
int arg = 123456;
float floatArg = 123456.125f;
@@ -2772,10 +2836,20 @@
m.instanceCharField = 'x';
assertIntEquals('x', $noinline$getInstanceCharFieldAnd0x1ffff(m));
+
+ assertIntEquals(0x7f, $noinline$bug68142795Byte((byte) 0x7f));
+ assertIntEquals((byte) 0x80, $noinline$bug68142795Byte((byte) 0x80));
+ assertIntEquals(0x7fff, $noinline$bug68142795Short((short) 0x7fff));
+ assertIntEquals((short) 0x8000, $noinline$bug68142795Short((short) 0x8000));
+ assertIntEquals(0, $noinline$bug68142795Boolean(false));
+ assertIntEquals(1, $noinline$bug68142795Boolean(true));
+ assertIntEquals(0x7f, $noinline$bug68142795Elaborate((byte) 0x7f));
+ assertIntEquals((byte) 0x80, $noinline$bug68142795Elaborate((byte) 0x80));
}
private static boolean $inline$true() { return true; }
private static boolean $inline$false() { return false; }
+ private static int $inline$get255() { return 255; }
public static boolean booleanField;