[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
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 @@
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::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 @@
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 @@
// 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 = ifInstruction->InputAt(0);
+ HInstruction* instanceOf = nullptr;
+ HBasicBlock* instanceOfTrueBlock = nullptr;
- HInstruction* ifInput = lastInstruction->InputAt(0);
- HInstruction* instanceOf;
- HBasicBlock* instanceOfTrueBlock;
+ // 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;
}
+ // 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);
- 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);
-
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) { }
+}
