Move intrinsic recognition logic in inliner.

The previous place `TryInlineAndReplace` did not cover all cases we try
inlining. Because we always want to intrinsify, move the code to
`TryBuildAndInline`, which is always called.

Test: test.py
Test: 638-checker-inline-cache-intrinsic
Change-Id: Id74b664f6139c00224473af6c72cb6fd858aec4c

10 files changed, 132 insertions, 59 deletions

diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index 1ba1c6cc75..719221ee32 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -929,6 +929,14 @@ HInstruction* HInliner::AddTypeGuard(HInstruction* receiver,
   return compare;
 }
 
+static void MaybeReplaceAndRemove(HInstruction* new_instruction, HInstruction* old_instruction) {
+  DCHECK(new_instruction != old_instruction);
+  if (new_instruction != nullptr) {
+    old_instruction->ReplaceWith(new_instruction);
+  }
+  old_instruction->GetBlock()->RemoveInstruction(old_instruction);
+}
+
 bool HInliner::TryInlinePolymorphicCall(
     HInvoke* invoke_instruction,
     const StackHandleScope<InlineCache::kIndividualCacheSize>& classes) {
@@ -993,13 +1001,7 @@ bool HInliner::TryInlinePolymorphicCall(
                                            invoke_instruction,
                                            deoptimize);
       if (deoptimize) {
-        if (return_replacement != nullptr) {
-          invoke_instruction->ReplaceWith(return_replacement);
-        }
-        invoke_instruction->GetBlock()->RemoveInstruction(invoke_instruction);
-        // Because the inline cache data can be populated concurrently, we force the end of the
-        // iteration. Otherwise, we could see a new receiver type.
-        break;
+        MaybeReplaceAndRemove(return_replacement, invoke_instruction);
       } else {
         CreateDiamondPatternForPolymorphicInline(compare, return_replacement, invoke_instruction);
       }
@@ -1202,11 +1204,8 @@ bool HInliner::TryInlinePolymorphicCallToSameTarget(
         invoke_instruction->GetDexPc());
     bb_cursor->InsertInstructionAfter(deoptimize, compare);
     deoptimize->CopyEnvironmentFrom(invoke_instruction->GetEnvironment());
-    if (return_replacement != nullptr) {
-      invoke_instruction->ReplaceWith(return_replacement);
-    }
+    MaybeReplaceAndRemove(return_replacement, invoke_instruction);
     receiver->ReplaceUsesDominatedBy(deoptimize, deoptimize);
-    invoke_instruction->GetBlock()->RemoveInstruction(invoke_instruction);
     deoptimize->SetReferenceTypeInfo(receiver->GetReferenceTypeInfo());
   }
 
