Merge "Fix ClinitCheck pruning."
diff --git a/compiler/optimizing/graph_visualizer.cc b/compiler/optimizing/graph_visualizer.cc
index 2b77901..af3ecb1 100644
--- a/compiler/optimizing/graph_visualizer.cc
+++ b/compiler/optimizing/graph_visualizer.cc
@@ -397,6 +397,9 @@
<< invoke->IsRecursive()
<< std::noboolalpha;
StartAttributeStream("intrinsic") << invoke->GetIntrinsic();
+ if (invoke->IsStatic()) {
+ StartAttributeStream("clinit_check") << invoke->GetClinitCheckRequirement();
+ }
}
void VisitUnresolvedInstanceFieldGet(HUnresolvedInstanceFieldGet* field_access) OVERRIDE {
diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc
index 73a44ee..0a39ff3 100644
--- a/compiler/optimizing/nodes.cc
+++ b/compiler/optimizing/nodes.cc
@@ -2068,6 +2068,19 @@
}
}
+std::ostream& operator<<(std::ostream& os, HInvokeStaticOrDirect::ClinitCheckRequirement rhs) {
+ switch (rhs) {
+ case HInvokeStaticOrDirect::ClinitCheckRequirement::kExplicit:
+ return os << "explicit";
+ case HInvokeStaticOrDirect::ClinitCheckRequirement::kImplicit:
+ return os << "implicit";
+ case HInvokeStaticOrDirect::ClinitCheckRequirement::kNone:
+ return os << "none";
+ default:
+ return os << "unknown:" << static_cast<int>(rhs);
+ }
+}
+
void HInstruction::RemoveEnvironmentUsers() {
for (HUseIterator<HEnvironment*> use_it(GetEnvUses()); !use_it.Done(); use_it.Advance()) {
HUseListNode<HEnvironment*>* user_node = use_it.Current();
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index e3c810e..b68ea0f 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -3505,20 +3505,19 @@
return GetInvokeType() == kStatic;
}
- // Remove the art::HLoadClass instruction set as last input by
- // art::PrepareForRegisterAllocation::VisitClinitCheck in lieu of
- // the initial art::HClinitCheck instruction (only relevant for
- // static calls with explicit clinit check).
- void RemoveLoadClassAsLastInput() {
+ // Remove the HClinitCheck or the replacement HLoadClass (set as last input by
+ // PrepareForRegisterAllocation::VisitClinitCheck() in lieu of the initial HClinitCheck)
+ // instruction; only relevant for static calls with explicit clinit check.
+ void RemoveExplicitClinitCheck(ClinitCheckRequirement new_requirement) {
DCHECK(IsStaticWithExplicitClinitCheck());
size_t last_input_index = InputCount() - 1;
HInstruction* last_input = InputAt(last_input_index);
DCHECK(last_input != nullptr);
- DCHECK(last_input->IsLoadClass()) << last_input->DebugName();
+ DCHECK(last_input->IsLoadClass() || last_input->IsClinitCheck()) << last_input->DebugName();
RemoveAsUserOfInput(last_input_index);
inputs_.pop_back();
- clinit_check_requirement_ = ClinitCheckRequirement::kImplicit;
- DCHECK(IsStaticWithImplicitClinitCheck());
+ clinit_check_requirement_ = new_requirement;
+ DCHECK(!IsStaticWithExplicitClinitCheck());
}
bool IsStringFactoryFor(HFakeString* str) const {
@@ -3539,7 +3538,7 @@
}
// Is this a call to a static method whose declaring class has an
- // explicit intialization check in the graph?
+ // explicit initialization check in the graph?
bool IsStaticWithExplicitClinitCheck() const {
return IsStatic() && (clinit_check_requirement_ == ClinitCheckRequirement::kExplicit);
}
@@ -3583,6 +3582,7 @@
DISALLOW_COPY_AND_ASSIGN(HInvokeStaticOrDirect);
};
+std::ostream& operator<<(std::ostream& os, HInvokeStaticOrDirect::ClinitCheckRequirement rhs);
class HInvokeVirtual : public HInvoke {
public:
diff --git a/compiler/optimizing/prepare_for_register_allocation.cc b/compiler/optimizing/prepare_for_register_allocation.cc
index ca928ae..f3d075c 100644
--- a/compiler/optimizing/prepare_for_register_allocation.cc
+++ b/compiler/optimizing/prepare_for_register_allocation.cc
@@ -48,12 +48,46 @@
}
void PrepareForRegisterAllocation::VisitClinitCheck(HClinitCheck* check) {
- HLoadClass* cls = check->GetLoadClass();
- check->ReplaceWith(cls);
- if (check->GetPrevious() == cls) {
+ // Try to find a static invoke from which this check originated.
+ HInvokeStaticOrDirect* invoke = nullptr;
+ for (HUseIterator<HInstruction*> it(check->GetUses()); !it.Done(); it.Advance()) {
+ HInstruction* user = it.Current()->GetUser();
+ if (user->IsInvokeStaticOrDirect() && CanMoveClinitCheck(check, user)) {
+ invoke = user->AsInvokeStaticOrDirect();
+ DCHECK(invoke->IsStaticWithExplicitClinitCheck());
+ invoke->RemoveExplicitClinitCheck(HInvokeStaticOrDirect::ClinitCheckRequirement::kImplicit);
+ break;
+ }
+ }
+ // If we found a static invoke for merging, remove the check from all other static invokes.
+ if (invoke != nullptr) {
+ for (HUseIterator<HInstruction*> it(check->GetUses()); !it.Done(); ) {
+ HInstruction* user = it.Current()->GetUser();
+ DCHECK(invoke->StrictlyDominates(user)); // All other uses must be dominated.
+ it.Advance(); // Advance before we remove the node, reference to the next node is preserved.
+ if (user->IsInvokeStaticOrDirect()) {
+ user->AsInvokeStaticOrDirect()->RemoveExplicitClinitCheck(
+ HInvokeStaticOrDirect::ClinitCheckRequirement::kNone);
+ }
+ }
+ }
+
+ HLoadClass* load_class = check->GetLoadClass();
+ bool can_merge_with_load_class = CanMoveClinitCheck(load_class, check);
+
+ check->ReplaceWith(load_class);
+
+ if (invoke != nullptr) {
+ // Remove the check from the graph. It has been merged into the invoke.
+ check->GetBlock()->RemoveInstruction(check);
+ // Check if we can merge the load class as well.
+ if (can_merge_with_load_class && !load_class->HasUses()) {
+ load_class->GetBlock()->RemoveInstruction(load_class);
+ }
+ } else if (can_merge_with_load_class) {
// Pass the initialization duty to the `HLoadClass` instruction,
// and remove the instruction from the graph.
- cls->SetMustGenerateClinitCheck(true);
+ load_class->SetMustGenerateClinitCheck(true);
check->GetBlock()->RemoveInstruction(check);
}
}
@@ -86,30 +120,60 @@
DCHECK(last_input != nullptr)
<< "Last input is not HLoadClass. It is " << last_input->DebugName();
- // Remove a load class instruction as last input of a static
- // invoke, which has been added (along with a clinit check,
- // removed by PrepareForRegisterAllocation::VisitClinitCheck
- // previously) by the graph builder during the creation of the
- // static invoke instruction, but is no longer required at this
- // stage (i.e., after inlining has been performed).
- invoke->RemoveLoadClassAsLastInput();
+ // Detach the explicit class initialization check from the invoke.
+ // Keeping track of the initializing instruction is no longer required
+ // at this stage (i.e., after inlining has been performed).
+ invoke->RemoveExplicitClinitCheck(HInvokeStaticOrDirect::ClinitCheckRequirement::kNone);
- // The static call will initialize the class so there's no need for a clinit check if
- // it's the first user.
- // There is one special case where we still need the clinit check, when inlining. Because
- // currently the callee is responsible for reporting parameters to the GC, the code
- // that walks the stack during `artQuickResolutionTrampoline` cannot be interrupted for GC.
- // Therefore we cannot allocate any object in that code, including loading a new class.
- if (last_input == invoke->GetPrevious() && !invoke->IsFromInlinedInvoke()) {
- last_input->SetMustGenerateClinitCheck(false);
+ // Merging with load class should have happened in VisitClinitCheck().
+ DCHECK(!CanMoveClinitCheck(last_input, invoke));
+ }
+}
- // If the load class instruction is no longer used, remove it from
- // the graph.
- if (!last_input->HasUses()) {
- last_input->GetBlock()->RemoveInstruction(last_input);
- }
+bool PrepareForRegisterAllocation::CanMoveClinitCheck(HInstruction* input, HInstruction* user) {
+ // Determine if input and user come from the same dex instruction, so that we can move
+ // the clinit check responsibility from one to the other, i.e. from HClinitCheck (user)
+ // to HLoadClass (input), or from HClinitCheck (input) to HInvokeStaticOrDirect (user).
+
+ // Start with a quick dex pc check.
+ if (user->GetDexPc() != input->GetDexPc()) {
+ return false;
+ }
+
+ // Now do a thorough environment check that this is really coming from the same instruction in
+ // the same inlined graph. Unfortunately, we have to go through the whole environment chain.
+ HEnvironment* user_environment = user->GetEnvironment();
+ HEnvironment* input_environment = input->GetEnvironment();
+ while (user_environment != nullptr || input_environment != nullptr) {
+ if (user_environment == nullptr || input_environment == nullptr) {
+ // Different environment chain length. This happens when a method is called
+ // once directly and once indirectly through another inlined method.
+ return false;
+ }
+ if (user_environment->GetDexPc() != input_environment->GetDexPc() ||
+ user_environment->GetMethodIdx() != input_environment->GetMethodIdx() ||
+ !IsSameDexFile(user_environment->GetDexFile(), input_environment->GetDexFile())) {
+ return false;
+ }
+ user_environment = user_environment->GetParent();
+ input_environment = input_environment->GetParent();
+ }
+
+ // Check for code motion taking the input to a different block.
+ if (user->GetBlock() != input->GetBlock()) {
+ return false;
+ }
+
+ // In debug mode, check that we have not inserted a throwing instruction
+ // or an instruction with side effects between input and user.
+ if (kIsDebugBuild) {
+ for (HInstruction* between = input->GetNext(); between != user; between = between->GetNext()) {
+ CHECK(between != nullptr); // User must be after input in the same block.
+ CHECK(!between->CanThrow());
+ CHECK(!between->HasSideEffects());
}
}
+ return true;
}
} // namespace art
diff --git a/compiler/optimizing/prepare_for_register_allocation.h b/compiler/optimizing/prepare_for_register_allocation.h
index d7f277f..a70fb30 100644
--- a/compiler/optimizing/prepare_for_register_allocation.h
+++ b/compiler/optimizing/prepare_for_register_allocation.h
@@ -41,6 +41,8 @@
void VisitCondition(HCondition* condition) OVERRIDE;
void VisitInvokeStaticOrDirect(HInvokeStaticOrDirect* invoke) OVERRIDE;
+ bool CanMoveClinitCheck(HInstruction* input, HInstruction* user);
+
DISALLOW_COPY_AND_ASSIGN(PrepareForRegisterAllocation);
};
diff --git a/test/478-checker-clinit-check-pruning/expected.txt b/test/478-checker-clinit-check-pruning/expected.txt
index 387e1a7..7de097f 100644
--- a/test/478-checker-clinit-check-pruning/expected.txt
+++ b/test/478-checker-clinit-check-pruning/expected.txt
@@ -4,3 +4,9 @@
Main$ClassWithClinit4's static initializer
Main$ClassWithClinit5's static initializer
Main$ClassWithClinit6's static initializer
+Main$ClassWithClinit7's static initializer
+Main$ClassWithClinit8's static initializer
+Main$ClassWithClinit9's static initializer
+Main$ClassWithClinit10's static initializer
+Main$ClassWithClinit11's static initializer
+Main$ClassWithClinit12's static initializer
diff --git a/test/478-checker-clinit-check-pruning/src/Main.java b/test/478-checker-clinit-check-pruning/src/Main.java
index cff6273..7993513 100644
--- a/test/478-checker-clinit-check-pruning/src/Main.java
+++ b/test/478-checker-clinit-check-pruning/src/Main.java
@@ -83,7 +83,7 @@
// before the next pass (liveness analysis) instead.
/// CHECK-START: void Main.invokeStaticNotInlined() liveness (before)
- /// CHECK: InvokeStaticOrDirect
+ /// CHECK: InvokeStaticOrDirect clinit_check:implicit
/// CHECK-START: void Main.invokeStaticNotInlined() liveness (before)
/// CHECK-NOT: LoadClass
@@ -269,7 +269,7 @@
/// CHECK-START: void Main.noClinitBecauseOfInvokeStatic() liveness (before)
/// CHECK-DAG: <<IntConstant:i\d+>> IntConstant 0
/// CHECK-DAG: <<LoadClass:l\d+>> LoadClass gen_clinit_check:false
- /// CHECK-DAG: InvokeStaticOrDirect
+ /// CHECK-DAG: InvokeStaticOrDirect clinit_check:implicit
/// CHECK-DAG: StaticFieldSet [<<LoadClass>>,<<IntConstant>>]
/// CHECK-START: void Main.noClinitBecauseOfInvokeStatic() liveness (before)
@@ -289,7 +289,7 @@
/// CHECK-DAG: <<IntConstant:i\d+>> IntConstant 0
/// CHECK-DAG: <<LoadClass:l\d+>> LoadClass gen_clinit_check:true
/// CHECK-DAG: StaticFieldSet [<<LoadClass>>,<<IntConstant>>]
- /// CHECK-DAG: InvokeStaticOrDirect
+ /// CHECK-DAG: InvokeStaticOrDirect clinit_check:none
/// CHECK-START: void Main.clinitBecauseOfFieldAccess() liveness (before)
/// CHECK-NOT: ClinitCheck
@@ -298,6 +298,206 @@
ClassWithClinit2.$noinline$staticMethod();
}
+ /*
+ * Verify that LoadClass from const-class is not merged with
+ * later invoke-static (or it's ClinitCheck).
+ */
+
+ /// CHECK-START: void Main.constClassAndInvokeStatic(java.lang.Iterable) liveness (before)
+ /// CHECK: LoadClass gen_clinit_check:false
+ /// CHECK: InvokeStaticOrDirect clinit_check:implicit
+
+ /// CHECK-START: void Main.constClassAndInvokeStatic(java.lang.Iterable) liveness (before)
+ /// CHECK-NOT: ClinitCheck
+
+ static void constClassAndInvokeStatic(Iterable it) {
+ $opt$inline$ignoreClass(ClassWithClinit7.class);
+ ClassWithClinit7.someStaticMethod(it);
+ }
+
+ static void $opt$inline$ignoreClass(Class c) {
+ }
+
+ static class ClassWithClinit7 {
+ static {
+ System.out.println("Main$ClassWithClinit7's static initializer");
+ }
+
+ // Note: not inlined from constClassAndInvokeStatic() but fully inlined from main().
+ static void someStaticMethod(Iterable it) {
+ // We're not inlining invoke-interface at the moment.
+ it.iterator();
+ }
+ }
+
+ /*
+ * Verify that LoadClass from sget is not merged with later invoke-static.
+ */
+
+ /// CHECK-START: void Main.sgetAndInvokeStatic(java.lang.Iterable) liveness (before)
+ /// CHECK: LoadClass gen_clinit_check:true
+ /// CHECK: InvokeStaticOrDirect clinit_check:none
+
+ /// CHECK-START: void Main.sgetAndInvokeStatic(java.lang.Iterable) liveness (before)
+ /// CHECK-NOT: ClinitCheck
+
+ static void sgetAndInvokeStatic(Iterable it) {
+ $opt$inline$ignoreInt(ClassWithClinit8.value);
+ ClassWithClinit8.someStaticMethod(it);
+ }
+
+ static void $opt$inline$ignoreInt(int i) {
+ }
+
+ static class ClassWithClinit8 {
+ public static int value = 0;
+ static {
+ System.out.println("Main$ClassWithClinit8's static initializer");
+ }
+
+ // Note: not inlined from sgetAndInvokeStatic() but fully inlined from main().
+ static void someStaticMethod(Iterable it) {
+ // We're not inlining invoke-interface at the moment.
+ it.iterator();
+ }
+ }
+
+ /*
+ * Verify that LoadClass from const-class, ClinitCheck from sget and
+ * InvokeStaticOrDirect from invoke-static are not merged.
+ */
+
+ /// CHECK-START: void Main.constClassSgetAndInvokeStatic(java.lang.Iterable) liveness (before)
+ /// CHECK: LoadClass gen_clinit_check:false
+ /// CHECK: ClinitCheck
+ /// CHECK: InvokeStaticOrDirect clinit_check:none
+
+ static void constClassSgetAndInvokeStatic(Iterable it) {
+ $opt$inline$ignoreClass(ClassWithClinit9.class);
+ $opt$inline$ignoreInt(ClassWithClinit9.value);
+ ClassWithClinit9.someStaticMethod(it);
+ }
+
+ static class ClassWithClinit9 {
+ public static int value = 0;
+ static {
+ System.out.println("Main$ClassWithClinit9's static initializer");
+ }
+
+ // Note: not inlined from constClassSgetAndInvokeStatic() but fully inlined from main().
+ static void someStaticMethod(Iterable it) {
+ // We're not inlining invoke-interface at the moment.
+ it.iterator();
+ }
+ }
+
+ /*
+ * Verify that LoadClass from a fully-inlined invoke-static is not merged
+ * with InvokeStaticOrDirect from a later invoke-static to the same method.
+ */
+
+ /// CHECK-START: void Main.inlinedInvokeStaticViaNonStatic(java.lang.Iterable) liveness (before)
+ /// CHECK: LoadClass gen_clinit_check:true
+ /// CHECK: InvokeStaticOrDirect clinit_check:none
+
+ /// CHECK-START: void Main.inlinedInvokeStaticViaNonStatic(java.lang.Iterable) liveness (before)
+ /// CHECK-NOT: ClinitCheck
+
+ static void inlinedInvokeStaticViaNonStatic(Iterable it) {
+ inlinedInvokeStaticViaNonStaticHelper(null);
+ inlinedInvokeStaticViaNonStaticHelper(it);
+ }
+
+ static void inlinedInvokeStaticViaNonStaticHelper(Iterable it) {
+ ClassWithClinit10.inlinedForNull(it);
+ }
+
+ static class ClassWithClinit10 {
+ public static int value = 0;
+ static {
+ System.out.println("Main$ClassWithClinit10's static initializer");
+ }
+
+ static void inlinedForNull(Iterable it) {
+ if (it != null) {
+ // We're not inlining invoke-interface at the moment.
+ it.iterator();
+ }
+ }
+ }
+
+ /*
+ * Check that the LoadClass from an invoke-static C.foo() doesn't get merged with
+ * an invoke-static inside C.foo(). This would mess up the stack walk in the
+ * resolution trampoline where we would have to load C (if C isn't loaded yet)
+ * which is not permitted there.
+ *
+ * Note: In case of failure, we would get an failed assertion during compilation,
+ * so we wouldn't really get to the checker tests below.
+ */
+
+ /// CHECK-START: void Main.inlinedInvokeStaticViaStatic(java.lang.Iterable) liveness (before)
+ /// CHECK: LoadClass gen_clinit_check:true
+ /// CHECK: InvokeStaticOrDirect clinit_check:none
+
+ /// CHECK-START: void Main.inlinedInvokeStaticViaStatic(java.lang.Iterable) liveness (before)
+ /// CHECK-NOT: ClinitCheck
+
+ static void inlinedInvokeStaticViaStatic(Iterable it) {
+ ClassWithClinit11.callInlinedForNull(it);
+ }
+
+ static class ClassWithClinit11 {
+ public static int value = 0;
+ static {
+ System.out.println("Main$ClassWithClinit11's static initializer");
+ }
+
+ static void callInlinedForNull(Iterable it) {
+ inlinedForNull(it);
+ }
+
+ static void inlinedForNull(Iterable it) {
+ // We're not inlining invoke-interface at the moment.
+ it.iterator();
+ }
+ }
+
+ /*
+ * A test similar to inlinedInvokeStaticViaStatic() but doing the indirect invoke
+ * twice with the first one to be fully inlined.
+ */
+
+ /// CHECK-START: void Main.inlinedInvokeStaticViaStaticTwice(java.lang.Iterable) liveness (before)
+ /// CHECK: LoadClass gen_clinit_check:true
+ /// CHECK: InvokeStaticOrDirect clinit_check:none
+
+ /// CHECK-START: void Main.inlinedInvokeStaticViaStaticTwice(java.lang.Iterable) liveness (before)
+ /// CHECK-NOT: ClinitCheck
+
+ static void inlinedInvokeStaticViaStaticTwice(Iterable it) {
+ ClassWithClinit12.callInlinedForNull(null);
+ ClassWithClinit12.callInlinedForNull(it);
+ }
+
+ static class ClassWithClinit12 {
+ public static int value = 0;
+ static {
+ System.out.println("Main$ClassWithClinit12's static initializer");
+ }
+
+ static void callInlinedForNull(Iterable it) {
+ inlinedForNull(it);
+ }
+
+ static void inlinedForNull(Iterable it) {
+ if (it != null) {
+ // We're not inlining invoke-interface at the moment.
+ it.iterator();
+ }
+ }
+ }
+
// TODO: Add a test for the case of a static method whose declaring
// class type index is not available (i.e. when `storage_index`
// equals `DexFile::kDexNoIndex` in
@@ -310,5 +510,12 @@
ClassWithClinit4.invokeStaticNotInlined();
SubClassOfClassWithClinit5.invokeStaticInlined();
SubClassOfClassWithClinit6.invokeStaticNotInlined();
+ Iterable it = new Iterable() { public java.util.Iterator iterator() { return null; } };
+ constClassAndInvokeStatic(it);
+ sgetAndInvokeStatic(it);
+ constClassSgetAndInvokeStatic(it);
+ inlinedInvokeStaticViaNonStatic(it);
+ inlinedInvokeStaticViaStatic(it);
+ inlinedInvokeStaticViaStaticTwice(it);
}
}