Fix System.arraycopy when doing same array copying.
At compile time, if constant source < constant destination, and we don't
know if the arrays are the same, then we must emit code that checks
if the two arrays are the same. If so, we jump to the slow path.
test:610-arraycopy
Change-Id: Ida67993d472b0ba4056d9c21c68f6e5239421f7d
diff --git a/compiler/optimizing/intrinsics_arm.cc b/compiler/optimizing/intrinsics_arm.cc
index f43f8ed..bfe4956 100644
--- a/compiler/optimizing/intrinsics_arm.cc
+++ b/compiler/optimizing/intrinsics_arm.cc
@@ -1451,20 +1451,26 @@
Label conditions_on_positions_validated;
SystemArrayCopyOptimizations optimizations(invoke);
- if (!optimizations.GetDestinationIsSource() &&
- (!src_pos.IsConstant() || !dest_pos.IsConstant())) {
- __ cmp(src, ShifterOperand(dest));
- }
// If source and destination are the same, we go to slow path if we need to do
// forward copying.
if (src_pos.IsConstant()) {
int32_t src_pos_constant = src_pos.GetConstant()->AsIntConstant()->GetValue();
if (dest_pos.IsConstant()) {
+ int32_t dest_pos_constant = dest_pos.GetConstant()->AsIntConstant()->GetValue();
+ if (optimizations.GetDestinationIsSource()) {
+ // Checked when building locations.
+ DCHECK_GE(src_pos_constant, dest_pos_constant);
+ } else if (src_pos_constant < dest_pos_constant) {
+ __ cmp(src, ShifterOperand(dest));
+ __ b(slow_path->GetEntryLabel(), EQ);
+ }
+
// Checked when building locations.
DCHECK(!optimizations.GetDestinationIsSource()
|| (src_pos_constant >= dest_pos.GetConstant()->AsIntConstant()->GetValue()));
} else {
if (!optimizations.GetDestinationIsSource()) {
+ __ cmp(src, ShifterOperand(dest));
__ b(&conditions_on_positions_validated, NE);
}
__ cmp(dest_pos.AsRegister<Register>(), ShifterOperand(src_pos_constant));
@@ -1472,6 +1478,7 @@
}
} else {
if (!optimizations.GetDestinationIsSource()) {
+ __ cmp(src, ShifterOperand(dest));
__ b(&conditions_on_positions_validated, NE);
}
if (dest_pos.IsConstant()) {
diff --git a/compiler/optimizing/intrinsics_arm64.cc b/compiler/optimizing/intrinsics_arm64.cc
index 1685cf9..549407e 100644
--- a/compiler/optimizing/intrinsics_arm64.cc
+++ b/compiler/optimizing/intrinsics_arm64.cc
@@ -2101,20 +2101,25 @@
vixl::Label conditions_on_positions_validated;
SystemArrayCopyOptimizations optimizations(invoke);
- if (!optimizations.GetDestinationIsSource() &&
- (!src_pos.IsConstant() || !dest_pos.IsConstant())) {
- __ Cmp(src, dest);
- }
// If source and destination are the same, we go to slow path if we need to do
// forward copying.
if (src_pos.IsConstant()) {
int32_t src_pos_constant = src_pos.GetConstant()->AsIntConstant()->GetValue();
if (dest_pos.IsConstant()) {
+ int32_t dest_pos_constant = dest_pos.GetConstant()->AsIntConstant()->GetValue();
+ if (optimizations.GetDestinationIsSource()) {
+ // Checked when building locations.
+ DCHECK_GE(src_pos_constant, dest_pos_constant);
+ } else if (src_pos_constant < dest_pos_constant) {
+ __ Cmp(src, dest);
+ __ B(slow_path->GetEntryLabel(), eq);
+ }
// Checked when building locations.
DCHECK(!optimizations.GetDestinationIsSource()
|| (src_pos_constant >= dest_pos.GetConstant()->AsIntConstant()->GetValue()));
} else {
if (!optimizations.GetDestinationIsSource()) {
+ __ Cmp(src, dest);
__ B(&conditions_on_positions_validated, ne);
}
__ Cmp(WRegisterFrom(dest_pos), src_pos_constant);
@@ -2122,6 +2127,7 @@
}
} else {
if (!optimizations.GetDestinationIsSource()) {
+ __ Cmp(src, dest);
__ B(&conditions_on_positions_validated, ne);
}
__ Cmp(RegisterFrom(src_pos, invoke->InputAt(1)->GetType()),
diff --git a/compiler/optimizing/intrinsics_x86_64.cc b/compiler/optimizing/intrinsics_x86_64.cc
index c5b44d4..5bd4964 100644
--- a/compiler/optimizing/intrinsics_x86_64.cc
+++ b/compiler/optimizing/intrinsics_x86_64.cc
@@ -1103,20 +1103,22 @@
NearLabel conditions_on_positions_validated;
SystemArrayCopyOptimizations optimizations(invoke);
- if (!optimizations.GetDestinationIsSource() &&
- (!src_pos.IsConstant() || !dest_pos.IsConstant())) {
- __ cmpl(src, dest);
- }
// If source and destination are the same, we go to slow path if we need to do
// forward copying.
if (src_pos.IsConstant()) {
int32_t src_pos_constant = src_pos.GetConstant()->AsIntConstant()->GetValue();
if (dest_pos.IsConstant()) {
- // Checked when building locations.
- DCHECK(!optimizations.GetDestinationIsSource()
- || (src_pos_constant >= dest_pos.GetConstant()->AsIntConstant()->GetValue()));
+ int32_t dest_pos_constant = dest_pos.GetConstant()->AsIntConstant()->GetValue();
+ if (optimizations.GetDestinationIsSource()) {
+ // Checked when building locations.
+ DCHECK_GE(src_pos_constant, dest_pos_constant);
+ } else if (src_pos_constant < dest_pos_constant) {
+ __ cmpl(src, dest);
+ __ j(kEqual, slow_path->GetEntryLabel());
+ }
} else {
if (!optimizations.GetDestinationIsSource()) {
+ __ cmpl(src, dest);
__ j(kNotEqual, &conditions_on_positions_validated);
}
__ cmpl(dest_pos.AsRegister<CpuRegister>(), Immediate(src_pos_constant));
@@ -1124,6 +1126,7 @@
}
} else {
if (!optimizations.GetDestinationIsSource()) {
+ __ cmpl(src, dest);
__ j(kNotEqual, &conditions_on_positions_validated);
}
if (dest_pos.IsConstant()) {
diff --git a/test/610-arraycopy/expected.txt b/test/610-arraycopy/expected.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/610-arraycopy/expected.txt
diff --git a/test/610-arraycopy/info.txt b/test/610-arraycopy/info.txt
new file mode 100644
index 0000000..a77190d
--- /dev/null
+++ b/test/610-arraycopy/info.txt
@@ -0,0 +1,2 @@
+Regression test for the System.arraycopy intrinsic, which had a bug
+when doing the copy on the same array.
diff --git a/test/610-arraycopy/src/Main.java b/test/610-arraycopy/src/Main.java
new file mode 100644
index 0000000..ee11c8e
--- /dev/null
+++ b/test/610-arraycopy/src/Main.java
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2016 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) {
+ Object[] a = new Object[5];
+ for (int i = 0; i < 5; i++) {
+ a[i] = new Integer(i);
+ }
+ $noinline$callArrayCopy(a, a);
+
+ expectEquals(0, ((Integer)a[0]).intValue());
+ expectEquals(0, ((Integer)a[1]).intValue());
+ expectEquals(1, ((Integer)a[2]).intValue());
+ expectEquals(2, ((Integer)a[3]).intValue());
+ expectEquals(4, ((Integer)a[4]).intValue());
+ }
+
+ public static void expectEquals(int expected, int actual) {
+ if (expected != actual) {
+ throw new Error("Expected " + expected + ", got " + actual);
+ }
+ }
+
+ public static void $noinline$callArrayCopy(Object[] a, Object[] b) {
+ System.arraycopy(a, 0, b, 1, 3);
+ if (doThrow) { throw new Error(); }
+ }
+
+ static boolean doThrow = false;
+}