@@ -1233,41 +1232,8 @@ bool HInliner::TryInlineAndReplace(HInvoke* invoke_instruction,
   uint32_t dex_pc = invoke_instruction->GetDexPc();
   HInstruction* cursor = invoke_instruction->GetPrevious();
   HBasicBlock* bb_cursor = invoke_instruction->GetBlock();
-  bool should_remove_invoke_instruction = false;
 
-  // 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()) {
-    MaybeRecordStat(stats_, MethodCompilationStat::kIntrinsicRecognized);
-    if (invoke_instruction->IsInvokeInterface()) {
-      // We don't intrinsify an invoke-interface directly.
-      // Replace the invoke-interface with an invoke-virtual.
-      HInvokeVirtual* new_invoke = new (graph_->GetAllocator()) HInvokeVirtual(
-          graph_->GetAllocator(),
-          invoke_instruction->GetNumberOfArguments(),
-          invoke_instruction->GetType(),
-          invoke_instruction->GetDexPc(),
-          invoke_instruction->GetMethodReference(),  // Use interface method's reference.
-          method,
-          MethodReference(method->GetDexFile(), method->GetDexMethodIndex()),
-          method->GetMethodIndex());
-      DCHECK_NE(new_invoke->GetIntrinsic(), Intrinsics::kNone);
-      HInputsRef inputs = invoke_instruction->GetInputs();
-      for (size_t index = 0; index != inputs.size(); ++index) {
-        new_invoke->SetArgumentAt(index, inputs[index]);
-      }
-      invoke_instruction->GetBlock()->InsertInstructionBefore(new_invoke, invoke_instruction);
-      new_invoke->CopyEnvironmentFrom(invoke_instruction->GetEnvironment());
-      if (invoke_instruction->GetType() == DataType::Type::kReference) {
-        new_invoke->SetReferenceTypeInfo(invoke_instruction->GetReferenceTypeInfo());
-      }
-      return_replacement = new_invoke;
-      // invoke_instruction is replaced with new_invoke.
-      should_remove_invoke_instruction = true;
-    } else {
-      invoke_instruction->SetResolvedMethod(method);
-    }
-  } else if (!TryBuildAndInline(invoke_instruction, method, receiver_type, &return_replacement)) {
+  if (!TryBuildAndInline(invoke_instruction, method, receiver_type, &return_replacement)) {
     if (invoke_instruction->IsInvokeInterface()) {
       DCHECK(!method->IsProxyMethod());
       // Turn an invoke-interface into an invoke-virtual. An invoke-virtual is always
@@ -1322,27 +1288,17 @@ bool HInliner::TryInlineAndReplace(HInvoke* invoke_instruction,
         new_invoke->SetReferenceTypeInfo(invoke_instruction->GetReferenceTypeInfo());
       }
       return_replacement = new_invoke;
-      // invoke_instruction is replaced with new_invoke.
-      should_remove_invoke_instruction = true;
     } else {
       // TODO: Consider sharpening an invoke virtual once it is not dependent on the
       // compiler driver.
       return false;
     }
-  } else {
-    // invoke_instruction is inlined.
-    should_remove_invoke_instruction = true;
   }
 
   if (cha_devirtualize) {
     AddCHAGuard(invoke_instruction, dex_pc, cursor, bb_cursor);
   }
-  if (return_replacement != nullptr) {
-    invoke_instruction->ReplaceWith(return_replacement);
-  }
-  if (should_remove_invoke_instruction) {
-    invoke_instruction->GetBlock()->RemoveInstruction(invoke_instruction);
-  }
+  MaybeReplaceAndRemove(return_replacement, invoke_instruction);
   FixUpReturnReferenceType(method, return_replacement);
   if (do_rtp && ReturnTypeMoreSpecific(invoke_instruction, return_replacement)) {
     // Actual return value has a more specific type than the method's declared
@@ -1468,6 +1424,35 @@ bool HInliner::TryBuildAndInline(HInvoke* invoke_instruction,
                                  ArtMethod* method,
                                  ReferenceTypeInfo receiver_type,
                                  HInstruction** return_replacement) {
+  // 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()) {
+    MaybeRecordStat(stats_, MethodCompilationStat::kIntrinsicRecognized);
+    // For simplicity, always create a new instruction to replace the existing
+    // invoke.
+    HInvokeVirtual* new_invoke = new (graph_->GetAllocator()) HInvokeVirtual(
+        graph_->GetAllocator(),
+        invoke_instruction->GetNumberOfArguments(),
+        invoke_instruction->GetType(),
+        invoke_instruction->GetDexPc(),
+        invoke_instruction->GetMethodReference(),  // Use existing invoke's method's reference.
+        method,
+        MethodReference(method->GetDexFile(), method->GetDexMethodIndex()),
+        method->GetMethodIndex());
+    DCHECK_NE(new_invoke->GetIntrinsic(), Intrinsics::kNone);
+    HInputsRef inputs = invoke_instruction->GetInputs();
+    for (size_t index = 0; index != inputs.size(); ++index) {
+      new_invoke->SetArgumentAt(index, inputs[index]);
+    }
+    invoke_instruction->GetBlock()->InsertInstructionBefore(new_invoke, invoke_instruction);
+    new_invoke->CopyEnvironmentFrom(invoke_instruction->GetEnvironment());
+    if (invoke_instruction->GetType() == DataType::Type::kReference) {
+      new_invoke->SetReferenceTypeInfo(invoke_instruction->GetReferenceTypeInfo());
+    }
+    *return_replacement = new_invoke;
+    return true;
+  }
+
   // 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 caller).
   if (!MayInline(codegen_->GetCompilerOptions(),
diff --git a/test/638-checker-inline-cache-intrinsic/run b/test/638-checker-inline-cache-intrinsic/run
index 814181d4a7..9016107e47 100644
--- a/test/638-checker-inline-cache-intrinsic/run
+++ b/test/638-checker-inline-cache-intrinsic/run
@@ -19,4 +19,4 @@
 # The test is for JIT, but we run in "optimizing" (AOT) mode, so that the Checker
 # stanzas in test/638-checker-inline-cache-intrinsic/src/Main.java will be checked.
 # Also pass a large JIT code cache size to avoid getting the inline caches GCed.
-exec ${RUN} --jit --runtime-option -Xjitinitialsize:32M --runtime-option -Xjitthreshold:1000 -Xcompiler-option --verbose-methods=inlineMonomorphic,knownReceiverType,stringEquals $@
+exec ${RUN} --jit --runtime-option -Xjitinitialsize:32M --runtime-option -Xjitthreshold:1000 -Xcompiler-option --verbose-methods=inlineMonomorphic,inlinePolymorphic,knownReceiverType,stringEquals $@
diff --git a/test/638-checker-inline-cache-intrinsic/src/Main.java b/test/638-checker-inline-cache-intrinsic/src/Main.java
index 738e13cf8d..f25d03a894 100644
--- a/test/638-checker-inline-cache-intrinsic/src/Main.java
+++ b/test/638-checker-inline-cache-intrinsic/src/Main.java
@@ -32,6 +32,26 @@ public class Main {
     return cs.charAt(0);
   }
 
+  /// CHECK-START: char Main.$noinline$inlinePolymorphic(java.lang.CharSequence) inliner (before)
+  /// CHECK:       InvokeInterface method_name:java.lang.CharSequence.charAt
+
+  /// CHECK-START: char Main.$noinline$inlinePolymorphic(java.lang.CharSequence) inliner (after)
+  /// CHECK:       InvokeVirtual method_name:java.lang.String.charAt intrinsic:StringCharAt
+  /// CHECK:       Deoptimize
+
+  /// CHECK-START: char Main.$noinline$inlinePolymorphic(java.lang.CharSequence) instruction_simplifier$after_inlining (after)
+  /// CHECK:       Deoptimize
+
+  /// CHECK-START: char Main.$noinline$inlinePolymorphic(java.lang.CharSequence) instruction_simplifier$after_inlining (after)
+  /// CHECK-NOT:   InvokeInterface
+
+  /// CHECK-START: char Main.$noinline$inlinePolymorphic(java.lang.CharSequence) instruction_simplifier$after_inlining (after)
+  /// CHECK-NOT:   InvokeVirtual method_name:java.lang.String.charAt
+
+  public static char $noinline$inlinePolymorphic(CharSequence cs) {
+    return cs.charAt(0);
+  }
+
   /// CHECK-START: char Main.$noinline$knownReceiverType() inliner (before)
   /// CHECK:       InvokeInterface method_name:java.lang.CharSequence.charAt
 
@@ -66,19 +86,27 @@ public class Main {
     ensureJitBaselineCompiled(Main.class, "$noinline$stringEquals");
     ensureJitBaselineCompiled(Main.class, "$noinline$inlineMonomorphic");
     ensureJitBaselineCompiled(Main.class, "$noinline$knownReceiverType");
+    ensureJitBaselineCompiled(Main.class, "$noinline$inlinePolymorphic");
     // Warm up inline cache.
     for (int i = 0; i < 600000; i++) {
       $noinline$inlineMonomorphic(str);
-    }
-    for (int i = 0; i < 600000; i++) {
       $noinline$stringEquals(str);
+      $noinline$inlinePolymorphic(str);
+      $noinline$inlinePolymorphic(strBuilder);
     }
     ensureJitCompiled(Main.class, "$noinline$stringEquals");
     ensureJitCompiled(Main.class, "$noinline$inlineMonomorphic");
+    ensureJitCompiled(Main.class, "$noinline$inlinePolymorphic");
     ensureJitCompiled(Main.class, "$noinline$knownReceiverType");
     if ($noinline$inlineMonomorphic(str) != 'x') {
       throw new Error("Expected x");
     }
+    if ($noinline$inlinePolymorphic(str) != 'x') {
+      throw new Error("Expected x");
+    }
+    if ($noinline$inlinePolymorphic(strBuilder) != 'a') {
+      throw new Error("Expected a");
+    }
     if ($noinline$knownReceiverType() != 'b') {
       throw new Error("Expected b");
     }
@@ -93,6 +121,7 @@ public class Main {
   }
 
   static String str = "xyz";
+  static StringBuilder strBuilder = new StringBuilder("abc");
 
   private static native void ensureJitBaselineCompiled(Class<?> itf, String method_name);
   private static native void ensureJitCompiled(Class<?> itf, String method_name);
diff --git a/test/729-checker-polymorphic-intrinsic/expected-stderr.txt b/test/729-checker-polymorphic-intrinsic/expected-stderr.txt
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/729-checker-polymorphic-intrinsic/expected-stderr.txt
diff --git a/test/729-checker-polymorphic-intrinsic/expected-stdout.txt b/test/729-checker-polymorphic-intrinsic/expected-stdout.txt
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/729-checker-polymorphic-intrinsic/expected-stdout.txt
diff --git a/test/729-checker-polymorphic-intrinsic/info.txt b/test/729-checker-polymorphic-intrinsic/info.txt
new file mode 100644
index 0000000000..bfe0f2d2d5
--- /dev/null
+++ b/test/729-checker-polymorphic-intrinsic/info.txt
@@ -0,0 +1 @@
+Test for a polymorphic inline cache containing an intrinsic call.
diff --git a/test/729-checker-polymorphic-intrinsic/profile b/test/729-checker-polymorphic-intrinsic/profile
new file mode 100644
index 0000000000..41598c3df4
--- /dev/null
+++ b/test/729-checker-polymorphic-intrinsic/profile
@@ -0,0 +1 @@
+SHLMain;->inlinePolymorphic(Ljava/lang/Object;)Ljava/lang/String;+Ljava/lang/StringBuilder;,Ljava/lang/String;
diff --git a/test/729-checker-polymorphic-intrinsic/run b/test/729-checker-polymorphic-intrinsic/run
new file mode 100644
index 0000000000..5fa72eddf1
--- /dev/null
+++ b/test/729-checker-polymorphic-intrinsic/run
@@ -0,0 +1,17 @@
+#!/bin/bash
+#
+# Copyright (C) 2021 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/729-checker-polymorphic-intrinsic/src/Main.java b/test/729-checker-polymorphic-intrinsic/src/Main.java
new file mode 100644
index 0000000000..e1b72acead
--- /dev/null
+++ b/test/729-checker-polymorphic-intrinsic/src/Main.java
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2021 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: java.lang.String Main.inlinePolymorphic(java.lang.Object) inliner (before)
+  /// CHECK:       InvokeVirtual method_name:java.lang.Object.toString
+
+  /// CHECK-START: java.lang.String Main.inlinePolymorphic(java.lang.Object) inliner (after)
+  /// CHECK-DAG:   InvokeVirtual method_name:java.lang.Object.toString
+  /// CHECK-DAG:   InvokeVirtual method_name:java.lang.StringBuilder.toString intrinsic:StringBuilderToString
+  public static String inlinePolymorphic(Object obj) {
+    return obj.toString();
+  }
+
+  public static void assertEquals(String actual, String expected) {
+    if (!expected.equals(actual)) {
+      throw new Error("Expected " + expected + ", got " + actual);
+    }
+  }
+
+  public static void main(String[] args) {
+    assertEquals(inlinePolymorphic(new StringBuilder("abc")), "abc");
+    assertEquals(inlinePolymorphic("def"), "def");
+  }
+}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index e3f4e885e2..9b21cf8842 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1002,6 +1002,7 @@
           "707-checker-invalid-profile",
           "714-invoke-custom-lambda-metafactory",
           "716-jli-jit-samples",
+          "729-checker-polymorphic-intrinsic",
           "800-smali",
           "801-VoidCheckCast",
           "802-deoptimization",