Merge "Port some debug-info tests to running debug tests."
diff --git a/src/main/java/com/android/tools/r8/D8.java b/src/main/java/com/android/tools/r8/D8.java
index 575b09f..2f5fb79 100644
--- a/src/main/java/com/android/tools/r8/D8.java
+++ b/src/main/java/com/android/tools/r8/D8.java
@@ -157,8 +157,8 @@
// Compute the marker to be placed in the main dex file.
private static Marker getMarker(InternalOptions options) {
- if (options.customizedMarker != null) {
- return options.customizedMarker;
+ if (options.hasMarker()) {
+ return options.getMarker();
}
return new Marker(Tool.D8)
.put("version", kVersion)
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 620faf8..84bcee7 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -82,8 +82,8 @@
// Compute the marker to be placed in the main dex file.
private static Marker getMarker(InternalOptions options) {
- if (options.customizedMarker != null) {
- return options.customizedMarker;
+ if (options.hasMarker()) {
+ return options.getMarker();
}
return new Marker(Tool.R8)
.put("version", kVersion)
diff --git a/src/main/java/com/android/tools/r8/dex/VirtualFile.java b/src/main/java/com/android/tools/r8/dex/VirtualFile.java
index f06aa45..cbca177 100644
--- a/src/main/java/com/android/tools/r8/dex/VirtualFile.java
+++ b/src/main/java/com/android/tools/r8/dex/VirtualFile.java
@@ -231,11 +231,6 @@
for (DexProgramClass clazz : application.classes()) {
VirtualFile file = new VirtualFile(nameToFileMap.size(), writer.namingLens);
nameToFileMap.put(nameToFileMap.size(), file);
- // Write the marker string to all files in files-per-class mode.
- // TODO(sgjesse): Get rid of this (currently some tests fails without it).
- if (writer.markerString != null) {
- file.transaction.addString(writer.markerString);
- }
file.addClass(clazz);
file.commitTransaction();
}
diff --git a/src/main/java/com/android/tools/r8/errors/InvalidDebugInfoException.java b/src/main/java/com/android/tools/r8/errors/InvalidDebugInfoException.java
new file mode 100644
index 0000000..f3f99e2
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/errors/InvalidDebugInfoException.java
@@ -0,0 +1,10 @@
+// Copyright (c) 2017, 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.errors;
+
+public class InvalidDebugInfoException extends InternalCompilerError {
+ public InvalidDebugInfoException(String message) {
+ super(message);
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/graph/JarCode.java b/src/main/java/com/android/tools/r8/graph/JarCode.java
index 9a367ff..6b9c06d 100644
--- a/src/main/java/com/android/tools/r8/graph/JarCode.java
+++ b/src/main/java/com/android/tools/r8/graph/JarCode.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.graph;
+import com.android.tools.r8.errors.InvalidDebugInfoException;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.ValueNumberGenerator;
import com.android.tools.r8.ir.conversion.IRBuilder;
@@ -81,19 +82,43 @@
@Override
public IRCode buildIR(DexEncodedMethod encodedMethod, InternalOptions options) {
triggerDelayedParsingIfNeccessary();
- JarSourceCode source = new JarSourceCode(clazz, node, application);
- IRBuilder builder = new IRBuilder(encodedMethod, source, options);
- return builder.build();
+ return options.debug
+ ? internalBuildWithLocals(encodedMethod, null, options)
+ : internalBuild(encodedMethod, null, options);
}
- public IRCode buildIR(DexEncodedMethod encodedMethod, ValueNumberGenerator generator,
- InternalOptions options) {
+ public IRCode buildIR(
+ DexEncodedMethod encodedMethod, ValueNumberGenerator generator, InternalOptions options) {
+ assert generator != null;
triggerDelayedParsingIfNeccessary();
- JarSourceCode source = new JarSourceCode(clazz, node, application);
- IRBuilder builder = new IRBuilder(encodedMethod, source, generator, options);
- return builder.build();
+ return options.debug
+ ? internalBuildWithLocals(encodedMethod, generator, options)
+ : internalBuild(encodedMethod, generator, options);
}
+ private IRCode internalBuildWithLocals(
+ DexEncodedMethod encodedMethod, ValueNumberGenerator generator, InternalOptions options) {
+ try {
+ return internalBuild(encodedMethod, generator, options);
+ } catch (InvalidDebugInfoException e) {
+ options.warningInvalidDebugInfo(encodedMethod, e);
+ node.localVariables.clear();
+ return internalBuild(encodedMethod, generator, options);
+ }
+ }
+
+ private IRCode internalBuild(
+ DexEncodedMethod encodedMethod, ValueNumberGenerator generator, InternalOptions options) {
+ if (!options.debug) {
+ node.localVariables.clear();
+ }
+ JarSourceCode source = new JarSourceCode(clazz, node, application);
+ IRBuilder builder =
+ (generator == null)
+ ? new IRBuilder(encodedMethod, source, options)
+ : new IRBuilder(encodedMethod, source, generator, options);
+ return builder.build();
+ }
@Override
public void registerReachableDefinitions(UseRegistry registry) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/Cmp.java b/src/main/java/com/android/tools/r8/ir/code/Cmp.java
index 3d20415..9b54353 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Cmp.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Cmp.java
@@ -143,7 +143,7 @@
@Override
public boolean canBeFolded() {
- return (leftValue().isConstant() && rightValue().isConstant()) || nonOverlapingRanges();
+ return (leftValue().isConstNumber() && rightValue().isConstNumber()) || nonOverlapingRanges();
}
@Override
@@ -151,7 +151,7 @@
assert canBeFolded();
int result;
if (type == NumericType.LONG) {
- if (leftValue().isConstant() && rightValue().isConstant()) {
+ if (leftValue().isConstNumber() && rightValue().isConstNumber()) {
long left = leftValue().getConstInstruction().asConstNumber().getLongValue();
long right = rightValue().getConstInstruction().asConstNumber().getLongValue();
result = Integer.signum(Long.compare(left, right));
diff --git a/src/main/java/com/android/tools/r8/ir/code/ConstString.java b/src/main/java/com/android/tools/r8/ir/code/ConstString.java
index bb1ff32..124ff54 100644
--- a/src/main/java/com/android/tools/r8/ir/code/ConstString.java
+++ b/src/main/java/com/android/tools/r8/ir/code/ConstString.java
@@ -64,6 +64,11 @@
}
@Override
+ public boolean isOutConstant() {
+ return true;
+ }
+
+ @Override
public boolean isConstString() {
return true;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Value.java b/src/main/java/com/android/tools/r8/ir/code/Value.java
index 035bf95..2ffb5a1 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Value.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Value.java
@@ -354,7 +354,7 @@
}
public boolean internalComputeNeedsRegister() {
- if (!isConstant()) {
+ if (!isConstNumber()) {
return true;
}
if (numberOfPhiUsers() > 0) {
@@ -422,6 +422,10 @@
return definition.getOutConstantConstInstruction();
}
+ public boolean isConstNumber() {
+ return isConstant() && getConstInstruction().isConstNumber();
+ }
+
public boolean isConstant() {
return definition.isOutConstant() && getLocalInfo() == null;
}
@@ -481,11 +485,11 @@
}
public boolean hasValueRange() {
- return valueRange != null || isConstant();
+ return valueRange != null || isConstNumber();
}
public boolean isValueInRange(int value) {
- if (isConstant()) {
+ if (isConstNumber()) {
return value == getConstInstruction().asConstNumber().getIntValue();
} else {
return valueRange != null && valueRange.containsValue(value);
@@ -493,7 +497,7 @@
}
public LongInterval getValueRange() {
- if (isConstant()) {
+ if (isConstNumber()) {
if (type == MoveType.SINGLE) {
int value = getConstInstruction().asConstNumber().getIntValue();
return new LongInterval(value, value);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
index 809e3ac..976020c2 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
@@ -7,6 +7,7 @@
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.InternalCompilerError;
+import com.android.tools.r8.errors.InvalidDebugInfoException;
import com.android.tools.r8.graph.DebugLocalInfo;
import com.android.tools.r8.graph.DexCallSite;
import com.android.tools.r8.graph.DexEncodedMethod;
@@ -1394,7 +1395,8 @@
Value in2 = readNumericRegister(right, type);
Value out = writeNumericRegister(dest, type, ThrowingInfo.NO_THROW);
Instruction instruction;
- if (in2.isConstant() && in2.getConstInstruction().asConstNumber().isIntegerNegativeOne(type)) {
+ if (in2.isConstNumber() &&
+ in2.getConstInstruction().asConstNumber().isIntegerNegativeOne(type)) {
instruction = new Not(type, out, in1);
} else {
instruction = new Xor(type, out, in1, in2);
@@ -1433,7 +1435,11 @@
DebugLocalInfo local = getCurrentLocal(register);
Value value = readRegister(register, currentBlock, EdgeType.NON_EDGE, type, local);
// Check that any information about a current-local is consistent with the read.
- assert local == null || value.getLocalInfo() == local || value.isUninitializedLocal();
+ if (local != null && value.getLocalInfo() != local && !value.isUninitializedLocal()) {
+ throw new InvalidDebugInfoException(
+ "Attempt to read local " + local
+ + " but no local information was associated with the value being read.");
+ }
// Check that any local information on the value is actually visible.
// If this assert triggers, the probable cause is that we end up reading an SSA value
// after it should have been ended on a fallthrough from a conditional jump or a trivial-phi
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index 6f94b1e..23b6b70 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -459,7 +459,7 @@
codeRewriter.commonSubexpressionElimination(code);
codeRewriter.simplifyArrayConstruction(code);
codeRewriter.rewriteMoveResult(code);
- codeRewriter.splitConstants(code);
+ codeRewriter.splitRangeInvokeConstants(code);
codeRewriter.foldConstants(code);
codeRewriter.rewriteSwitch(code);
codeRewriter.simplifyIf(code);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
index 12d8b6a..484354a 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
@@ -308,6 +308,7 @@
}
}
computeBlockEntryJarStates(builder);
+ state.setBuilding();
}
private void computeBlockEntryJarStates(IRBuilder builder) {
@@ -360,6 +361,7 @@
}
}
}
+ state.restoreState(0);
}
private void updateStateForLocalVariableEnd(AbstractInsnNode insn) {
@@ -1146,9 +1148,11 @@
state.pop();
Type elementType = state.pop(JarState.ARRAY_TYPE).getArrayElementType();
if (elementType == null) {
- // We propagate the null type, which will then get resolved to an
- // actual type if we have a non-null type on another flow edge.
- elementType = JarState.NULL_TYPE;
+ // We propagate the byte-or-bool type, which will then get resolved to an
+ // actual type if we have a concrete byte type or bool type on another flow edge.
+ elementType = (Opcodes.BALOAD == opcode)
+ ? JarState.BYTE_OR_BOOL_TYPE
+ : getArrayElementTypeForOpcode(opcode);
}
state.push(elementType);
break;
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/JarState.java b/src/main/java/com/android/tools/r8/ir/conversion/JarState.java
index 9d11fae..5009d5e 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/JarState.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/JarState.java
@@ -3,7 +3,9 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.conversion;
+import com.android.tools.r8.errors.InvalidDebugInfoException;
import com.android.tools.r8.graph.DebugLocalInfo;
+import com.android.tools.r8.utils.DescriptorUtils;
import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Multimap;
@@ -36,6 +38,10 @@
// Type representative for the null value (non-existent but works for tracking the types here).
public static final Type NULL_TYPE = Type.getObjectType("<null>");
+ // TODO(zerny): Define an internal Type wrapping ASM types so that we can define an actual value.
+ // Type representative for a value that may be either a boolean or a byte.
+ public static final Type BYTE_OR_BOOL_TYPE = null;
+
// Typed mapping from a local slot or stack slot to a virtual register.
public static class Slot {
public final int register;
@@ -78,6 +84,9 @@
assert type != REFERENCE_TYPE;
assert type != OBJECT_TYPE;
assert type != ARRAY_TYPE;
+ if (type == BYTE_OR_BOOL_TYPE) {
+ type = Type.BYTE_TYPE;
+ }
int sort = type.getSort();
int otherSort = other.getSort();
if (isReferenceCompatible(type, other)) {
@@ -183,6 +192,9 @@
private final Map<Integer, Snapshot> targetStates = new HashMap<>();
+ // Mode denoting that the state setup is done and we are now emitting IR.
+ // Concretely we treat all remaining byte-or-bool types as bytes (no actual type can flow there).
+ private boolean building = false;
public JarState(int maxLocals, Map<LocalVariableNode, DebugLocalInfo> localVariables) {
int localsRegistersSize = maxLocals * 3;
@@ -205,6 +217,39 @@
}
}
+ public void setBuilding() {
+ assert stack.isEmpty();
+ building = true;
+ for (int i = 0; i < locals.length; i++) {
+ Local local = locals[i];
+ if (local != null && local.slot.type == BYTE_OR_BOOL_TYPE) {
+ locals[i] = new Local(new Slot(local.slot.register, Type.BYTE_TYPE), local.info);
+ }
+ }
+ for (Entry<Integer, Snapshot> entry : targetStates.entrySet()) {
+ Local[] locals = entry.getValue().locals;
+ for (int i = 0; i < locals.length; i++) {
+ Local local = locals[i];
+ if (local != null && local.slot.type == BYTE_OR_BOOL_TYPE) {
+ locals[i] = new Local(new Slot(local.slot.register, Type.BYTE_TYPE), local.info);
+ }
+ }
+ ImmutableList.Builder<Slot> builder = ImmutableList.builder();
+ boolean found = false;
+ for (Slot slot : entry.getValue().stack) {
+ if (slot.type == BYTE_OR_BOOL_TYPE) {
+ found = true;
+ builder.add(new Slot(slot.register, Type.BYTE_TYPE));
+ } else {
+ builder.add(slot);
+ }
+ }
+ if (found) {
+ entry.setValue(new Snapshot(locals, builder.build()));
+ }
+ }
+ }
+
// Local variable procedures.
public List<Local> openLocals(LabelNode label) {
@@ -253,6 +298,10 @@
int getLocalRegister(int index, Type type) {
assert index < localsSize;
+ if (type == BYTE_OR_BOOL_TYPE) {
+ assert Slot.isCategory1(type);
+ return index + localsSize;
+ }
if (type.getSort() == Type.OBJECT || type.getSort() == Type.ARRAY) {
return index;
}
@@ -287,28 +336,44 @@
}
public int writeLocal(int index, Type type) {
- assert type != null;
+ assert nonNullType(type);
Local local = getLocal(index, type);
- assert local == null || local.info == null || local.slot.isCompatibleWith(type);
+ if (local != null && local.info != null && !local.slot.isCompatibleWith(type)) {
+ throw new InvalidDebugInfoException(
+ "Attempt to write value of type " + prettyType(type) + " to local " + local.info);
+ }
// We cannot assume consistency for writes because we do not have complete information about the
// scopes of locals. We assume the program to be verified and overwrite if the types mismatch.
- if (local == null || (local.info == null && !local.slot.type.equals(type))) {
+ if (local == null || (local.info == null && !typeEquals(local.slot.type, type))) {
local = setLocal(index, type, null);
}
return local.slot.register;
}
+ public boolean typeEquals(Type type1, Type type2) {
+ return (type1 == BYTE_OR_BOOL_TYPE && type2 == BYTE_OR_BOOL_TYPE)
+ || (type1 != null && type1.equals(type2));
+ }
+
public Slot readLocal(int index, Type type) {
Local local = getLocal(index, type);
assert local != null;
+ if (local.info != null && !local.slot.isCompatibleWith(type)) {
+ throw new InvalidDebugInfoException(
+ "Attempt to read value of type " + prettyType(type) + " from local " + local.info);
+ }
assert local.slot.isCompatibleWith(type);
return local.slot;
}
+ public boolean nonNullType(Type type) {
+ return type != null || !building;
+ }
+
// Stack procedures.
public int push(Type type) {
- assert type != null;
+ assert nonNullType(type);
int top = topOfStack;
// For simplicity, every stack slot (and local variable) is wide (uses two registers).
topOfStack += 2;
@@ -332,14 +397,19 @@
// For simplicity, every stack slot (and local variable) is wide (uses two registers).
topOfStack -= 2;
Slot slot = stack.pop();
- assert slot.type != null;
+ assert nonNullType(slot.type);
assert slot.register == topOfStack;
return slot;
}
public Slot pop(Type type) {
Slot slot = pop();
- assert slot.isCompatibleWith(type);
+ boolean compatible = slot.isCompatibleWith(type);
+ if (!compatible && !localVariables.isEmpty()) {
+ throw new InvalidDebugInfoException("Expected to read stack value of type " + prettyType(type)
+ + " but found value of type " + prettyType(slot.type));
+ }
+ assert compatible;
return slot;
}
@@ -416,13 +486,17 @@
return true;
}
+ private boolean isRefinement(Type current, Type other) {
+ return (current == JarState.NULL_TYPE && other != JarState.NULL_TYPE)
+ || (current == JarState.BYTE_OR_BOOL_TYPE && other != JarState.BYTE_OR_BOOL_TYPE);
+ }
+
private ImmutableList<Slot> mergeStacks(
ImmutableList<Slot> currentStack, ImmutableList<Slot> newStack) {
assert currentStack.size() == newStack.size();
List<Slot> mergedStack = null;
for (int i = 0; i < currentStack.size(); i++) {
- if (currentStack.get(i).type == JarState.NULL_TYPE &&
- newStack.get(i).type != JarState.NULL_TYPE) {
+ if (isRefinement(currentStack.get(i).type, newStack.get(i).type)) {
if (mergedStack == null) {
mergedStack = new ArrayList<>();
mergedStack.addAll(currentStack.subList(0, i));
@@ -448,8 +522,7 @@
// If this assert triggers we can get different debug information for the same local
// on different control-flow paths and we will have to merge them.
assert currentLocal.info == newLocal.info;
- if (currentLocal.slot.type == JarState.NULL_TYPE &&
- newLocal.slot.type != JarState.NULL_TYPE) {
+ if (isRefinement(currentLocal.slot.type, newLocal.slot.type)) {
if (mergedLocals == null) {
mergedLocals = new Local[currentLocals.length];
System.arraycopy(currentLocals, 0, mergedLocals, 0, i);
@@ -463,15 +536,6 @@
return mergedLocals != null ? mergedLocals : currentLocals;
}
- private static boolean verifyStack(List<Slot> stack, List<Slot> other) {
- assert stack.size() == other.size();
- int i = 0;
- for (Slot slot : stack) {
- assert slot.isCompatibleWith(other.get(i++).type);
- }
- return true;
- }
-
// Other helpers.
private static boolean verifySlots(Slot[] slots, Type type) {
@@ -490,7 +554,11 @@
public static String stackToString(Collection<Slot> stack) {
List<String> strings = new ArrayList<>(stack.size());
for (Slot slot : stack) {
- strings.add(slot.type.toString());
+ if (slot.type == BYTE_OR_BOOL_TYPE) {
+ strings.add("<byte|bool>");
+ } else {
+ strings.add(slot.type.toString());
+ }
}
StringBuilder builder = new StringBuilder("{ ");
for (int i = strings.size() - 1; i >= 0; i--) {
@@ -516,6 +584,8 @@
builder.append("_");
} else if (local.info != null) {
builder.append(local.info);
+ } else if (local.slot.type == BYTE_OR_BOOL_TYPE) {
+ builder.append("<byte|bool>");
} else {
builder.append(local.slot.type.toString());
}
@@ -523,4 +593,17 @@
builder.append(" }");
return builder.toString();
}
+
+ private String prettyType(Type type) {
+ if (type == BYTE_OR_BOOL_TYPE) {
+ return "<byte|bool>";
+ }
+ if (type == ARRAY_TYPE) {
+ return type.getElementType().getInternalName();
+ }
+ if (type == REFERENCE_TYPE || type == OBJECT_TYPE || type == NULL_TYPE) {
+ return type.getInternalName();
+ }
+ return DescriptorUtils.descriptorToJavaType(type.getDescriptor());
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index 981f673..48a64b8 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -20,6 +20,7 @@
import com.android.tools.r8.ir.code.CheckCast;
import com.android.tools.r8.ir.code.Cmp;
import com.android.tools.r8.ir.code.Cmp.Bias;
+import com.android.tools.r8.ir.code.ConstInstruction;
import com.android.tools.r8.ir.code.ConstNumber;
import com.android.tools.r8.ir.code.ConstString;
import com.android.tools.r8.ir.code.DominatorTree;
@@ -32,6 +33,7 @@
import com.android.tools.r8.ir.code.InstructionListIterator;
import com.android.tools.r8.ir.code.Invoke;
import com.android.tools.r8.ir.code.InvokeMethod;
+import com.android.tools.r8.ir.code.InvokeNewArray;
import com.android.tools.r8.ir.code.InvokeVirtual;
import com.android.tools.r8.ir.code.MemberType;
import com.android.tools.r8.ir.code.MoveType;
@@ -61,7 +63,6 @@
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
-import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
@@ -650,68 +651,35 @@
assert code.isConsistentSSA();
}
- // Constants are canonicalized in the entry block. We split some of them when it is likely
- // that having them canonicalized in the entry block will lead to poor code quality.
- public void splitConstants(IRCode code) {
+ // Split constants that flow into ranged invokes. This gives the register allocator more
+ // freedom in assigning register to ranged invokes which can greatly reduce the number
+ // of register needed (and thereby code size as well).
+ public void splitRangeInvokeConstants(IRCode code) {
for (BasicBlock block : code.blocks) {
- // Split constants that flow into phis. It is likely that these constants will have moves
- // generated for them anyway and we might as well insert a const instruction in the right
- // predecessor block.
- splitPhiConstants(code, block);
- // Split constants that flow into ranged invokes. This gives the register allocator more
- // freedom in assigning register to ranged invokes which can greatly reduce the number
- // of register needed (and thereby code size as well).
- splitRangedInvokeConstants(code, block);
- }
- }
-
- private void splitRangedInvokeConstants(IRCode code, BasicBlock block) {
- InstructionListIterator it = block.listIterator();
- while (it.hasNext()) {
- Instruction current = it.next();
- if (current.isInvoke() && current.asInvoke().requiredArgumentRegisters() > 5) {
- Invoke invoke = current.asInvoke();
- it.previous();
- Map<ConstNumber, ConstNumber> oldToNew = new HashMap<>();
- for (int i = 0; i < invoke.inValues().size(); i++) {
- Value value = invoke.inValues().get(i);
- if (value.isConstant() && value.numberOfUsers() > 1) {
- ConstNumber definition = value.getConstInstruction().asConstNumber();
- Value originalValue = definition.outValue();
- ConstNumber newNumber = oldToNew.get(definition);
- if (newNumber == null) {
- newNumber = ConstNumber.copyOf(code, definition);
- it.add(newNumber);
- oldToNew.put(definition, newNumber);
+ InstructionListIterator it = block.listIterator();
+ while (it.hasNext()) {
+ Instruction current = it.next();
+ if (current.isInvoke() && current.asInvoke().requiredArgumentRegisters() > 5) {
+ Invoke invoke = current.asInvoke();
+ it.previous();
+ Map<ConstNumber, ConstNumber> oldToNew = new HashMap<>();
+ for (int i = 0; i < invoke.inValues().size(); i++) {
+ Value value = invoke.inValues().get(i);
+ if (value.isConstNumber() && value.numberOfUsers() > 1) {
+ ConstNumber definition = value.getConstInstruction().asConstNumber();
+ Value originalValue = definition.outValue();
+ ConstNumber newNumber = oldToNew.get(definition);
+ if (newNumber == null) {
+ newNumber = ConstNumber.copyOf(code, definition);
+ it.add(newNumber);
+ oldToNew.put(definition, newNumber);
+ }
+ invoke.inValues().set(i, newNumber.outValue());
+ originalValue.removeUser(invoke);
+ newNumber.outValue().addUser(invoke);
}
- invoke.inValues().set(i, newNumber.outValue());
- originalValue.removeUser(invoke);
- newNumber.outValue().addUser(invoke);
}
- }
- it.next();
- }
- }
- }
-
- private void splitPhiConstants(IRCode code, BasicBlock block) {
- for (int i = 0; i < block.getPredecessors().size(); i++) {
- Map<ConstNumber, ConstNumber> oldToNew = new IdentityHashMap<>();
- BasicBlock predecessor = block.getPredecessors().get(i);
- for (Phi phi : block.getPhis()) {
- Value operand = phi.getOperand(i);
- if (!operand.isPhi() && operand.isConstant()) {
- ConstNumber definition = operand.getConstInstruction().asConstNumber();
- ConstNumber newNumber = oldToNew.get(definition);
- Value originalValue = definition.outValue();
- if (newNumber == null) {
- newNumber = ConstNumber.copyOf(code, definition);
- oldToNew.put(definition, newNumber);
- insertConstantInBlock(newNumber, predecessor);
- }
- phi.getOperands().set(i, newNumber.outValue());
- originalValue.removePhiUser(phi);
- newNumber.outValue().addPhiUser(phi);
+ it.next();
}
}
}
@@ -728,7 +696,7 @@
List<Instruction> toInsertInThisBlock = new ArrayList<>();
while (it.hasNext()) {
Instruction instruction = it.next();
- if (instruction.isConstNumber()) {
+ if (instruction.isConstNumber() && instruction.outValue().numberOfAllUsers() != 0) {
// Collect the blocks for all users of the constant.
List<BasicBlock> userBlocks = new LinkedList<>();
for (Instruction user : instruction.outValue().uniqueUsers()) {
@@ -814,18 +782,16 @@
insertAt.add(instruction);
}
- private short[] computeArrayFilledData(
- NewArrayEmpty newArray, int size, BasicBlock block, int elementSize) {
- ConstNumber[] values = computeConstantArrayValues(newArray, block, size);
+ private short[] computeArrayFilledData(ConstInstruction[] values, int size, int elementSize) {
if (values == null) {
return null;
}
if (elementSize == 1) {
short[] result = new short[(size + 1) / 2];
for (int i = 0; i < size; i += 2) {
- short value = (short) (values[i].getIntValue() & 0xFF);
+ short value = (short) (values[i].asConstNumber().getIntValue() & 0xFF);
if (i + 1 < size) {
- value |= (short) ((values[i + 1].getIntValue() & 0xFF) << 8);
+ value |= (short) ((values[i + 1].asConstNumber().getIntValue() & 0xFF) << 8);
}
result[i / 2] = value;
}
@@ -835,7 +801,7 @@
int shortsPerConstant = elementSize / 2;
short[] result = new short[size * shortsPerConstant];
for (int i = 0; i < size; i++) {
- long value = values[i].getRawValue();
+ long value = values[i].asConstNumber().getRawValue();
for (int part = 0; part < shortsPerConstant; part++) {
result[i * shortsPerConstant + part] = (short) ((value >> (16 * part)) & 0xFFFFL);
}
@@ -843,12 +809,12 @@
return result;
}
- private ConstNumber[] computeConstantArrayValues(
+ private ConstInstruction[] computeConstantArrayValues(
NewArrayEmpty newArray, BasicBlock block, int size) {
if (size > MAX_FILL_ARRAY_SIZE) {
return null;
}
- ConstNumber[] values = new ConstNumber[size];
+ ConstInstruction[] values = new ConstInstruction[size];
int remaining = size;
Set<Instruction> users = newArray.outValue().uniqueUsers();
// We allow the array instantiations to cross block boundaries as long as it hasn't encountered
@@ -860,8 +826,9 @@
Instruction instruction = it.next();
// If we encounter an instruction that can throw an exception we need to bail out of the
// optimization so that we do not transform half-initialized arrays into fully initialized
- // arrays on exceptional edges.
- if (instruction.instructionInstanceCanThrow()) {
+ // arrays on exceptional edges. If the block has no handlers it is not observable so
+ // we perform the rewriting.
+ if (block.hasCatchHandlers() && instruction.instructionInstanceCanThrow()) {
return null;
}
if (!users.contains(instruction)) {
@@ -873,16 +840,15 @@
return null;
}
ArrayPut arrayPut = instruction.asArrayPut();
- if (!arrayPut.source().isConstant()) {
+ if (!(arrayPut.source().isConstant() && arrayPut.index().isConstNumber())) {
return null;
}
- assert arrayPut.index().isConstant();
int index = arrayPut.index().getConstInstruction().asConstNumber().getIntValue();
assert index >= 0 && index < values.length;
if (values[index] != null) {
return null;
}
- ConstNumber value = arrayPut.source().getConstInstruction().asConstNumber();
+ ConstInstruction value = arrayPut.source().getConstInstruction();
values[index] = value;
--remaining;
if (remaining == 0) {
@@ -895,7 +861,7 @@
return null;
}
- private boolean isPrimitiveNewArrayWithConstantPositiveSize(Instruction instruction) {
+ private boolean isPrimitiveOrStringNewArrayWithPositiveSize(Instruction instruction) {
if (!(instruction instanceof NewArrayEmpty)) {
return false;
}
@@ -903,48 +869,69 @@
if (!newArray.size().isConstant()) {
return false;
}
+ assert newArray.size().isConstNumber();
int size = newArray.size().getConstInstruction().asConstNumber().getIntValue();
if (size < 1) {
return false;
}
- if (!newArray.type.isPrimitiveArrayType()) {
- return false;
- }
- return true;
+ return newArray.type.isPrimitiveArrayType() || newArray.type == dexItemFactory.stringArrayType;
}
/**
- * Replace NewArrayEmpty followed by stores of constants to all entries with NewArrayEmpty
- * and FillArrayData.
+ * Replace new-array followed by stores of constants to all entries with new-array
+ * and fill-array-data / filled-new-array.
*/
public void simplifyArrayConstruction(IRCode code) {
for (BasicBlock block : code.blocks) {
// Map from the array value to the number of array put instruction to remove for that value.
+ Map<Value, Instruction> instructionToInsertForArray = new HashMap<>();
Map<Value, Integer> storesToRemoveForArray = new HashMap<>();
// First pass: identify candidates and insert fill array data instruction.
InstructionListIterator it = block.listIterator();
while (it.hasNext()) {
Instruction instruction = it.next();
- if (!isPrimitiveNewArrayWithConstantPositiveSize(instruction)) {
+ if (!isPrimitiveOrStringNewArrayWithPositiveSize(instruction)) {
continue;
}
NewArrayEmpty newArray = instruction.asNewArrayEmpty();
int size = newArray.size().getConstInstruction().asConstNumber().getIntValue();
- // If there is only one element it is typically smaller to generate the array put
- // instruction instead of fill array data.
- if (size == 1) {
+ ConstInstruction[] values = computeConstantArrayValues(newArray, block, size);
+ if (values == null) {
continue;
}
- int elementSize = newArray.type.elementSizeForPrimitiveArrayType();
- short[] contents = computeArrayFilledData(newArray, size, block, elementSize);
- if (contents == null) {
- continue;
+ if (newArray.type == dexItemFactory.stringArrayType) {
+ // Don't replace with filled-new-array if it requires more than 200 consecutive registers.
+ if (size > 200) {
+ continue;
+ }
+ List<Value> stringValues = new ArrayList<>(size);
+ for (ConstInstruction value : values) {
+ stringValues.add(value.outValue());
+ }
+ InvokeNewArray invoke = new InvokeNewArray(
+ dexItemFactory.stringArrayType, newArray.outValue(), stringValues);
+ it.detach();
+ for (Value value : newArray.inValues()) {
+ value.removeUser(newArray);
+ }
+ instructionToInsertForArray.put(newArray.outValue(), invoke);
+ } else {
+ // If there is only one element it is typically smaller to generate the array put
+ // instruction instead of fill array data.
+ if (size == 1) {
+ continue;
+ }
+ int elementSize = newArray.type.elementSizeForPrimitiveArrayType();
+ short[] contents = computeArrayFilledData(values, size, elementSize);
+ if (contents == null) {
+ continue;
+ }
+ int arraySize = newArray.size().getConstInstruction().asConstNumber().getIntValue();
+ NewArrayFilledData fillArray = new NewArrayFilledData(
+ newArray.outValue(), elementSize, arraySize, contents);
+ it.add(fillArray);
}
storesToRemoveForArray.put(newArray.outValue(), size);
- int arraySize = newArray.size().getConstInstruction().asConstNumber().getIntValue();
- NewArrayFilledData fillArray = new NewArrayFilledData(
- newArray.outValue(), elementSize, arraySize, contents);
- it.add(fillArray);
}
// Second pass: remove all the array put instructions for the array for which we have
// inserted a fill array data instruction instead.
@@ -956,9 +943,18 @@
if (instruction.isArrayPut()) {
Value array = instruction.asArrayPut().array();
Integer toRemoveCount = storesToRemoveForArray.get(array);
- if (toRemoveCount != null && toRemoveCount > 0) {
- storesToRemoveForArray.put(array, toRemoveCount - 1);
- it.remove();
+ if (toRemoveCount != null) {
+ if (toRemoveCount > 0) {
+ storesToRemoveForArray.put(array, --toRemoveCount);
+ it.remove();
+ }
+ if (toRemoveCount == 0) {
+ storesToRemoveForArray.put(array, --toRemoveCount);
+ Instruction construction = instructionToInsertForArray.get(array);
+ if (construction != null) {
+ it.add(construction);
+ }
+ }
}
}
}
@@ -1082,8 +1078,8 @@
If theIf = block.exit().asIf();
List<Value> inValues = theIf.inValues();
int cond;
- if (inValues.get(0).isConstant()
- && (theIf.isZeroTest() || inValues.get(1).isConstant())) {
+ if (inValues.get(0).isConstNumber()
+ && (theIf.isZeroTest() || inValues.get(1).isConstNumber())) {
// Zero test with a constant of comparison between between two constants.
if (theIf.isZeroTest()) {
cond = inValues.get(0).getConstInstruction().asConstNumber().getIntValue();
@@ -1143,8 +1139,8 @@
List<Value> inValues = theIf.inValues();
Value leftValue = inValues.get(0);
Value rightValue = inValues.get(1);
- if (leftValue.isConstant() || rightValue.isConstant()) {
- if (leftValue.isConstant()) {
+ if (leftValue.isConstNumber() || rightValue.isConstNumber()) {
+ if (leftValue.isConstNumber()) {
int left = leftValue.getConstInstruction().asConstNumber().getIntValue();
if (left == 0) {
If ifz = new If(theIf.getType().forSwappedOperands(), rightValue);
@@ -1152,7 +1148,6 @@
assert block.exit() == ifz;
}
} else {
- assert rightValue.isConstant();
int right = rightValue.getConstInstruction().asConstNumber().getIntValue();
if (right == 0) {
If ifz = new If(theIf.getType(), leftValue);
@@ -1231,7 +1226,7 @@
iterator.previous();
iterator.add(zero);
- // Then replace the invoke instruction with NewArrayEmpty instruction.
+ // Then replace the invoke instruction with new-array instruction.
Instruction next = iterator.next();
assert current == next;
NewArrayEmpty newArray = new NewArrayEmpty(destValue, zero.outValue(),
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 38cdb29..7b7e5e7 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
@@ -1587,7 +1587,7 @@
newActive.add(splitChild);
// If the constant is split before its first actual use, mark the constant as being
// spilled. That will allows us to remove it afterwards if it is rematerializable.
- if (intervals.getValue().isConstant()
+ if (intervals.getValue().isConstNumber()
&& intervals.getStart() == intervals.getValue().definition.getNumber()
&& intervals.getUses().size() == 1) {
intervals.setSpilled(true);
@@ -1598,7 +1598,9 @@
LiveIntervals splitOfSplit = splitChild.splitBefore(splitChild.getFirstUse());
splitOfSplit.setRegister(intervals.getRegister());
inactive.add(splitOfSplit);
- } else if (intervals.getValue().isConstant()) {
+ } else if (intervals.getValue().isConstNumber()) {
+ // TODO(ager): Do this for all constants. Currently we only rematerialize const
+ // number and therefore we only do it for numbers at this point.
splitRangesForSpilledConstant(splitChild, registerNumber);
} else if (intervals.isArgumentInterval()) {
splitRangesForSpilledArgument(splitChild);
@@ -1627,7 +1629,7 @@
// Spilling a non-pinned, non-rematerializable value. We use the value in the spill
// register for as long as possible to avoid further moves.
assert spilled.isSpilled();
- assert !spilled.getValue().isConstant();
+ assert !spilled.getValue().isConstNumber();
assert !spilled.isLinked() || spilled.isArgumentInterval();
if (spilled.isArgumentInterval()) {
registerNumber = Constants.U16BIT_MAX;
@@ -1658,7 +1660,7 @@
// spill we are running low on registers and this constant should get out of the way
// as much as possible.
assert spilled.isSpilled();
- assert spilled.getValue().isConstant();
+ assert spilled.getValue().isConstNumber();
assert !spilled.isLinked() || spilled.isArgumentInterval();
// Do not split range if constant is reused by one of the eleven following instruction.
int maxGapSize = 11 * INSTRUCTION_NUMBER_DELTA;
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/LiveIntervals.java b/src/main/java/com/android/tools/r8/ir/regalloc/LiveIntervals.java
index 06b33f4..9a810d9 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/LiveIntervals.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/LiveIntervals.java
@@ -85,7 +85,8 @@
}
public boolean isRematerializable(LinearScanRegisterAllocator registerAllocator) {
- if (!value.isConstant()) {
+ // TODO(ager): rematerialize const string as well.
+ if (!value.isConstNumber()) {
return false;
}
// If one of the non-spilled splits uses a register that is higher than U8BIT_MAX we cannot
@@ -95,10 +96,22 @@
// these computations. We use the unadjusted real register number to make sure that
// isRematerializable for the same intervals does not change from one phase of
// compilation to the next.
+ if (getMaxNonSpilledRegister() == NO_REGISTER) {
+ assert allSplitsAreSpilled();
+ return true;
+ }
int max = registerAllocator.unadjustedRealRegisterFromAllocated(getMaxNonSpilledRegister());
return max < Constants.U8BIT_MAX;
}
+ private boolean allSplitsAreSpilled() {
+ assert isSpilled();
+ for (LiveIntervals splitChild : splitChildren) {
+ assert splitChild.isSpilled();
+ }
+ return true;
+ }
+
public boolean isSpilledAndRematerializable(LinearScanRegisterAllocator allocator) {
return isSpilled() && isRematerializable(allocator);
}
@@ -420,7 +433,7 @@
}
public boolean isConstantNumberInterval() {
- return value.definition != null && value.isConstant();
+ return value.definition != null && value.isConstNumber();
}
public int numberOfUsesWithConstraint() {
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 541b622..70a2e53 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -6,6 +6,7 @@
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.dex.Marker;
import com.android.tools.r8.errors.CompilationError;
+import com.android.tools.r8.errors.InvalidDebugInfoException;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.shaking.ProguardConfigurationRule;
@@ -47,7 +48,22 @@
public boolean quiet = false;
// Hidden marker for classes.dex
- public Marker customizedMarker;
+ private boolean hasMarker = false;
+ private Marker marker;
+
+ public boolean hasMarker() {
+ return hasMarker;
+ }
+
+ public void setMarker(Marker marker) {
+ this.hasMarker = true;
+ this.marker = marker;
+ }
+
+ public Marker getMarker() {
+ assert hasMarker();
+ return marker;
+ }
public List<String> methodsFilter = ImmutableList.of();
public int minApiLevel = Constants.DEFAULT_ANDROID_API;
@@ -104,6 +120,12 @@
public boolean warningMissingEnclosingMember = false;
+ public int warningInvalidDebugInfoCount = 0;
+
+ public void warningInvalidDebugInfo(DexEncodedMethod method, InvalidDebugInfoException e) {
+ warningInvalidDebugInfoCount++;
+ }
+
public boolean printWarnings() {
boolean printed = false;
boolean printOutdatedToolchain = false;
@@ -111,6 +133,13 @@
System.out.println("Warning: " + warningInvalidParameterAnnotations);
printed = true;
}
+ if (warningInvalidDebugInfoCount > 0) {
+ System.out.println("Warning: stripped invalid locals information from "
+ + warningInvalidDebugInfoCount
+ + (warningInvalidDebugInfoCount == 1 ? " method." : " methods."));
+ printed = true;
+ printOutdatedToolchain = true;
+ }
if (warningMissingEnclosingMember) {
System.out.println(
"Warning: InnerClass annotations are missing corresponding EnclosingMember annotations."
diff --git a/src/main/java/com/android/tools/r8/utils/StringUtils.java b/src/main/java/com/android/tools/r8/utils/StringUtils.java
index 268228f..8404703 100644
--- a/src/main/java/com/android/tools/r8/utils/StringUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/StringUtils.java
@@ -12,6 +12,8 @@
public class StringUtils {
+ public final static String LINE_SEPARATOR = System.getProperty("line.separator");
+
private final static char[] IDENTIFIER_LETTERS
= "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ_".toCharArray();
private final static int NUMBER_OF_LETTERS = IDENTIFIER_LETTERS.length;
@@ -107,7 +109,7 @@
return join(collection, separator, BraceType.NONE);
}
- public static String join(String separator, String... strings) {
+ public static <T> String join(String separator, T... strings) {
return join(Arrays.asList(strings), separator, BraceType.NONE);
}
@@ -122,6 +124,18 @@
return builder.toString();
}
+ public static <T> String lines(T... lines) {
+ StringBuilder builder = new StringBuilder();
+ for (T line : lines) {
+ builder.append(line).append(LINE_SEPARATOR);
+ }
+ return builder.toString();
+ }
+
+ public static <T> String joinLines(T... lines) {
+ return join(LINE_SEPARATOR, lines);
+ }
+
public static String zeroPrefix(int i, int width) {
return zeroPrefixString(Integer.toString(i), width);
}
diff --git a/src/test/java/com/android/tools/r8/D8LazyRunExamplesAndroidOTest.java b/src/test/java/com/android/tools/r8/D8LazyRunExamplesAndroidOTest.java
index 9c80561..d707614 100644
--- a/src/test/java/com/android/tools/r8/D8LazyRunExamplesAndroidOTest.java
+++ b/src/test/java/com/android/tools/r8/D8LazyRunExamplesAndroidOTest.java
@@ -20,13 +20,16 @@
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Iterator;
import java.util.List;
import java.util.stream.Collectors;
import org.junit.Test;
public class D8LazyRunExamplesAndroidOTest
extends D8IncrementalRunExamplesAndroidOTest {
+
+ // Please note that all tool specific markers have been eliminated in the resulting
+ // dex applications. This allows for byte-wise comparison of the results.
+
class D8LazyTestRunner extends D8IncrementalTestRunner {
D8LazyTestRunner(String testName, String packageName, String mainClass) {
@@ -53,7 +56,9 @@
@Override
D8IncrementalTestRunner test(String testName, String packageName, String mainClass) {
- return new D8LazyTestRunner(testName, packageName, mainClass);
+ D8IncrementalTestRunner result = new D8LazyTestRunner(testName, packageName, mainClass);
+ result.withOptionConsumer(options -> options.setMarker(null));
+ return result;
}
@Test
@@ -75,7 +80,10 @@
.build();
fullBuildResult = ToolHelper.runD8(
- command, (options) -> options.interfaceMethodDesugaring = OffOrAuto.Auto);
+ command, options -> {
+ options.interfaceMethodDesugaring = OffOrAuto.Auto;
+ options.setMarker(null);
+ });
}
// Build each class individually using tmpClassesDir as classpath for desugaring.
@@ -94,7 +102,10 @@
AndroidApp individualResult =
ToolHelper.runD8(
builder.build(),
- (options) -> options.interfaceMethodDesugaring = OffOrAuto.Auto);
+ options -> {
+ options.interfaceMethodDesugaring = OffOrAuto.Auto;
+ options.setMarker(null);
+ });
individalDexes.add(individualResult.getDexProgramResources().get(0));
}
AndroidApp mergedResult = mergeDexResources(minAPILevel, individalDexes);
@@ -111,7 +122,8 @@
for (Resource resource : individalDexes) {
builder.addDexProgramData(readFromResource(resource));
}
- AndroidApp mergedResult = ToolHelper.runD8(builder.build());
+ AndroidApp mergedResult = ToolHelper.runD8(builder.build(),
+ options -> options.setMarker(null));
return mergedResult;
}
diff --git a/src/test/java/com/android/tools/r8/D8NonLazyRunExamplesAndroidOTest.java b/src/test/java/com/android/tools/r8/D8NonLazyRunExamplesAndroidOTest.java
index 42d3c47..5b2977c 100644
--- a/src/test/java/com/android/tools/r8/D8NonLazyRunExamplesAndroidOTest.java
+++ b/src/test/java/com/android/tools/r8/D8NonLazyRunExamplesAndroidOTest.java
@@ -29,6 +29,10 @@
@Override
D8IncrementalTestRunner test(String testName, String packageName, String mainClass) {
- return new D8LazyTestRunner(testName, packageName, mainClass);
+ D8IncrementalTestRunner result = new D8LazyTestRunner(testName, packageName, mainClass);
+ // Eliminate the tool specific marker in the resulting dex applications.
+ // This allows for byte-wise comparison of the results.
+ result.withOptionConsumer(options -> options.setMarker(null));
+ return result;
}
}
diff --git a/src/test/java/com/android/tools/r8/ToolHelper.java b/src/test/java/com/android/tools/r8/ToolHelper.java
index 562518f..fd4cdccb 100644
--- a/src/test/java/com/android/tools/r8/ToolHelper.java
+++ b/src/test/java/com/android/tools/r8/ToolHelper.java
@@ -21,6 +21,7 @@
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.ListUtils;
+import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.ThreadUtils;
import com.android.tools.r8.utils.Timing;
import com.google.common.collect.ImmutableList;
@@ -61,7 +62,7 @@
public static final String EXAMPLES_ANDROID_O_BUILD_DIR = BUILD_DIR + "test/examplesAndroidO/";
public static final String SMALI_BUILD_DIR = BUILD_DIR + "test/smali/";
- public static final String LINE_SEPARATOR = System.getProperty("line.separator");
+ public static final String LINE_SEPARATOR = StringUtils.LINE_SEPARATOR;
private static final String ANDROID_JAR_PATTERN = "third_party/android_jar/lib-v%d/android.jar";
private static final int DEFAULT_MIN_SDK = Constants.ANDROID_I_API;
diff --git a/src/test/java/com/android/tools/r8/d8/D8FrameworkTest.java b/src/test/java/com/android/tools/r8/d8/D8FrameworkTest.java
index 2878ed0..a449e21 100644
--- a/src/test/java/com/android/tools/r8/d8/D8FrameworkTest.java
+++ b/src/test/java/com/android/tools/r8/d8/D8FrameworkTest.java
@@ -68,7 +68,7 @@
Marker selfie = Marker.parse(marker.toString());
assert marker.equals(selfie);
AndroidApp app = ToolHelper.runD8(command, options -> {
- options.customizedMarker = marker;
+ options.setMarker(marker);
options.numberOfThreads = threads;
});
DexApplication dexApp =
diff --git a/src/test/java/com/android/tools/r8/jasmin/InvalidDebugInfoTests.java b/src/test/java/com/android/tools/r8/jasmin/InvalidDebugInfoTests.java
index 155438f..81d299e 100644
--- a/src/test/java/com/android/tools/r8/jasmin/InvalidDebugInfoTests.java
+++ b/src/test/java/com/android/tools/r8/jasmin/InvalidDebugInfoTests.java
@@ -6,6 +6,7 @@
import static org.junit.Assert.assertEquals;
import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.utils.StringUtils;
import com.google.common.collect.ImmutableList;
import org.junit.Test;
@@ -82,7 +83,7 @@
JasminBuilder builder = new JasminBuilder();
JasminBuilder.ClassBuilder clazz = builder.addClass("Test");
- clazz.addStaticMethod("foo", ImmutableList.of("I","I"), "V",
+ clazz.addStaticMethod("foo", ImmutableList.of("I", "I"), "V",
".limit stack 2",
".limit locals 3",
".var 0 is x I from LabelInit to LabelExit",
@@ -123,4 +124,165 @@
assertEquals(expected, artResult);
}
+ @Test
+ public void invalidInfoBug63412730_onWrite() throws Throwable {
+ JasminBuilder builder = new JasminBuilder();
+ JasminBuilder.ClassBuilder clazz = builder.addClass("Test");
+ clazz.addStaticMethod("bar", ImmutableList.of(), "V",
+ ".limit stack 3",
+ ".limit locals 2",
+ ".var 1 is i I from LI to End",
+ ".var 0 is f F from LF to End",
+ "Init:",
+ " ldc 42",
+ " istore 0",
+ "LI:",
+ " ldc 7.5",
+ " fstore 1",
+ "LF:",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " dup",
+ " iload 0",
+ " invokevirtual java/io/PrintStream/println(I)V",
+ " fload 1",
+ " invokevirtual java/io/PrintStream/println(F)V",
+ " return",
+ "End:");
+
+ clazz.addMainMethod(
+ ".limit stack 1",
+ ".limit locals 1",
+ " invokestatic Test/bar()V",
+ " return");
+
+ String expected = StringUtils.lines("42", "7.5");
+ String javaResult = runOnJava(builder, clazz.name);
+ assertEquals(expected, javaResult);
+ String artResult = runOnArtD8(builder, clazz.name);
+ assertEquals(expected, artResult);
+ }
+
+ @Test
+ public void invalidInfoBug63412730_onRead() throws Throwable {
+ JasminBuilder builder = new JasminBuilder();
+ JasminBuilder.ClassBuilder clazz = builder.addClass("Test");
+ clazz.addStaticMethod("bar", ImmutableList.of(), "V",
+ ".limit stack 3",
+ ".limit locals 2",
+ ".var 1 is i I from Locals to End",
+ ".var 0 is f F from Locals to End",
+ "Init:",
+ " ldc 42",
+ " istore 0",
+ " ldc 7.5",
+ " fstore 1",
+ "Locals:",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " dup",
+ " iload 0",
+ " invokevirtual java/io/PrintStream/println(I)V",
+ " fload 1",
+ " invokevirtual java/io/PrintStream/println(F)V",
+ " return",
+ "End:");
+
+ clazz.addMainMethod(
+ ".limit stack 1",
+ ".limit locals 1",
+ " invokestatic Test/bar()V",
+ " return");
+
+ String expected = StringUtils.lines("42", "7.5");
+ String javaResult = runOnJava(builder, clazz.name);
+ assertEquals(expected, javaResult);
+ String artResult = runOnArtD8(builder, clazz.name);
+ assertEquals(expected, artResult);
+ }
+
+ @Test
+ public void invalidInfoBug63412730_onMove() throws Throwable {
+ JasminBuilder builder = new JasminBuilder();
+ JasminBuilder.ClassBuilder clazz = builder.addClass("Test");
+ clazz.addStaticMethod("bar", ImmutableList.of(), "V",
+ ".limit stack 3",
+ ".limit locals 2",
+ ".var 1 is i I from LI to End",
+ ".var 0 is j F from LJ to End",
+ "Init:",
+ " ldc 42",
+ " istore 0",
+ "LI:",
+ " ldc 0",
+ " ldc 0",
+ " ifeq LZ",
+ " ldc 75",
+ " istore 1",
+ "LJ:",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " iload 1",
+ " invokevirtual java/io/PrintStream/println(I)V",
+ " return",
+ "LZ:",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " iload 0",
+ " invokevirtual java/io/PrintStream/println(I)V",
+ " return",
+ "End:");
+
+ clazz.addMainMethod(
+ ".limit stack 1",
+ ".limit locals 1",
+ " invokestatic Test/bar()V",
+ " return");
+
+ String expected = StringUtils.lines("42");
+ String javaResult = runOnJava(builder, clazz.name);
+ assertEquals(expected, javaResult);
+ String artResult = runOnArtD8(builder, clazz.name);
+ assertEquals(expected, artResult);
+ }
+
+ @Test
+ public void invalidInfoBug63412730_onPop() throws Throwable {
+ JasminBuilder builder = new JasminBuilder();
+ JasminBuilder.ClassBuilder clazz = builder.addClass("Test");
+ clazz.addStaticMethod("bar", ImmutableList.of(), "V",
+ ".limit stack 3",
+ ".limit locals 2",
+ ".var 1 is a [Ljava/lang/Object; from Locals to End",
+ ".var 0 is o LObject; from Locals to End",
+ "Init:",
+ " ldc 1",
+ " anewarray java/lang/Object",
+ " astore 0",
+ " new java/lang/Integer",
+ " dup",
+ " ldc 42",
+ " invokespecial java/lang/Integer/<init>(I)V",
+ " astore 1",
+ "Locals:",
+ " aload 0",
+ " ldc 0",
+ " aload 1",
+ " aastore",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " aload 0",
+ " ldc 0",
+ " aaload",
+ " invokevirtual java/io/PrintStream/println(Ljava/lang/Object;)V",
+ " return",
+ "End:");
+
+ clazz.addMainMethod(
+ ".limit stack 1",
+ ".limit locals 1",
+ " invokestatic Test/bar()V",
+ " return");
+
+ String expected = StringUtils.lines("42");
+ String javaResult = runOnJava(builder, clazz.name);
+ assertEquals(expected, javaResult);
+ String artResult = runOnArtD8(builder, clazz.name);
+ assertEquals(expected, artResult);
+ }
}
diff --git a/src/test/java/com/android/tools/r8/smali/IfSimplificationTest.java b/src/test/java/com/android/tools/r8/smali/IfSimplificationTest.java
index 6b4a431..919f569 100644
--- a/src/test/java/com/android/tools/r8/smali/IfSimplificationTest.java
+++ b/src/test/java/com/android/tools/r8/smali/IfSimplificationTest.java
@@ -180,8 +180,8 @@
":return",
" return v0");
DexCode code = method.getCode().asDexCode();
- assertEquals(12, code.instructions.length);
- assertTrue(code.instructions[11] instanceof Return);
+ assertEquals(10, code.instructions.length);
+ assertTrue(code.instructions[9] instanceof Return);
}
@Test
@@ -443,6 +443,6 @@
DexCode code = method.getCode().asDexCode();
// TODO(sgjesse): Maybe this test is too fragile, as it leaves quite a lot of code, so the
// expectation might need changing with other optimizations.
- assertEquals(29, code.instructions.length);
+ assertEquals(27, code.instructions.length);
}
}
diff --git a/src/test/java/com/android/tools/r8/smali/SwitchRewritingTest.java b/src/test/java/com/android/tools/r8/smali/SwitchRewritingTest.java
index f10add0..7f89da7 100644
--- a/src/test/java/com/android/tools/r8/smali/SwitchRewritingTest.java
+++ b/src/test/java/com/android/tools/r8/smali/SwitchRewritingTest.java
@@ -7,7 +7,6 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
-import com.android.tools.r8.R8;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.code.Const;
import com.android.tools.r8.code.Const4;
@@ -292,12 +291,11 @@
DexCode code = method.getCode().asDexCode();
if (key == 0) {
assertEquals(5, code.instructions.length);
- assertTrue(code.instructions[0] instanceof IfEqz);
+ assertTrue(code.instructions[2] instanceof IfEqz);
} else {
assertEquals(6, code.instructions.length);
- assertTrue(some16BitConst(code.instructions[0]));
- assertTrue(code.instructions[1] instanceof IfEq);
- assertTrue(code.instructions[2] instanceof Const4);
+ assertTrue(some16BitConst(code.instructions[2]));
+ assertTrue(code.instructions[3] instanceof IfEq);
}
}
@@ -351,9 +349,9 @@
DexEncodedMethod method = getMethod(app, signature);
DexCode code = method.getCode().asDexCode();
if (twoCaseWillUsePackedSwitch(key1, key2)) {
- assertTrue(code.instructions[0] instanceof PackedSwitch);
+ assertTrue(code.instructions[3] instanceof PackedSwitch);
} else {
- assertTrue(code.instructions[0] instanceof SparseSwitch);
+ assertTrue(code.instructions[3] instanceof SparseSwitch);
}
}
@@ -423,10 +421,22 @@
MethodSignature signature = new MethodSignature("Test", "test", "int", ImmutableList.of("int"));
DexEncodedMethod method = getMethod(app, signature);
DexCode code = method.getCode().asDexCode();
+ int packedSwitchCount = 0;
+ int sparseSwitchCount = 0;
+ for (Instruction instruction : code.instructions) {
+ if (instruction instanceof PackedSwitch) {
+ packedSwitchCount++;
+ }
+ if (instruction instanceof SparseSwitch) {
+ sparseSwitchCount++;
+ }
+ }
if (keyStep <= 2) {
- assertTrue(code.instructions[0] instanceof PackedSwitch);
+ assertEquals(1, packedSwitchCount);
+ assertEquals(0, sparseSwitchCount);
} else {
- assertTrue(code.instructions[0] instanceof SparseSwitch);
+ assertEquals(0, packedSwitchCount);
+ assertEquals(1, sparseSwitchCount);
}
}
diff --git a/tools/create_jctf_tests.py b/tools/create_jctf_tests.py
index ff9d797..b7b449d 100755
--- a/tools/create_jctf_tests.py
+++ b/tools/create_jctf_tests.py
@@ -16,10 +16,12 @@
import utils
-JCTFROOT = 'third_party/jctf'
-DESTINATION_DIR = 'build/generated/test/java/com/android/tools/r8/jctf'
+JCTFROOT = join(utils.REPO_ROOT, 'third_party', 'jctf')
+DESTINATION_DIR = join(utils.REPO_ROOT, 'build', 'generated', 'test', 'java',
+ 'com', 'android', 'tools', 'r8', 'jctf')
PACKAGE_PREFIX = 'com.google.jctf.test.lib.java.'
-RELATIVE_TESTDIR = 'LibTests/src/com/google/jctf/test/lib/java'
+RELATIVE_TESTDIR = join('LibTests', 'src', 'com', 'google', 'jctf', 'test',
+ 'lib', 'java')
TESTDIR = join(JCTFROOT, RELATIVE_TESTDIR)
TEMPLATE = Template(
"""// Copyright (c) 2017, the R8 project authors. Please see the AUTHORS file
@@ -58,12 +60,17 @@
EXIT_FAILURE = 1
RE_PACKAGE = re.compile('package\\s+(com[^\\s;]*)')
+def fix_long_path(p):
+ if os.name == 'nt':
+ p = ('\\\\?\\' + p).decode('utf-8')
+ return p
+
def file_contains_string(filepath, search_string):
- with open(filepath) as f:
+ with open(fix_long_path(filepath)) as f:
return search_string in f.read()
def read_package_from_java_file(filepath):
- with open(filepath) as f:
+ with open(fix_long_path(filepath)) as f:
for line in f:
m = RE_PACKAGE.search(line)
if m:
@@ -74,7 +81,7 @@
def generate_test(class_name, compiler_under_test, compiler_under_test_enum,
relative_package):
filename = join(DESTINATION_DIR, compiler_under_test,
- relative_package.replace('.', '/'), class_name + '.java')
+ relative_package.replace('.', os.sep), class_name + '.java')
utils.makedirs_if_needed(dirname(filename))
full_class_name = '{}{}.{}'.format(PACKAGE_PREFIX, relative_package,
@@ -88,7 +95,7 @@
classFile = full_class_name.replace('.', '/') + '.class',
nameWithoutPackagePrefix = '{}.{}'.format(relative_package, class_name))
- with open(filename, 'w') as f:
+ with open(fix_long_path(filename), 'w') as f:
f.write(contents)
def Main():
@@ -98,7 +105,7 @@
return EXIT_FAILURE
for tool in ['d8', 'r8']:
- p = join(DESTINATION_DIR, tool)
+ p = fix_long_path(join(DESTINATION_DIR, tool))
if exists(p):
rmtree(p)
makedirs(p)
diff --git a/tools/test.py b/tools/test.py
index ba459e7..8e016c7 100755
--- a/tools/test.py
+++ b/tools/test.py
@@ -68,6 +68,7 @@
utils.upload_html_to_cloud_storage(upload_dir, destination)
url = 'http://storage.googleapis.com/%s/%s/test/index.html' % (BUCKET, u_dir)
print 'Test results available at: %s' % url
+ print '@@@STEP_LINK@Test failures@%s@@@' % url
def Main():
(options, args) = ParseOptions()
@@ -112,7 +113,7 @@
return_code = gradle.RunGradle(gradle_args + ['-Pdex_vm=%s' % art_vm],
throw_on_failure=False)
if return_code != 0:
- if options.archive_failures:
+ if options.archive_failures and os.name != 'nt':
archive_failures()
return return_code
diff --git a/tools/test_framework.py b/tools/test_framework.py
index 1ade73d..a153847 100755
--- a/tools/test_framework.py
+++ b/tools/test_framework.py
@@ -71,7 +71,7 @@
if args.tool == 'goyt':
tool_file = GOYT_EXE
- tool_args = ['--num-threads=4'] + tool_args
+ tool_args = ['--num-threads=8'] + tool_args
elif args.tool == 'dx':
tool_file = DX_JAR
else:
diff --git a/tools/utils.py b/tools/utils.py
index b7abb45..dab1dd1 100644
--- a/tools/utils.py
+++ b/tools/utils.py
@@ -56,7 +56,7 @@
def upload_html_to_cloud_storage(directory, destination):
# Upload and make the content encoding right for viewing directly
- cmd = [sys.executable, 'gsutil.py', 'cp', '-z', 'html', '-a',
+ cmd = ['gsutil.py', 'cp', '-z', 'html', '-a',
'public-read', '-R', directory, destination]
PrintCmd(cmd)
subprocess.check_call(cmd)
