[optimzing] Fix codegen bug and improve type propagation

- don't bound the type if there are no relevant uses
- insert the bound type in the bounded block (this allows for condition
materialization without changing the logic there).
- add more comments
- add tests for BoundType generation
- fix GenerateTestAndBranch

Change-Id: I5c1fdda104da4a46775d207270220d410234a472

5 files changed, 116 insertions, 37 deletions

diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc
index 87c74fb..86e84ac 100644
--- a/compiler/optimizing/code_generator_x86.cc
+++ b/compiler/optimizing/code_generator_x86.cc
@@ -877,7 +877,7 @@ void InstructionCodeGeneratorX86::GenerateTestAndBranch(HInstruction* instructio
       if (rhs.IsRegister()) {
         __ cmpl(lhs.AsRegister<Register>(), rhs.AsRegister<Register>());
       } else if (rhs.IsConstant()) {
-        int32_t constant = rhs.GetConstant()->AsIntConstant()->GetValue();
+        int32_t constant = CodeGenerator::GetInt32ValueOf(rhs.GetConstant());
         if (constant == 0) {
           __ testl(lhs.AsRegister<Register>(), lhs.AsRegister<Register>());
         } else {
diff --git a/compiler/optimizing/reference_type_propagation.cc b/compiler/optimizing/reference_type_propagation.cc
index 31ddbb7..de6941c 100644
--- a/compiler/optimizing/reference_type_propagation.cc
+++ b/compiler/optimizing/reference_type_propagation.cc
@@ -58,11 +58,11 @@ void ReferenceTypePropagation::VisitBasicBlock(HBasicBlock* block) {
 }
 
 void ReferenceTypePropagation::BoundTypeForIfNotNull(HBasicBlock* block) {
-  HInstruction* lastInstruction = block->GetLastInstruction();
-  if (!lastInstruction->IsIf()) {
+  HIf* ifInstruction = block->GetLastInstruction()->AsIf();
+  if (ifInstruction == nullptr) {
     return;
   }
-  HInstruction* ifInput = lastInstruction->InputAt(0);
+  HInstruction* ifInput = ifInstruction->InputAt(0);
   if (!ifInput->IsNotEqual() && !ifInput->IsEqual()) {
     return;
   }
@@ -78,16 +78,20 @@ void ReferenceTypePropagation::BoundTypeForIfNotNull(HBasicBlock* block) {
     return;
   }
 
-  HBoundType* bound_type =
-      new (graph_->GetArena()) HBoundType(obj, ReferenceTypeInfo::CreateTop(false));
-
-  block->InsertInstructionBefore(bound_type, lastInstruction);
+  // We only need to bound the type if we have uses in the relevant block.
+  // So start with null and create the HBoundType lazily, only if it's needed.
+  HBoundType* bound_type = nullptr;
   HBasicBlock* notNullBlock = ifInput->IsNotEqual()
-      ? lastInstruction->AsIf()->IfTrueSuccessor()
-      : lastInstruction->AsIf()->IfFalseSuccessor();
+      ? ifInstruction->IfTrueSuccessor()
+      : ifInstruction->IfFalseSuccessor();
+
   for (HUseIterator<HInstruction*> it(obj->GetUses()); !it.Done(); it.Advance()) {
     HInstruction* user = it.Current()->GetUser();
     if (notNullBlock->Dominates(user->GetBlock())) {
+      if (bound_type == nullptr) {
+        bound_type = new (graph_->GetArena()) HBoundType(obj, ReferenceTypeInfo::CreateTop(false));
+        notNullBlock->InsertInstructionBefore(bound_type, notNullBlock->GetFirstInstruction());
+      }
       user->ReplaceInput(bound_type, it.Current()->GetIndex());
     }
   }
@@ -98,47 +102,58 @@ void ReferenceTypePropagation::BoundTypeForIfNotNull(HBasicBlock* block) {
 // If that's the case insert an HBoundType instruction to bound the type of `x`
 // to `ClassX` in the scope of the dominated blocks.
 void ReferenceTypePropagation::BoundTypeForIfInstanceOf(HBasicBlock* block) {
-  HInstruction* lastInstruction = block->GetLastInstruction();
-  if (!lastInstruction->IsIf()) {
+  HIf* ifInstruction = block->GetLastInstruction()->AsIf();
+  if (ifInstruction == nullptr) {
     return;
   }
-
-  HInstruction* ifInput = lastInstruction->InputAt(0);
-  HInstruction* instanceOf;
-  HBasicBlock* instanceOfTrueBlock;
+  HInstruction* ifInput = ifInstruction->InputAt(0);
+  HInstruction* instanceOf = nullptr;
+  HBasicBlock* instanceOfTrueBlock = nullptr;
+
+  // The instruction simplifier has transformed:
+  //   - `if (a instanceof A)` into an HIf with an HInstanceOf input
+  //   - `if (!(a instanceof A)` into an HIf with an HBooleanNot input (which in turn
+  //     has an HInstanceOf input)
+  // So we should not see the usual HEqual here.
   if (ifInput->IsInstanceOf()) {
     instanceOf = ifInput;
-    instanceOfTrueBlock = lastInstruction->AsIf()->IfTrueSuccessor();
+    instanceOfTrueBlock = ifInstruction->IfTrueSuccessor();
   } else if (ifInput->IsBooleanNot() && ifInput->InputAt(0)->IsInstanceOf()) {
     instanceOf = ifInput->InputAt(0);
-    instanceOfTrueBlock = lastInstruction->AsIf()->IfFalseSuccessor();
+    instanceOfTrueBlock = ifInstruction->IfFalseSuccessor();
   } else {
     return;
   }
 
-  HInstruction* obj = instanceOf->InputAt(0);
-  HLoadClass* load_class = instanceOf->InputAt(1)->AsLoadClass();
-
-  ReferenceTypeInfo obj_rti = obj->GetReferenceTypeInfo();
-  ReferenceTypeInfo class_rti = load_class->GetLoadedClassRTI();
-  HBoundType* bound_type = new (graph_->GetArena()) HBoundType(obj, class_rti);
-
-  // Narrow the type as much as possible.
-  {
-    ScopedObjectAccess soa(Thread::Current());
-    if (!load_class->IsResolved() || class_rti.IsSupertypeOf(obj_rti)) {
-      bound_type->SetReferenceTypeInfo(obj_rti);
-    } else {
-      bound_type->SetReferenceTypeInfo(
-          ReferenceTypeInfo::Create(class_rti.GetTypeHandle(), /* is_exact */ false));
-    }
-  }
-
-  block->InsertInstructionBefore(bound_type, lastInstruction);
+  // We only need to bound the type if we have uses in the relevant block.
+  // So start with null and create the HBoundType lazily, only if it's needed.
+  HBoundType* bound_type = nullptr;
 
+  HInstruction* obj = instanceOf->InputAt(0);
   for (HUseIterator<HInstruction*> it(obj->GetUses()); !it.Done(); it.Advance()) {
     HInstruction* user = it.Current()->GetUser();
     if (instanceOfTrueBlock->Dominates(user->GetBlock())) {
+      if (bound_type == nullptr) {
+        HLoadClass* load_class = instanceOf->InputAt(1)->AsLoadClass();
+
+        ReferenceTypeInfo obj_rti = obj->GetReferenceTypeInfo();
+        ReferenceTypeInfo class_rti = load_class->GetLoadedClassRTI();
+        bound_type = new (graph_->GetArena()) HBoundType(obj, class_rti);
+
+        // Narrow the type as much as possible.
+        {
+          ScopedObjectAccess soa(Thread::Current());
+          if (!load_class->IsResolved() || class_rti.IsSupertypeOf(obj_rti)) {
+            bound_type->SetReferenceTypeInfo(obj_rti);
+          } else {
+            bound_type->SetReferenceTypeInfo(
+                ReferenceTypeInfo::Create(class_rti.GetTypeHandle(), /* is_exact */ false));
+          }
+        }
+
+        instanceOfTrueBlock->InsertInstructionBefore(
+            bound_type, instanceOfTrueBlock->GetFirstInstruction());
+      }
       user->ReplaceInput(bound_type, it.Current()->GetIndex());
     }
   }
diff --git a/test/477-checker-bound-type/expected.txt b/test/477-checker-bound-type/expected.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/477-checker-bound-type/expected.txt
diff --git a/test/477-checker-bound-type/info.txt b/test/477-checker-bound-type/info.txt
new file mode 100644
index 0000000..68c774a
--- /dev/null
+++ b/test/477-checker-bound-type/info.txt
@@ -0,0 +1,3 @@
+Tests that we only generate a bound type if we have relevant users.
+It also tests a code generator regression for GenerateTestAndBranch which
+didn't take into account NullConstants.
diff --git a/test/477-checker-bound-type/src/Main.java b/test/477-checker-bound-type/src/Main.java
new file mode 100644
index 0000000..b30028d
--- /dev/null
+++ b/test/477-checker-bound-type/src/Main.java
@@ -0,0 +1,61 @@
+/*
+* Copyright (C) 2015 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 {
+
+  // CHECK-START: java.lang.Object Main.boundTypeForIf(java.lang.Object) reference_type_propagation (after)
+  // CHECK:     BoundType
+  public static Object boundTypeForIf(Object a) {
+    if (a != null) {
+      return a.toString();
+    } else {
+      return null;
+    }
+  }
+
+  // CHECK-START: java.lang.Object Main.boundTypeForInstanceOf(java.lang.Object) reference_type_propagation (after)
+  // CHECK:     BoundType
+  public static Object boundTypeForInstanceOf(Object a) {
+    if (a instanceof Main) {
+      return (Main)a;
+    } else {
+      return null;
+    }
+  }
+
+  // CHECK-START: java.lang.Object Main.noBoundTypeForIf(java.lang.Object) reference_type_propagation (after)
+  // CHECK-NOT: BoundType
+  public static Object noBoundTypeForIf(Object a) {
+    if (a == null) {
+      return new Object();
+    } else {
+      return null;
+    }
+  }
+
+  // CHECK-START: java.lang.Object Main.noBoundTypeForInstanceOf(java.lang.Object) reference_type_propagation (after)
+  // CHECK-NOT: BoundType
+  public static Object noBoundTypeForInstanceOf(Object a) {
+    if (a instanceof Main) {
+      return new Object();
+    } else {
+      return null;
+    }
+  }
+
+  public static void main(String[] args) {  }
+}