From ccc61972ca31bbb3bf82cdc30656c13bebfbe6a9 Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Thu, 1 Oct 2015 14:34:20 +0100 Subject: [PATCH] Do more inlining when JITting. We now check the verification status of the class. This triggered a bug when an inlined method was not compiled (typically in a JIT configuration), and the path for deopting in StackVisitor was using the wrong ArtMethod in order to know the compiler that compiled the current frame. Change-Id: I81d3ca0cf5cd3864b83b63dd954c58e1f2adaad4 --- compiler/optimizing/inliner.cc | 14 ++++---- runtime/stack.cc | 23 ++++++------- runtime/stack.h | 4 +++ test/535-deopt-and-inlining/expected.txt | 0 test/535-deopt-and-inlining/info.txt | 2 ++ test/535-deopt-and-inlining/src/Main.java | 55 +++++++++++++++++++++++++++++++ 6 files changed, 81 insertions(+), 17 deletions(-) create mode 100644 test/535-deopt-and-inlining/expected.txt create mode 100644 test/535-deopt-and-inlining/info.txt create mode 100644 test/535-deopt-and-inlining/src/Main.java diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc index 039029aa5..94437ca79 100644 --- a/compiler/optimizing/inliner.cc +++ b/compiler/optimizing/inliner.cc @@ -247,12 +247,14 @@ bool HInliner::TryInline(HInvoke* invoke_instruction) { return false; } - uint16_t class_def_idx = resolved_method->GetDeclaringClass()->GetDexClassDefIndex(); - if (!compiler_driver_->IsMethodVerifiedWithoutFailures( - resolved_method->GetDexMethodIndex(), class_def_idx, *resolved_method->GetDexFile())) { - VLOG(compiler) << "Method " << PrettyMethod(method_index, caller_dex_file) - << " couldn't be verified, so it cannot be inlined"; - return false; + if (!resolved_method->GetDeclaringClass()->IsVerified()) { + uint16_t class_def_idx = resolved_method->GetDeclaringClass()->GetDexClassDefIndex(); + if (!compiler_driver_->IsMethodVerifiedWithoutFailures( + resolved_method->GetDexMethodIndex(), class_def_idx, *resolved_method->GetDexFile())) { + VLOG(compiler) << "Method " << PrettyMethod(method_index, caller_dex_file) + << " couldn't be verified, so it cannot be inlined"; + return false; + } } if (invoke_instruction->IsInvokeStaticOrDirect() && diff --git a/runtime/stack.cc b/runtime/stack.cc index 7f72f8ab6..1d21a6494 100644 --- a/runtime/stack.cc +++ b/runtime/stack.cc @@ -110,7 +110,7 @@ StackVisitor::StackVisitor(Thread* thread, } InlineInfo StackVisitor::GetCurrentInlineInfo() const { - ArtMethod* outer_method = *GetCurrentQuickFrame(); + ArtMethod* outer_method = GetOuterMethod(); uint32_t native_pc_offset = outer_method->NativeQuickPcOffset(cur_quick_frame_pc_); CodeInfo code_info = outer_method->GetOptimizedCodeInfo(); StackMapEncoding encoding = code_info.ExtractEncoding(); @@ -194,11 +194,12 @@ size_t StackVisitor::GetNativePcOffset() const { } bool StackVisitor::IsReferenceVReg(ArtMethod* m, uint16_t vreg) { + DCHECK_EQ(m, GetMethod()); // Process register map (which native and runtime methods don't have) if (m->IsNative() || m->IsRuntimeMethod() || m->IsProxyMethod()) { return false; } - if (m->IsOptimized(sizeof(void*))) { + if (GetOuterMethod()->IsOptimized(sizeof(void*))) { return true; // TODO: Implement. } const uint8_t* native_gc_map = m->GetNativeGcMap(sizeof(void*)); @@ -251,7 +252,7 @@ bool StackVisitor::GetVReg(ArtMethod* m, uint16_t vreg, VRegKind kind, uint32_t* if (GetVRegFromDebuggerShadowFrame(vreg, kind, val)) { return true; } - if (m->IsOptimized(sizeof(void*))) { + if (GetOuterMethod()->IsOptimized(sizeof(void*))) { return GetVRegFromOptimizedCode(m, vreg, kind, val); } else { return GetVRegFromQuickCode(m, vreg, kind, val); @@ -288,15 +289,15 @@ bool StackVisitor::GetVRegFromQuickCode(ArtMethod* m, uint16_t vreg, VRegKind ki bool StackVisitor::GetVRegFromOptimizedCode(ArtMethod* m, uint16_t vreg, VRegKind kind, uint32_t* val) const { + ArtMethod* outer_method = GetOuterMethod(); + const void* code_pointer = outer_method->GetQuickOatCodePointer(sizeof(void*)); + DCHECK(code_pointer != nullptr); DCHECK_EQ(m, GetMethod()); const DexFile::CodeItem* code_item = m->GetCodeItem(); DCHECK(code_item != nullptr) << PrettyMethod(m); // Can't be null or how would we compile // its instructions? uint16_t number_of_dex_registers = code_item->registers_size_; DCHECK_LT(vreg, code_item->registers_size_); - ArtMethod* outer_method = *GetCurrentQuickFrame(); - const void* code_pointer = outer_method->GetQuickOatCodePointer(sizeof(void*)); - DCHECK(code_pointer != nullptr); CodeInfo code_info = outer_method->GetOptimizedCodeInfo(); StackMapEncoding encoding = code_info.ExtractEncoding(); @@ -405,7 +406,7 @@ bool StackVisitor::GetVRegPair(ArtMethod* m, uint16_t vreg, VRegKind kind_lo, if (cur_quick_frame_ != nullptr) { DCHECK(context_ != nullptr); // You can't reliably read registers without a context. DCHECK(m == GetMethod()); - if (m->IsOptimized(sizeof(void*))) { + if (GetOuterMethod()->IsOptimized(sizeof(void*))) { return GetVRegPairFromOptimizedCode(m, vreg, kind_lo, kind_hi, val); } else { return GetVRegPairFromQuickCode(m, vreg, kind_lo, kind_hi, val); @@ -481,7 +482,7 @@ bool StackVisitor::SetVReg(ArtMethod* m, uint16_t vreg, uint32_t new_value, if (cur_quick_frame_ != nullptr) { DCHECK(context_ != nullptr); // You can't reliably write registers without a context. DCHECK(m == GetMethod()); - if (m->IsOptimized(sizeof(void*))) { + if (GetOuterMethod()->IsOptimized(sizeof(void*))) { return false; } else { return SetVRegFromQuickCode(m, vreg, new_value, kind); @@ -590,7 +591,7 @@ bool StackVisitor::SetVRegPair(ArtMethod* m, uint16_t vreg, uint64_t new_value, if (cur_quick_frame_ != nullptr) { DCHECK(context_ != nullptr); // You can't reliably write registers without a context. DCHECK(m == GetMethod()); - if (m->IsOptimized(sizeof(void*))) { + if (GetOuterMethod()->IsOptimized(sizeof(void*))) { return false; } else { return SetVRegPairFromQuickCode(m, vreg, new_value, kind_lo, kind_hi); @@ -724,14 +725,14 @@ void StackVisitor::SetFPR(uint32_t reg, uintptr_t value) { uintptr_t StackVisitor::GetReturnPc() const { uint8_t* sp = reinterpret_cast(GetCurrentQuickFrame()); DCHECK(sp != nullptr); - uint8_t* pc_addr = sp + GetMethod()->GetReturnPcOffset().SizeValue(); + uint8_t* pc_addr = sp + GetOuterMethod()->GetReturnPcOffset().SizeValue(); return *reinterpret_cast(pc_addr); } void StackVisitor::SetReturnPc(uintptr_t new_ret_pc) { uint8_t* sp = reinterpret_cast(GetCurrentQuickFrame()); CHECK(sp != nullptr); - uint8_t* pc_addr = sp + GetMethod()->GetReturnPcOffset().SizeValue(); + uint8_t* pc_addr = sp + GetOuterMethod()->GetReturnPcOffset().SizeValue(); *reinterpret_cast(pc_addr) = new_ret_pc; } diff --git a/runtime/stack.h b/runtime/stack.h index 292c74509..31acf0eb6 100644 --- a/runtime/stack.h +++ b/runtime/stack.h @@ -473,6 +473,10 @@ class StackVisitor { ArtMethod* GetMethod() const SHARED_REQUIRES(Locks::mutator_lock_); + ArtMethod* GetOuterMethod() const { + return *GetCurrentQuickFrame(); + } + bool IsShadowFrame() const { return cur_shadow_frame_ != nullptr; } diff --git a/test/535-deopt-and-inlining/expected.txt b/test/535-deopt-and-inlining/expected.txt new file mode 100644 index 000000000..e69de29bb diff --git a/test/535-deopt-and-inlining/info.txt b/test/535-deopt-and-inlining/info.txt new file mode 100644 index 000000000..717612a1a --- /dev/null +++ b/test/535-deopt-and-inlining/info.txt @@ -0,0 +1,2 @@ +Stress test for deoptimization and JIT, to ensure the +stack visitor uses the right ArtMethod when deopting. diff --git a/test/535-deopt-and-inlining/src/Main.java b/test/535-deopt-and-inlining/src/Main.java new file mode 100644 index 000000000..c231bf0e8 --- /dev/null +++ b/test/535-deopt-and-inlining/src/Main.java @@ -0,0 +1,55 @@ +/* + * 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 { + + public static void run() { + // Loop enough to get JIT compilation. + for (int i = 0; i < 10000; ++i) { + doCall(new int[0]); + } + } + + public static void main(String[] args) throws Exception { + run(); + } + + public static void doCall(int[] array) { + try { + deopt(array); + } catch (IndexOutOfBoundsException ioobe) { + // Expected + } + } + + public static void deopt(int[] array) { + // Invoke `deopt` much more than `$inline$deopt` so that only `deopt` gets + // initially JITted. + if (call == 100) { + call = 0; + $inline$deopt(array); + } else { + call++; + } + } + + public static void $inline$deopt(int[] array) { + array[0] = 1; + array[1] = 1; + } + + static int call = 0; +} -- 2.11.0