summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Mathieu Chartier <mathieuc@google.com> 2016-08-30 10:23:01 -0700
committer Mathieu Chartier <mathieuc@google.com> 2016-10-24 10:48:23 -0700
commit1ceb37c75544c5285fb87f27e1d9fa7261ced60c (patch)
treea2c672170bb8424a148298706c6fd71d892a1b6e
parent3667e26de4856cccf24bcbab54ad3349a05267c0 (diff)
Remove unnecessary load class for new instance
Remove the load class for new instance if the load class has only one use and can not throw. Previously many were not removed due to MarkInDexCache nulling out the environment of the HLoadClass and causing CanMoveClinitCheck to fail. Also keep track of initialized HLoadClass and always remove clinit checks for these. Added checker regression test. Code size savings: ARM64 CC boot.oat: 47896936 -> 47642488 (-0.53%) Savings from IsInitialized optimization: 65984 bytes Performance unmeasured, probably faster due to removing unnecessary work. Test: test-art-host with CC baker Bug: 29516974 Change-Id: I43358762ffb380ebe7e6518d0d440a5e1cc03b61
-rw-r--r--compiler/optimizing/prepare_for_register_allocation.cc33
-rw-r--r--compiler/optimizing/sharpening.cc1
-rw-r--r--test/621-checker-new-instance/expected.txt0
-rw-r--r--test/621-checker-new-instance/info.txt1
-rw-r--r--test/621-checker-new-instance/src/Main.java53
5 files changed, 74 insertions, 14 deletions
diff --git a/compiler/optimizing/prepare_for_register_allocation.cc b/compiler/optimizing/prepare_for_register_allocation.cc
index a4df9e5503..7b66ef3627 100644
--- a/compiler/optimizing/prepare_for_register_allocation.cc
+++ b/compiler/optimizing/prepare_for_register_allocation.cc
@@ -129,6 +129,7 @@ void PrepareForRegisterAllocation::VisitClinitCheck(HClinitCheck* check) {
} else if (can_merge_with_load_class && !load_class->NeedsAccessCheck()) {
// Pass the initialization duty to the `HLoadClass` instruction,
// and remove the instruction from the graph.
+ DCHECK(load_class->HasEnvironment());
load_class->SetMustGenerateClinitCheck(true);
check->GetBlock()->RemoveInstruction(check);
}
@@ -136,7 +137,7 @@ void PrepareForRegisterAllocation::VisitClinitCheck(HClinitCheck* check) {
void PrepareForRegisterAllocation::VisitNewInstance(HNewInstance* instruction) {
HLoadClass* load_class = instruction->InputAt(0)->AsLoadClass();
- bool has_only_one_use = load_class->HasOnlyOneNonEnvironmentUse();
+ const bool has_only_one_use = load_class->HasOnlyOneNonEnvironmentUse();
// Change the entrypoint to kQuickAllocObject if either:
// - the class is finalizable (only kQuickAllocObject handles finalizable classes),
// - the class needs access checks (we do not know if it's finalizable),
@@ -144,19 +145,25 @@ void PrepareForRegisterAllocation::VisitNewInstance(HNewInstance* instruction) {
if (instruction->IsFinalizable() || has_only_one_use || load_class->NeedsAccessCheck()) {
instruction->SetEntrypoint(kQuickAllocObject);
instruction->ReplaceInput(GetGraph()->GetIntConstant(load_class->GetTypeIndex()), 0);
- // The allocation entry point that deals with access checks does not work with inlined
- // methods, so we need to check whether this allocation comes from an inlined method.
- // We also need to make the same check as for moving clinit check, whether the HLoadClass
- // has the clinit check responsibility or not (HLoadClass can throw anyway).
- if (has_only_one_use &&
- !instruction->GetEnvironment()->IsFromInlinedInvoke() &&
- CanMoveClinitCheck(load_class, instruction)) {
- // We can remove the load class from the graph. If it needed access checks, we delegate
- // the access check to the allocation.
- if (load_class->NeedsAccessCheck()) {
- instruction->SetEntrypoint(kQuickAllocObjectWithAccessCheck);
+ if (has_only_one_use) {
+ // We've just removed the only use of the HLoadClass. Since we don't run DCE after this pass,
+ // do it manually if possible.
+ if (!load_class->CanThrow()) {
+ // If the load class can not throw, it has no side effects and can be removed if there is
+ // only one use.
+ load_class->GetBlock()->RemoveInstruction(load_class);
+ } else if (!instruction->GetEnvironment()->IsFromInlinedInvoke() &&
+ CanMoveClinitCheck(load_class, instruction)) {
+ // The allocation entry point that deals with access checks does not work with inlined
+ // methods, so we need to check whether this allocation comes from an inlined method.
+ // We also need to make the same check as for moving clinit check, whether the HLoadClass
+ // has the clinit check responsibility or not (HLoadClass can throw anyway).
+ // If it needed access checks, we delegate the access check to the allocation.
+ if (load_class->NeedsAccessCheck()) {
+ instruction->SetEntrypoint(kQuickAllocObjectWithAccessCheck);
+ }
+ load_class->GetBlock()->RemoveInstruction(load_class);
}
- load_class->GetBlock()->RemoveInstruction(load_class);
}
}
}
diff --git a/compiler/optimizing/sharpening.cc b/compiler/optimizing/sharpening.cc
index df1b351249..d938a70579 100644
--- a/compiler/optimizing/sharpening.cc
+++ b/compiler/optimizing/sharpening.cc
@@ -162,7 +162,6 @@ void HSharpening::ProcessLoadClass(HLoadClass* load_class) {
? compilation_unit_.GetDexCache()
: hs.NewHandle(class_linker->FindDexCache(soa.Self(), dex_file));
mirror::Class* klass = dex_cache->GetResolvedType(type_index);
-
if (codegen_->GetCompilerOptions().IsBootImage()) {
// Compiling boot image. Check if the class is a boot image class.
DCHECK(!runtime->UseJitCompilation());
diff --git a/test/621-checker-new-instance/expected.txt b/test/621-checker-new-instance/expected.txt
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/621-checker-new-instance/expected.txt
diff --git a/test/621-checker-new-instance/info.txt b/test/621-checker-new-instance/info.txt
new file mode 100644
index 0000000000..c27c45ca7f
--- /dev/null
+++ b/test/621-checker-new-instance/info.txt
@@ -0,0 +1 @@
+Tests for removing useless load class.
diff --git a/test/621-checker-new-instance/src/Main.java b/test/621-checker-new-instance/src/Main.java
new file mode 100644
index 0000000000..68a46449f0
--- /dev/null
+++ b/test/621-checker-new-instance/src/Main.java
@@ -0,0 +1,53 @@
+/*
+ * 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 {
+ /// CHECK-START: java.lang.Object Main.newObject() prepare_for_register_allocation (before)
+ /// CHECK: LoadClass
+ /// CHECK: NewInstance
+
+ /// CHECK-START: java.lang.Object Main.newObject() prepare_for_register_allocation (after)
+ /// CHECK-NOT: LoadClass
+ /// CHECK: NewInstance
+ public static Object newObject() {
+ return new Object();
+ }
+
+ /// CHECK-START: java.lang.Object Main.newFinalizableMayThrow() prepare_for_register_allocation (after)
+ /// CHECK: LoadClass
+ /// CHECK: NewInstance
+ public static Object newFinalizableMayThrow() {
+ return $inline$newFinalizableMayThrow();
+ }
+
+ public static Object $inline$newFinalizableMayThrow() {
+ return new FinalizableMayThrow();
+ }
+
+ public static void main(String[] args) {
+ newFinalizableMayThrow();
+ newObject();
+ }
+}
+
+class FinalizableMayThrow {
+ // clinit may throw OOME.
+ static Object o = new Object();
+ static String s;
+ public void finalize() {
+ s = "Test";
+ }
+}