diff options
| author | 2022-10-06 20:31:46 +0100 | |
|---|---|---|
| committer | 2022-10-10 14:04:24 +0000 | |
| commit | 629c065a416260a2a1e87c034ca0b42fbda0fe44 (patch) | |
| tree | f21b68faeba647ea389ff9abc6b3ef481058c9df | |
| parent | 2096c1fca7a744634ce079ecbda6de28ba1cdc81 (diff) | |
Don't mark speculatively failed inlines as always throws
If we don't inline an always throwing invoke through inline caches,
don't mark it as 'always throwing' since it may not always throw
in other instances.
Bug: 241199625
Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b
Change-Id: I4cb7edaa4716bd1722fba2bef39f31903bf9238e
| -rw-r--r-- | compiler/optimizing/inliner.cc | 49 | ||||
| -rw-r--r-- | compiler/optimizing/inliner.h | 14 | ||||
| -rw-r--r-- | test/638-checker-inline-caches/expected-stdout.txt | 1 | ||||
| -rw-r--r-- | test/638-checker-inline-caches/profile | 1 | ||||
| -rw-r--r-- | test/638-checker-inline-caches/src-multidex/SubC.java | 1 | ||||
| -rw-r--r-- | test/638-checker-inline-caches/src/Main.java | 31 | ||||
| -rw-r--r-- | test/638-checker-inline-caches/src/Super.java | 1 |
7 files changed, 74 insertions, 24 deletions
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc index b739f3f838..4dd4395cd5 100644 --- a/compiler/optimizing/inliner.cc +++ b/compiler/optimizing/inliner.cc @@ -481,7 +481,8 @@ bool HInliner::TryInline(HInvoke* invoke_instruction) { bool result = TryInlineAndReplace(invoke_instruction, actual_method, ReferenceTypeInfo::CreateInvalid(), - /* do_rtp= */ true); + /* do_rtp= */ true, + /* is_speculative= */ false); if (result) { MaybeRecordStat(stats_, MethodCompilationStat::kInlinedInvokeVirtualOrInterface); if (outermost_graph_ == graph_) { @@ -543,7 +544,8 @@ bool HInliner::TryInlineFromCHA(HInvoke* invoke_instruction) { if (!TryInlineAndReplace(invoke_instruction, method, ReferenceTypeInfo::CreateInvalid(), - /* do_rtp= */ true)) { + /* do_rtp= */ true, + /* is_speculative= */ true)) { return false; } AddCHAGuard(invoke_instruction, dex_pc, cursor, bb_cursor); @@ -811,7 +813,8 @@ bool HInliner::TryInlineMonomorphicCall( if (!TryInlineAndReplace(invoke_instruction, resolved_method, ReferenceTypeInfo::Create(monomorphic_type, /* is_exact= */ true), - /* do_rtp= */ false)) { + /* do_rtp= */ false, + /* is_speculative= */ true)) { return false; } @@ -1006,7 +1009,8 @@ bool HInliner::TryInlinePolymorphicCall( !TryBuildAndInline(invoke_instruction, method, ReferenceTypeInfo::Create(handle, /* is_exact= */ true), - &return_replacement)) { + &return_replacement, + /* is_speculative= */ true)) { all_targets_inlined = false; } else { one_target_inlined = true; @@ -1183,7 +1187,8 @@ bool HInliner::TryInlinePolymorphicCallToSameTarget( if (!TryBuildAndInline(invoke_instruction, actual_method, ReferenceTypeInfo::CreateInvalid(), - &return_replacement)) { + &return_replacement, + /* is_speculative= */ true)) { return false; } @@ -1337,11 +1342,13 @@ bool HInliner::TryDevirtualize(HInvoke* invoke_instruction, bool HInliner::TryInlineAndReplace(HInvoke* invoke_instruction, ArtMethod* method, ReferenceTypeInfo receiver_type, - bool do_rtp) { + bool do_rtp, + bool is_speculative) { DCHECK(!invoke_instruction->IsIntrinsic()); HInstruction* return_replacement = nullptr; - if (!TryBuildAndInline(invoke_instruction, method, receiver_type, &return_replacement)) { + if (!TryBuildAndInline( + invoke_instruction, method, receiver_type, &return_replacement, is_speculative)) { return false; } @@ -1490,7 +1497,8 @@ bool HInliner::IsInliningBudgetAvailable(ArtMethod* method, bool HInliner::TryBuildAndInline(HInvoke* invoke_instruction, ArtMethod* method, ReferenceTypeInfo receiver_type, - HInstruction** return_replacement) { + HInstruction** return_replacement, + bool is_speculative) { // If invoke_instruction is devirtualized to a different method, give intrinsics // another chance before we try to inline it. if (invoke_instruction->GetResolvedMethod() != method && method->IsIntrinsic()) { @@ -1553,7 +1561,8 @@ bool HInliner::TryBuildAndInline(HInvoke* invoke_instruction, return false; } - if (!TryBuildAndInlineHelper(invoke_instruction, method, receiver_type, return_replacement)) { + if (!TryBuildAndInlineHelper( + invoke_instruction, method, receiver_type, return_replacement, is_speculative)) { return false; } @@ -1865,7 +1874,8 @@ void HInliner::SubstituteArguments(HGraph* callee_graph, // the number of instructions in the inlined body. bool HInliner::CanInlineBody(const HGraph* callee_graph, HInvoke* invoke, - size_t* out_number_of_instructions) const { + size_t* out_number_of_instructions, + bool is_speculative) const { const HBasicBlock* target_block = invoke->GetBlock(); ArtMethod* const resolved_method = callee_graph->GetArtMethod(); @@ -1916,11 +1926,15 @@ bool HInliner::CanInlineBody(const HGraph* callee_graph, } if (!has_one_return) { - // If we know that the method always throws with the particular parameters, set it as such. This - // is better than using the dex instructions as we have more information about this particular - // call. - invoke->SetAlwaysThrows(/* always_throws= */ true); - graph_->SetHasAlwaysThrowingInvokes(/* value= */ true); + if (!is_speculative) { + // If we know that the method always throws with the particular parameters, set it as such. + // This is better than using the dex instructions as we have more information about this + // particular call. We don't mark speculative inlines (e.g. the ones from the inline cache) as + // always throwing since they might not throw when executed. + invoke->SetAlwaysThrows(/* always_throws= */ true); + graph_->SetHasAlwaysThrowingInvokes(/* value= */ true); + } + LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedAlwaysThrows) << "Method " << resolved_method->PrettyMethod() << " could not be inlined because it always throws"; @@ -2021,7 +2035,8 @@ bool HInliner::CanInlineBody(const HGraph* callee_graph, bool HInliner::TryBuildAndInlineHelper(HInvoke* invoke_instruction, ArtMethod* resolved_method, ReferenceTypeInfo receiver_type, - HInstruction** return_replacement) { + HInstruction** return_replacement, + bool is_speculative) { DCHECK(!(resolved_method->IsStatic() && receiver_type.IsValid())); const dex::CodeItem* code_item = resolved_method->GetCodeItem(); const DexFile& callee_dex_file = *resolved_method->GetDexFile(); @@ -2125,7 +2140,7 @@ bool HInliner::TryBuildAndInlineHelper(HInvoke* invoke_instruction, try_catch_inlining_allowed_for_recursive_inline); size_t number_of_instructions = 0; - if (!CanInlineBody(callee_graph, invoke_instruction, &number_of_instructions)) { + if (!CanInlineBody(callee_graph, invoke_instruction, &number_of_instructions, is_speculative)) { return false; } diff --git a/compiler/optimizing/inliner.h b/compiler/optimizing/inliner.h index 712d3b5323..581f52422f 100644 --- a/compiler/optimizing/inliner.h +++ b/compiler/optimizing/inliner.h @@ -80,20 +80,23 @@ class HInliner : public HOptimization { bool TryInlineAndReplace(HInvoke* invoke_instruction, ArtMethod* resolved_method, ReferenceTypeInfo receiver_type, - bool do_rtp) + bool do_rtp, + bool is_speculative) REQUIRES_SHARED(Locks::mutator_lock_); bool TryBuildAndInline(HInvoke* invoke_instruction, ArtMethod* resolved_method, ReferenceTypeInfo receiver_type, - HInstruction** return_replacement) + HInstruction** return_replacement, + bool is_speculative) REQUIRES_SHARED(Locks::mutator_lock_); bool TryBuildAndInlineHelper(HInvoke* invoke_instruction, ArtMethod* resolved_method, ReferenceTypeInfo receiver_type, - HInstruction** return_replacement) - REQUIRES_SHARED(Locks::mutator_lock_); + HInstruction** return_replacement, + bool is_speculative) + REQUIRES_SHARED(Locks::mutator_lock_); // Substitutes parameters in the callee graph with their values from the caller. void SubstituteArguments(HGraph* callee_graph, @@ -144,7 +147,8 @@ class HInliner : public HOptimization { // inlining, such as inlining a throw instruction into a try block. bool CanInlineBody(const HGraph* callee_graph, HInvoke* invoke, - size_t* out_number_of_instructions) const + size_t* out_number_of_instructions, + bool is_speculative) const REQUIRES_SHARED(Locks::mutator_lock_); // Create a new HInstanceFieldGet. diff --git a/test/638-checker-inline-caches/expected-stdout.txt b/test/638-checker-inline-caches/expected-stdout.txt index e69de29bb2..8f1cb06856 100644 --- a/test/638-checker-inline-caches/expected-stdout.txt +++ b/test/638-checker-inline-caches/expected-stdout.txt @@ -0,0 +1 @@ +I don't throw diff --git a/test/638-checker-inline-caches/profile b/test/638-checker-inline-caches/profile index 7756a160ba..4630a8d27e 100644 --- a/test/638-checker-inline-caches/profile +++ b/test/638-checker-inline-caches/profile @@ -4,3 +4,4 @@ HSLMain;->inlinePolymophicCrossDexSubASubC(LSuper;)I+LSubA;,LSubC; HSLMain;->inlineMegamorphic(LSuper;)I+LSubA;,LSubB;,LSubC;,LSubD;,LSubE; HSLMain;->inlineMissingTypes(LSuper;)I+missing_types HSLMain;->noInlineCache(LSuper;)I +HSLMain;->noInlineSomeSubclassesThrow(LSuper;)V+LSubA; diff --git a/test/638-checker-inline-caches/src-multidex/SubC.java b/test/638-checker-inline-caches/src-multidex/SubC.java index f7e3c08ff9..e0f571e33a 100644 --- a/test/638-checker-inline-caches/src-multidex/SubC.java +++ b/test/638-checker-inline-caches/src-multidex/SubC.java @@ -16,4 +16,5 @@ public class SubC extends Super { public int getValue() { return 24; } + void someSubclassesThrow() { System.out.println("I don't throw"); } } diff --git a/test/638-checker-inline-caches/src/Main.java b/test/638-checker-inline-caches/src/Main.java index f104e6aea8..7a0fb2645e 100644 --- a/test/638-checker-inline-caches/src/Main.java +++ b/test/638-checker-inline-caches/src/Main.java @@ -16,18 +16,22 @@ class SubA extends Super { int getValue() { return 42; } + void someSubclassesThrow() throws Error { throw new Error("I always throw"); } } class SubB extends Super { int getValue() { return 38; } + void someSubclassesThrow() { System.out.println("I don't throw"); } } class SubD extends Super { int getValue() { return 10; } + void someSubclassesThrow() { System.out.println("I don't throw"); } } class SubE extends Super { int getValue() { return -4; } + void someSubclassesThrow() { System.out.println("I don't throw"); } } public class Main { @@ -134,6 +138,20 @@ public class Main { return a.getValue(); } + // We shouldn't inline `someSubclassesThrow` since we are trying a monomorphic inline and it + // always throws for SubA. However, we shouldn't mark it as `always_throws` since we speculatively + // tried to inline and other subclasses (e.g. SubB) can call noInlineSomeSubclassesThrow and they + // don't throw. + + /// CHECK-START: void Main.noInlineSomeSubclassesThrow(Super) inliner (before) + /// CHECK: InvokeVirtual method_name:Super.someSubclassesThrow always_throws:false + + /// CHECK-START: void Main.noInlineSomeSubclassesThrow(Super) inliner (after) + /// CHECK: InvokeVirtual method_name:Super.someSubclassesThrow always_throws:false + public static void noInlineSomeSubclassesThrow(Super a) throws Error { + a.someSubclassesThrow(); + } + public static void testInlineMonomorphic() { if (inlineMonomorphicSubA(new SubA()) != 42) { throw new Error("Expected 42"); @@ -186,11 +204,20 @@ public class Main { } } - public static void main(String[] args) { + private static void $noinline$testsomeSubclassesThrow() throws Exception { + try { + noInlineSomeSubclassesThrow(new SubA()); + throw new Exception("Unreachable"); + } catch (Error expected) { + } + noInlineSomeSubclassesThrow(new SubB()); + } + + public static void main(String[] args) throws Exception { testInlineMonomorphic(); testInlinePolymorhic(); testInlineMegamorphic(); testNoInlineCache(); + $noinline$testsomeSubclassesThrow(); } - } diff --git a/test/638-checker-inline-caches/src/Super.java b/test/638-checker-inline-caches/src/Super.java index 30cdf30dbf..1964866f11 100644 --- a/test/638-checker-inline-caches/src/Super.java +++ b/test/638-checker-inline-caches/src/Super.java @@ -16,4 +16,5 @@ public abstract class Super { abstract int getValue(); + abstract void someSubclassesThrow(); } |