Version 1.3.50
Merge: Implement workaround for arm64 Art codegen for long-to-int.
CL: https://r8-review.googlesource.com/c/r8/+/32420
R=ricow@google.com
Bug: 80262475
Change-Id: I85b591879dfeb10317ccb0aae790fdc63b158150
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index e3cb075..ecf2c9a 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.3.49";
+ public static final String LABEL = "1.3.50";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/NumberConversion.java b/src/main/java/com/android/tools/r8/ir/code/NumberConversion.java
index d676216..e8dc599 100644
--- a/src/main/java/com/android/tools/r8/ir/code/NumberConversion.java
+++ b/src/main/java/com/android/tools/r8/ir/code/NumberConversion.java
@@ -37,6 +37,10 @@
this.to = to;
}
+ public boolean isLongToIntConversion() {
+ return from == NumericType.LONG && to == NumericType.INT;
+ }
+
@Override
public void buildDex(DexBuilder builder) {
com.android.tools.r8.code.Instruction instruction;
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java b/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
index 9fd568b..5133dbe 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
@@ -1422,7 +1422,7 @@
}
private boolean needsSingleResultOverlappingLongOperandsWorkaround(LiveIntervals intervals) {
- if (!options.canHaveCmpLongBug()) {
+ if (!options.canHaveCmpLongBug() && !options.canHaveLongToIntBug()) {
return false;
}
if (intervals.requiredRegisters() == 2) {
@@ -1439,7 +1439,11 @@
return false;
}
Instruction definition = intervals.getValue().definition;
- return definition.isCmp() && definition.asCmp().inValues().get(0).outType().isWide();
+ if (definition.isCmp()) {
+ return definition.inValues().get(0).outType().isWide();
+ }
+ return definition.isNumberConversion()
+ && definition.asNumberConversion().isLongToIntConversion();
}
private boolean singleOverlappingLong(int register1, int register2) {
@@ -1450,15 +1454,23 @@
// allocating for the result?
private boolean isSingleResultOverlappingLongOperands(LiveIntervals intervals, int register) {
assert needsSingleResultOverlappingLongOperandsWorkaround(intervals);
- Value left = intervals.getValue().definition.asCmp().leftValue();
- Value right = intervals.getValue().definition.asCmp().rightValue();
- int leftReg =
- left.getLiveIntervals().getSplitCovering(intervals.getStart()).getRegister();
- int rightReg =
- right.getLiveIntervals().getSplitCovering(intervals.getStart()).getRegister();
- assert leftReg != NO_REGISTER;
- assert rightReg != NO_REGISTER;
- return singleOverlappingLong(register, leftReg) || singleOverlappingLong(register, rightReg);
+ if (intervals.getValue().definition.isCmp()) {
+ Value left = intervals.getValue().definition.asCmp().leftValue();
+ Value right = intervals.getValue().definition.asCmp().rightValue();
+ int leftReg =
+ left.getLiveIntervals().getSplitCovering(intervals.getStart()).getRegister();
+ int rightReg =
+ right.getLiveIntervals().getSplitCovering(intervals.getStart()).getRegister();
+ assert leftReg != NO_REGISTER;
+ assert rightReg != NO_REGISTER;
+ return singleOverlappingLong(register, leftReg) || singleOverlappingLong(register, rightReg);
+ } else {
+ assert intervals.getValue().definition.isNumberConversion();
+ Value inputValue = intervals.getValue().definition.asNumberConversion().inValues().get(0);
+ int inputReg
+ = inputValue.getLiveIntervals().getSplitCovering(intervals.getStart()).getRegister();
+ return register == inputReg;
+ }
}
// The dalvik jit had a bug where the long operations add, sub, or, xor and and would write
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 5e23686..9b7fbf4 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -751,7 +751,7 @@
//
// We used to insert a empty loop at the end, however, mediatek has an optimizer
// on lollipop devices that cannot deal with an unreachable infinite loop, so we
- // couldn't do that. See b/119895393.
+ // couldn't do that. See b/119895393.2
public boolean canHaveTracingPastInstructionsStreamBug() {
return minApiLevel < AndroidApiLevel.L.getLevel();
}
@@ -783,4 +783,27 @@
// TODO(ager): Update this with an actual bound when the issue has been fixed.
return true;
}
+
+ // Some Art Lollipop version do not deal correctly with long-to-int conversions.
+ //
+ // In particular, the following code performs an out of bounds array access when the
+ // long loaded from the long array is very large (has non-zero values in the upper 32 bits).
+ //
+ // aget-wide v9, v3, v1
+ // long-to-int v9, v9
+ // aget-wide v10, v3, v9
+ //
+ // The issue seems to be that the higher bits of the 64-bit register holding the long
+ // are not cleared and the integer is therefore a 64-bit integer that is not truncated
+ // to 32 bits.
+ //
+ // As a workaround, we do not allow long-to-int to have the same source and target register
+ // for min-apis where lollipop devices could be targeted.
+ //
+ // See b/80262475.
+ public boolean canHaveLongToIntBug() {
+ // We have only seen this happening on Lollipop arm64 backends. We have tested on
+ // Marshmallow and Nougat arm64 devices and they do not have the bug.
+ return minApiLevel < AndroidApiLevel.M.getLevel();
+ }
}
diff --git a/src/test/java/com/android/tools/r8/regress/b80262475/B80262475.java b/src/test/java/com/android/tools/r8/regress/b80262475/B80262475.java
new file mode 100644
index 0000000..b3fc08f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/regress/b80262475/B80262475.java
@@ -0,0 +1,69 @@
+// Copyright (c) 2018 the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.regress.b80262475;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static junit.framework.TestCase.assertTrue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertFalse;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.code.Instruction;
+import com.android.tools.r8.code.LongToInt;
+import com.android.tools.r8.code.Return;
+import com.android.tools.r8.code.ReturnVoid;
+import com.android.tools.r8.code.ReturnWide;
+import com.android.tools.r8.code.Throw;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+import org.junit.Test;
+
+class TestClass {
+ public static void f(long[] a) {
+ int i = (int) a[0];
+ a[i] = a[0];
+ }
+}
+
+public class B80262475 extends TestBase {
+
+ private boolean overlappingLongToIntInputAndOutput(Instruction instruction) {
+ if (instruction instanceof LongToInt) {
+ LongToInt longToInt = (LongToInt) instruction;
+ return longToInt.A == longToInt.B;
+ }
+ return false;
+ }
+
+ @Test
+ public void testLongToIntOverlap()
+ throws IOException, CompilationFailedException, ExecutionException {
+ MethodSubject method = getMethodSubject(AndroidApiLevel.L);
+ Instruction[] instructions = method.getMethod().getCode().asDexCode().instructions;
+ for (Instruction instruction : instructions) {
+ assertFalse(overlappingLongToIntInputAndOutput(instruction));
+ }
+ }
+
+ private MethodSubject getMethodSubject(AndroidApiLevel level)
+ throws CompilationFailedException, IOException, ExecutionException {
+ CodeInspector inspector = testForD8()
+ .addProgramClasses(TestClass.class)
+ .setMinApi(level)
+ .compile()
+ .inspector();
+ ClassSubject clazz = inspector.clazz(TestClass.class);
+ assertThat(clazz, isPresent());
+ MethodSubject method = clazz.method("void", "f", ImmutableList.of("long[]"));
+ assertThat(method, isPresent());
+ return method;
+ }
+}
