diff options
| -rw-r--r-- | compiler/optimizing/inliner.cc | 38 | ||||
| -rw-r--r-- | compiler/optimizing/inliner.h | 3 | ||||
| -rw-r--r-- | test/638-checker-inline-caches/src/Main.java | 75 | ||||
| -rw-r--r-- | test/644-checker-deopt/expected.txt | 0 | ||||
| -rw-r--r-- | test/644-checker-deopt/info.txt | 2 | ||||
| -rw-r--r-- | test/644-checker-deopt/profile | 2 | ||||
| -rw-r--r-- | test/644-checker-deopt/run | 17 | ||||
| -rw-r--r-- | test/644-checker-deopt/src/Main.java | 74 | ||||
| -rw-r--r-- | test/knownfailures.json | 1 |
9 files changed, 71 insertions, 141 deletions
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc index 8674e727bb..ac5cbf9b51 100644 --- a/compiler/optimizing/inliner.cc +++ b/compiler/optimizing/inliner.cc @@ -470,6 +470,33 @@ static Handle<mirror::ObjectArray<mirror::Class>> AllocateInlineCacheHolder( return inline_cache; } +bool HInliner::UseOnlyPolymorphicInliningWithNoDeopt() { + // If we are compiling AOT or OSR, pretend the call using inline caches is polymorphic and + // do not generate a deopt. + // + // For AOT: + // Generating a deopt does not ensure that we will actually capture the new types; + // and the danger is that we could be stuck in a loop with "forever" deoptimizations. + // Take for example the following scenario: + // - we capture the inline cache in one run + // - the next run, we deoptimize because we miss a type check, but the method + // never becomes hot again + // In this case, the inline cache will not be updated in the profile and the AOT code + // will keep deoptimizing. + // Another scenario is if we use profile compilation for a process which is not allowed + // to JIT (e.g. system server). If we deoptimize we will run interpreted code for the + // rest of the lifetime. + // TODO(calin): + // This is a compromise because we will most likely never update the inline cache + // in the profile (unless there's another reason to deopt). So we might be stuck with + // a sub-optimal inline cache. + // We could be smarter when capturing inline caches to mitigate this. + // (e.g. by having different thresholds for new and old methods). + // + // For OSR: + // We may come from the interpreter and it may have seen different receiver types. + return Runtime::Current()->IsAotCompiler() || outermost_graph_->IsCompilingOsr(); +} bool HInliner::TryInlineFromInlineCache(const DexFile& caller_dex_file, HInvoke* invoke_instruction, ArtMethod* resolved_method) @@ -503,9 +530,7 @@ bool HInliner::TryInlineFromInlineCache(const DexFile& caller_dex_file, case kInlineCacheMonomorphic: { MaybeRecordStat(kMonomorphicCall); - if (outermost_graph_->IsCompilingOsr()) { - // If we are compiling OSR, we pretend this call is polymorphic, as we may come from the - // interpreter and it may have seen different receiver types. + if (UseOnlyPolymorphicInliningWithNoDeopt()) { return TryInlinePolymorphicCall(invoke_instruction, resolved_method, inline_cache); } else { return TryInlineMonomorphicCall(invoke_instruction, resolved_method, inline_cache); @@ -926,14 +951,11 @@ bool HInliner::TryInlinePolymorphicCall(HInvoke* invoke_instruction, // If we have inlined all targets before, and this receiver is the last seen, // we deoptimize instead of keeping the original invoke instruction. - bool deoptimize = all_targets_inlined && + bool deoptimize = !UseOnlyPolymorphicInliningWithNoDeopt() && + all_targets_inlined && (i != InlineCache::kIndividualCacheSize - 1) && (classes->Get(i + 1) == nullptr); - if (outermost_graph_->IsCompilingOsr()) { - // We do not support HDeoptimize in OSR methods. - deoptimize = false; - } HInstruction* compare = AddTypeGuard(receiver, cursor, bb_cursor, diff --git a/compiler/optimizing/inliner.h b/compiler/optimizing/inliner.h index 9e4685cbf4..67476b6956 100644 --- a/compiler/optimizing/inliner.h +++ b/compiler/optimizing/inliner.h @@ -180,6 +180,9 @@ class HInliner : public HOptimization { Handle<mirror::ObjectArray<mirror::Class>> classes) REQUIRES_SHARED(Locks::mutator_lock_); + // Returns whether or not we should use only polymorphic inlining with no deoptimizations. + bool UseOnlyPolymorphicInliningWithNoDeopt(); + // Try CHA-based devirtualization to change virtual method calls into // direct calls. // Returns the actual method that resolved_method can be devirtualized to. diff --git a/test/638-checker-inline-caches/src/Main.java b/test/638-checker-inline-caches/src/Main.java index 680bd14dbc..f104e6aea8 100644 --- a/test/638-checker-inline-caches/src/Main.java +++ b/test/638-checker-inline-caches/src/Main.java @@ -36,16 +36,17 @@ public class Main { /// CHECK: InvokeVirtual method_name:Super.getValue /// CHECK-START: int Main.inlineMonomorphicSubA(Super) inliner (after) - /// CHECK-NOT: InvokeVirtual method_name:Super.getValue - - /// CHECK-START: int Main.inlineMonomorphicSubA(Super) inliner (after) /// CHECK: <<SubARet:i\d+>> IntConstant 42 /// CHECK: <<Obj:l\d+>> NullCheck /// CHECK: <<ObjClass:l\d+>> InstanceFieldGet [<<Obj>>] field_name:java.lang.Object.shadow$_klass_ /// CHECK: <<InlineClass:l\d+>> LoadClass class_name:SubA /// CHECK: <<Test:z\d+>> NotEqual [<<InlineClass>>,<<ObjClass>>] - /// CHECK: Deoptimize [<<Test>>,<<Obj>>] - /// CHECK: Return [<<SubARet>>] + /// CHECK: <<DefaultRet:i\d+>> InvokeVirtual [<<Obj>>] method_name:Super.getValue + + /// CHECK: <<Ret:i\d+>> Phi [<<SubARet>>,<<DefaultRet>>] + /// CHECK: Return [<<Ret>>] + + /// CHECK-NOT: Deoptimize public static int inlineMonomorphicSubA(Super a) { return a.getValue(); } @@ -53,27 +54,27 @@ public class Main { /// CHECK-START: int Main.inlinePolymophicSubASubB(Super) inliner (before) /// CHECK: InvokeVirtual method_name:Super.getValue - /// CHECK-START: int Main.inlinePolymophicSubASubB(Super) inliner (after) - /// CHECK-NOT: InvokeVirtual method_name:Super.getValue - // Note that the order in which the types are added to the inline cache in the profile matters. /// CHECK-START: int Main.inlinePolymophicSubASubB(Super) inliner (after) /// CHECK-DAG: <<SubARet:i\d+>> IntConstant 42 /// CHECK-DAG: <<SubBRet:i\d+>> IntConstant 38 - /// CHECK: <<Obj:l\d+>> NullCheck - /// CHECK: <<ObjClassSubA:l\d+>> InstanceFieldGet [<<Obj>>] field_name:java.lang.Object.shadow$_klass_ - /// CHECK: <<InlineClassSubA:l\d+>> LoadClass class_name:SubA - /// CHECK: <<TestSubA:z\d+>> NotEqual [<<InlineClassSubA>>,<<ObjClassSubA>>] - /// CHECK: If [<<TestSubA>>] - - /// CHECK: <<ObjClassSubB:l\d+>> InstanceFieldGet field_name:java.lang.Object.shadow$_klass_ - /// CHECK: <<InlineClassSubB:l\d+>> LoadClass class_name:SubB - /// CHECK: <<TestSubB:z\d+>> NotEqual [<<InlineClassSubB>>,<<ObjClassSubB>>] - /// CHECK: Deoptimize [<<TestSubB>>,<<Obj>>] - - /// CHECK: <<Ret:i\d+>> Phi [<<SubARet>>,<<SubBRet>>] - /// CHECK: Return [<<Ret>>] + /// CHECK-DAG: <<Obj:l\d+>> NullCheck + /// CHECK-DAG: <<ObjClassSubA:l\d+>> InstanceFieldGet [<<Obj>>] field_name:java.lang.Object.shadow$_klass_ + /// CHECK-DAG: <<InlineClassSubA:l\d+>> LoadClass class_name:SubA + /// CHECK-DAG: <<TestSubA:z\d+>> NotEqual [<<InlineClassSubA>>,<<ObjClassSubA>>] + /// CHECK-DAG: If [<<TestSubA>>] + + /// CHECK-DAG: <<ObjClassSubB:l\d+>> InstanceFieldGet field_name:java.lang.Object.shadow$_klass_ + /// CHECK-DAG: <<InlineClassSubB:l\d+>> LoadClass class_name:SubB + /// CHECK-DAG: <<TestSubB:z\d+>> NotEqual [<<InlineClassSubB>>,<<ObjClassSubB>>] + /// CHECK-DAG: <<DefaultRet:i\d+>> InvokeVirtual [<<Obj>>] method_name:Super.getValue + + /// CHECK-DAG: <<FirstMerge:i\d+>> Phi [<<SubBRet>>,<<DefaultRet>>] + /// CHECK-DAG: <<Ret:i\d+>> Phi [<<SubARet>>,<<FirstMerge>>] + /// CHECK-DAG: Return [<<Ret>>] + + /// CHECK-NOT: Deoptimize public static int inlinePolymophicSubASubB(Super a) { return a.getValue(); } @@ -81,27 +82,27 @@ public class Main { /// CHECK-START: int Main.inlinePolymophicCrossDexSubASubC(Super) inliner (before) /// CHECK: InvokeVirtual method_name:Super.getValue - /// CHECK-START: int Main.inlinePolymophicCrossDexSubASubC(Super) inliner (after) - /// CHECK-NOT: InvokeVirtual method_name:Super.getValue - // Note that the order in which the types are added to the inline cache in the profile matters. /// CHECK-START: int Main.inlinePolymophicCrossDexSubASubC(Super) inliner (after) /// CHECK-DAG: <<SubARet:i\d+>> IntConstant 42 /// CHECK-DAG: <<SubCRet:i\d+>> IntConstant 24 - /// CHECK: <<Obj:l\d+>> NullCheck - /// CHECK: <<ObjClassSubA:l\d+>> InstanceFieldGet [<<Obj>>] field_name:java.lang.Object.shadow$_klass_ - /// CHECK: <<InlineClassSubA:l\d+>> LoadClass class_name:SubA - /// CHECK: <<TestSubA:z\d+>> NotEqual [<<InlineClassSubA>>,<<ObjClassSubA>>] - /// CHECK: If [<<TestSubA>>] - - /// CHECK: <<ObjClassSubC:l\d+>> InstanceFieldGet field_name:java.lang.Object.shadow$_klass_ - /// CHECK: <<InlineClassSubC:l\d+>> LoadClass class_name:SubC - /// CHECK: <<TestSubC:z\d+>> NotEqual [<<InlineClassSubC>>,<<ObjClassSubC>>] - /// CHECK: Deoptimize [<<TestSubC>>,<<Obj>>] - - /// CHECK: <<Ret:i\d+>> Phi [<<SubARet>>,<<SubCRet>>] - /// CHECK: Return [<<Ret>>] + /// CHECK-DAG: <<Obj:l\d+>> NullCheck + /// CHECK-DAG: <<ObjClassSubA:l\d+>> InstanceFieldGet [<<Obj>>] field_name:java.lang.Object.shadow$_klass_ + /// CHECK-DAG: <<InlineClassSubA:l\d+>> LoadClass class_name:SubA + /// CHECK-DAG: <<TestSubA:z\d+>> NotEqual [<<InlineClassSubA>>,<<ObjClassSubA>>] + /// CHECK-DAG: If [<<TestSubA>>] + + /// CHECK-DAG: <<ObjClassSubC:l\d+>> InstanceFieldGet field_name:java.lang.Object.shadow$_klass_ + /// CHECK-DAG: <<InlineClassSubC:l\d+>> LoadClass class_name:SubC + /// CHECK-DAG: <<TestSubC:z\d+>> NotEqual [<<InlineClassSubC>>,<<ObjClassSubC>>] + /// CHECK-DAG: <<DefaultRet:i\d+>> InvokeVirtual [<<Obj>>] method_name:Super.getValue + + /// CHECK-DAG: <<FirstMerge:i\d+>> Phi [<<SubCRet>>,<<DefaultRet>>] + /// CHECK-DAG: <<Ret:i\d+>> Phi [<<SubARet>>,<<FirstMerge>>] + /// CHECK-DAG: Return [<<Ret>>] + + /// CHECK-NOT: Deoptimize public static int inlinePolymophicCrossDexSubASubC(Super a) { return a.getValue(); } diff --git a/test/644-checker-deopt/expected.txt b/test/644-checker-deopt/expected.txt deleted file mode 100644 index e69de29bb2..0000000000 --- a/test/644-checker-deopt/expected.txt +++ /dev/null diff --git a/test/644-checker-deopt/info.txt b/test/644-checker-deopt/info.txt deleted file mode 100644 index c5fb12c570..0000000000 --- a/test/644-checker-deopt/info.txt +++ /dev/null @@ -1,2 +0,0 @@ -Regression test for making sure HDeoptimize is executed before -the code it should have prevented executing. diff --git a/test/644-checker-deopt/profile b/test/644-checker-deopt/profile deleted file mode 100644 index cb261cc694..0000000000 --- a/test/644-checker-deopt/profile +++ /dev/null @@ -1,2 +0,0 @@ -LMain;->inlineMonomorphic(LMain;)I+LMain; -LMain;->inlinePolymorphic(LMain;)I+LMain;,LSubMain; diff --git a/test/644-checker-deopt/run b/test/644-checker-deopt/run deleted file mode 100644 index 146e180000..0000000000 --- a/test/644-checker-deopt/run +++ /dev/null @@ -1,17 +0,0 @@ -#!/bin/bash -# -# Copyright (C) 2017 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. - -exec ${RUN} $@ --profile -Xcompiler-option --compiler-filter=speed-profile diff --git a/test/644-checker-deopt/src/Main.java b/test/644-checker-deopt/src/Main.java deleted file mode 100644 index 17c80a6057..0000000000 --- a/test/644-checker-deopt/src/Main.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright (C) 2017 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: int Main.inlineMonomorphic(Main) inliner (before) - /// CHECK: InvokeVirtual method_name:Main.getValue - - /// CHECK-START: int Main.inlineMonomorphic(Main) inliner (after) - /// CHECK-NOT: InvokeVirtual method_name:Main.getValue - - /// CHECK-START: int Main.inlineMonomorphic(Main) licm (before) - /// CHECK: <<Deopt:l\d+>> Deoptimize - /// CHECK: InstanceFieldGet [<<Deopt>>] field_name:Main.value - - /// CHECK-START: int Main.inlineMonomorphic(Main) licm (after) - /// CHECK: <<Deopt:l\d+>> Deoptimize - /// CHECK: InstanceFieldGet [<<Deopt>>] field_name:Main.value - - public static int inlineMonomorphic(Main a) { - if (a == null) { - return 42; - } - int i = 0; - while (i < 100) { - i += a.getValue(); - } - return i; - } - - /// CHECK-START: int Main.inlinePolymorphic(Main) inliner (before) - /// CHECK: InvokeVirtual method_name:Main.getValue - - /// CHECK-START: int Main.inlinePolymorphic(Main) inliner (after) - /// CHECK-NOT: InvokeVirtual method_name:Main.getValue - - /// CHECK-START: int Main.inlineMonomorphic(Main) licm (before) - /// CHECK: <<Deopt:l\d+>> Deoptimize - /// CHECK: InstanceFieldGet [<<Deopt>>] field_name:Main.value - - /// CHECK-START: int Main.inlineMonomorphic(Main) licm (after) - /// CHECK: <<Deopt:l\d+>> Deoptimize - /// CHECK: InstanceFieldGet [<<Deopt>>] field_name:Main.value - public static int inlinePolymorphic(Main a) { - return a.getValue(); - } - - public int getValue() { - return value; - } - - public static void main(String[] args) { - inlineMonomorphic(new Main()); - } - - int value = 1; -} - -// Add a subclass of 'Main' to write the polymorphic inline cache in the profile. -class SubMain extends Main { -} diff --git a/test/knownfailures.json b/test/knownfailures.json index 4b44df7958..168b673110 100644 --- a/test/knownfailures.json +++ b/test/knownfailures.json @@ -508,7 +508,6 @@ "640-checker-short-simd", "641-checker-arraycopy", "643-checker-bogus-ic", - "644-checker-deopt", "645-checker-abs-simd", "706-checker-scheduler"], "description": ["Checker tests are not compatible with jvmti."], |