ART: Fix bug in LSE
LSE will not remove a load if the type of the heap value does not
match the type of the load. This was a workaround for b/22538329 but
backfires for integers. For example, 'IntConstant 0' has type int
but can be retrieved from a boolean field. The corresponding store is
removed but not the load, loading uninitialized memory. This fixes the
issue until the workaround is not needed any more.
Change-Id: I2a47783e8d5f93104854e5216b69b6c220832c76
diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc
index adde004..2a2221a 100644
--- a/compiler/optimizing/load_store_elimination.cc
+++ b/compiler/optimizing/load_store_elimination.cc
@@ -655,6 +655,16 @@
}
}
+ static bool IsIntFloatAlias(Primitive::Type type1, Primitive::Type type2) {
+ return (type1 == Primitive::kPrimFloat && type2 == Primitive::kPrimInt) ||
+ (type2 == Primitive::kPrimFloat && type1 == Primitive::kPrimInt);
+ }
+
+ static bool IsLongDoubleAlias(Primitive::Type type1, Primitive::Type type2) {
+ return (type1 == Primitive::kPrimDouble && type2 == Primitive::kPrimLong) ||
+ (type2 == Primitive::kPrimDouble && type1 == Primitive::kPrimLong);
+ }
+
void VisitGetLocation(HInstruction* instruction,
HInstruction* ref,
size_t offset,
@@ -686,7 +696,8 @@
if ((heap_value != kUnknownHeapValue) &&
// Keep the load due to possible I/F, J/D array aliasing.
// See b/22538329 for details.
- (heap_value->GetType() == instruction->GetType())) {
+ !IsIntFloatAlias(heap_value->GetType(), instruction->GetType()) &&
+ !IsLongDoubleAlias(heap_value->GetType(), instruction->GetType())) {
removed_loads_.push_back(instruction);
substitute_instructions_for_loads_.push_back(heap_value);
TryRemovingNullCheck(instruction);
diff --git a/test/530-checker-lse/src/Main.java b/test/530-checker-lse/src/Main.java
index 98251e4..7d89372 100644
--- a/test/530-checker-lse/src/Main.java
+++ b/test/530-checker-lse/src/Main.java
@@ -52,6 +52,11 @@
int j;
}
+class TestClass3 {
+ float floatField = 8.0f;
+ boolean test1 = true;
+}
+
class Finalizable {
static boolean sVisited = false;
static final int VALUE = 0xbeef;
@@ -577,6 +582,37 @@
return obj.i;
}
+ /// CHECK-START: float Main.test24() load_store_elimination (before)
+ /// CHECK-DAG: <<True:i\d+>> IntConstant 1
+ /// CHECK-DAG: <<Float8:f\d+>> FloatConstant 8
+ /// CHECK-DAG: <<Float42:f\d+>> FloatConstant 42
+ /// CHECK-DAG: <<Obj:l\d+>> NewInstance
+ /// CHECK-DAG: InstanceFieldSet [<<Obj>>,<<True>>]
+ /// CHECK-DAG: InstanceFieldSet [<<Obj>>,<<Float8>>]
+ /// CHECK-DAG: <<GetTest:z\d+>> InstanceFieldGet [<<Obj>>]
+ /// CHECK-DAG: If [<<GetTest>>]
+ /// CHECK-DAG: <<GetField:f\d+>> InstanceFieldGet [<<Obj>>]
+ /// CHECK-DAG: <<Phi:f\d+>> Phi [<<Float42>>,<<GetField>>]
+ /// CHECK-DAG: Return [<<Phi>>]
+
+ /// CHECK-START: float Main.test24() load_store_elimination (after)
+ /// CHECK-DAG: <<True:i\d+>> IntConstant 1
+ /// CHECK-DAG: <<Float8:f\d+>> FloatConstant 8
+ /// CHECK-DAG: <<Float42:f\d+>> FloatConstant 42
+ /// CHECK-DAG: <<Obj:l\d+>> NewInstance
+ /// CHECK-DAG: If [<<True>>]
+ /// CHECK-DAG: <<Phi:f\d+>> Phi [<<Float42>>,<<Float8>>]
+ /// CHECK-DAG: Return [<<Phi>>]
+
+ static float test24() {
+ float a = 42.0f;
+ TestClass3 obj = new TestClass3();
+ if (obj.test1) {
+ a = obj.floatField;
+ }
+ return a;
+ }
+
/// CHECK-START: void Main.testFinalizable() load_store_elimination (before)
/// CHECK: NewInstance
/// CHECK: InstanceFieldSet
@@ -683,6 +719,7 @@
assertIntEquals(test22(), 13);
assertIntEquals(test23(true), 4);
assertIntEquals(test23(false), 5);
+ assertFloatEquals(test24(), 8.0f);
testFinalizableByForcingGc();
}
}
