summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Santiago Aboy Solanes <solanes@google.com> 2022-10-06 20:31:46 +0100
committer Santiago Aboy Solanes <solanes@google.com> 2022-10-10 14:04:24 +0000
commit629c065a416260a2a1e87c034ca0b42fbda0fe44 (patch)
treef21b68faeba647ea389ff9abc6b3ef481058c9df
parent2096c1fca7a744634ce079ecbda6de28ba1cdc81 (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.cc49
-rw-r--r--compiler/optimizing/inliner.h14
-rw-r--r--test/638-checker-inline-caches/expected-stdout.txt1
-rw-r--r--test/638-checker-inline-caches/profile1
-rw-r--r--test/638-checker-inline-caches/src-multidex/SubC.java1
-rw-r--r--test/638-checker-inline-caches/src/Main.java31
-rw-r--r--test/638-checker-inline-caches/src/Super.java1
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();
}