ART: Disable vectorization for debuggable graphs.
SuspendCheck environment is incorrectly initialized with
a stale version of the loop induction variable (a pre-loop one)
for vectorized loops. The value can be retrieved from a
corresponding stack maps only in case of asynchronous
deoptimization in debuggable mode. Thus this workaround forbids
loop optimizations on debuggable graphs so the bug is never
triggered.
Test: test-art-target, test-art-host.
Bug: 138601207
Change-Id: Ica9f61f471c024146b7823214ef952e1db2a4663
diff --git a/compiler/optimizing/loop_optimization.cc b/compiler/optimizing/loop_optimization.cc
index 9914127..3e1c42c 100644
--- a/compiler/optimizing/loop_optimization.cc
+++ b/compiler/optimizing/loop_optimization.cc
@@ -763,6 +763,11 @@
}
// Vectorize loop, if possible and valid.
if (kEnableVectorization &&
+ // Disable vectorization for debuggable graphs: this is a workaround for the bug
+ // in 'GenerateNewLoop' which caused the SuspendCheck environment to be invalid.
+ // TODO: b/138601207, investigate other possible cases with wrong environment values and
+ // possibly switch back vectorization on for debuggable graphs.
+ !graph_->IsDebuggable() &&
TrySetSimpleLoopHeader(header, &main_phi) &&
ShouldVectorize(node, body, trip_count) &&
TryAssignLastValue(node->loop_info, main_phi, preheader, /*collect_loop_uses*/ true)) {
diff --git a/test/597-deopt-busy-loop/expected.txt b/test/597-deopt-busy-loop/expected.txt
index 6e2fc8e..c8ab17d 100644
--- a/test/597-deopt-busy-loop/expected.txt
+++ b/test/597-deopt-busy-loop/expected.txt
@@ -1,3 +1,4 @@
JNI_OnLoad called
Simple loop finishing
Float loop finishing
+Simd loop finishing
diff --git a/test/597-deopt-busy-loop/src/Main.java b/test/597-deopt-busy-loop/src/Main.java
index d15c6c7..fc2821e 100644
--- a/test/597-deopt-busy-loop/src/Main.java
+++ b/test/597-deopt-busy-loop/src/Main.java
@@ -31,5 +31,9 @@
undeoptimizeAll();
ensureJitCompiled(FloatLoop.class, "$noinline$busyLoop");
FloatLoop.main();
+
+ undeoptimizeAll();
+ ensureJitCompiled(SimdLoop.class, "$noinline$busyLoop");
+ SimdLoop.main();
}
}
diff --git a/test/597-deopt-busy-loop/src/SimdLoop.java b/test/597-deopt-busy-loop/src/SimdLoop.java
new file mode 100644
index 0000000..ecddfb9
--- /dev/null
+++ b/test/597-deopt-busy-loop/src/SimdLoop.java
@@ -0,0 +1,92 @@
+/*
+ * Copyright (C) 2019 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.
+ */
+
+// This test checks that values (e.g. induction variable) are properly set for SuspendCheck
+// environment in the SIMD loop; by causing asynchronous deoptimization in debuggable mode we
+// we observe the values.
+public class SimdLoop implements Runnable {
+ static final int numberOfThreads = 2;
+ volatile static boolean sExitFlag = false;
+ volatile static boolean sEntered = false;
+ int threadIndex;
+
+ SimdLoop(int index) {
+ threadIndex = index;
+ }
+
+ public static void main() throws Exception {
+ final Thread[] threads = new Thread[numberOfThreads];
+ for (int t = 0; t < threads.length; t++) {
+ threads[t] = new Thread(new SimdLoop(t));
+ threads[t].start();
+ }
+ for (Thread t : threads) {
+ t.join();
+ }
+
+ System.out.println("Simd loop finishing");
+ }
+
+ static final int kArraySize = 10000000;
+
+ public void expectEqual(int value, int expected) {
+ if (value != expected) {
+ throw new Error("Expected: " + expected + ", found: " + value);
+ }
+ }
+
+ public void $noinline$busyLoop() {
+ Main.assertIsManaged();
+
+ int[] array = new int[kArraySize];
+ sEntered = true;
+
+ // On Arm64:
+ // This loop is likely to be vectorized; when deoptimizing to interpreter the induction
+ // variable i will be set to wrong value (== 0).
+ for (int i = 0; i < kArraySize; i++) {
+ array[i]++;
+ }
+
+ // We might have managed to execute the whole loop before deoptimizeAll() happend.
+ if (sExitFlag) {
+ Main.assertIsInterpreted();
+ }
+
+ // Regression: the value of the induction variable might have been set to 0 when
+ // deoptimizing causing to have another array[0]++.
+ expectEqual(array[0], 1);
+ }
+
+ public void run() {
+ if (threadIndex == 0) {
+ while (!sEntered) {
+ Thread.yield();
+ }
+
+ try {
+ Thread.sleep(1L);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+
+ Main.deoptimizeAll();
+ sExitFlag = true;
+ } else {
+ $noinline$busyLoop();
+ }
+ }
+}
