Ensure object ArraySet with null value does not need a type check.

The art::PrepareForRegisterAllocation visitor can remove an
art::BoundType instruction as value input of an
art::ArraySet instruction, possibly replacing it with an
art::NullConstant.  If this happens, remove the need for a
type check in this art::ArraySet.

Bug: 27638110
Change-Id: I6270f8a8e22822a24d8a5919df427ca9c64d121b

6 files changed, 96 insertions, 0 deletions

diff --git a/compiler/optimizing/graph_visualizer.cc b/compiler/optimizing/graph_visualizer.cc
index 4f1e90cd7f..3a9d242df2 100644
--- a/compiler/optimizing/graph_visualizer.cc
+++ b/compiler/optimizing/graph_visualizer.cc
@@ -382,6 +382,8 @@ class HGraphVisualizerPrinter : public HGraphDelegateVisitor {
   void VisitArraySet(HArraySet* array_set) OVERRIDE {
     StartAttributeStream("value_can_be_null") << std::boolalpha
         << array_set->GetValueCanBeNull() << std::noboolalpha;
+    StartAttributeStream("needs_type_check") << std::boolalpha
+        << array_set->NeedsTypeCheck() << std::noboolalpha;
   }
 
   void VisitCompare(HCompare* compare) OVERRIDE {
diff --git a/compiler/optimizing/prepare_for_register_allocation.cc b/compiler/optimizing/prepare_for_register_allocation.cc
index 0ad104eaa7..fc72727196 100644
--- a/compiler/optimizing/prepare_for_register_allocation.cc
+++ b/compiler/optimizing/prepare_for_register_allocation.cc
@@ -47,6 +47,19 @@ void PrepareForRegisterAllocation::VisitBoundType(HBoundType* bound_type) {
   bound_type->GetBlock()->RemoveInstruction(bound_type);
 }
 
+void PrepareForRegisterAllocation::VisitArraySet(HArraySet* instruction) {
+  HInstruction* value = instruction->GetValue();
+  // PrepareForRegisterAllocation::VisitBoundType may have replaced a
+  // BoundType (as value input of this ArraySet) with a NullConstant.
+  // If so, this ArraySet no longer needs a type check.
+  if (value->IsNullConstant()) {
+    DCHECK_EQ(value->GetType(), Primitive::kPrimNot);
+    if (instruction->NeedsTypeCheck()) {
+      instruction->ClearNeedsTypeCheck();
+    }
+  }
+}
+
 void PrepareForRegisterAllocation::VisitClinitCheck(HClinitCheck* check) {
   // Try to find a static invoke or a new-instance from which this check originated.
   HInstruction* implicit_clinit = nullptr;
diff --git a/compiler/optimizing/prepare_for_register_allocation.h b/compiler/optimizing/prepare_for_register_allocation.h
index c90724c251..a6791482a7 100644
--- a/compiler/optimizing/prepare_for_register_allocation.h
+++ b/compiler/optimizing/prepare_for_register_allocation.h
@@ -40,6 +40,7 @@ class PrepareForRegisterAllocation : public HGraphDelegateVisitor {
   void VisitDivZeroCheck(HDivZeroCheck* check) OVERRIDE;
   void VisitBoundsCheck(HBoundsCheck* check) OVERRIDE;
   void VisitBoundType(HBoundType* bound_type) OVERRIDE;
+  void VisitArraySet(HArraySet* instruction) OVERRIDE;
   void VisitClinitCheck(HClinitCheck* check) OVERRIDE;
   void VisitCondition(HCondition* condition) OVERRIDE;
   void VisitInvokeStaticOrDirect(HInvokeStaticOrDirect* invoke) OVERRIDE;
diff --git a/test/590-checker-array-set-null-regression/expected.txt b/test/590-checker-array-set-null-regression/expected.txt
new file mode 100644
index 0000000000..b0aad4deb5
--- /dev/null
+++ b/test/590-checker-array-set-null-regression/expected.txt
@@ -0,0 +1 @@
+passed
diff --git a/test/590-checker-array-set-null-regression/info.txt b/test/590-checker-array-set-null-regression/info.txt
new file mode 100644
index 0000000000..fe173a334d
--- /dev/null
+++ b/test/590-checker-array-set-null-regression/info.txt
@@ -0,0 +1,11 @@
+Regression test for art::PrepareForRegisterAllocation, which replaces
+
+  ArraySet[array, index, BoundType[NullConstant]]
+
+with
+
+  ArraySet[array, index, NullConstant]
+
+but used to forget to remove the "need for a type check" bit in the
+ArraySet, thus failing "!may_need_runtime_call_for_type_check"
+assertions in code generators.
diff --git a/test/590-checker-array-set-null-regression/src/Main.java b/test/590-checker-array-set-null-regression/src/Main.java
new file mode 100644
index 0000000000..792ee4ecd6
--- /dev/null
+++ b/test/590-checker-array-set-null-regression/src/Main.java
@@ -0,0 +1,68 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class Main {
+
+  public static void main(String args[]) {
+    Element[] elements = new Element[51];
+    testArraySetCheckCastNull(elements);
+
+    System.out.println("passed");
+  }
+
+  /// CHECK-START: void Main.testArraySetCheckCastNull(Main$Element[]) builder (after)
+  /// CHECK:         <<Array:l\d+>>         ParameterValue
+  /// CHECK-DAG:     <<Index:i\d+>>         IntConstant 42
+  /// CHECK-DAG:     <<Null:l\d+>>          NullConstant
+  /// CHECK-DAG:     <<Class:l\d+>>         LoadClass
+  /// CHECK-DAG:                            CheckCast [<<Null>>,<<Class>>]
+  /// CHECK-DAG:     <<CheckedValue:l\d+>>  BoundType [<<Null>>] klass:Main$Element can_be_null:true
+  /// CHECK-DAG:     <<CheckedArray:l\d+>>  NullCheck [<<Array>>]
+  /// CHECK-DAG:     <<Length:i\d+>>        ArrayLength [<<CheckedArray>>]
+  /// CHECK-DAG:     <<CheckedIndex:i\d+>>  BoundsCheck [<<Index>>,<<Length>>]
+  /// CHECK-DAG:     <<ArraySet:v\d+>>      ArraySet [<<CheckedArray>>,<<CheckedIndex>>,<<CheckedValue>>] needs_type_check:true
+
+  /// CHECK-START: void Main.testArraySetCheckCastNull(Main$Element[]) instruction_simplifier (after)
+  /// CHECK-NOT:                            CheckCast
+
+  /// CHECK-START: void Main.testArraySetCheckCastNull(Main$Element[]) prepare_for_register_allocation (before)
+  /// CHECK:         <<Array:l\d+>>         ParameterValue
+  /// CHECK-DAG:     <<Index:i\d+>>         IntConstant 42
+  /// CHECK-DAG:     <<Null:l\d+>>          NullConstant
+  /// CHECK-DAG:     <<Class:l\d+>>         LoadClass
+  /// CHECK-DAG:     <<CheckedValue:l\d+>>  BoundType [<<Null>>]
+  /// CHECK-DAG:     <<CheckedArray:l\d+>>  NullCheck [<<Array>>]
+  /// CHECK-DAG:     <<Length:i\d+>>        ArrayLength [<<CheckedArray>>]
+  /// CHECK-DAG:     <<CheckedIndex:i\d+>>  BoundsCheck [<<Index>>,<<Length>>]
+  /// CHECK-DAG:     <<ArraySet:v\d+>>      ArraySet [<<CheckedArray>>,<<CheckedIndex>>,<<CheckedValue>>] needs_type_check:true
+
+  /// CHECK-START: void Main.testArraySetCheckCastNull(Main$Element[]) prepare_for_register_allocation (after)
+  /// CHECK:         <<Array:l\d+>>         ParameterValue
+  /// CHECK-DAG:     <<Index:i\d+>>         IntConstant 42
+  /// CHECK-DAG:     <<Null:l\d+>>          NullConstant
+  /// CHECK-DAG:     <<Class:l\d+>>         LoadClass
+  /// CHECK-DAG:     <<Length:i\d+>>        ArrayLength [<<Array>>]
+  /// CHECK-DAG:     <<ArraySet:v\d+>>      ArraySet [<<Array>>,<<Index>>,<<Null>>] needs_type_check:false
+
+  static void testArraySetCheckCastNull(Element[] elements) {
+    Object object = null;
+    Element element = (Element) object;
+    elements[42] = element;
+  }
+
+  class Element {}
+
+}