diff options
| -rw-r--r-- | compiler/optimizing/graph_visualizer.cc | 3 | ||||
| -rw-r--r-- | compiler/optimizing/nodes.cc | 13 | ||||
| -rw-r--r-- | compiler/optimizing/nodes.h | 18 | ||||
| -rw-r--r-- | compiler/optimizing/prepare_for_register_allocation.cc | 116 | ||||
| -rw-r--r-- | compiler/optimizing/prepare_for_register_allocation.h | 2 | ||||
| -rw-r--r-- | test/478-checker-clinit-check-pruning/expected.txt | 6 | ||||
| -rw-r--r-- | test/478-checker-clinit-check-pruning/src/Main.java | 213 |
7 files changed, 333 insertions, 38 deletions
diff --git a/compiler/optimizing/graph_visualizer.cc b/compiler/optimizing/graph_visualizer.cc index 2b7790184a..af3ecb1031 100644 --- a/compiler/optimizing/graph_visualizer.cc +++ b/compiler/optimizing/graph_visualizer.cc @@ -397,6 +397,9 @@ class HGraphVisualizerPrinter : public HGraphDelegateVisitor { << 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 73a44ee2cb..0a39ff31bf 100644 --- a/compiler/optimizing/nodes.cc +++ b/compiler/optimizing/nodes.cc @@ -2068,6 +2068,19 @@ void HInvokeStaticOrDirect::RemoveInputAt(size_t index) { } } +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 2878ac9899..2160601e06 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -3505,20 +3505,19 @@ class HInvokeStaticOrDirect : public HInvoke { 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 @@ class HInvokeStaticOrDirect : public HInvoke { } // 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 @@ class HInvokeStaticOrDirect : public HInvoke { 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 ca928ae0f2..f3d075caaa 100644 --- a/compiler/optimizing/prepare_for_register_allocation.cc +++ b/compiler/optimizing/prepare_for_register_allocation.cc @@ -48,12 +48,46 @@ void PrepareForRegisterAllocation::VisitBoundType(HBoundType* bound_type) { } 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 @@ void PrepareForRegisterAllocation::VisitInvokeStaticOrDirect(HInvokeStaticOrDire 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(); - - // 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); - - // If the load class instruction is no longer used, remove it from - // the graph. - if (!last_input->HasUses()) { - last_input->GetBlock()->RemoveInstruction(last_input); - } + // 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); + + // Merging with load class should have happened in VisitClinitCheck(). + DCHECK(!CanMoveClinitCheck(last_input, invoke)); + } +} + +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 d7f277fa0d..a70fb309df 100644 --- a/compiler/optimizing/prepare_for_register_allocation.h +++ b/compiler/optimizing/prepare_for_register_allocation.h @@ -41,6 +41,8 @@ class PrepareForRegisterAllocation : public HGraphDelegateVisitor { 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 387e1a7cb1..7de097f666 100644 --- a/test/478-checker-clinit-check-pruning/expected.txt +++ b/test/478-checker-clinit-check-pruning/expected.txt @@ -4,3 +4,9 @@ Main$ClassWithClinit3's static initializer 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 cff627373d..79935134b4 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 @@ public class Main { // 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 @@ public class Main { /// 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 @@ public class Main { /// 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 @@ public class Main { 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 @@ public class Main { 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); } } |