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
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index b739f3f..4dd4395 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -481,7 +481,8 @@
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 @@
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 @@
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 @@
!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 @@
if (!TryBuildAndInline(invoke_instruction,
actual_method,
ReferenceTypeInfo::CreateInvalid(),
- &return_replacement)) {
+ &return_replacement,
+ /* is_speculative= */ true)) {
return false;
}
@@ -1337,11 +1342,13 @@
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::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 @@
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 @@
// 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 @@
}
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::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 @@
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 712d3b5..581f524 100644
--- a/compiler/optimizing/inliner.h
+++ b/compiler/optimizing/inliner.h
@@ -80,20 +80,23 @@
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 @@
// 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 e69de29..8f1cb06 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 7756a16..4630a8d 100644
--- a/test/638-checker-inline-caches/profile
+++ b/test/638-checker-inline-caches/profile
@@ -4,3 +4,4 @@
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 f7e3c08..e0f571e 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 f104e6a..7a0fb26 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 @@
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 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 30cdf30..1964866 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();
}