ART: Naive NullCheck elimination in InstructionBuilder

Save a little bit of memory by not generating trivially redundant
HNullCheck instructions. This patch builds on the fact that the
InstructionBuilder now directly generates SSA form and looks at the
input of the NullCheck. For obvious cases, such as NewInstance/Array
or `this`, the NullCheck generation is completely avoided.

Bug: 28173563
Change-Id: I1fdf3b096c7a939f7b8586c02a2a6b44dfa43443

4 files changed, 33 insertions, 46 deletions

diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc
index 12cb826395..00cd2f659d 100644
--- a/compiler/optimizing/instruction_builder.cc
+++ b/compiler/optimizing/instruction_builder.cc
@@ -215,6 +215,17 @@ void HInstructionBuilder::InitializeInstruction(HInstruction* instruction) {
   }
 }
 
+HInstruction* HInstructionBuilder::LoadNullCheckedLocal(uint32_t register_index, uint32_t dex_pc) {
+  HInstruction* ref = LoadLocal(register_index, Primitive::kPrimNot);
+  if (!ref->CanBeNull()) {
+    return ref;
+  }
+
+  HNullCheck* null_check = new (arena_) HNullCheck(ref, dex_pc);
+  AppendInstruction(null_check);
+  return null_check;
+}
+
 void HInstructionBuilder::SetLoopHeaderPhiInputs() {
   for (size_t i = loop_headers_.size(); i > 0; --i) {
     HBasicBlock* block = loop_headers_[i - 1];
@@ -1084,10 +1095,9 @@ bool HInstructionBuilder::HandleInvoke(HInvoke* invoke,
   size_t start_index = 0;
   size_t argument_index = 0;
   if (invoke->GetOriginalInvokeType() != InvokeType::kStatic) {  // Instance call.
-    HInstruction* arg = LoadLocal(is_range ? register_index : args[0], Primitive::kPrimNot);
-    HNullCheck* null_check = new (arena_) HNullCheck(arg, invoke->GetDexPc());
-    AppendInstruction(null_check);
-    invoke->SetArgumentAt(0, null_check);
+    HInstruction* arg = LoadNullCheckedLocal(is_range ? register_index : args[0],
+                                             invoke->GetDexPc());
+    invoke->SetArgumentAt(0, arg);
     start_index = 1;
     argument_index = 1;
   }
@@ -1193,9 +1203,7 @@ bool HInstructionBuilder::BuildInstanceFieldAccess(const Instruction& instructio
       compiler_driver_->ComputeInstanceFieldInfo(field_index, dex_compilation_unit_, is_put, soa);
 
 
-  HInstruction* object = LoadLocal(obj_reg, Primitive::kPrimNot);
-  HInstruction* null_check = new (arena_) HNullCheck(object, dex_pc);
-  AppendInstruction(null_check);
+  HInstruction* object = LoadNullCheckedLocal(obj_reg, dex_pc);
 
   Primitive::Type field_type = (resolved_field == nullptr)
       ? GetFieldAccessType(*dex_file_, field_index)
@@ -1205,14 +1213,14 @@ bool HInstructionBuilder::BuildInstanceFieldAccess(const Instruction& instructio
     HInstruction* field_set = nullptr;
     if (resolved_field == nullptr) {
       MaybeRecordStat(MethodCompilationStat::kUnresolvedField);
-      field_set = new (arena_) HUnresolvedInstanceFieldSet(null_check,
+      field_set = new (arena_) HUnresolvedInstanceFieldSet(object,
                                                            value,
                                                            field_type,
                                                            field_index,
                                                            dex_pc);
     } else {
       uint16_t class_def_index = resolved_field->GetDeclaringClass()->GetDexClassDefIndex();
-      field_set = new (arena_) HInstanceFieldSet(null_check,
+      field_set = new (arena_) HInstanceFieldSet(object,
                                                  value,
                                                  field_type,
                                                  resolved_field->GetOffset(),
@@ -1228,13 +1236,13 @@ bool HInstructionBuilder::BuildInstanceFieldAccess(const Instruction& instructio
     HInstruction* field_get = nullptr;
     if (resolved_field == nullptr) {
       MaybeRecordStat(MethodCompilationStat::kUnresolvedField);
-      field_get = new (arena_) HUnresolvedInstanceFieldGet(null_check,
+      field_get = new (arena_) HUnresolvedInstanceFieldGet(object,
                                                            field_type,
                                                            field_index,
                                                            dex_pc);
     } else {
       uint16_t class_def_index = resolved_field->GetDeclaringClass()->GetDexClassDefIndex();
-      field_get = new (arena_) HInstanceFieldGet(null_check,
+      field_get = new (arena_) HInstanceFieldGet(object,
                                                  field_type,
                                                  resolved_field->GetOffset(),
                                                  resolved_field->IsVolatile(),
@@ -1449,10 +1457,7 @@ void HInstructionBuilder::BuildArrayAccess(const Instruction& instruction,
   uint8_t array_reg = instruction.VRegB_23x();
   uint8_t index_reg = instruction.VRegC_23x();
 
-  HInstruction* object = LoadLocal(array_reg, Primitive::kPrimNot);
-  object = new (arena_) HNullCheck(object, dex_pc);
-  AppendInstruction(object);
-
+  HInstruction* object = LoadNullCheckedLocal(array_reg, dex_pc);
   HInstruction* length = new (arena_) HArrayLength(object, dex_pc);
   AppendInstruction(length);
   HInstruction* index = LoadLocal(index_reg, Primitive::kPrimInt);
@@ -1527,11 +1532,8 @@ void HInstructionBuilder::BuildFillArrayData(HInstruction* object,
 }
 
 void HInstructionBuilder::BuildFillArrayData(const Instruction& instruction, uint32_t dex_pc) {
-  HInstruction* array = LoadLocal(instruction.VRegA_31t(), Primitive::kPrimNot);
-  HNullCheck* null_check = new (arena_) HNullCheck(array, dex_pc);
-  AppendInstruction(null_check);
-
-  HInstruction* length = new (arena_) HArrayLength(null_check, dex_pc);
+  HInstruction* array = LoadNullCheckedLocal(instruction.VRegA_31t(), dex_pc);
+  HInstruction* length = new (arena_) HArrayLength(array, dex_pc);
   AppendInstruction(length);
 
   int32_t payload_offset = instruction.VRegB_31t() + dex_pc;
@@ -1547,28 +1549,28 @@ void HInstructionBuilder::BuildFillArrayData(const Instruction& instruction, uin
 
   switch (payload->element_width) {
     case 1:
-      BuildFillArrayData(null_check,
+      BuildFillArrayData(array,
                          reinterpret_cast<const int8_t*>(data),
                          element_count,
                          Primitive::kPrimByte,
                          dex_pc);
       break;
     case 2:
-      BuildFillArrayData(null_check,
+      BuildFillArrayData(array,
                          reinterpret_cast<const int16_t*>(data),
                          element_count,
                          Primitive::kPrimShort,
                          dex_pc);
       break;
     case 4:
-      BuildFillArrayData(null_check,
+      BuildFillArrayData(array,
                          reinterpret_cast<const int32_t*>(data),
                          element_count,
                          Primitive::kPrimInt,
                          dex_pc);
       break;
     case 8:
-      BuildFillWideArrayData(null_check,
+      BuildFillWideArrayData(array,
                              reinterpret_cast<const int64_t*>(data),
                              element_count,
                              dex_pc);
@@ -2575,9 +2577,7 @@ bool HInstructionBuilder::ProcessDexInstruction(const Instruction& instruction,
     ARRAY_XX(_SHORT, Primitive::kPrimShort);
 
     case Instruction::ARRAY_LENGTH: {
-      HInstruction* object = LoadLocal(instruction.VRegB_12x(), Primitive::kPrimNot);
-      object = new (arena_) HNullCheck(object, dex_pc);
-      AppendInstruction(object);
+      HInstruction* object = LoadNullCheckedLocal(instruction.VRegB_12x(), dex_pc);
       AppendInstruction(new (arena_) HArrayLength(object, dex_pc));
       UpdateLocal(instruction.VRegA_12x(), current_block_->GetLastInstruction());
       break;
diff --git a/compiler/optimizing/instruction_builder.h b/compiler/optimizing/instruction_builder.h
index 070f7da80e..0e3e5a7c34 100644
--- a/compiler/optimizing/instruction_builder.h
+++ b/compiler/optimizing/instruction_builder.h
@@ -87,6 +87,7 @@ class HInstructionBuilder : public ValueObject {
   ArenaVector<HInstruction*>* GetLocalsFor(HBasicBlock* block);
   HInstruction* ValueOfLocalAt(HBasicBlock* block, size_t local);
   HInstruction* LoadLocal(uint32_t register_index, Primitive::Type type) const;
+  HInstruction* LoadNullCheckedLocal(uint32_t register_index, uint32_t dex_pc);
   void UpdateLocal(uint32_t register_index, HInstruction* instruction);
 
   void AppendInstruction(HInstruction* instruction);
diff --git a/test/444-checker-nce/src/Main.java b/test/444-checker-nce/src/Main.java
index c96b18c71b..ddc2f77e89 100644
--- a/test/444-checker-nce/src/Main.java
+++ b/test/444-checker-nce/src/Main.java
@@ -28,10 +28,6 @@ public class Main {
   }
 
   /// CHECK-START: Main Main.thisTest() builder (after)
-  /// CHECK:         NullCheck
-  /// CHECK:         InvokeStaticOrDirect
-
-  /// CHECK-START: Main Main.thisTest() instruction_simplifier (after)
   /// CHECK-NOT:     NullCheck
   /// CHECK:         InvokeStaticOrDirect
   public Main thisTest() {
@@ -40,12 +36,10 @@ public class Main {
 
   /// CHECK-START: Main Main.newInstanceRemoveTest() builder (after)
   /// CHECK:         NewInstance
-  /// CHECK:         NullCheck
   /// CHECK:         InvokeStaticOrDirect
-  /// CHECK:         NullCheck
   /// CHECK:         InvokeStaticOrDirect
 
-  /// CHECK-START: Main Main.newInstanceRemoveTest() instruction_simplifier (after)
+  /// CHECK-START: Main Main.newInstanceRemoveTest() builder (after)
   /// CHECK-NOT:     NullCheck
   public Main newInstanceRemoveTest() {
     Main m = new Main();
@@ -54,13 +48,10 @@ public class Main {
 
   /// CHECK-START: Main Main.newArrayRemoveTest() builder (after)
   /// CHECK:         NewArray
-  /// CHECK:         NullCheck
   /// CHECK:         ArrayGet
 
-  /// CHECK-START: Main Main.newArrayRemoveTest() instruction_simplifier (after)
-  /// CHECK:         NewArray
+  /// CHECK-START: Main Main.newArrayRemoveTest() builder (after)
   /// CHECK-NOT:     NullCheck
-  /// CHECK:         ArrayGet
   public Main newArrayRemoveTest() {
     Main[] ms = new Main[1];
     return ms[0];
@@ -179,9 +170,6 @@ public class Main {
   }
 
   /// CHECK-START: Main Main.scopeRemoveTest(int, Main) builder (after)
-  /// CHECK:         NullCheck
-
-  /// CHECK-START: Main Main.scopeRemoveTest(int, Main) instruction_simplifier (after)
   /// CHECK-NOT:     NullCheck
   public Main scopeRemoveTest(int count, Main a) {
     Main m = null;
diff --git a/test/572-checker-array-get-regression/src/Main.java b/test/572-checker-array-get-regression/src/Main.java
index b55be706f4..89b97ed316 100644
--- a/test/572-checker-array-get-regression/src/Main.java
+++ b/test/572-checker-array-get-regression/src/Main.java
@@ -25,13 +25,11 @@ public class Main {
   /// CHECK-DAG:     <<Const2P19:i\d+>>    IntConstant 524288
   /// CHECK-DAG:     <<ConstM1:i\d+>>      IntConstant -1
   /// CHECK-DAG:     <<Array:l\d+>>        NewArray [<<Const2P19>>,<<Method>>]
-  /// CHECK-DAG:     <<NullCheck1:l\d+>>   NullCheck [<<Array>>]
-  /// CHECK-DAG:     <<Length1:i\d+>>      ArrayLength [<<NullCheck1>>]
+  /// CHECK-DAG:     <<Length1:i\d+>>      ArrayLength [<<Array>>]
   /// CHECK-DAG:     <<Index:i\d+>>        Add [<<Length1>>,<<ConstM1>>]
-  /// CHECK-DAG:     <<NullCheck2:l\d+>>   NullCheck [<<Array>>]
-  /// CHECK-DAG:     <<Length2:i\d+>>      ArrayLength [<<NullCheck2>>]
+  /// CHECK-DAG:     <<Length2:i\d+>>      ArrayLength [<<Array>>]
   /// CHECK-DAG:     <<BoundsCheck:i\d+>>  BoundsCheck [<<Index>>,<<Length2>>]
-  /// CHECK-DAG:     <<LastElement:l\d+>>  ArrayGet [<<NullCheck2>>,<<BoundsCheck>>]
+  /// CHECK-DAG:     <<LastElement:l\d+>>  ArrayGet [<<Array>>,<<BoundsCheck>>]
   /// CHECK-DAG:                           Return [<<LastElement>>]