diff options
author | 2016-05-04 14:00:12 +0100 | |
---|---|---|
committer | 2016-05-10 09:18:30 +0100 | |
commit | bb2c93b8d833db3b872fe7130712d73260c5503f (patch) | |
tree | deabb2a88a55deda1dd3d3bf9945ec64d806853a | |
parent | ceeb3b73f84e8b11f362605007d382405d08d95c (diff) |
Relax the DCHECK in load store elimination.
The DCHECK was too strong, as we could come from a field or array
get on null, instead of null directly.
bug:27831001
(cherry picked from commit 65fef30952bb92acec7ed36f7f431d93f7ce88b3)
Change-Id: Ia3ba1235e95408d66349a02fc438df9c2cf9e255
-rw-r--r-- | compiler/optimizing/load_store_elimination.cc | 15 | ||||
-rw-r--r-- | test/586-checker-null-array-get/src/Main.java | 49 |
2 files changed, 49 insertions, 15 deletions
diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc index 1b23622abd..b4d93add9c 100644 --- a/compiler/optimizing/load_store_elimination.cc +++ b/compiler/optimizing/load_store_elimination.cc @@ -732,19 +732,14 @@ class LSEVisitor : public HGraphVisitor { if (Primitive::PrimitiveKind(heap_value->GetType()) != Primitive::PrimitiveKind(instruction->GetType())) { // The only situation where the same heap location has different type is when - // we do an array get from a null constant. In order to stay properly typed - // we do not merge the array gets. + // we do an array get on an instruction that originates from the null constant + // (the null could be behind a field access, an array access, a null check or + // a bound type). + // In order to stay properly typed on primitive types, we do not eliminate + // the array gets. if (kIsDebugBuild) { DCHECK(heap_value->IsArrayGet()) << heap_value->DebugName(); DCHECK(instruction->IsArrayGet()) << instruction->DebugName(); - HInstruction* array = instruction->AsArrayGet()->GetArray(); - DCHECK(array->IsNullCheck()) << array->DebugName(); - HInstruction* input = HuntForOriginalReference(array->InputAt(0)); - DCHECK(input->IsNullConstant()) << input->DebugName(); - array = heap_value->AsArrayGet()->GetArray(); - DCHECK(array->IsNullCheck()) << array->DebugName(); - input = HuntForOriginalReference(array->InputAt(0)); - DCHECK(input->IsNullConstant()) << input->DebugName(); } return; } diff --git a/test/586-checker-null-array-get/src/Main.java b/test/586-checker-null-array-get/src/Main.java index 332cfb0a53..e0782bc84d 100644 --- a/test/586-checker-null-array-get/src/Main.java +++ b/test/586-checker-null-array-get/src/Main.java @@ -14,10 +14,20 @@ * limitations under the License. */ +class Test1 { + int[] iarr; +} + +class Test2 { + float[] farr; +} + public class Main { public static Object[] getObjectArray() { return null; } public static long[] getLongArray() { return null; } public static Object getNull() { return null; } + public static Test1 getNullTest1() { return null; } + public static Test2 getNullTest2() { return null; } public static void main(String[] args) { try { @@ -26,13 +36,25 @@ public class Main { } catch (NullPointerException e) { // Expected. } + try { + bar(); + throw new Error("Expected NullPointerException"); + } catch (NullPointerException e) { + // Expected. + } + try { + test1(); + throw new Error("Expected NullPointerException"); + } catch (NullPointerException e) { + // Expected. + } } /// CHECK-START: void Main.foo() load_store_elimination (after) - /// CHECK-DAG: <<Null:l\d+>> NullConstant - /// CHECK-DAG: <<Check:l\d+>> NullCheck [<<Null>>] - /// CHECK-DAG: <<Get1:j\d+>> ArrayGet [<<Check>>,{{i\d+}}] - /// CHECK-DAG: <<Get2:l\d+>> ArrayGet [<<Check>>,{{i\d+}}] + /// CHECK-DAG: <<Null:l\d+>> NullConstant + /// CHECK-DAG: <<Check:l\d+>> NullCheck [<<Null>>] + /// CHECK-DAG: <<Get1:j\d+>> ArrayGet [<<Check>>,{{i\d+}}] + /// CHECK-DAG: <<Get2:l\d+>> ArrayGet [<<Check>>,{{i\d+}}] public static void foo() { longField = getLongArray()[0]; objectField = getObjectArray()[0]; @@ -56,7 +78,7 @@ public class Main { // elimination pass to add a HDeoptimize. Not having the bounds check helped // the load store elimination think it could merge two ArrayGet with different // types. - String[] array = ((String[])getNull()); + String[] array = (String[])getNull(); objectField = array[0]; objectField = array[1]; objectField = array[2]; @@ -68,6 +90,23 @@ public class Main { longField = longArray[3]; } + /// CHECK-START: float Main.test1() load_store_elimination (after) + /// CHECK-DAG: <<Null:l\d+>> NullConstant + /// CHECK-DAG: <<Check1:l\d+>> NullCheck [<<Null>>] + /// CHECK-DAG: <<FieldGet1:l\d+>> InstanceFieldGet [<<Check1>>] field_name:Test1.iarr + /// CHECK-DAG: <<Check2:l\d+>> NullCheck [<<FieldGet1>>] + /// CHECK-DAG: <<ArrayGet1:i\d+>> ArrayGet [<<Check2>>,{{i\d+}}] + /// CHECK-DAG: <<ArrayGet2:f\d+>> ArrayGet [<<Check2>>,{{i\d+}}] + /// CHECK-DAG: Return [<<ArrayGet2>>] + public static float test1() { + Test1 test1 = getNullTest1(); + Test2 test2 = getNullTest2();; + int[] iarr = test1.iarr; + float[] farr = test2.farr; + iarr[0] = iarr[1]; + return farr[0]; + } + public static long longField; public static Object objectField; } |