Do not deopt when using AOT inline caches

Generating a deopt does not ensure that we will actually capture the new
types; and the danger is that we could be stuck in a loop with "forever"
deoptimizations. For example, if the method never becomes hot again, the
inline cache will not be updated and the AOT code will keep
deoptimizing.

This is a compromise because we will most likely never update the inline
cache (unless there's another reason to deopt). So we might be stuck
with a sub-optimal inline cache. As a TODO, we could be smarter when
capturing inline caches to mitigate this. (e.g. by having different
thresholds for new and old methods).

Delete test 644 which was testing a regression involving deopts which is
no longer applicable.

Bug: 38412648
Test: m test-art-host
Change-Id: Ib84559e84f07ec976feeb3a3120fa486d5bee3bf
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index 8674e72..ac5cbf9 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -470,6 +470,33 @@
   return inline_cache;
 }
 
+bool HInliner::UseOnlyPolymorphicInliningWithNoDeopt() {
+  // If we are compiling AOT or OSR, pretend the call using inline caches is polymorphic and
+  // do not generate a deopt.
+  //
+  // For AOT:
+  //    Generating a deopt does not ensure that we will actually capture the new types;
+  //    and the danger is that we could be stuck in a loop with "forever" deoptimizations.
+  //    Take for example the following scenario:
+  //      - we capture the inline cache in one run
+  //      - the next run, we deoptimize because we miss a type check, but the method
+  //        never becomes hot again
+  //    In this case, the inline cache will not be updated in the profile and the AOT code
+  //    will keep deoptimizing.
+  //    Another scenario is if we use profile compilation for a process which is not allowed
+  //    to JIT (e.g. system server). If we deoptimize we will run interpreted code for the
+  //    rest of the lifetime.
+  // TODO(calin):
+  //    This is a compromise because we will most likely never update the inline cache
+  //    in the profile (unless there's another reason to deopt). So we might be stuck with
+  //    a sub-optimal inline cache.
+  //    We could be smarter when capturing inline caches to mitigate this.
+  //    (e.g. by having different thresholds for new and old methods).
+  //
+  // For OSR:
+  //     We may come from the interpreter and it may have seen different receiver types.
+  return Runtime::Current()->IsAotCompiler() || outermost_graph_->IsCompilingOsr();
+}
 bool HInliner::TryInlineFromInlineCache(const DexFile& caller_dex_file,
                                         HInvoke* invoke_instruction,
                                         ArtMethod* resolved_method)
