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;
   }