Forbid HDeoptimize instructions in OSR methods.
Otherwise dominated instructions will assume something that
isn't necessarily correct if coming from the interpreter.
bug:28335959
bug:28249238
bug:28348878
bug:28080135
Change-Id: I842bd1c6a919aff48cf6048d2ea51cf2d40f3c1d
diff --git a/compiler/optimizing/bounds_check_elimination.cc b/compiler/optimizing/bounds_check_elimination.cc
index 659c6f8..b65e98a 100644
--- a/compiler/optimizing/bounds_check_elimination.cc
+++ b/compiler/optimizing/bounds_check_elimination.cc
@@ -552,7 +552,11 @@
DCHECK(!IsAddedBlock(block));
first_index_bounds_check_map_.clear();
HGraphVisitor::VisitBasicBlock(block);
- AddComparesWithDeoptimization(block);
+ // We should never deoptimize from an osr method, otherwise we might wrongly optimize
+ // code dominated by the deoptimization.
+ if (!GetGraph()->IsCompilingOsr()) {
+ AddComparesWithDeoptimization(block);
+ }
}
void Finish() {
@@ -1358,6 +1362,11 @@
if (loop->IsIrreducible()) {
return false;
}
+ // We should never deoptimize from an osr method, otherwise we might wrongly optimize
+ // code dominated by the deoptimization.
+ if (GetGraph()->IsCompilingOsr()) {
+ return false;
+ }
// A try boundary preheader is hard to handle.
// TODO: remove this restriction.
if (loop->GetPreHeader()->GetLastInstruction()->IsTryBoundary()) {
diff --git a/compiler/optimizing/graph_checker.cc b/compiler/optimizing/graph_checker.cc
index 96837a8..968e267 100644
--- a/compiler/optimizing/graph_checker.cc
+++ b/compiler/optimizing/graph_checker.cc
@@ -258,6 +258,15 @@
VisitInstruction(check);
}
+void GraphChecker::VisitDeoptimize(HDeoptimize* deopt) {
+ if (GetGraph()->IsCompilingOsr()) {
+ AddError(StringPrintf("A graph compiled OSR cannot have a HDeoptimize instruction"));
+ }
+
+ // Perform the instruction base checks too.
+ VisitInstruction(deopt);
+}
+
void GraphChecker::VisitTryBoundary(HTryBoundary* try_boundary) {
ArrayRef<HBasicBlock* const> handlers = try_boundary->GetExceptionHandlers();
diff --git a/compiler/optimizing/graph_checker.h b/compiler/optimizing/graph_checker.h
index 83b1984..3060c80 100644
--- a/compiler/optimizing/graph_checker.h
+++ b/compiler/optimizing/graph_checker.h
@@ -57,6 +57,7 @@
void VisitCheckCast(HCheckCast* check) OVERRIDE;
void VisitCondition(HCondition* op) OVERRIDE;
void VisitConstant(HConstant* instruction) OVERRIDE;
+ void VisitDeoptimize(HDeoptimize* instruction) OVERRIDE;
void VisitIf(HIf* instruction) OVERRIDE;
void VisitInstanceOf(HInstanceOf* check) OVERRIDE;
void VisitInvokeStaticOrDirect(HInvokeStaticOrDirect* invoke) OVERRIDE;
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index 836bcfa..bb199cf 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -322,7 +322,13 @@
return false;
} else if (ic.IsMonomorphic()) {
MaybeRecordStat(kMonomorphicCall);
- return TryInlineMonomorphicCall(invoke_instruction, resolved_method, ic);
+ 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.
+ return TryInlinePolymorphicCall(invoke_instruction, resolved_method, ic);
+ } else {
+ return TryInlineMonomorphicCall(invoke_instruction, resolved_method, ic);
+ }
} else if (ic.IsPolymorphic()) {
MaybeRecordStat(kPolymorphicCall);
return TryInlinePolymorphicCall(invoke_instruction, resolved_method, ic);
@@ -510,6 +516,11 @@
bool deoptimize = all_targets_inlined &&
(i != InlineCache::kIndividualCacheSize - 1) &&
(ic.GetTypeAt(i + 1) == nullptr);
+
+ if (outermost_graph_->IsCompilingOsr()) {
+ // We do not support HDeoptimize in OSR methods.
+ deoptimize = false;
+ }
HInstruction* compare = AddTypeGuard(
receiver, cursor, bb_cursor, class_index, is_referrer, invoke_instruction, deoptimize);
if (deoptimize) {
@@ -672,7 +683,8 @@
HInstruction* cursor = invoke_instruction->GetPrevious();
HBasicBlock* bb_cursor = invoke_instruction->GetBlock();
- if (!TryInlineAndReplace(invoke_instruction, actual_method, /* do_rtp */ false)) {
+ HInstruction* return_replacement = nullptr;
+ if (!TryBuildAndInline(invoke_instruction, actual_method, &return_replacement)) {
return false;
}
@@ -701,9 +713,6 @@
}
HNotEqual* compare = new (graph_->GetArena()) HNotEqual(class_table_get, constant);
- HDeoptimize* deoptimize = new (graph_->GetArena()) HDeoptimize(
- compare, invoke_instruction->GetDexPc());
- // TODO: Extend reference type propagation to understand the guard.
if (cursor != nullptr) {
bb_cursor->InsertInstructionAfter(receiver_class, cursor);
} else {
@@ -711,8 +720,19 @@
}
bb_cursor->InsertInstructionAfter(class_table_get, receiver_class);
bb_cursor->InsertInstructionAfter(compare, class_table_get);
- bb_cursor->InsertInstructionAfter(deoptimize, compare);
- deoptimize->CopyEnvironmentFrom(invoke_instruction->GetEnvironment());
+
+ if (outermost_graph_->IsCompilingOsr()) {
+ CreateDiamondPatternForPolymorphicInline(compare, return_replacement, invoke_instruction);
+ } else {
+ // TODO: Extend reference type propagation to understand the guard.
+ HDeoptimize* deoptimize = new (graph_->GetArena()) HDeoptimize(
+ compare, invoke_instruction->GetDexPc());
+ bb_cursor->InsertInstructionAfter(deoptimize, compare);
+ deoptimize->CopyEnvironmentFrom(invoke_instruction->GetEnvironment());
+ if (return_replacement != nullptr) {
+ invoke_instruction->ReplaceWith(return_replacement);
+ }
+ }
// Run type propagation to get the guard typed.
ReferenceTypePropagation rtp_fixup(graph_,
@@ -744,6 +764,12 @@
HInstruction** return_replacement) {
const DexFile& caller_dex_file = *caller_compilation_unit_.GetDexFile();
+ if (method->IsProxyMethod()) {
+ VLOG(compiler) << "Method " << PrettyMethod(method)
+ << " is not inlined because of unimplemented inline support for proxy methods.";
+ return false;
+ }
+
// Check whether we're allowed to inline. The outermost compilation unit is the relevant
// dex file here (though the transitivity of an inline chain would allow checking the calller).
if (!compiler_driver_->MayInline(method->GetDexFile(),
@@ -1260,6 +1286,8 @@
size_t HInliner::RunOptimizations(HGraph* callee_graph,
const DexFile::CodeItem* code_item,
const DexCompilationUnit& dex_compilation_unit) {
+ // Note: if the outermost_graph_ is being compiled OSR, we should not run any
+ // optimization that could lead to a HDeoptimize. The following optimizations do not.
HDeadCodeElimination dce(callee_graph, stats_);
HConstantFolding fold(callee_graph);
HSharpening sharpening(callee_graph, codegen_, dex_compilation_unit, compiler_driver_);
diff --git a/compiler/optimizing/reference_type_propagation.cc b/compiler/optimizing/reference_type_propagation.cc
index 04c9ff9..6eebe7f 100644
--- a/compiler/optimizing/reference_type_propagation.cc
+++ b/compiler/optimizing/reference_type_propagation.cc
@@ -664,12 +664,6 @@
}
if (phi->GetBlock()->IsLoopHeader()) {
- if (!is_first_run_ && graph_->IsCompilingOsr()) {
- // Don't update the type of a loop phi when compiling OSR: we may have done
- // speculative optimizations dominating that phi, that do not hold at the
- // point the interpreter jumps to that loop header.
- return;
- }
// Set the initial type for the phi. Use the non back edge input for reaching
// a fixed point faster.
HInstruction* first_input = phi->InputAt(0);
diff --git a/test/570-checker-osr/src/Main.java b/test/570-checker-osr/src/Main.java
index 200b54a..11a6fd4 100644
--- a/test/570-checker-osr/src/Main.java
+++ b/test/570-checker-osr/src/Main.java
@@ -61,6 +61,18 @@
throw new Error("Unexpected return value");
}
+ $noinline$inlineCache2(new Main(), /* isSecondInvocation */ false);
+ if ($noinline$inlineCache2(new SubMain(), /* isSecondInvocation */ true) != SubMain.class) {
+ throw new Error("Unexpected return value");
+ }
+
+ // Test polymorphic inline cache to the same target (inlineCache3).
+ $noinline$inlineCache3(new Main(), /* isSecondInvocation */ false);
+ $noinline$inlineCache3(new SubMain(), /* isSecondInvocation */ false);
+ if ($noinline$inlineCache3(new SubMain(), /* isSecondInvocation */ true) != null) {
+ throw new Error("Unexpected return value");
+ }
+
$noinline$stackOverflow(new Main(), /* isSecondInvocation */ false);
$noinline$stackOverflow(new SubMain(), /* isSecondInvocation */ true);
@@ -147,10 +159,76 @@
return other.returnClass();
}
+ public static Class $noinline$inlineCache2(Main m, boolean isSecondInvocation) {
+ // If we are running in non-JIT mode, or were unlucky enough to get this method
+ // already JITted, just return the expected value.
+ if (!isInInterpreter("$noinline$inlineCache2")) {
+ return SubMain.class;
+ }
+
+ ensureHasProfilingInfo("$noinline$inlineCache2");
+
+ // Ensure that we have OSR code to jump to.
+ if (isSecondInvocation) {
+ ensureHasOsrCode("$noinline$inlineCache2");
+ }
+
+ // This call will be optimized in the OSR compiled code
+ // to check and deoptimize if m is not of type 'Main'.
+ Main other = m.inlineCache2();
+
+ // Jump to OSR compiled code. The second run
+ // of this method will have 'm' as a SubMain, and the compiled
+ // code we are jumping to will have wrongly optimize other as being null.
+ if (isSecondInvocation) {
+ while (!isInOsrCode("$noinline$inlineCache2")) {}
+ }
+
+ // We used to wrongly optimize this code and assume 'other' was always null.
+ return (other == null) ? null : other.returnClass();
+ }
+
+ public static Class $noinline$inlineCache3(Main m, boolean isSecondInvocation) {
+ // If we are running in non-JIT mode, or were unlucky enough to get this method
+ // already JITted, just return the expected value.
+ if (!isInInterpreter("$noinline$inlineCache3")) {
+ return SubMain.class;
+ }
+
+ ensureHasProfilingInfo("$noinline$inlineCache3");
+
+ // Ensure that we have OSR code to jump to.
+ if (isSecondInvocation) {
+ ensureHasOsrCode("$noinline$inlineCache3");
+ }
+
+ // This call will be optimized in the OSR compiled code
+ // to check and deoptimize if m is not of type 'Main'.
+ Main other = m.inlineCache3();
+
+ // Jump to OSR compiled code. The second run
+ // of this method will have 'm' as a SubMain, and the compiled
+ // code we are jumping to will have wrongly optimize other as being null.
+ if (isSecondInvocation) {
+ while (!isInOsrCode("$noinline$inlineCache3")) {}
+ }
+
+ // We used to wrongly optimize this code and assume 'other' was always null.
+ return (other == null) ? null : other.returnClass();
+ }
+
public Main inlineCache() {
return new Main();
}
+ public Main inlineCache2() {
+ return null;
+ }
+
+ public Main inlineCache3() {
+ return null;
+ }
+
public Class returnClass() {
return Main.class;
}
@@ -235,6 +313,10 @@
return new SubMain();
}
+ public Main inlineCache2() {
+ return new SubMain();
+ }
+
public void otherInlineCache() {
return;
}