@@ -503,9 +530,7 @@
 
     case kInlineCacheMonomorphic: {
       MaybeRecordStat(kMonomorphicCall);
-      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.
+      if (UseOnlyPolymorphicInliningWithNoDeopt()) {
         return TryInlinePolymorphicCall(invoke_instruction, resolved_method, inline_cache);
       } else {
         return TryInlineMonomorphicCall(invoke_instruction, resolved_method, inline_cache);
@@ -926,14 +951,11 @@
 
       // If we have inlined all targets before, and this receiver is the last seen,
       // we deoptimize instead of keeping the original invoke instruction.
-      bool deoptimize = all_targets_inlined &&
+      bool deoptimize = !UseOnlyPolymorphicInliningWithNoDeopt() &&
+          all_targets_inlined &&
           (i != InlineCache::kIndividualCacheSize - 1) &&
           (classes->Get(i + 1) == nullptr);
 
-      if (outermost_graph_->IsCompilingOsr()) {
-        // We do not support HDeoptimize in OSR methods.
-        deoptimize = false;
-      }
       HInstruction* compare = AddTypeGuard(receiver,
                                            cursor,
                                            bb_cursor,
diff --git a/compiler/optimizing/inliner.h b/compiler/optimizing/inliner.h
index 9e4685c..67476b6 100644
--- a/compiler/optimizing/inliner.h
+++ b/compiler/optimizing/inliner.h
@@ -180,6 +180,9 @@
                                             Handle<mirror::ObjectArray<mirror::Class>> classes)
     REQUIRES_SHARED(Locks::mutator_lock_);
 
+  // Returns whether or not we should use only polymorphic inlining with no deoptimizations.
+  bool UseOnlyPolymorphicInliningWithNoDeopt();
+
   // Try CHA-based devirtualization to change virtual method calls into
   // direct calls.
   // Returns the actual method that resolved_method can be devirtualized to.
diff --git a/test/638-checker-inline-caches/src/Main.java b/test/638-checker-inline-caches/src/Main.java
index 680bd14..f104e6a 100644
--- a/test/638-checker-inline-caches/src/Main.java
+++ b/test/638-checker-inline-caches/src/Main.java
@@ -36,16 +36,17 @@
   /// CHECK:       InvokeVirtual method_name:Super.getValue
 
   /// CHECK-START: int Main.inlineMonomorphicSubA(Super) inliner (after)
-  /// CHECK-NOT:   InvokeVirtual method_name:Super.getValue
-
-  /// CHECK-START: int Main.inlineMonomorphicSubA(Super) inliner (after)
   /// CHECK:  <<SubARet:i\d+>>      IntConstant 42
   /// CHECK:  <<Obj:l\d+>>          NullCheck
   /// CHECK:  <<ObjClass:l\d+>>     InstanceFieldGet [<<Obj>>] field_name:java.lang.Object.shadow$_klass_
   /// CHECK:  <<InlineClass:l\d+>>  LoadClass class_name:SubA
   /// CHECK:  <<Test:z\d+>>         NotEqual [<<InlineClass>>,<<ObjClass>>]
-  /// CHECK:                        Deoptimize [<<Test>>,<<Obj>>]
-  /// CHECK:                        Return [<<SubARet>>]
+  /// CHECK:  <<DefaultRet:i\d+>>   InvokeVirtual [<<Obj>>] method_name:Super.getValue
+
+  /// CHECK:  <<Ret:i\d+>>          Phi [<<SubARet>>,<<DefaultRet>>]
+  /// CHECK:                        Return [<<Ret>>]
+
+  /// CHECK-NOT:                    Deoptimize
   public static int inlineMonomorphicSubA(Super a) {
     return a.getValue();
   }
@@ -53,27 +54,27 @@
   /// CHECK-START: int Main.inlinePolymophicSubASubB(Super) inliner (before)
   /// CHECK:       InvokeVirtual method_name:Super.getValue
 
-  /// CHECK-START: int Main.inlinePolymophicSubASubB(Super) inliner (after)
-  /// CHECK-NOT:   InvokeVirtual method_name:Super.getValue
-
   // Note that the order in which the types are added to the inline cache in the profile matters.
 
   /// CHECK-START: int Main.inlinePolymophicSubASubB(Super) inliner (after)
   /// CHECK-DAG:  <<SubARet:i\d+>>          IntConstant 42
   /// CHECK-DAG:  <<SubBRet:i\d+>>          IntConstant 38
-  /// CHECK:      <<Obj:l\d+>>              NullCheck
-  /// CHECK:      <<ObjClassSubA:l\d+>>     InstanceFieldGet [<<Obj>>] field_name:java.lang.Object.shadow$_klass_
-  /// CHECK:      <<InlineClassSubA:l\d+>>  LoadClass class_name:SubA
-  /// CHECK:      <<TestSubA:z\d+>>         NotEqual [<<InlineClassSubA>>,<<ObjClassSubA>>]
-  /// CHECK:                                If [<<TestSubA>>]
+  /// CHECK-DAG:   <<Obj:l\d+>>             NullCheck
+  /// CHECK-DAG:   <<ObjClassSubA:l\d+>>    InstanceFieldGet [<<Obj>>] field_name:java.lang.Object.shadow$_klass_
+  /// CHECK-DAG:   <<InlineClassSubA:l\d+>> LoadClass class_name:SubA
+  /// CHECK-DAG:   <<TestSubA:z\d+>>        NotEqual [<<InlineClassSubA>>,<<ObjClassSubA>>]
+  /// CHECK-DAG:                            If [<<TestSubA>>]
 
-  /// CHECK:      <<ObjClassSubB:l\d+>>     InstanceFieldGet field_name:java.lang.Object.shadow$_klass_
-  /// CHECK:      <<InlineClassSubB:l\d+>>  LoadClass class_name:SubB
-  /// CHECK:      <<TestSubB:z\d+>>         NotEqual [<<InlineClassSubB>>,<<ObjClassSubB>>]
-  /// CHECK:                                Deoptimize [<<TestSubB>>,<<Obj>>]
+  /// CHECK-DAG:   <<ObjClassSubB:l\d+>>    InstanceFieldGet field_name:java.lang.Object.shadow$_klass_
+  /// CHECK-DAG:   <<InlineClassSubB:l\d+>> LoadClass class_name:SubB
+  /// CHECK-DAG:   <<TestSubB:z\d+>>        NotEqual [<<InlineClassSubB>>,<<ObjClassSubB>>]
+  /// CHECK-DAG:   <<DefaultRet:i\d+>>      InvokeVirtual [<<Obj>>] method_name:Super.getValue
 
-  /// CHECK:      <<Ret:i\d+>>              Phi [<<SubARet>>,<<SubBRet>>]
-  /// CHECK:                                Return [<<Ret>>]
+  /// CHECK-DAG:  <<FirstMerge:i\d+>>       Phi [<<SubBRet>>,<<DefaultRet>>]
+  /// CHECK-DAG:  <<Ret:i\d+>>              Phi [<<SubARet>>,<<FirstMerge>>]
+  /// CHECK-DAG:                            Return [<<Ret>>]
+
+  /// CHECK-NOT:                            Deoptimize
   public static int inlinePolymophicSubASubB(Super a) {
     return a.getValue();
   }
@@ -81,27 +82,27 @@
   /// CHECK-START: int Main.inlinePolymophicCrossDexSubASubC(Super) inliner (before)
   /// CHECK:       InvokeVirtual method_name:Super.getValue
 
-  /// CHECK-START: int Main.inlinePolymophicCrossDexSubASubC(Super) inliner (after)
-  /// CHECK-NOT:   InvokeVirtual method_name:Super.getValue
-
   // Note that the order in which the types are added to the inline cache in the profile matters.
 
   /// CHECK-START: int Main.inlinePolymophicCrossDexSubASubC(Super) inliner (after)
   /// CHECK-DAG:  <<SubARet:i\d+>>          IntConstant 42
   /// CHECK-DAG:  <<SubCRet:i\d+>>          IntConstant 24
-  /// CHECK:      <<Obj:l\d+>>              NullCheck
-  /// CHECK:      <<ObjClassSubA:l\d+>>     InstanceFieldGet [<<Obj>>] field_name:java.lang.Object.shadow$_klass_
-  /// CHECK:      <<InlineClassSubA:l\d+>>  LoadClass class_name:SubA
-  /// CHECK:      <<TestSubA:z\d+>>         NotEqual [<<InlineClassSubA>>,<<ObjClassSubA>>]
-  /// CHECK:                                If [<<TestSubA>>]
+  /// CHECK-DAG:  <<Obj:l\d+>>              NullCheck
+  /// CHECK-DAG:  <<ObjClassSubA:l\d+>>     InstanceFieldGet [<<Obj>>] field_name:java.lang.Object.shadow$_klass_
+  /// CHECK-DAG:  <<InlineClassSubA:l\d+>>  LoadClass class_name:SubA
+  /// CHECK-DAG:  <<TestSubA:z\d+>>         NotEqual [<<InlineClassSubA>>,<<ObjClassSubA>>]
+  /// CHECK-DAG:                            If [<<TestSubA>>]
 
-  /// CHECK:      <<ObjClassSubC:l\d+>>     InstanceFieldGet field_name:java.lang.Object.shadow$_klass_
-  /// CHECK:      <<InlineClassSubC:l\d+>>  LoadClass class_name:SubC
-  /// CHECK:      <<TestSubC:z\d+>>         NotEqual [<<InlineClassSubC>>,<<ObjClassSubC>>]
-  /// CHECK:                                Deoptimize [<<TestSubC>>,<<Obj>>]
+  /// CHECK-DAG:  <<ObjClassSubC:l\d+>>     InstanceFieldGet field_name:java.lang.Object.shadow$_klass_
+  /// CHECK-DAG:  <<InlineClassSubC:l\d+>>  LoadClass class_name:SubC
+  /// CHECK-DAG:  <<TestSubC:z\d+>>         NotEqual [<<InlineClassSubC>>,<<ObjClassSubC>>]
+  /// CHECK-DAG:  <<DefaultRet:i\d+>>       InvokeVirtual [<<Obj>>] method_name:Super.getValue
 
-  /// CHECK:      <<Ret:i\d+>>              Phi [<<SubARet>>,<<SubCRet>>]
-  /// CHECK:                                Return [<<Ret>>]
+  /// CHECK-DAG:  <<FirstMerge:i\d+>>       Phi [<<SubCRet>>,<<DefaultRet>>]
+  /// CHECK-DAG:  <<Ret:i\d+>>              Phi [<<SubARet>>,<<FirstMerge>>]
+  /// CHECK-DAG:                            Return [<<Ret>>]
+
+  /// CHECK-NOT:                            Deoptimize
   public static int inlinePolymophicCrossDexSubASubC(Super a) {
     return a.getValue();
   }
diff --git a/test/644-checker-deopt/expected.txt b/test/644-checker-deopt/expected.txt
deleted file mode 100644
index e69de29..0000000
--- a/test/644-checker-deopt/expected.txt
+++ /dev/null
diff --git a/test/644-checker-deopt/info.txt b/test/644-checker-deopt/info.txt
deleted file mode 100644
index c5fb12c..0000000
--- a/test/644-checker-deopt/info.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-Regression test for making sure HDeoptimize is executed before
-the code it should have prevented executing.
diff --git a/test/644-checker-deopt/profile b/test/644-checker-deopt/profile
deleted file mode 100644
index cb261cc..0000000
--- a/test/644-checker-deopt/profile
+++ /dev/null
@@ -1,2 +0,0 @@
-LMain;->inlineMonomorphic(LMain;)I+LMain;
-LMain;->inlinePolymorphic(LMain;)I+LMain;,LSubMain;
diff --git a/test/644-checker-deopt/run b/test/644-checker-deopt/run
deleted file mode 100644
index 146e180..0000000
--- a/test/644-checker-deopt/run
+++ /dev/null
@@ -1,17 +0,0 @@
-#!/bin/bash
-#
-# Copyright (C) 2017 The Android Open Source Project
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-exec ${RUN} $@ --profile -Xcompiler-option --compiler-filter=speed-profile
diff --git a/test/644-checker-deopt/src/Main.java b/test/644-checker-deopt/src/Main.java
deleted file mode 100644
index 17c80a6..0000000
--- a/test/644-checker-deopt/src/Main.java
+++ /dev/null
@@ -1,74 +0,0 @@
-/*
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-public class Main {
-
-  /// CHECK-START: int Main.inlineMonomorphic(Main) inliner (before)
-  /// CHECK:       InvokeVirtual method_name:Main.getValue
-
-  /// CHECK-START: int Main.inlineMonomorphic(Main) inliner (after)
-  /// CHECK-NOT:   InvokeVirtual method_name:Main.getValue
-
-  /// CHECK-START: int Main.inlineMonomorphic(Main) licm (before)
-  /// CHECK:   <<Deopt:l\d+>> Deoptimize
-  /// CHECK:                  InstanceFieldGet [<<Deopt>>] field_name:Main.value
-
-  /// CHECK-START: int Main.inlineMonomorphic(Main) licm (after)
-  /// CHECK:   <<Deopt:l\d+>> Deoptimize
-  /// CHECK:                  InstanceFieldGet [<<Deopt>>] field_name:Main.value
-
-  public static int inlineMonomorphic(Main a) {
-    if (a == null) {
-      return 42;
-    }
-    int i = 0;
-    while (i < 100) {
-      i += a.getValue();
-    }
-    return i;
-  }
-
-  /// CHECK-START: int Main.inlinePolymorphic(Main) inliner (before)
-  /// CHECK:       InvokeVirtual method_name:Main.getValue
-
-  /// CHECK-START: int Main.inlinePolymorphic(Main) inliner (after)
-  /// CHECK-NOT:   InvokeVirtual method_name:Main.getValue
-
-  /// CHECK-START: int Main.inlineMonomorphic(Main) licm (before)
-  /// CHECK:   <<Deopt:l\d+>> Deoptimize
-  /// CHECK:                  InstanceFieldGet [<<Deopt>>] field_name:Main.value
-
-  /// CHECK-START: int Main.inlineMonomorphic(Main) licm (after)
-  /// CHECK:   <<Deopt:l\d+>> Deoptimize
-  /// CHECK:                  InstanceFieldGet [<<Deopt>>] field_name:Main.value
-  public static int inlinePolymorphic(Main a) {
-    return a.getValue();
-  }
-
-  public int getValue() {
-    return value;
-  }
-
-  public static void main(String[] args) {
-    inlineMonomorphic(new Main());
-  }
-
-  int value = 1;
-}
-
-// Add a subclass of 'Main' to write the polymorphic inline cache in the profile.
-class SubMain extends Main {
-}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 4b44df7..168b673 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -508,7 +508,6 @@
             "640-checker-short-simd",
             "641-checker-arraycopy",
             "643-checker-bogus-ic",
-            "644-checker-deopt",
             "645-checker-abs-simd",
             "706-checker-scheduler"],
         "description": ["Checker tests are not compatible with jvmti."],