Fix crash when inlining intrinsics with specialized HIR

During the inliner phase if we recognize an intrinsic, we insert it.
This is problematic since there are some intrinsics which we only
expect during the instruction builder phase. This CL skips
inlining those intrinsics.

Potentially, we could generate the graphs for those intrinsics and
inline it, but it needs refactoring of inliner.cc.

Bug: 319045458
Fixes: 319045458
Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b
Test: Locally compiling the app in the bug
Change-Id: Ied3ec87e5655cec3bdfd978eb5c7411ddb102360

7 files changed, 57 insertions, 9 deletions

diff --git a/compiler/optimizing/graph_checker.cc b/compiler/optimizing/graph_checker.cc
index 8f2f25355d..d4b0bf2e81 100644
--- a/compiler/optimizing/graph_checker.cc
+++ b/compiler/optimizing/graph_checker.cc
@@ -739,14 +739,7 @@ void GraphChecker::VisitInvoke(HInvoke* invoke) {
 
   // Check for intrinsics which should have been replaced by intermediate representation in the
   // instruction builder.
-  if (IsIntrinsicWithSpecializedHir(invoke->GetIntrinsic()) &&
-      // FIXME: The inliner can currently create graphs with any of the intrinsics with HIR.
-      // However, we are able to compensate for `StringCharAt` and `StringLength` in the
-      // `HInstructionSimplifier`, so we're allowing these two intrinsics for now, preserving
-      // the old behavior. Besides fixing the bug, we should also clean up the simplifier
-      // and remove `SimplifyStringCharAt` and `SimplifyStringLength`. Bug: 319045458
-      invoke->GetIntrinsic() != Intrinsics::kStringCharAt &&
-      invoke->GetIntrinsic() != Intrinsics::kStringLength) {
+  if (!IsValidIntrinsicAfterBuilder(invoke->GetIntrinsic())) {
     AddError(
         StringPrintf("The graph contains the instrinsic %d which should have been replaced in the "
                      "instruction builder: %s:%d in block %d.",
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index e0bf028138..16abf0b0de 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -1543,7 +1543,9 @@ bool HInliner::TryBuildAndInline(HInvoke* invoke_instruction,
                                  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()) {
+  if (invoke_instruction->GetResolvedMethod() != method &&
+      method->IsIntrinsic() &&
+      IsValidIntrinsicAfterBuilder(static_cast<Intrinsics>(method->GetIntrinsic()))) {
     MaybeRecordStat(stats_, MethodCompilationStat::kIntrinsicRecognized);
     // For simplicity, always create a new instruction to replace the existing
     // invoke.
diff --git a/compiler/optimizing/intrinsics.h b/compiler/optimizing/intrinsics.h
index d14d264204..0a431b8aa8 100644
--- a/compiler/optimizing/intrinsics.h
+++ b/compiler/optimizing/intrinsics.h
@@ -177,6 +177,17 @@ static inline bool IsIntrinsicWithSpecializedHir(Intrinsics intrinsic) {
   }
 }
 
+static inline bool IsValidIntrinsicAfterBuilder(Intrinsics intrinsic) {
+  return !IsIntrinsicWithSpecializedHir(intrinsic) ||
+         // FIXME: The inliner can currently create graphs with any of the intrinsics with HIR.
+         // However, we are able to compensate for `StringCharAt` and `StringLength` in the
+         // `HInstructionSimplifier`, so we're allowing these two intrinsics for now, preserving
+         // the old behavior. Besides fixing the bug, we should also clean up the simplifier
+         // and remove `SimplifyStringCharAt` and `SimplifyStringLength`. Bug: 319045458
+         intrinsic == Intrinsics::kStringCharAt ||
+         intrinsic == Intrinsics::kStringLength;
+}
+
 #define GENERIC_OPTIMIZATION(name, bit)                \
 public:                                                \
 void Set##name() { SetBit(k##name); }                  \
diff --git a/test/2273-checker-unreachable-intrinsics/expected-stderr.txt b/test/2273-checker-unreachable-intrinsics/expected-stderr.txt
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/2273-checker-unreachable-intrinsics/expected-stderr.txt
diff --git a/test/2273-checker-unreachable-intrinsics/expected-stdout.txt b/test/2273-checker-unreachable-intrinsics/expected-stdout.txt
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/2273-checker-unreachable-intrinsics/expected-stdout.txt
diff --git a/test/2273-checker-unreachable-intrinsics/info.txt b/test/2273-checker-unreachable-intrinsics/info.txt
new file mode 100644
index 0000000000..ac5b617580
--- /dev/null
+++ b/test/2273-checker-unreachable-intrinsics/info.txt
@@ -0,0 +1 @@
+Tests we don't inline intrinsics with specialized HIR
diff --git a/test/2273-checker-unreachable-intrinsics/src/Main.java b/test/2273-checker-unreachable-intrinsics/src/Main.java
new file mode 100644
index 0000000000..d2b6cdec0e
--- /dev/null
+++ b/test/2273-checker-unreachable-intrinsics/src/Main.java
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2024 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 {
+    public static void main(String[] args) {
+        assertEquals(false, $noinline$testIsEmpty("Hello"));
+        assertEquals(true, $noinline$testIsEmpty(""));
+    }
+
+    // Even though `str` is of type String, we keep the CharSequence.isEmpty since String.IsEmpty is
+    // an intrinsic with specialized HIR.
+
+    /// CHECK-START: boolean Main.$noinline$testIsEmpty(java.lang.String) inliner (after)
+    /// CHECK: InvokeInterface method_name:java.lang.CharSequence.isEmpty
+    private static boolean $noinline$testIsEmpty(String str) {
+        return $inline$IsEmpty(str);
+    }
+
+    private static boolean $inline$IsEmpty(CharSequence chr) {
+        return chr.isEmpty();
+    }
+
+    private static void assertEquals(boolean expected, boolean actual) {
+        if (expected != actual) {
+            throw new AssertionError("Wrong result: " + expected + " != " + actual);
+        }
+    }
+}