Merge "Do not translate File.separatorChar to / when reading from archives."
diff --git a/build.gradle b/build.gradle
index f379bbf..8ea2b3f 100644
--- a/build.gradle
+++ b/build.gradle
@@ -1239,6 +1239,18 @@
}
test {
+ if (project.hasProperty('generate_golden_files_to')) {
+ systemProperty 'generate_golden_files_to', project.property('generate_golden_files_to')
+ assert project.hasProperty('HEAD_sha1')
+ systemProperty 'test_git_HEAD_sha1', project.property('HEAD_sha1')
+ }
+
+ if (project.hasProperty('use_golden_files_in')) {
+ systemProperty 'use_golden_files_in', project.property('use_golden_files_in')
+ assert project.hasProperty('HEAD_sha1')
+ systemProperty 'test_git_HEAD_sha1', project.property('HEAD_sha1')
+ }
+
dependsOn getJarsFromSupportLibs
testLogging.exceptionFormat = 'full'
if (project.hasProperty('print_test_stdout')) {
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 0ba40b3..fb3b067 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.2.20-dev";
+ public static final String LABEL = "1.2.21-dev";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index 9f97f88..831edbe 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -43,12 +43,9 @@
import com.android.tools.r8.naming.NamingLens;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.InternalOptions;
-import com.google.common.collect.ImmutableMap;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
-import java.util.Map;
-import java.util.Set;
import java.util.function.Consumer;
public class DexEncodedMethod extends KeyedDexItem<DexMethod> implements ResolutionResult {
@@ -582,6 +579,14 @@
return m1.method.slowCompareTo(m2.method);
}
+ public static class ClassInlinerEligibility {
+ public final boolean returnsReceiver;
+
+ public ClassInlinerEligibility(boolean returnsReceiver) {
+ this.returnsReceiver = returnsReceiver;
+ }
+ }
+
public static class OptimizationInfo {
private int returnedArgument = -1;
@@ -593,8 +598,9 @@
private boolean useIdentifierNameString = false;
private boolean checksNullReceiverBeforeAnySideEffect = false;
private boolean triggersClassInitBeforeAnySideEffect = false;
- private Set<DexField> receiverOnlyUsedForReadingFields = null;
- private Map<DexField, Integer> onlyInitializesFieldsWithNoOtherSideEffects = null;
+ // Stores information about instance methods and constructors for
+ // class inliner, null value indicates that the method is not eligible.
+ private ClassInlinerEligibility classInlinerEligibility = null;
private OptimizationInfo() {
// Intentionally left empty.
@@ -631,13 +637,12 @@
return returnsConstant;
}
- public boolean isReceiverOnlyUsedForReadingFields(Set<DexField> fields) {
- return receiverOnlyUsedForReadingFields != null &&
- fields.containsAll(receiverOnlyUsedForReadingFields);
+ private void setClassInlinerEligibility(ClassInlinerEligibility eligibility) {
+ this.classInlinerEligibility = eligibility;
}
- public Map<DexField, Integer> onlyInitializesFieldsWithNoOtherSideEffects() {
- return onlyInitializesFieldsWithNoOtherSideEffects;
+ public ClassInlinerEligibility getClassInlinerEligibility() {
+ return this.classInlinerEligibility;
}
public long getReturnedConstant() {
@@ -681,19 +686,6 @@
returnedConstant = value;
}
- private void markReceiverOnlyUsedForReadingFields(Set<DexField> fields) {
- receiverOnlyUsedForReadingFields = fields;
- }
-
- private void markOnlyInitializesFieldsWithNoOtherSideEffects(Map<DexField, Integer> mapping) {
- if (mapping == null) {
- onlyInitializesFieldsWithNoOtherSideEffects = null;
- } else {
- onlyInitializesFieldsWithNoOtherSideEffects = mapping.isEmpty()
- ? Collections.emptyMap() : ImmutableMap.copyOf(mapping);
- }
- }
-
private void markForceInline() {
forceInline = true;
}
@@ -751,13 +743,8 @@
ensureMutableOI().markReturnsConstant(value);
}
- synchronized public void markReceiverOnlyUsedForReadingFields(Set<DexField> fields) {
- ensureMutableOI().markReceiverOnlyUsedForReadingFields(fields);
- }
-
- synchronized public void markOnlyInitializesFieldsWithNoOtherSideEffects(
- Map<DexField, Integer> mapping) {
- ensureMutableOI().markOnlyInitializesFieldsWithNoOtherSideEffects(mapping);
+ synchronized public void setClassInlinerEligibility(ClassInlinerEligibility eligibility) {
+ ensureMutableOI().setClassInlinerEligibility(eligibility);
}
synchronized public void markForceInline() {
diff --git a/src/main/java/com/android/tools/r8/ir/code/NewInstance.java b/src/main/java/com/android/tools/r8/ir/code/NewInstance.java
index 85a2509..a5599fa 100644
--- a/src/main/java/com/android/tools/r8/ir/code/NewInstance.java
+++ b/src/main/java/com/android/tools/r8/ir/code/NewInstance.java
@@ -19,6 +19,7 @@
public class NewInstance extends Instruction {
public final DexType clazz;
+ private boolean allowSpilling = true;
public NewInstance(DexType clazz, Value dest) {
super(dest);
@@ -109,4 +110,12 @@
public boolean triggersInitializationOfClass(DexType klass) {
return clazz == klass;
}
+
+ public void markNoSpilling() {
+ allowSpilling = false;
+ }
+
+ public boolean isSpillingAllowed() {
+ return allowSpilling;
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Phi.java b/src/main/java/com/android/tools/r8/ir/code/Phi.java
index 186839b..51850a0 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Phi.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Phi.java
@@ -76,6 +76,10 @@
}
public void addOperands(List<Value> operands) {
+ addOperands(operands, true);
+ }
+
+ public void addOperands(List<Value> operands, boolean removeTrivialPhi) {
// Phi operands are only filled in once to complete the phi. Some phis are incomplete for a
// period of time to break cycles. When the cycle has been resolved they are completed
// exactly once by adding the operands.
@@ -91,7 +95,9 @@
if (!canBeNull) {
markNeverNull();
}
- removeTrivialPhi();
+ if (removeTrivialPhi) {
+ removeTrivialPhi();
+ }
}
public void addDebugValue(Value value) {
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 90a9898..81dc436 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
@@ -169,7 +169,8 @@
}
this.classInliner =
(options.enableClassInlining && options.enableInlining && inliner != null)
- ? new ClassInliner(appInfo.dexItemFactory) : null;
+ ? new ClassInliner(appInfo.dexItemFactory, options.classInliningInstructionLimit)
+ : null;
}
/**
@@ -627,6 +628,10 @@
printC1VisualizerHeader(method);
printMethod(code, "Initial IR (SSA)");
+ if (options.canHaveArtStringNewInitBug()) {
+ codeRewriter.ensureDirectStringNewToInit(code);
+ }
+
if (options.debug) {
codeRewriter.simplifyDebugLocals(code);
}
@@ -752,10 +757,7 @@
}
// Analysis must be done after method is rewritten by logArgumentTypes()
- codeRewriter.identifyReceiverOnlyUsedForReadingFields(method, code, feedback);
- if (method.isInstanceInitializer()) {
- codeRewriter.identifyOnlyInitializesFieldsWithNoOtherSideEffects(method, code, feedback);
- }
+ codeRewriter.identifyClassInlinerEligibility(method, code, feedback);
printMethod(code, "Optimized IR (SSA)");
finalizeIR(method, code, feedback);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedback.java b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedback.java
index 26ca6b2..16a77c8 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedback.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedback.java
@@ -5,10 +5,8 @@
package com.android.tools.r8.ir.conversion;
import com.android.tools.r8.graph.DexEncodedMethod;
-import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexEncodedMethod.ClassInlinerEligibility;
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
-import java.util.Map;
-import java.util.Set;
public interface OptimizationFeedback {
void methodReturnsArgument(DexEncodedMethod method, int argument);
@@ -18,7 +16,5 @@
void markProcessed(DexEncodedMethod method, Constraint state);
void markCheckNullReceiverBeforeAnySideEffect(DexEncodedMethod method, boolean mark);
void markTriggerClassInitBeforeAnySideEffect(DexEncodedMethod method, boolean mark);
- void markReceiverOnlyUsedForReadingFields(DexEncodedMethod method, Set<DexField> fields);
- void markOnlyInitializesFieldsWithNoOtherSideEffects(
- DexEncodedMethod method, Map<DexField, Integer> mapping);
+ void setClassInlinerEligibility(DexEncodedMethod method, ClassInlinerEligibility eligibility);
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackDirect.java b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackDirect.java
index 645df64..7303b6b 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackDirect.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackDirect.java
@@ -5,10 +5,8 @@
package com.android.tools.r8.ir.conversion;
import com.android.tools.r8.graph.DexEncodedMethod;
-import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexEncodedMethod.ClassInlinerEligibility;
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
-import java.util.Map;
-import java.util.Set;
public class OptimizationFeedbackDirect implements OptimizationFeedback {
@@ -48,13 +46,8 @@
}
@Override
- public void markReceiverOnlyUsedForReadingFields(DexEncodedMethod method, Set<DexField> fields) {
- method.markReceiverOnlyUsedForReadingFields(fields);
- }
-
- @Override
- public void markOnlyInitializesFieldsWithNoOtherSideEffects(DexEncodedMethod method,
- Map<DexField, Integer> mapping) {
- method.markOnlyInitializesFieldsWithNoOtherSideEffects(mapping);
+ public void setClassInlinerEligibility(
+ DexEncodedMethod method, ClassInlinerEligibility eligibility) {
+ method.setClassInlinerEligibility(eligibility);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackIgnore.java b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackIgnore.java
index 97fe052..6915a2f 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackIgnore.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackIgnore.java
@@ -5,10 +5,8 @@
package com.android.tools.r8.ir.conversion;
import com.android.tools.r8.graph.DexEncodedMethod;
-import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexEncodedMethod.ClassInlinerEligibility;
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
-import java.util.Map;
-import java.util.Set;
public class OptimizationFeedbackIgnore implements OptimizationFeedback {
@@ -34,11 +32,7 @@
public void markTriggerClassInitBeforeAnySideEffect(DexEncodedMethod method, boolean mark) {}
@Override
- public void markReceiverOnlyUsedForReadingFields(DexEncodedMethod method, Set<DexField> fields) {
- }
-
- @Override
- public void markOnlyInitializesFieldsWithNoOtherSideEffects(DexEncodedMethod method,
- Map<DexField, Integer> mapping) {
+ public void setClassInlinerEligibility(
+ DexEncodedMethod method, ClassInlinerEligibility eligibility) {
}
}
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 3ea434a..bd3442c 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
@@ -12,6 +12,7 @@
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexEncodedMethod.ClassInlinerEligibility;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
@@ -47,8 +48,6 @@
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.If;
import com.android.tools.r8.ir.code.If.Type;
-import com.android.tools.r8.ir.code.InstanceGet;
-import com.android.tools.r8.ir.code.InstancePut;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionIterator;
import com.android.tools.r8.ir.code.InstructionListIterator;
@@ -60,6 +59,7 @@
import com.android.tools.r8.ir.code.MemberType;
import com.android.tools.r8.ir.code.NewArrayEmpty;
import com.android.tools.r8.ir.code.NewArrayFilledData;
+import com.android.tools.r8.ir.code.NewInstance;
import com.android.tools.r8.ir.code.NumericType;
import com.android.tools.r8.ir.code.Phi;
import com.android.tools.r8.ir.code.Position;
@@ -97,9 +97,12 @@
import it.unimi.dsi.fastutil.ints.IntLists;
import it.unimi.dsi.fastutil.objects.Object2IntLinkedOpenHashMap;
import it.unimi.dsi.fastutil.objects.Object2IntMap;
+import it.unimi.dsi.fastutil.objects.Reference2IntMap;
+import it.unimi.dsi.fastutil.objects.Reference2IntOpenHashMap;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Comparator;
+import java.util.Deque;
import java.util.HashMap;
import java.util.HashSet;
import java.util.IdentityHashMap;
@@ -736,120 +739,72 @@
}
}
- public void identifyReceiverOnlyUsedForReadingFields(
+ public void identifyClassInlinerEligibility(
DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) {
- if (!method.isNonAbstractVirtualMethod()) {
+ // Method eligibility is calculated in similar way for regular method
+ // and for the constructor. To be eligible method should only be using its
+ // receiver in the following ways:
+ //
+ // (1) as a receiver of reads/writes of instance fields of the holder class
+ // (2) as a return value
+ // (3) as a receiver of a call to the superclass initializer
+ //
+ boolean instanceInitializer = method.isInstanceInitializer();
+ if (method.accessFlags.isNative() ||
+ (!method.isNonAbstractVirtualMethod() && !instanceInitializer)) {
return;
}
- feedback.markReceiverOnlyUsedForReadingFields(method, null);
+ feedback.setClassInlinerEligibility(method, null); // To allow returns below.
- Value instance = code.getThis();
- if (instance.numberOfPhiUsers() > 0) {
- return;
- }
-
- Set<DexField> fields = Sets.newIdentityHashSet();
- for (Instruction insn : instance.uniqueUsers()) {
- if (!insn.isInstanceGet()) {
- return;
- }
- InstanceGet get = insn.asInstanceGet();
- if (get.dest() == instance || get.object() != instance) {
- return;
- }
- fields.add(get.getField());
- }
- feedback.markReceiverOnlyUsedForReadingFields(method, fields);
- }
-
- public void identifyOnlyInitializesFieldsWithNoOtherSideEffects(
- DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) {
- assert method.isInstanceInitializer();
-
- feedback.markOnlyInitializesFieldsWithNoOtherSideEffects(method, null);
-
- if (code.hasCatchHandlers()) {
- return;
- }
-
- List<Value> args = code.collectArguments(true /* not interested in receiver */);
- Map<DexField, Integer> mapping = new IdentityHashMap<>();
Value receiver = code.getThis();
-
- InstructionIterator it = code.instructionIterator();
- while (it.hasNext()) {
- Instruction instruction = it.next();
-
- // Mark an argument.
- if (instruction.isArgument()) {
- continue;
- }
-
- // Allow super call to java.lang.Object.<init>() on 'this'.
- if (instruction.isInvokeDirect()) {
- InvokeDirect invokedDirect = instruction.asInvokeDirect();
- if (invokedDirect.getInvokedMethod() != dexItemFactory.objectMethods.constructor ||
- invokedDirect.getReceiver() != receiver) {
- return;
- }
- continue;
- }
-
- // Allow final return.
- if (instruction.isReturn()) {
- continue;
- }
-
- // Allow assignment to this class' fields. If the assigned value is an argument
- // reference update the mep. Otherwise just allow assigning any value, since all
- // invalid values should be filtered out at the definitions.
- if (instruction.isInstancePut()) {
- InstancePut instancePut = instruction.asInstancePut();
- DexField field = instancePut.getField();
- if (instancePut.object() != receiver) {
- return;
- }
-
- Value value = instancePut.value();
- if (value.isArgument() && !value.isThis()) {
- assert (args.contains(value));
- mapping.put(field, args.indexOf(value));
- } else {
- mapping.remove(field);
- }
- continue;
- }
-
- // Allow non-throwing constants.
- if (instruction.isConstInstruction()) {
- if (instruction.instructionTypeCanThrow()) {
- return;
- }
- continue;
- }
-
- // Allow goto instructions jumping to the next block.
- if (instruction.isGoto()) {
- if (instruction.getBlock().getNumber() + 1 !=
- instruction.asGoto().getTarget().getNumber()) {
- return;
- }
- continue;
- }
-
- // Allow binary and unary instructions if they don't throw.
- if (instruction.isBinop() || instruction.isUnop()) {
- if (instruction.instructionTypeCanThrow()) {
- return;
- }
- continue;
- }
-
+ if (receiver.numberOfPhiUsers() > 0) {
return;
}
- feedback.markOnlyInitializesFieldsWithNoOtherSideEffects(method, mapping);
+ boolean receiverUsedAsReturnValue = false;
+ boolean seenSuperInitCall = false;
+ for (Instruction insn : receiver.uniqueUsers()) {
+ if (insn.isReturn()) {
+ receiverUsedAsReturnValue = true;
+ continue;
+ }
+
+ if (insn.isInstanceGet() ||
+ (insn.isInstancePut() && insn.asInstancePut().object() == receiver)) {
+ DexField field = insn.asFieldInstruction().getField();
+ if (field.clazz == method.method.holder) {
+ // Since class inliner currently only supports classes directly extending
+ // java.lang.Object, we don't need to worry about fields defined in superclasses.
+ continue;
+ }
+ return;
+ }
+
+ // If this is an instance initializer allow one call
+ // to java.lang.Object.<init>() on 'this'.
+ if (instanceInitializer && insn.isInvokeDirect()) {
+ InvokeDirect invokedDirect = insn.asInvokeDirect();
+ if (invokedDirect.getInvokedMethod() == dexItemFactory.objectMethods.constructor &&
+ invokedDirect.getReceiver() == receiver &&
+ !seenSuperInitCall) {
+ seenSuperInitCall = true;
+ continue;
+ }
+ return;
+ }
+
+ // Other receiver usages make the method not eligible.
+ return;
+ }
+
+ if (instanceInitializer && !seenSuperInitCall) {
+ // Call to super constructor not found?
+ return;
+ }
+
+ feedback.setClassInlinerEligibility(
+ method, new ClassInlinerEligibility(receiverUsedAsReturnValue));
}
/**
@@ -2683,4 +2638,147 @@
// When we fall out of the loop the iterator is in the last eol block.
iterator.add(new InvokeVirtual(printLn, null, ImmutableList.of(out, empty)));
}
+
+ public static void ensureDirectStringNewToInit(IRCode code) {
+ DexItemFactory factory = code.options.itemFactory;
+ for (BasicBlock block : code.blocks) {
+ for (InstructionListIterator it = block.listIterator(); it.hasNext(); ) {
+ Instruction instruction = it.next();
+ if (instruction.isInvokeDirect()) {
+ InvokeDirect invoke = instruction.asInvokeDirect();
+ DexMethod method = invoke.getInvokedMethod();
+ if (factory.isConstructor(method)
+ && method.holder == factory.stringType
+ && invoke.getReceiver().isPhi()) {
+ NewInstance newInstance = findNewInstance(invoke.getReceiver().asPhi());
+ replaceTrivialNewInstancePhis(newInstance.outValue());
+ if (invoke.getReceiver().isPhi()) {
+ throw new CompilationError(
+ "Failed to remove trivial phis between new-instance and <init>");
+ }
+ newInstance.markNoSpilling();
+ }
+ }
+ }
+ }
+ }
+
+ private static NewInstance findNewInstance(Phi phi) {
+ Set<Phi> seen = new HashSet<>();
+ Set<Value> values = new HashSet<>();
+ recursiveAddOperands(phi, seen, values);
+ if (values.size() != 1) {
+ throw new CompilationError("Failed to identify unique new-instance for <init>");
+ }
+ Value newInstanceValue = values.iterator().next();
+ if (newInstanceValue.definition == null || !newInstanceValue.definition.isNewInstance()) {
+ throw new CompilationError("Invalid defining value for call to <init>");
+ }
+ return newInstanceValue.definition.asNewInstance();
+ }
+
+ private static void recursiveAddOperands(Phi phi, Set<Phi> seen, Set<Value> values) {
+ for (Value operand : phi.getOperands()) {
+ if (!operand.isPhi()) {
+ values.add(operand);
+ } else {
+ Phi phiOp = operand.asPhi();
+ if (seen.add(phiOp)) {
+ recursiveAddOperands(phiOp, seen, values);
+ }
+ }
+ }
+ }
+
+ // If an <init> call takes place on a phi the code must contain an irreducible loop between the
+ // new-instance and the <init>. Assuming the code is verifiable, new-instance must flow to a
+ // unique <init>. Here we compute the set of strongly connected phis making use of the
+ // new-instance value and replace all trivial ones by the new-instance value.
+ // This is a simplified variant of the removeRedundantPhis algorithm in Section 3.2 of:
+ // http://compilers.cs.uni-saarland.de/papers/bbhlmz13cc.pdf
+ private static void replaceTrivialNewInstancePhis(Value newInstanceValue) {
+ List<Set<Value>> components = new SCC().computeSCC(newInstanceValue);
+ for (int i = components.size() - 1; i >= 0; i--) {
+ Set<Value> component = components.get(i);
+ if (component.size() == 1 && component.iterator().next() == newInstanceValue) {
+ continue;
+ }
+ Set<Phi> trivialPhis = new HashSet<>();
+ for (Value value : component) {
+ boolean isTrivial = true;
+ Phi p = value.asPhi();
+ for (Value op : p.getOperands()) {
+ if (op != newInstanceValue && !component.contains(op)) {
+ isTrivial = false;
+ break;
+ }
+ }
+ if (isTrivial) {
+ trivialPhis.add(p);
+ }
+ }
+ for (Phi trivialPhi : trivialPhis) {
+ for (Value op : trivialPhi.getOperands()) {
+ op.removePhiUser(trivialPhi);
+ }
+ trivialPhi.replaceUsers(newInstanceValue);
+ trivialPhi.getBlock().removePhi(trivialPhi);
+ }
+ }
+ }
+
+ // Dijkstra's path-based strongly-connected components algorithm.
+ // https://en.wikipedia.org/wiki/Path-based_strong_component_algorithm
+ private static class SCC {
+
+ private int currentTime = 0;
+ private final Reference2IntMap<Value> discoverTime = new Reference2IntOpenHashMap<>();
+ private final Set<Value> unassignedSet = new HashSet<>();
+ private final Deque<Value> unassignedStack = new ArrayDeque<>();
+ private final Deque<Value> preorderStack = new ArrayDeque<>();
+ private final List<Set<Value>> components = new ArrayList<>();
+
+ public List<Set<Value>> computeSCC(Value v) {
+ assert currentTime == 0;
+ dfs(v);
+ return components;
+ }
+
+ private void dfs(Value value) {
+ discoverTime.put(value, currentTime++);
+ unassignedSet.add(value);
+ unassignedStack.push(value);
+ preorderStack.push(value);
+ for (Phi phi : value.uniquePhiUsers()) {
+ if (!discoverTime.containsKey(phi)) {
+ // If not seen yet, continue the search.
+ dfs(phi);
+ } else if (unassignedSet.contains(phi)) {
+ // If seen already and the element is on the unassigned stack we have found a cycle.
+ // Pop off everything discovered later than the target from the preorder stack. This may
+ // not coincide with the cycle as an outer cycle may already have popped elements off.
+ int discoverTimeOfPhi = discoverTime.getInt(phi);
+ while (discoverTimeOfPhi < discoverTime.getInt(preorderStack.peek())) {
+ preorderStack.pop();
+ }
+ }
+ }
+ if (preorderStack.peek() == value) {
+ // If the current element is the top of the preorder stack, then we are at entry to a
+ // strongly-connected component consisting of this element and every element above this
+ // element on the stack.
+ Set<Value> component = new HashSet<>(unassignedStack.size());
+ while (true) {
+ Value member = unassignedStack.pop();
+ unassignedSet.remove(member);
+ component.add(member);
+ if (member == value) {
+ components.add(component);
+ break;
+ }
+ }
+ preorderStack.pop();
+ }
+ }
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
index 7a6a846..92bda1d 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
@@ -86,6 +86,7 @@
});
}
+ code.removeAllTrivialPhis();
assert code.isConsistentSSA();
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
index 845e17b..01dcf0d 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -315,6 +315,12 @@
}
@Override
+ public boolean isValidTarget(InvokeMethod invoke, DexEncodedMethod target, IRCode inlinee) {
+ return !target.isInstanceInitializer()
+ || inliner.legalConstructorInline(method, invoke, inlinee);
+ }
+
+ @Override
public boolean exceededAllowance() {
return instructionAllowance < 0;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
index 86d40d9..b56dc21 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
@@ -61,6 +61,11 @@
}
@Override
+ public boolean isValidTarget(InvokeMethod invoke, DexEncodedMethod target, IRCode inlinee) {
+ return true;
+ }
+
+ @Override
public ListIterator<BasicBlock> updateTypeInformationIfNeeded(IRCode inlinee,
ListIterator<BasicBlock> blockIterator, BasicBlock block, BasicBlock invokeSuccessor) {
return blockIterator;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index b75597d..25cd969 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -285,7 +285,7 @@
return numOfInstructions;
}
- private boolean legalConstructorInline(DexEncodedMethod method,
+ boolean legalConstructorInline(DexEncodedMethod method,
InvokeMethod invoke, IRCode code) {
// In the Java VM Specification section "4.10.2.4. Instance Initialization Methods and
@@ -447,8 +447,7 @@
// Make sure constructor inlining is legal.
assert !target.isClassInitializer();
- if (target.isInstanceInitializer()
- && !legalConstructorInline(method, invoke, inlinee)) {
+ if (!strategy.isValidTarget(invoke, target, inlinee)) {
continue;
}
DexType downcast = createDowncastIfNeeded(strategy, invoke, target);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java b/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java
index 7b88b38..2443d11 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java
@@ -18,6 +18,8 @@
void ensureMethodProcessed(DexEncodedMethod target, IRCode inlinee);
+ boolean isValidTarget(InvokeMethod invoke, DexEncodedMethod target, IRCode inlinee);
+
ListIterator<BasicBlock> updateTypeInformationIfNeeded(IRCode inlinee,
ListIterator<BasicBlock> blockIterator, BasicBlock block, BasicBlock invokeSuccessor);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
index 9ff44eb..7bdff33 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
@@ -9,68 +9,75 @@
import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexEncodedMethod.ClassInlinerEligibility;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.ir.code.BasicBlock;
+import com.android.tools.r8.ir.code.ConstNumber;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.InstanceGet;
import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.InstructionListIterator;
import com.android.tools.r8.ir.code.InvokeDirect;
import com.android.tools.r8.ir.code.InvokeMethodWithReceiver;
import com.android.tools.r8.ir.code.NewInstance;
+import com.android.tools.r8.ir.code.Phi;
import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.ir.code.ValueType;
import com.android.tools.r8.ir.optimize.Inliner.InliningInfo;
import com.android.tools.r8.ir.optimize.Inliner.Reason;
import com.google.common.collect.Streams;
-import java.util.Collections;
+import java.util.ArrayList;
import java.util.IdentityHashMap;
+import java.util.LinkedList;
import java.util.List;
import java.util.Map;
-import java.util.Set;
+import java.util.Map.Entry;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Predicate;
import java.util.stream.Collectors;
public final class ClassInliner {
private final DexItemFactory factory;
+ private final int totalMethodInstructionLimit;
private final ConcurrentHashMap<DexType, Boolean> knownClasses = new ConcurrentHashMap<>();
- private static final Map<DexField, Integer> NO_MAPPING = new IdentityHashMap<>();
-
public interface InlinerAction {
void inline(Map<InvokeMethodWithReceiver, InliningInfo> methods);
}
- public ClassInliner(DexItemFactory factory) {
+ public ClassInliner(DexItemFactory factory, int totalMethodInstructionLimit) {
this.factory = factory;
+ this.totalMethodInstructionLimit = totalMethodInstructionLimit;
}
// Process method code and inline eligible class instantiations, in short:
//
// - collect all 'new-instance' instructions in the original code. Note that class
- // inlining, if happens, mutates code and can add 'new-instance' instructions.
- // Processing them as well is possible, but does not seem to have much value.
+ // inlining, if happens, mutates code and may add new 'new-instance' instructions.
+ // Processing them as well is possible, but does not seem to bring much value.
//
// - for each 'new-instance' we check if it is eligible for inlining, i.e:
// -> the class of the new instance is 'eligible' (see computeClassEligible(...))
- // -> the instance is initialized with 'eligible' constructor (see
- // onlyInitializesFieldsWithNoOtherSideEffects flag in method's optimization
- // info); eligible constructor also defines a set of instance fields directly
- // initialized with parameter values, called field initialization mapping below
+ // -> the instance is initialized with 'eligible' constructor (see comments in
+ // CodeRewriter::identifyClassInlinerEligibility(...))
// -> has only 'eligible' uses, i.e:
- // * it is a receiver of a field read if the field is present in the
- // field initialization mapping
- // * it is a receiver of virtual or interface call with single target being
- // a method only reading fields in the current field initialization mapping
+ // * as a receiver of a field read/write for a field defined in same class
+ // as method.
+ // * as a receiver of virtual or interface call with single target being
+ // an eligible method according to identifyClassInlinerEligibility(...);
+ // NOTE: if method receiver is used as a return value, the method call
+ // should ignore return value
//
// - inline eligible 'new-instance' instructions, i.e:
- // -> force inline methods called on the instance (which may introduce additional
- // instance field reads, but only for fields present in the current field
- // initialization mapping)
- // -> replace instance field reads with appropriate values passed to the constructor
- // according to field initialization mapping
- // -> remove constructor call
+ // -> force inline methods called on the instance (including the initializer);
+ // (this may introduce additional instance field reads/writes on the receiver)
+ // -> replace instance field reads with appropriate values calculated based on
+ // fields writes
+ // -> remove the call to superclass initializer
+ // -> remove all field writes
// -> remove 'new-instance' instructions
//
// For example:
@@ -117,7 +124,6 @@
.map(Instruction::asNewInstance)
.collect(Collectors.toList());
- nextNewInstance:
for (NewInstance newInstance : newInstances) {
Value eligibleInstance = newInstance.outValue();
if (eligibleInstance == null) {
@@ -129,177 +135,281 @@
continue;
}
- // No Phi users.
- if (eligibleInstance.numberOfPhiUsers() > 0) {
+ Map<InvokeMethodWithReceiver, InliningInfo> methodCalls = checkInstanceUsersEligibility(
+ appInfo, method, isProcessedConcurrently, newInstance, eligibleInstance, eligibleClass);
+ if (methodCalls == null) {
continue;
}
- Set<Instruction> uniqueUsers = eligibleInstance.uniqueUsers();
+ if (getTotalEstimatedMethodSize(methodCalls) >= totalMethodInstructionLimit) {
+ continue;
+ }
- // Find an initializer invocation.
- InvokeDirect eligibleInitCall = null;
- Map<DexField, Integer> mappings = null;
- for (Instruction user : uniqueUsers) {
- if (!user.isInvokeDirect()) {
+ // Inline the class instance.
+ forceInlineAllMethodInvocations(inliner, methodCalls);
+ removeSuperClassInitializerAndFieldReads(code, newInstance, eligibleInstance);
+ removeFieldWrites(eligibleInstance, eligibleClass);
+ removeInstruction(newInstance);
+
+ // Restore normality.
+ code.removeAllTrivialPhis();
+ assert code.isConsistentSSA();
+ }
+ }
+
+ private Map<InvokeMethodWithReceiver, InliningInfo> checkInstanceUsersEligibility(
+ AppInfoWithSubtyping appInfo, DexEncodedMethod method,
+ Predicate<DexEncodedMethod> isProcessedConcurrently,
+ NewInstance newInstanceInsn, Value receiver, DexType clazz) {
+
+ // No Phi users.
+ if (receiver.numberOfPhiUsers() > 0) {
+ return null; // Not eligible.
+ }
+
+ Map<InvokeMethodWithReceiver, InliningInfo> methodCalls = new IdentityHashMap<>();
+
+ for (Instruction user : receiver.uniqueUsers()) {
+ // Field read/write.
+ if (user.isInstanceGet() ||
+ (user.isInstancePut() && user.asInstancePut().value() != receiver)) {
+ if (user.asFieldInstruction().getField().clazz == newInstanceInsn.clazz) {
+ // Eligible field read or write. Note: as long as we consider only classes eligible
+ // if they directly extend java.lang.Object we don't need to check if the field
+ // really exists in the class.
continue;
}
+ return null; // Not eligible.
+ }
- InvokeDirect candidate = user.asInvokeDirect();
- DexMethod candidateInit = candidate.getInvokedMethod();
- if (factory.isConstructor(candidateInit) &&
- candidate.inValues().lastIndexOf(eligibleInstance) == 0) {
+ // Eligible constructor call.
+ if (user.isInvokeDirect()) {
+ InliningInfo inliningInfo = isEligibleConstructorCall(appInfo, method,
+ user.asInvokeDirect(), receiver, clazz, isProcessedConcurrently);
+ if (inliningInfo != null) {
+ methodCalls.put(user.asInvokeDirect(), inliningInfo);
+ continue;
+ }
+ return null; // Not eligible.
+ }
- if (candidateInit.holder != eligibleClass) {
- // Inlined constructor call? We won't get field initialization mapping in this
- // case, but since we only support eligible classes extending java.lang.Object,
- // it's safe to assume an empty mapping.
- if (candidateInit.holder == factory.objectType) {
- mappings = Collections.emptyMap();
- }
+ // Eligible virtual method call.
+ if (user.isInvokeVirtual() || user.isInvokeInterface()) {
+ InliningInfo inliningInfo = isEligibleMethodCall(
+ appInfo, method, user.asInvokeMethodWithReceiver(),
+ receiver, clazz, isProcessedConcurrently);
+ if (inliningInfo != null) {
+ methodCalls.put(user.asInvokeMethodWithReceiver(), inliningInfo);
+ continue;
+ }
+ return null; // Not eligible.
+ }
- } else {
- // Is it a call to an *eligible* constructor?
- mappings = getConstructorFieldMappings(appInfo, candidateInit, isProcessedConcurrently);
- }
+ return null; // Not eligible.
+ }
+ return methodCalls;
+ }
- eligibleInitCall = candidate;
+ // Remove call to superclass initializer, replace field reads with appropriate
+ // values, insert phis when needed.
+ private void removeSuperClassInitializerAndFieldReads(
+ IRCode code, NewInstance newInstance, Value eligibleInstance) {
+ Map<DexField, FieldValueHelper> fieldHelpers = new IdentityHashMap<>();
+ for (Instruction user : eligibleInstance.uniqueUsers()) {
+ // Remove the call to superclass constructor.
+ if (user.isInvokeDirect() &&
+ user.asInvokeDirect().getInvokedMethod() == factory.objectMethods.constructor) {
+ removeInstruction(user);
+ continue;
+ }
+
+ if (user.isInstanceGet()) {
+ // Replace a field read with appropriate value.
+ replaceFieldRead(code, newInstance, user.asInstanceGet(), fieldHelpers);
+ continue;
+ }
+
+ if (user.isInstancePut()) {
+ // Skip in this iteration since these instructions are needed to
+ // properly calculate what value should field reads be replaced with.
+ continue;
+ }
+
+ throw new Unreachable("Unexpected usage left after method inlining: " + user);
+ }
+ }
+
+ private void removeFieldWrites(Value receiver, DexType clazz) {
+ for (Instruction user : receiver.uniqueUsers()) {
+ if (!user.isInstancePut()) {
+ throw new Unreachable("Unexpected usage left after field reads removed: " + user);
+ }
+ if (user.asInstancePut().getField().clazz != clazz) {
+ throw new Unreachable("Unexpected field write left after field reads removed: " + user);
+ }
+ removeInstruction(user);
+ }
+ }
+
+ private int getTotalEstimatedMethodSize(Map<InvokeMethodWithReceiver, InliningInfo> methodCalls) {
+ int totalSize = 0;
+ for (InliningInfo info : methodCalls.values()) {
+ totalSize += info.target.getCode().estimatedSizeForInlining();
+ }
+ return totalSize;
+ }
+
+ private void replaceFieldRead(IRCode code, NewInstance newInstance,
+ InstanceGet fieldRead, Map<DexField, FieldValueHelper> fieldHelpers) {
+
+ Value value = fieldRead.outValue();
+ if (value != null) {
+ FieldValueHelper helper = fieldHelpers.computeIfAbsent(
+ fieldRead.getField(), field -> new FieldValueHelper(field, code, newInstance));
+ Value newValue = helper.getValueForFieldRead(fieldRead.getBlock(), fieldRead);
+ value.replaceUsers(newValue);
+ for (FieldValueHelper fieldValueHelper : fieldHelpers.values()) {
+ fieldValueHelper.replaceValue(value, newValue);
+ }
+ assert value.numberOfAllUsers() == 0;
+ }
+ removeInstruction(fieldRead);
+ }
+
+ // Describes and caches what values are supposed to be used instead of field reads.
+ private static final class FieldValueHelper {
+ final DexField field;
+ final IRCode code;
+ final NewInstance newInstance;
+
+ private Value defaultValue = null;
+ private final Map<BasicBlock, Value> ins = new IdentityHashMap<>();
+ private final Map<BasicBlock, Value> outs = new IdentityHashMap<>();
+
+ private FieldValueHelper(DexField field, IRCode code, NewInstance newInstance) {
+ this.field = field;
+ this.code = code;
+ this.newInstance = newInstance;
+ }
+
+ void replaceValue(Value oldValue, Value newValue) {
+ for (Entry<BasicBlock, Value> entry : ins.entrySet()) {
+ if (entry.getValue() == oldValue) {
+ entry.setValue(newValue);
+ }
+ }
+ for (Entry<BasicBlock, Value> entry : outs.entrySet()) {
+ if (entry.getValue() == oldValue) {
+ entry.setValue(newValue);
+ }
+ }
+ }
+
+ Value getValueForFieldRead(BasicBlock block, Instruction valueUser) {
+ assert valueUser != null;
+ Value value = getValueDefinedInTheBlock(block, valueUser);
+ return value != null ? value : getOrCreateInValue(block);
+ }
+
+ private Value getOrCreateOutValue(BasicBlock block) {
+ Value value = outs.get(block);
+ if (value != null) {
+ return value;
+ }
+
+ value = getValueDefinedInTheBlock(block, null);
+ if (value == null) {
+ // No value defined in the block.
+ value = getOrCreateInValue(block);
+ }
+
+ assert value != null;
+ outs.put(block, value);
+ return value;
+ }
+
+ private Value getOrCreateInValue(BasicBlock block) {
+ Value value = ins.get(block);
+ if (value != null) {
+ return value;
+ }
+
+ List<BasicBlock> predecessors = block.getPredecessors();
+ if (predecessors.size() == 1) {
+ value = getOrCreateOutValue(predecessors.get(0));
+ ins.put(block, value);
+ } else {
+ // Create phi, add it to the block, cache in ins map for future use.
+ Phi phi = new Phi(code.valueNumberGenerator.next(),
+ block, ValueType.fromDexType(field.type), null);
+ ins.put(block, phi);
+
+ List<Value> operands = new ArrayList<>();
+ for (BasicBlock predecessor : block.getPredecessors()) {
+ operands.add(getOrCreateOutValue(predecessor));
+ }
+ // Add phi, but don't remove trivial phis; since we cache the phi
+ // we just created for future use we should delay removing trivial
+ // phis until we are done with replacing fields reads.
+ phi.addOperands(operands, false);
+ value = phi;
+ }
+
+ assert value != null;
+ return value;
+ }
+
+ private Value getValueDefinedInTheBlock(BasicBlock block, Instruction stopAt) {
+ InstructionListIterator iterator = stopAt == null ?
+ block.listIterator(block.getInstructions().size()) : block.listIterator(stopAt);
+
+ Instruction valueProducingInsn = null;
+ while (iterator.hasPrevious()) {
+ Instruction instruction = iterator.previous();
+ assert instruction != null;
+
+ if (instruction == newInstance ||
+ (instruction.isInstancePut() &&
+ instruction.asInstancePut().getField() == field &&
+ instruction.asInstancePut().object() == newInstance.outValue())) {
+ valueProducingInsn = instruction;
break;
}
}
- if (mappings == null) {
- continue;
+ if (valueProducingInsn == null) {
+ return null;
+ }
+ if (valueProducingInsn.isInstancePut()) {
+ return valueProducingInsn.asInstancePut().value();
}
- // Check all regular users.
- Map<InvokeMethodWithReceiver, InliningInfo> methodCalls = new IdentityHashMap<>();
-
- for (Instruction user : uniqueUsers) {
- if (user == eligibleInitCall) {
- continue /* next user */;
- }
-
- if (user.isInstanceGet()) {
- InstanceGet instanceGet = user.asInstanceGet();
- if (mappings.containsKey(instanceGet.getField())) {
- continue /* next user */;
- }
-
- // Not replaceable field read.
- continue nextNewInstance;
- }
-
- if (user.isInvokeVirtual() || user.isInvokeInterface()) {
- InvokeMethodWithReceiver invoke = user.asInvokeMethodWithReceiver();
- if (invoke.inValues().lastIndexOf(eligibleInstance) > 0) {
- continue nextNewInstance; // Instance must only be passes as a receiver.
- }
-
- DexEncodedMethod singleTarget =
- findSingleTarget(appInfo, invoke, eligibleClass);
- if (singleTarget == null) {
- continue nextNewInstance;
- }
- if (isProcessedConcurrently.test(singleTarget)) {
- continue nextNewInstance;
- }
- if (method == singleTarget) {
- continue nextNewInstance; // Don't inline itself.
- }
-
- if (!singleTarget.getOptimizationInfo()
- .isReceiverOnlyUsedForReadingFields(mappings.keySet())) {
- continue nextNewInstance; // Target must be trivial.
- }
-
- if (!singleTarget.isInliningCandidate(method, Reason.SIMPLE, appInfo)) {
- // We won't be able to inline it here.
-
- // Note that there may be some false negatives here since the method may
- // reference private fields of its class which are supposed to be replaced
- // with arguments after inlining. We should try and improve it later.
-
- // Using -allowaccessmodification mitigates this.
- continue nextNewInstance;
- }
-
- methodCalls.put(invoke, new InliningInfo(singleTarget, eligibleClass));
- continue /* next user */;
- }
-
- continue nextNewInstance; // Unsupported user.
+ assert newInstance == valueProducingInsn;
+ if (defaultValue == null) {
+ // If we met newInstance it means that default value is supposed to be used.
+ defaultValue = code.createValue(ValueType.fromDexType(field.type));
+ ConstNumber defaultValueInsn = new ConstNumber(defaultValue, 0);
+ defaultValueInsn.setPosition(newInstance.getPosition());
+ LinkedList<Instruction> instructions = block.getInstructions();
+ instructions.add(instructions.indexOf(newInstance) + 1, defaultValueInsn);
+ defaultValueInsn.setBlock(block);
}
-
- // Force-inline of method invocation if any.
- inlineAllCalls(inliner, methodCalls);
- assert assertOnlyConstructorAndFieldReads(eligibleInstance, eligibleInitCall, mappings);
-
- // Replace all field reads with arguments passed to the constructor.
- patchFieldReads(eligibleInstance, eligibleInitCall, mappings);
- assert assertOnlyConstructor(eligibleInstance, eligibleInitCall);
-
- // Remove constructor call and new-instance instructions.
- removeInstruction(eligibleInitCall);
- removeInstruction(newInstance);
- code.removeAllTrivialPhis();
+ return defaultValue;
}
}
- private void inlineAllCalls(
+ private void forceInlineAllMethodInvocations(
InlinerAction inliner, Map<InvokeMethodWithReceiver, InliningInfo> methodCalls) {
if (!methodCalls.isEmpty()) {
inliner.inline(methodCalls);
}
}
- private void patchFieldReads(
- Value instance, InvokeDirect invokeMethod, Map<DexField, Integer> mappings) {
- for (Instruction user : instance.uniqueUsers()) {
- if (!user.isInstanceGet()) {
- continue;
- }
- InstanceGet fieldRead = user.asInstanceGet();
-
- // Replace the field read with
- assert mappings.containsKey(fieldRead.getField());
- Value arg = invokeMethod.inValues().get(1 + mappings.get(fieldRead.getField()));
- assert arg != null;
- Value value = fieldRead.outValue();
- if (value != null) {
- value.replaceUsers(arg);
- assert value.numberOfAllUsers() == 0;
- }
-
- // Remove instruction.
- removeInstruction(fieldRead);
- }
- }
-
private void removeInstruction(Instruction instruction) {
instruction.inValues().forEach(v -> v.removeUser(instruction));
instruction.getBlock().removeInstruction(instruction);
}
- private boolean assertOnlyConstructorAndFieldReads(
- Value instance, InvokeDirect invokeMethod, Map<DexField, Integer> mappings) {
- for (Instruction user : instance.uniqueUsers()) {
- if (user != invokeMethod &&
- !(user.isInstanceGet() && mappings.containsKey(user.asFieldInstruction().getField()))) {
- throw new Unreachable("Not all calls are inlined!");
- }
- }
- return true;
- }
-
- private boolean assertOnlyConstructor(Value instance, InvokeDirect invokeMethod) {
- for (Instruction user : instance.uniqueUsers()) {
- if (user != invokeMethod) {
- throw new Unreachable("Not all field reads are substituted!");
- }
- }
- return true;
- }
-
private DexEncodedMethod findSingleTarget(
AppInfo appInfo, InvokeMethodWithReceiver invoke, DexType instanceType) {
@@ -322,24 +432,94 @@
return null;
}
- private Map<DexField, Integer> getConstructorFieldMappings(
- AppInfo appInfo, DexMethod init, Predicate<DexEncodedMethod> isProcessedConcurrently) {
- assert isClassEligible(appInfo, init.holder);
+ private InliningInfo isEligibleConstructorCall(
+ AppInfoWithSubtyping appInfo,
+ DexEncodedMethod method,
+ InvokeDirect initInvoke,
+ Value receiver,
+ DexType inlinedClass,
+ Predicate<DexEncodedMethod> isProcessedConcurrently) {
+
+ // Must be a constructor of the exact same class.
+ DexMethod init = initInvoke.getInvokedMethod();
+ if (!factory.isConstructor(init)) {
+ return null;
+ }
+ // Must be a constructor called on the receiver.
+ if (initInvoke.inValues().lastIndexOf(receiver) != 0) {
+ return null;
+ }
+
+ assert init.holder == inlinedClass
+ : "Inlined constructor? [invoke: " + initInvoke +
+ ", expected class: " + inlinedClass + "]";
DexEncodedMethod definition = appInfo.definitionFor(init);
- if (definition == null) {
- return NO_MAPPING;
+ if (definition == null || isProcessedConcurrently.test(definition)) {
+ return null;
}
- if (isProcessedConcurrently.test(definition)) {
- return NO_MAPPING;
+ if (!definition.isInliningCandidate(method, Reason.SIMPLE, appInfo)) {
+ // We won't be able to inline it here.
+
+ // Note that there may be some false negatives here since the method may
+ // reference private fields of its class which are supposed to be replaced
+ // with arguments after inlining. We should try and improve it later.
+
+ // Using -allowaccessmodification mitigates this.
+ return null;
}
- if (definition.accessFlags.isAbstract() || definition.accessFlags.isNative()) {
- return NO_MAPPING;
+ return definition.getOptimizationInfo().getClassInlinerEligibility() != null
+ ? new InliningInfo(definition, inlinedClass) : null;
+ }
+
+ private InliningInfo isEligibleMethodCall(
+ AppInfoWithSubtyping appInfo,
+ DexEncodedMethod method,
+ InvokeMethodWithReceiver invoke,
+ Value receiver,
+ DexType inlinedClass,
+ Predicate<DexEncodedMethod> isProcessedConcurrently) {
+
+ if (invoke.inValues().lastIndexOf(receiver) > 0) {
+ return null; // Instance passed as an argument.
}
- return definition.getOptimizationInfo().onlyInitializesFieldsWithNoOtherSideEffects();
+ DexEncodedMethod singleTarget =
+ findSingleTarget(appInfo, invoke, inlinedClass);
+ if (singleTarget == null || isProcessedConcurrently.test(singleTarget)) {
+ return null;
+ }
+ if (method == singleTarget) {
+ return null; // Don't inline itself.
+ }
+
+ ClassInlinerEligibility eligibility =
+ singleTarget.getOptimizationInfo().getClassInlinerEligibility();
+ if (eligibility == null) {
+ return null;
+ }
+
+ // If the method returns receiver and the return value is actually
+ // used in the code the method is not eligible.
+ if (eligibility.returnsReceiver &&
+ invoke.outValue() != null && invoke.outValue().numberOfAllUsers() > 0) {
+ return null;
+ }
+
+ if (!singleTarget.isInliningCandidate(method, Reason.SIMPLE, appInfo)) {
+ // We won't be able to inline it here.
+
+ // Note that there may be some false negatives here since the method may
+ // reference private fields of its class which are supposed to be replaced
+ // with arguments after inlining. We should try and improve it later.
+
+ // Using -allowaccessmodification mitigates this.
+ return null;
+ }
+
+ return new InliningInfo(singleTarget, inlinedClass);
}
private boolean isClassEligible(AppInfo appInfo, DexType clazz) {
@@ -354,7 +534,7 @@
}
// Class is eligible for this optimization. Eligibility implementation:
- // - not an abstract or interface
+ // - is not an abstract class or interface
// - directly extends java.lang.Object
// - does not declare finalizer
// - does not trigger any static initializers
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 0976087..d2c7701 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
@@ -1819,6 +1819,10 @@
// we allow ourselves to look at monitor spill candidates as well. Registers holding objects
// used as monitors should not be spilled if we can avoid it. Spilling them can lead
// to Art lock verification issues.
+ // Also, at this point we still don't allow splitting any string new-instance instructions
+ // that have been explicitly blocked. Doing so could lead to a behavioral bug on some ART
+ // runtimes (b/80118070). To remove this restriction, we would need to know when the call to
+ // <init> has definitely happened, and would be safe to split the value after that point.
if (candidate == REGISTER_CANDIDATE_NOT_FOUND) {
candidate = getLargestValidCandidate(
unhandledInterval, registerConstraint, needsRegisterPair, usePositions, Type.MONITOR);
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 c8e6e4c..bf65310 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
@@ -522,6 +522,13 @@
return usedInMonitorOperations;
}
+ public boolean isNewStringInstanceDisallowingSpilling() {
+ // Due to b/80118070 some String new-instances must not be spilled.
+ return value.definition != null
+ && value.definition.isNewInstance()
+ && !value.definition.asNewInstance().isSpillingAllowed();
+ }
+
public int numberOfUsesWithConstraint() {
int count = 0;
for (LiveIntervalsUse use : getUses()) {
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/RegisterPositions.java b/src/main/java/com/android/tools/r8/ir/regalloc/RegisterPositions.java
index 459114b..eff8bea 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/RegisterPositions.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/RegisterPositions.java
@@ -23,6 +23,7 @@
private int[] backing;
private final BitSet registerHoldsConstant;
private final BitSet registerHoldsMonitor;
+ private final BitSet registerHoldsNewStringInstanceDisallowingSpilling;
public RegisterPositions(int limit) {
this.limit = limit;
@@ -32,6 +33,7 @@
}
registerHoldsConstant = new BitSet(limit);
registerHoldsMonitor = new BitSet(limit);
+ registerHoldsNewStringInstanceDisallowingSpilling = new BitSet(limit);
}
public boolean hasType(int index, Type type) {
@@ -41,7 +43,9 @@
case CONST_NUMBER:
return holdsConstant(index);
case OTHER:
- return !holdsMonitor(index) && !holdsConstant(index);
+ return !holdsMonitor(index)
+ && !holdsConstant(index)
+ && !holdsNewStringInstanceDisallowingSpilling(index);
case ANY:
return true;
default:
@@ -55,6 +59,10 @@
private boolean holdsMonitor(int index) { return registerHoldsMonitor.get(index); }
+ private boolean holdsNewStringInstanceDisallowingSpilling(int index) {
+ return registerHoldsNewStringInstanceDisallowingSpilling.get(index);
+ }
+
public void set(int index, int value) {
if (index >= backing.length) {
grow(index + 1);
@@ -66,6 +74,8 @@
set(index, value);
registerHoldsConstant.set(index, intervals.isConstantNumberInterval());
registerHoldsMonitor.set(index, intervals.usedInMonitorOperation());
+ registerHoldsNewStringInstanceDisallowingSpilling.set(
+ index, intervals.isNewStringInstanceDisallowingSpilling());
}
public int get(int index) {
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 68796ac..5f0ec45 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -101,6 +101,7 @@
public boolean enableInlining =
!Version.isDev() || System.getProperty("com.android.tools.r8.disableinlining") == null;
public boolean enableClassInlining = true;
+ public int classInliningInstructionLimit = 50;
public int inliningInstructionLimit = 5;
public boolean enableSwitchMapRemoval = true;
public final OutlineOptions outline = new OutlineOptions();
@@ -620,4 +621,12 @@
public boolean canHaveDex2OatLinkedListBug() {
return minApiLevel < AndroidApiLevel.N.getLevel();
}
+
+ // Art 7.0.0 and later Art JIT may perform an invalid optimization if a string new-instance does
+ // not flow directly to the init call.
+ //
+ // See b/78493232 and b/80118070.
+ public boolean canHaveArtStringNewInitBug() {
+ return minApiLevel <= AndroidApiLevel.P.getLevel();
+ }
}
diff --git a/src/test/java/com/android/tools/r8/AsmTestBase.java b/src/test/java/com/android/tools/r8/AsmTestBase.java
index 9cc2b48..8038c98 100644
--- a/src/test/java/com/android/tools/r8/AsmTestBase.java
+++ b/src/test/java/com/android/tools/r8/AsmTestBase.java
@@ -64,6 +64,10 @@
Assert.assertEquals(javaResult.stdout, d8Result.stdout);
Assert.assertEquals(javaResult.stdout, r8Result.stdout);
Assert.assertEquals(javaResult.stdout, r8ShakenResult.stdout);
+ Assert.assertEquals(0, javaResult.exitCode);
+ Assert.assertEquals(0, d8Result.exitCode);
+ Assert.assertEquals(0, r8Result.exitCode);
+ Assert.assertEquals(0, r8ShakenResult.exitCode);
}
protected void ensureR8FailsWithCompilationError(String main, byte[]... classes)
diff --git a/src/test/java/com/android/tools/r8/D8RunExamplesAndroidOTest.java b/src/test/java/com/android/tools/r8/D8RunExamplesAndroidOTest.java
index ede7e53..e01373c 100644
--- a/src/test/java/com/android/tools/r8/D8RunExamplesAndroidOTest.java
+++ b/src/test/java/com/android/tools/r8/D8RunExamplesAndroidOTest.java
@@ -15,6 +15,7 @@
import org.hamcrest.core.IsInstanceOf;
import org.hamcrest.core.StringContains;
import org.junit.Assert;
+import org.junit.Assume;
import org.junit.Test;
import org.junit.internal.matchers.ThrowableMessageMatcher;
@@ -156,16 +157,20 @@
// TODO check compilation warnings are correctly reported
// Missing interface B is causing the wrong code to be executed.
- if (ToolHelper.artSupported()) {
- thrown.expect(AssertionError.class);
- execute(
- "testMissingInterfaceDesugared2AndroidK",
- "desugaringwithmissingclasstest2.Main",
- new Path[] {
- lib1.getInputJar(), lib2.getInputJar(), lib3.getInputJar(), test.getInputJar()
- },
- new Path[] {lib1Dex, lib2Dex, lib3Dex, testDex});
+ if (!ToolHelper.artSupported() && !ToolHelper.compareAgaintsGoldenFiles()) {
+ return;
}
+ if (ToolHelper.artSupported() && !ToolHelper.compareAgaintsGoldenFiles()) {
+ thrown.expect(AssertionError.class);
+ }
+ execute(
+ "testMissingInterfaceDesugared2AndroidK",
+ "desugaringwithmissingclasstest2.Main",
+ new Path[] {
+ lib1.getInputJar(), lib2.getInputJar(), lib3.getInputJar(), test.getInputJar()
+ },
+ new Path[] {lib1Dex, lib2Dex, lib3Dex, testDex});
+
}
@Test
@@ -256,17 +261,20 @@
Path testDex = test.build();
// TODO check compilation warnings are correctly reported
+ Assume.assumeTrue(ToolHelper.artSupported() || ToolHelper.compareAgaintsGoldenFiles());
+
// Missing interface B is causing the wrong method to be executed.
- if (ToolHelper.artSupported()) {
+ if (ToolHelper.artSupported() && !ToolHelper.compareAgaintsGoldenFiles()) {
thrown.expect(AssertionError.class);
- execute(
- "testCallToMissingSuperInterfaceDesugaredAndroidK",
- "desugaringwithmissingclasstest3.Main",
- new Path[] {
- lib1.getInputJar(), lib2.getInputJar(), lib3.getInputJar(), test.getInputJar()
- },
- new Path[] {lib1Dex, lib2Dex, lib3Dex, testDex});
}
+ execute(
+ "testCallToMissingSuperInterfaceDesugaredAndroidK",
+ "desugaringwithmissingclasstest3.Main",
+ new Path[] {
+ lib1.getInputJar(), lib2.getInputJar(), lib3.getInputJar(), test.getInputJar()
+ },
+ new Path[] {lib1Dex, lib2Dex, lib3Dex, testDex});
+
}
@Test
diff --git a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
index ca6feed..2032866 100644
--- a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.ToolHelper.DexVm;
import com.android.tools.r8.ToolHelper.DexVm.Kind;
import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.dex.Marker.Tool;
import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.shaking.ProguardRuleParserException;
@@ -22,6 +23,7 @@
import com.android.tools.r8.utils.FileUtils;
import com.android.tools.r8.utils.InternalOptions.LineNumberOptimization;
import com.android.tools.r8.utils.ListUtils;
+import com.android.tools.r8.utils.TestDescriptionWatcher;
import com.google.common.base.Charsets;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
@@ -1010,8 +1012,7 @@
// Test depends on exception produced for missing method or similar cases, but
// after class inlining removes class instantiations and references the exception
// is not produced.
- "042-new-instance",
- "075-verification-error"
+ "435-new-instance"
);
private static List<String> hasMissingClasses = ImmutableList.of(
@@ -1363,6 +1364,9 @@
@Rule
public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
+ @Rule
+ public TestDescriptionWatcher watcher = new TestDescriptionWatcher();
+
public R8RunArtTestsTest(String name, DexTool toolchain) {
this.name = name;
this.toolchain = toolchain;
@@ -1554,6 +1558,8 @@
if (disableClassInlining) {
options.enableClassInlining = false;
}
+ // Make sure we don't depend on this settings.
+ options.classInliningInstructionLimit = 10000;
options.lineNumberOptimization = LineNumberOptimization.OFF;
// Some tests actually rely on missing classes for what they test.
options.ignoreMissingClasses = hasMissingClasses;
@@ -1768,7 +1774,7 @@
specification.disableInlining, specification.disableClassInlining,
specification.hasMissingClasses);
- if (!ToolHelper.artSupported()) {
+ if (!ToolHelper.artSupported() && !ToolHelper.dealsWithGoldenFiles()) {
return;
}
@@ -1789,9 +1795,11 @@
boolean compileOnly = System.getProperty("jctf_compile_only", "0").equals("1");
if (compileOnly || specification.skipArt) {
- // verify dex code instead of running it
- Path oatFile = temp.getRoot().toPath().resolve("all.oat");
- ToolHelper.runDex2Oat(processedFile.toPath(), oatFile);
+ if (ToolHelper.isDex2OatSupported()) {
+ // verify dex code instead of running it
+ Path oatFile = temp.getRoot().toPath().resolve("all.oat");
+ ToolHelper.runDex2Oat(processedFile.toPath(), oatFile);
+ }
return;
}
@@ -1963,7 +1971,7 @@
specification.hasMissingClasses);
}
- if (!specification.skipArt && ToolHelper.artSupported()) {
+ if (!specification.skipArt && (ToolHelper.artSupported() || ToolHelper.dealsWithGoldenFiles())) {
File originalFile;
File processedFile;
diff --git a/src/test/java/com/android/tools/r8/R8RunExamplesCommon.java b/src/test/java/com/android/tools/r8/R8RunExamplesCommon.java
index 62dd4af..fe5ae72 100644
--- a/src/test/java/com/android/tools/r8/R8RunExamplesCommon.java
+++ b/src/test/java/com/android/tools/r8/R8RunExamplesCommon.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.utils.ExceptionUtils;
import com.android.tools.r8.utils.InternalOptions.LineNumberOptimization;
+import com.android.tools.r8.utils.TestDescriptionWatcher;
import java.io.IOException;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
@@ -21,6 +22,7 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
+import org.junit.Assume;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -69,6 +71,9 @@
@Rule
public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
+ @Rule
+ public TestDescriptionWatcher watcher = new TestDescriptionWatcher();
+
private final Input input;
private final CompilerUnderTest compiler;
private final CompilationMode mode;
@@ -183,9 +188,8 @@
@Test
public void outputIsIdentical() throws IOException, InterruptedException, ExecutionException {
- if (!ToolHelper.artSupported()) {
- return;
- }
+ Assume.assumeTrue(ToolHelper.artSupported() || ToolHelper.compareAgaintsGoldenFiles());
+
DexVm vm = ToolHelper.getDexVm();
diff --git a/src/test/java/com/android/tools/r8/R8RunSmaliTestsTest.java b/src/test/java/com/android/tools/r8/R8RunSmaliTestsTest.java
index fc373d2..0f3cadc 100644
--- a/src/test/java/com/android/tools/r8/R8RunSmaliTestsTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunSmaliTestsTest.java
@@ -8,6 +8,7 @@
import com.android.tools.r8.ToolHelper.DexVm;
import com.android.tools.r8.ToolHelper.DexVm.Version;
import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.TestDescriptionWatcher;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import java.nio.file.Path;
@@ -91,6 +92,9 @@
@Rule
public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
+ @Rule
+ public TestDescriptionWatcher watcher = new TestDescriptionWatcher();
+
@Parameters(name = "{0}")
public static Collection<String[]> data() {
return Arrays.asList(new String[][]{
diff --git a/src/test/java/com/android/tools/r8/RunExamplesAndroidNTest.java b/src/test/java/com/android/tools/r8/RunExamplesAndroidNTest.java
index bb8c686..83df845 100644
--- a/src/test/java/com/android/tools/r8/RunExamplesAndroidNTest.java
+++ b/src/test/java/com/android/tools/r8/RunExamplesAndroidNTest.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.OffOrAuto;
+import com.android.tools.r8.utils.TestDescriptionWatcher;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import java.nio.file.Path;
@@ -73,7 +74,7 @@
build(inputFile, out);
- if (!ToolHelper.artSupported()) {
+ if (!ToolHelper.artSupported() && !ToolHelper.dealsWithGoldenFiles()) {
return;
}
@@ -118,6 +119,8 @@
@Rule public ExpectedException thrown = ExpectedException.none();
+ @Rule public TestDescriptionWatcher watcher = new TestDescriptionWatcher();
+
@Test
public void staticInterfaceMethods() throws Throwable {
test("staticinterfacemethods", "interfacemethods", "StaticInterfaceMethods")
diff --git a/src/test/java/com/android/tools/r8/RunExamplesAndroidOTest.java b/src/test/java/com/android/tools/r8/RunExamplesAndroidOTest.java
index 9fdbf0a..2c6a72a 100644
--- a/src/test/java/com/android/tools/r8/RunExamplesAndroidOTest.java
+++ b/src/test/java/com/android/tools/r8/RunExamplesAndroidOTest.java
@@ -21,6 +21,7 @@
import com.android.tools.r8.utils.DexInspector.InvokeInstructionSubject;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.OffOrAuto;
+import com.android.tools.r8.utils.TestDescriptionWatcher;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.io.ByteStreams;
@@ -47,6 +48,7 @@
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.rules.TemporaryFolder;
+import org.junit.rules.TestWatcher;
public abstract class RunExamplesAndroidOTest
<B extends BaseCommand.Builder<? extends BaseCommand, B>> {
@@ -148,7 +150,7 @@
build(inputFile, out);
- if (!ToolHelper.artSupported()) {
+ if (!ToolHelper.artSupported() && !ToolHelper.dealsWithGoldenFiles()) {
return;
}
@@ -270,6 +272,9 @@
@Rule
public ExpectedException thrown = ExpectedException.none();
+ @Rule
+ public TestDescriptionWatcher watcher = new TestDescriptionWatcher();
+
boolean failsOn(Map<ToolHelper.DexVm.Version, List<String>> failsOn, String name) {
Version vmVersion = ToolHelper.getDexVm().getVersion();
return failsOn.containsKey(vmVersion)
@@ -553,14 +558,14 @@
throws IOException {
boolean expectedToFail = expectedToFail(testName);
- if (expectedToFail) {
+ if (expectedToFail && !ToolHelper.compareAgaintsGoldenFiles()) {
thrown.expect(Throwable.class);
}
String output = ToolHelper.runArtNoVerificationErrors(
Arrays.stream(dexes).map(path -> path.toString()).collect(Collectors.toList()),
qualifiedMainClass,
null);
- if (!expectedToFail && !skipRunningOnJvm(testName)) {
+ if (!expectedToFail && !skipRunningOnJvm(testName) && !ToolHelper.compareAgaintsGoldenFiles()) {
ToolHelper.ProcessResult javaResult =
ToolHelper.runJava(ImmutableList.copyOf(jars), qualifiedMainClass);
assertEquals("JVM run failed", javaResult.exitCode, 0);
diff --git a/src/test/java/com/android/tools/r8/RunExamplesAndroidPTest.java b/src/test/java/com/android/tools/r8/RunExamplesAndroidPTest.java
index bc898bd..182fe80 100644
--- a/src/test/java/com/android/tools/r8/RunExamplesAndroidPTest.java
+++ b/src/test/java/com/android/tools/r8/RunExamplesAndroidPTest.java
@@ -19,6 +19,7 @@
import com.android.tools.r8.utils.DexInspector.InstructionSubject;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.OffOrAuto;
+import com.android.tools.r8.utils.TestDescriptionWatcher;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.io.ByteStreams;
@@ -143,7 +144,7 @@
build(inputFile, out);
- if (!ToolHelper.artSupported()) {
+ if (!ToolHelper.artSupported() && !ToolHelper.dealsWithGoldenFiles()) {
return;
}
@@ -193,6 +194,9 @@
@Rule
public ExpectedException thrown = ExpectedException.none();
+ @Rule
+ public TestDescriptionWatcher watcher = new TestDescriptionWatcher();
+
boolean failsOn(Map<DexVm.Version, List<String>> failsOn, String name) {
DexVm.Version vmVersion = ToolHelper.getDexVm().getVersion();
return failsOn.containsKey(vmVersion)
diff --git a/src/test/java/com/android/tools/r8/RunExamplesJava9Test.java b/src/test/java/com/android/tools/r8/RunExamplesJava9Test.java
index 281b0da..284ed68 100644
--- a/src/test/java/com/android/tools/r8/RunExamplesJava9Test.java
+++ b/src/test/java/com/android/tools/r8/RunExamplesJava9Test.java
@@ -19,6 +19,7 @@
import com.android.tools.r8.utils.DexInspector.InstructionSubject;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.OffOrAuto;
+import com.android.tools.r8.utils.TestDescriptionWatcher;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.io.ByteStreams;
@@ -143,7 +144,7 @@
build(inputFile, out);
- if (!ToolHelper.artSupported()) {
+ if (!ToolHelper.artSupported() && !ToolHelper.dealsWithGoldenFiles()) {
return;
}
@@ -224,6 +225,9 @@
@Rule
public ExpectedException thrown = ExpectedException.none();
+ @Rule
+ public TestDescriptionWatcher watcher = new TestDescriptionWatcher();
+
boolean failsOn(Map<DexVm.Version, List<String>> failsOn, String name) {
DexVm.Version vmVersion = ToolHelper.getDexVm().getVersion();
return failsOn.containsKey(vmVersion)
diff --git a/src/test/java/com/android/tools/r8/TestBase.java b/src/test/java/com/android/tools/r8/TestBase.java
index 331af0a..add86c2 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -26,6 +26,7 @@
import com.android.tools.r8.utils.FileUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.PreloadedClassFileProvider;
+import com.android.tools.r8.utils.TestDescriptionWatcher;
import com.android.tools.r8.utils.ZipUtils;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
@@ -59,6 +60,9 @@
@Rule
public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
+ @Rule
+ public TestDescriptionWatcher watcher = new TestDescriptionWatcher();
+
/**
* Check if tests should also run Proguard when applicable.
*/
diff --git a/src/test/java/com/android/tools/r8/ToolHelper.java b/src/test/java/com/android/tools/r8/ToolHelper.java
index 0a4416a..000076e 100644
--- a/src/test/java/com/android/tools/r8/ToolHelper.java
+++ b/src/test/java/com/android/tools/r8/ToolHelper.java
@@ -23,18 +23,23 @@
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.AndroidAppConsumers;
import com.android.tools.r8.utils.DefaultDiagnosticsHandler;
+import com.android.tools.r8.utils.FileUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.Reporter;
import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.Timing;
+import com.android.tools.r8.utils.ZipUtils;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.io.ByteStreams;
import com.google.common.io.CharStreams;
+import com.google.gson.Gson;
import java.io.File;
+import java.io.FileReader;
+import java.io.FileWriter;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
@@ -702,6 +707,10 @@
return true;
}
+ public static boolean isDex2OatSupported() {
+ return !isWindows();
+ }
+
public static Path getClassPathForTests() {
return Paths.get(BUILD_DIR, "classes", "test");
}
@@ -1021,17 +1030,197 @@
return runArtRaw(files, mainClass, extras, null);
}
+ // Index used to name directory aimed at storing dex files and process result
+ // for one invokation of runArtRaw() in order to avoid conflicts in case of
+ // multiple calls within the same test.
+ private static int testOutputPathIndex = 0;
+
public static ProcessResult runArtRaw(List<String> files, String mainClass,
Consumer<ArtCommandBuilder> extras, DexVm version)
throws IOException {
- ArtCommandBuilder builder =
- version != null ? new ArtCommandBuilder(version) : new ArtCommandBuilder();
- files.forEach(builder::appendClasspath);
- builder.setMainClass(mainClass);
- if (extras != null) {
- extras.accept(builder);
+
+ ArtCommandBuilder builder =
+ version != null ? new ArtCommandBuilder(version) : new ArtCommandBuilder();
+ files.forEach(builder::appendClasspath);
+ builder.setMainClass(mainClass);
+ if (extras != null) {
+ extras.accept(builder);
+ }
+
+ ProcessResult processResult = null;
+
+ // Whenever we start a new test method we reset the index count.
+ String reset_output_index = System.getProperty("reset_output_index");
+ if (reset_output_index != null) {
+ System.clearProperty("reset_output_index");
+ testOutputPathIndex = 0;
+ } else {
+ assert testOutputPathIndex >= 0;
+ testOutputPathIndex++;
+ }
+
+ String goldenFilesDirInProp = System.getProperty("use_golden_files_in");
+ if (goldenFilesDirInProp != null) {
+ File goldenFileDir = new File(goldenFilesDirInProp);
+ assert goldenFileDir.isDirectory();
+ processResult = compareAgainstGoldenFiles(
+ files.stream().map(f -> new File(f)).collect(Collectors.toList()), goldenFileDir);
+ if (processResult.exitCode == 0) {
+ processResult = readProcessResult(goldenFileDir);
+ }
+ } else {
+ processResult = runArtProcessRaw(builder);
+ }
+
+ String goldenFilesDirToProp = System.getProperty("generate_golden_files_to");
+ if (goldenFilesDirToProp != null) {
+ File goldenFileDir = new File(goldenFilesDirToProp);
+ assert goldenFileDir.isDirectory();
+ storeAsGoldenFiles(files.stream().map(f -> new File(f)).collect(Collectors.toList()),
+ goldenFileDir);
+ storeProcessResult(processResult, goldenFileDir);
+ }
+
+ return processResult;
+ }
+
+ private static Path findNonConflictingDestinationFilePath(Path testOutputPath) {
+ int index = 0;
+ Path destFilePath;
+ do {
+ destFilePath = Paths.get(testOutputPath.toString(),
+ "classes-" + String.format("%03d", index) + FileUtils.DEX_EXTENSION);
+ index++;
+ } while (destFilePath.toFile().exists());
+
+ return destFilePath;
+ }
+
+ private static Path getTestOutputPath(File destDir) throws IOException {
+ assert destDir.exists();
+ assert destDir.isDirectory();
+
+ String testClassName = System.getProperty("test_class_name");
+ String testName = System.getProperty("test_name");
+ String headSha1 = System.getProperty("test_git_HEAD_sha1");
+
+ assert testClassName != null;
+ assert testName != null;
+ assert headSha1 != null;
+
+ return Files.createDirectories(
+ Paths.get(destDir.getAbsolutePath(), headSha1, testClassName, testName + "-" + String
+ .format("%03d", testOutputPathIndex)));
+ }
+
+ private static List<File> unzipDexFilesArchive(File zipFile) throws IOException {
+ File tmpDir = Files.createTempDirectory("r8-test-").toFile();
+ tmpDir.deleteOnExit();
+ return ZipUtils.unzip(zipFile.getAbsolutePath(), tmpDir);
+ }
+
+ private static void storeAsGoldenFiles(List<File> files, File destDir) throws IOException {
+ Path testOutputPath = getTestOutputPath(destDir);
+
+ for (File f : files) {
+ Path filePath = f.toPath();
+ // TODO(jmhenaff): Check it's been produced by D8/R8?
+ List<File> testFiles = Collections.singletonList(f);
+ if (FileUtils.isArchive(filePath)) {
+ testFiles = unzipDexFilesArchive(f);
+ }
+ for (File testFile : testFiles) {
+ Path testFilePath = testFile.toPath();
+ if (FileUtils.isDexFile(testFilePath)) {
+ Path destFile = findNonConflictingDestinationFilePath(testOutputPath);
+ FileUtils.writeToFile(destFile, null, Files.readAllBytes(testFilePath));
+ }
+ }
}
- return runArtProcessRaw(builder);
+ }
+
+ @SuppressWarnings("unchecked")
+ private static void storeProcessResult(ProcessResult processResult, File dest)
+ throws IOException {
+ Gson gson = new Gson();
+ Path testOutputPath = getTestOutputPath(dest);
+ try (FileWriter fw = new FileWriter(new File(testOutputPath.toFile(), "processResult.json"))) {
+ gson.toJson(processResult, ProcessResult.class, fw);
+ }
+ }
+
+ private static ProcessResult readProcessResult(File dest) throws IOException {
+ File processResultFile = new File(getTestOutputPath(dest).toFile(), "processResult.json");
+ Gson gson = new Gson();
+ try (FileReader fr = new FileReader(processResultFile)) {
+ return gson.fromJson(fr, ProcessResult.class);
+ }
+ }
+
+ private static ProcessResult compareAgainstGoldenFiles(List<File> files, File destDir)
+ throws IOException {
+ Path testOutputPath = getTestOutputPath(destDir);
+
+ int index = 0;
+ String stdErr = "";
+ boolean passed = true;
+ for (File f : files) {
+ Path filePath = f.toPath();
+
+ List<File> testFiles = Collections.singletonList(f);
+ if (FileUtils.isArchive(filePath)) {
+ testFiles = unzipDexFilesArchive(f);
+ }
+
+ for (File testFile : testFiles) {
+ Path testFilePath = testFile.toPath();
+ // TODO(jmhenaff): Check it's been produced by D8/R8?
+ if (FileUtils.isDexFile(testFilePath)) {
+ File goldenFile = Paths.get(testOutputPath.toString(),
+ "classes-" + String.format("%03d", index) + FileUtils.DEX_EXTENSION).toFile();
+ if (!goldenFile.exists()) {
+ String fileDesc = "'" + testFile.getAbsolutePath() + "'";
+ if (FileUtils.isZipFile(filePath)) {
+ fileDesc += " (extracted from '" + f.getAbsolutePath() + "')";
+ }
+ stdErr += "Cannot find golden file '" + goldenFile.getAbsolutePath()
+ + "' to compare against test file " + fileDesc + "\n";
+ passed = false;
+ } else if (!com.google.common.io.Files.equal(testFile, goldenFile)) {
+ String fileDesc = "'" + testFile.getAbsolutePath() + "'";
+ if (FileUtils.isZipFile(filePath)) {
+ fileDesc += " (extracted from '" + f.getAbsolutePath() + "')";
+ }
+ stdErr +=
+ "File " + fileDesc + " differs from golden file '" + goldenFile.getAbsolutePath()
+ + "'\n";
+ passed = false;
+ }
+ index++;
+ }
+ }
+ }
+ // Ensure we processed as many files as there are golden files
+ File goldenFile = Paths.get(testOutputPath.toString(),
+ "classes-" + String.format("%03d", index) + FileUtils.DEX_EXTENSION).toFile();
+ if (goldenFile.exists()) {
+ stdErr += "Less dex files have been produced: there is at least one more golden file ('"
+ + goldenFile.getAbsolutePath() + "'\n";
+ passed = false;
+ }
+ return new ProcessResult(passed ? 0 : -1, "", stdErr);
+ }
+
+ public static boolean dealsWithGoldenFiles() {
+ return compareAgaintsGoldenFiles() || generateGoldenFiles();
+ }
+
+ public static boolean compareAgaintsGoldenFiles() {
+ return System.getProperty("use_golden_files_in") != null;
+ }
+
+ public static boolean generateGoldenFiles() {
+ return System.getProperty("generate_golden_files_to") != null;
}
public static ProcessResult runArtNoVerificationErrorsRaw(String file, String mainClass)
@@ -1086,7 +1275,7 @@
}
private static ProcessResult runArtProcessRaw(ArtCommandBuilder builder) throws IOException {
- Assume.assumeTrue(ToolHelper.artSupported());
+ Assume.assumeTrue(artSupported() || dealsWithGoldenFiles());
ProcessResult result;
if (builder.isForDevice()) {
try {
@@ -1161,7 +1350,7 @@
}
public static void runDex2Oat(Path file, Path outFile, DexVm vm) throws IOException {
- Assume.assumeTrue(ToolHelper.artSupported());
+ Assume.assumeTrue(ToolHelper.isDex2OatSupported());
// TODO(jmhenaff): find a way to run this on windows (push dex and run on device/emulator?)
Assume.assumeTrue(!ToolHelper.isWindows());
assert Files.exists(file);
diff --git a/src/test/java/com/android/tools/r8/VmTestRunner.java b/src/test/java/com/android/tools/r8/VmTestRunner.java
index 978cbe7..a3e76a4 100644
--- a/src/test/java/com/android/tools/r8/VmTestRunner.java
+++ b/src/test/java/com/android/tools/r8/VmTestRunner.java
@@ -68,7 +68,7 @@
@Override
protected boolean isIgnored(FrameworkMethod child) {
// Do not run VM tests if running VMs is not even supported.
- if (!ToolHelper.artSupported()) {
+ if (!ToolHelper.artSupported() && !ToolHelper.dealsWithGoldenFiles()) {
return true;
}
if (super.isIgnored(child)) {
diff --git a/src/test/java/com/android/tools/r8/cf/AlwaysNullGetItemTestRunner.java b/src/test/java/com/android/tools/r8/cf/AlwaysNullGetItemTestRunner.java
index a24e95a..7d88649 100644
--- a/src/test/java/com/android/tools/r8/cf/AlwaysNullGetItemTestRunner.java
+++ b/src/test/java/com/android/tools/r8/cf/AlwaysNullGetItemTestRunner.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.ToolHelper.ProcessResult;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.DescriptorUtils;
+import com.android.tools.r8.utils.TestDescriptionWatcher;
import java.nio.file.Path;
import org.junit.Rule;
import org.junit.Test;
@@ -25,6 +26,9 @@
@Rule
public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
+ @Rule
+ public TestDescriptionWatcher watcher = new TestDescriptionWatcher();
+
@Test
public void test() throws Exception {
ProcessResult runInput =
diff --git a/src/test/java/com/android/tools/r8/cf/MethodHandleTestRunner.java b/src/test/java/com/android/tools/r8/cf/MethodHandleTestRunner.java
index e43ffd7..6069b4c 100644
--- a/src/test/java/com/android/tools/r8/cf/MethodHandleTestRunner.java
+++ b/src/test/java/com/android/tools/r8/cf/MethodHandleTestRunner.java
@@ -31,6 +31,7 @@
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
+
@RunWith(Parameterized.class)
public class MethodHandleTestRunner extends TestBase {
static final Class<?> CLASS = MethodHandleTest.class;
@@ -143,8 +144,8 @@
if (runInput.exitCode != runDex.exitCode) {
System.out.println(runDex.stderr);
}
- assertEquals(runInput.stdout, runDex.stdout);
assertEquals(runInput.exitCode, runDex.exitCode);
+ assertEquals(runInput.stdout, runDex.stdout);
}
private void build(ProgramConsumer programConsumer) throws Exception {
diff --git a/src/test/java/com/android/tools/r8/debug/DebugTestBase.java b/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
index 25356ae..d41ee84 100644
--- a/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
+++ b/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
@@ -16,6 +16,7 @@
import com.android.tools.r8.naming.MemberNaming.Signature;
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.DescriptorUtils;
+import com.android.tools.r8.utils.TestDescriptionWatcher;
import com.google.common.collect.ImmutableList;
import it.unimi.dsi.fastutil.longs.LongArrayList;
import it.unimi.dsi.fastutil.longs.LongList;
@@ -113,6 +114,9 @@
@Rule
public TestName testName = new TestName();
+ @Rule
+ public TestDescriptionWatcher watcher = new TestDescriptionWatcher();
+
protected static final boolean supportsDefaultMethod(DebugTestConfig config) {
return config.isCfRuntime()
|| ToolHelper.getMinApiLevelForDexVm().getLevel() >= AndroidApiLevel.N.getLevel();
diff --git a/src/test/java/com/android/tools/r8/debuginfo/DebugInfoTestBase.java b/src/test/java/com/android/tools/r8/debuginfo/DebugInfoTestBase.java
index d0ad1df..6f85b18 100644
--- a/src/test/java/com/android/tools/r8/debuginfo/DebugInfoTestBase.java
+++ b/src/test/java/com/android/tools/r8/debuginfo/DebugInfoTestBase.java
@@ -15,6 +15,7 @@
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.AndroidAppConsumers;
+import com.android.tools.r8.utils.TestDescriptionWatcher;
import com.google.common.collect.ImmutableList;
import java.io.IOException;
import java.nio.file.Path;
@@ -31,6 +32,9 @@
@Rule
public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
+ @Rule
+ public TestDescriptionWatcher watcher = new TestDescriptionWatcher();
+
static AndroidApp compileWithD8(Class... classes) throws IOException, CompilationFailedException {
D8Command.Builder builder = D8Command.builder();
for (Class clazz : classes) {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/R8InliningRegressionTests.java b/src/test/java/com/android/tools/r8/ir/optimize/R8InliningRegressionTests.java
index 6760bdd..cb6df88 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/R8InliningRegressionTests.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/R8InliningRegressionTests.java
@@ -35,7 +35,7 @@
}
private void buildAndTest(String folder, String mainClass) throws Exception {
- Assume.assumeTrue(ToolHelper.artSupported());
+ Assume.assumeTrue(ToolHelper.artSupported() || ToolHelper.compareAgaintsGoldenFiles());
Path proguardRules = Paths.get(ToolHelper.EXAMPLES_DIR, folder, "keep-rules.txt");
Path jarFile =
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
index e9319d2..d36e6ad 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
@@ -16,6 +16,11 @@
import com.android.tools.r8.VmTestRunner;
import com.android.tools.r8.code.NewInstance;
import com.android.tools.r8.graph.DexCode;
+import com.android.tools.r8.ir.optimize.classinliner.builders.BuildersTestClass;
+import com.android.tools.r8.ir.optimize.classinliner.builders.ControlFlow;
+import com.android.tools.r8.ir.optimize.classinliner.builders.Pair;
+import com.android.tools.r8.ir.optimize.classinliner.builders.PairBuilder;
+import com.android.tools.r8.ir.optimize.classinliner.builders.Tuple;
import com.android.tools.r8.ir.optimize.classinliner.trivial.ClassWithFinal;
import com.android.tools.r8.ir.optimize.classinliner.trivial.CycleReferenceAB;
import com.android.tools.r8.ir.optimize.classinliner.trivial.CycleReferenceBA;
@@ -75,8 +80,7 @@
collectNewInstanceTypes(clazz, "testConstructorMapping1"));
assertEquals(
- Collections.singleton(
- "com.android.tools.r8.ir.optimize.classinliner.trivial.ReferencedFields"),
+ Collections.emptySet(),
collectNewInstanceTypes(clazz, "testConstructorMapping2"));
assertEquals(
@@ -120,6 +124,60 @@
assertFalse(inspector.clazz(CycleReferenceBA.class).isPresent());
}
+ @Test
+ public void testBuilders() throws Exception {
+ byte[][] classes = {
+ ToolHelper.getClassAsBytes(BuildersTestClass.class),
+ ToolHelper.getClassAsBytes(BuildersTestClass.Pos.class),
+ ToolHelper.getClassAsBytes(Tuple.class),
+ ToolHelper.getClassAsBytes(Pair.class),
+ ToolHelper.getClassAsBytes(PairBuilder.class),
+ ToolHelper.getClassAsBytes(ControlFlow.class),
+ };
+ String main = BuildersTestClass.class.getCanonicalName();
+ ProcessResult javaOutput = runOnJava(main, classes);
+ assertEquals(0, javaOutput.exitCode);
+
+ AndroidApp app = runR8(buildAndroidApp(classes), BuildersTestClass.class);
+
+ DexInspector inspector = new DexInspector(app);
+ ClassSubject clazz = inspector.clazz(BuildersTestClass.class);
+
+ assertEquals(
+ Sets.newHashSet(
+ "com.android.tools.r8.ir.optimize.classinliner.builders.Pair",
+ "java.lang.StringBuilder"),
+ collectNewInstanceTypes(clazz, "testSimpleBuilder"));
+
+ // Note that Pair created instances were also inlined in the following method since
+ // we use 'System.out.println(pX.toString())', if we used 'System.out.println(pX)'
+ // as in the above method, the instance of pair would be passed to println() which
+ // would make it not eligible for inlining.
+ assertEquals(
+ Collections.singleton("java.lang.StringBuilder"),
+ collectNewInstanceTypes(clazz, "testSimpleBuilderWithMultipleBuilds"));
+
+ assertFalse(inspector.clazz(PairBuilder.class).isPresent());
+
+ assertEquals(
+ Collections.singleton("java.lang.StringBuilder"),
+ collectNewInstanceTypes(clazz, "testBuilderConstructors"));
+
+ assertFalse(inspector.clazz(Tuple.class).isPresent());
+
+ assertEquals(
+ Collections.singleton("java.lang.StringBuilder"),
+ collectNewInstanceTypes(clazz, "testWithControlFlow"));
+
+ assertFalse(inspector.clazz(ControlFlow.class).isPresent());
+
+ assertEquals(
+ Collections.emptySet(),
+ collectNewInstanceTypes(clazz, "testWithMoreControlFlow"));
+
+ assertFalse(inspector.clazz(BuildersTestClass.Pos.class).isPresent());
+ }
+
private Set<String> collectNewInstanceTypes(
ClassSubject clazz, String methodName, String... params) {
assertNotNull(clazz);
@@ -135,7 +193,11 @@
+ "-dontobfuscate\n"
+ "-allowaccessmodification";
- AndroidApp compiled = compileWithR8(app, config, o -> o.enableClassInlining = true);
+ AndroidApp compiled = compileWithR8(app, config,
+ o -> {
+ o.enableClassInlining = true;
+ o.classInliningInstructionLimit = 10000;
+ });
// Materialize file for execution.
Path generatedDexFile = temp.getRoot().toPath().resolve("classes.jar");
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/BuildersTestClass.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/BuildersTestClass.java
new file mode 100644
index 0000000..1516e14
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/BuildersTestClass.java
@@ -0,0 +1,95 @@
+// 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.ir.optimize.classinliner.builders;
+
+public class BuildersTestClass {
+ private static int ID = 0;
+
+ private static int nextInt() {
+ return ID++;
+ }
+
+ private static String next() {
+ return Integer.toString(nextInt());
+ }
+
+ public static void main(String[] args) {
+ BuildersTestClass test = new BuildersTestClass();
+ test.testSimpleBuilder();
+ test.testSimpleBuilderWithMultipleBuilds();
+ test.testBuilderConstructors();
+ test.testWithControlFlow();
+ test.testWithMoreControlFlow();
+ }
+
+ private synchronized void testSimpleBuilder() {
+ System.out.println(
+ new PairBuilder<String, String>().setFirst("f-" + next()).build().toString());
+ testSimpleBuilder2();
+ testSimpleBuilder3();
+ }
+
+ private synchronized void testSimpleBuilder2() {
+ System.out.println(
+ new PairBuilder<String, String>().setSecond("s-" + next()).build().toString());
+ }
+
+ private synchronized void testSimpleBuilder3() {
+ System.out.println(new PairBuilder<String, String>()
+ .setFirst("f-" + next()).setSecond("s-" + next()).build().toString());
+ }
+
+ private synchronized void testSimpleBuilderWithMultipleBuilds() {
+ PairBuilder<String, String> builder = new PairBuilder<>();
+ Pair p1 = builder.build();
+ System.out.println(p1.toString());
+ builder.setFirst("f-" + next());
+ Pair p2 = builder.build();
+ System.out.println(p2.toString());
+ builder.setSecond("s-" + next());
+ Pair p3 = builder.build();
+ System.out.println(p3.toString());
+ }
+
+ private synchronized void testBuilderConstructors() {
+ System.out.println(new Tuple().toString());
+ System.out.println(new Tuple(true, (byte) 77, (short) 9977, '#', 42,
+ 987654321123456789L, -12.34f, 43210.98765, "s-" + next() + "-s").toString());
+ }
+
+ private synchronized void testWithControlFlow() {
+ ControlFlow flow = new ControlFlow(-1, 2, 7);
+ for (int k = 0; k < 25; k++) {
+ if (k % 3 == 0) {
+ flow.foo(k);
+ } else if (k % 3 == 1) {
+ flow.bar(nextInt(), nextInt(), nextInt(), nextInt());
+ }
+ }
+ System.out.println("flow = " + flow.toString());
+ }
+
+ private synchronized void testWithMoreControlFlow() {
+ String str = "1234567890";
+ Pos pos = new Pos();
+ while (pos.y < str.length()) {
+ pos.x = pos.y;
+ pos.y = pos.x;
+
+ if (str.charAt(pos.x) != '*') {
+ if ('0' <= str.charAt(pos.y) && str.charAt(pos.y) <= '9') {
+ while (pos.y < str.length() && '0' <= str.charAt(pos.y) && str.charAt(pos.y) <= '9') {
+ pos.y++;
+ }
+ }
+ }
+ }
+ }
+
+ public static class Pos {
+ public int x = 0;
+ public int y = 0;
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/ControlFlow.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/ControlFlow.java
new file mode 100644
index 0000000..7b95dc1
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/ControlFlow.java
@@ -0,0 +1,54 @@
+// 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.ir.optimize.classinliner.builders;
+
+public class ControlFlow {
+ int a;
+ int b;
+ int c = 1234;
+ int d;
+ String s = ">";
+
+ ControlFlow(int b, int c, int d) {
+ this.s += this.a++ + ">";
+ this.s += this.b + ">";
+ this.b = b;
+ this.s += this.b + ">";
+ this.s += this.c + ">";
+ this.c += c;
+ this.s += this.c + ">";
+ this.s += (this.d = d) + ">";
+ }
+
+ public void foo(int count) {
+ for (int i = 0; i < count; i++) {
+ switch (i % 4) {
+ case 0:
+ this.s += ++this.a + ">";
+ break;
+ case 1:
+ this.c += this.b;
+ this.s += this.c + ">";
+ break;
+ case 2:
+ this.d += this.d++ + this.c++ + this.b++ + this.a++;
+ this.s += this.d + ">";
+ break;
+ }
+ }
+ }
+
+ public void bar(int a, int b, int c, int d) {
+ this.a += a;
+ this.b += b;
+ this.c += c;
+ this.d += d;
+ }
+
+ @Override
+ public String toString() {
+ return s;
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/Pair.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/Pair.java
new file mode 100644
index 0000000..af45cab
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/Pair.java
@@ -0,0 +1,22 @@
+// 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.ir.optimize.classinliner.builders;
+
+public class Pair<F, S> {
+ public final F first;
+ public final S second;
+
+ public Pair(F first, S second) {
+ this.first = first;
+ this.second = second;
+ }
+
+ @Override
+ public String toString() {
+ return "Pair(" +
+ (first == null ? "<null>" : first) + ", " +
+ (second == null ? "<null>" : second) + ")";
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/PairBuilder.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/PairBuilder.java
new file mode 100644
index 0000000..0c80c53
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/PairBuilder.java
@@ -0,0 +1,29 @@
+// 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.ir.optimize.classinliner.builders;
+
+public class PairBuilder<F, S> {
+ public F first;
+ public S second = null;
+
+ public PairBuilder<F, S> setFirst(F first) {
+ System.out.println("[before] first = " + this.first);
+ this.first = first;
+ System.out.println("[after] first = " + this.first);
+ return this;
+ }
+
+ public PairBuilder<F, S> setSecond(S second) {
+ System.out.println("[before] second = " + this.second);
+ this.second = second;
+ System.out.println("[after] second = " + this.second);
+ return this;
+ }
+
+ public Pair<F, S> build() {
+ return new Pair<>(first, second);
+ }
+}
+
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/Tuple.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/Tuple.java
new file mode 100644
index 0000000..3a77cc2
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/Tuple.java
@@ -0,0 +1,39 @@
+// 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.ir.optimize.classinliner.builders;
+
+public class Tuple {
+ public boolean z;
+ public byte b;
+ public short s;
+ public char c;
+ public int i;
+ public long l;
+ public float f;
+ public double d;
+ public Object o;
+
+ Tuple() {
+ }
+
+ Tuple(boolean z, byte b, short s, char c, int i, long l, float f, double d, Object o) {
+ this.z = z;
+ this.b = b;
+ this.s = s;
+ this.c = c;
+ this.i = i;
+ this.l = l;
+ this.f = f;
+ this.d = d;
+ this.o = o;
+ }
+
+ @Override
+ public String toString() {
+ return "Tuple1(" + z + ", " + b + ", " + s + ", " +
+ ((int) c) + ", " + i + ", " + l + ", " + f + ", " +
+ d + ", " + (o == null ? "<null>" : o) + ")";
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/devirtualize/InvokeInterfaceToInvokeVirtualTest.java b/src/test/java/com/android/tools/r8/ir/optimize/devirtualize/InvokeInterfaceToInvokeVirtualTest.java
index 57d694d..86c15a2 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/devirtualize/InvokeInterfaceToInvokeVirtualTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/devirtualize/InvokeInterfaceToInvokeVirtualTest.java
@@ -87,4 +87,4 @@
assertEquals(0, artOutput.exitCode);
assertEquals(javaOutput.stdout.trim(), artOutput.stdout.trim());
}
-}
\ No newline at end of file
+}
diff --git a/src/test/java/com/android/tools/r8/jdwp/RunJdwpTests.java b/src/test/java/com/android/tools/r8/jdwp/RunJdwpTests.java
index bdedf43..3028525 100644
--- a/src/test/java/com/android/tools/r8/jdwp/RunJdwpTests.java
+++ b/src/test/java/com/android/tools/r8/jdwp/RunJdwpTests.java
@@ -16,6 +16,7 @@
import com.android.tools.r8.ToolHelper.DexVm.Version;
import com.android.tools.r8.ToolHelper.ProcessResult;
import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.TestDescriptionWatcher;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import java.io.File;
@@ -30,6 +31,7 @@
import org.junit.Assume;
import org.junit.BeforeClass;
import org.junit.ClassRule;
+import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
@@ -267,6 +269,9 @@
@ClassRule
public static TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
+ @Rule
+ public TestDescriptionWatcher watcher = new TestDescriptionWatcher();
+
@BeforeClass
public static void compileLibraries() throws Exception {
// Selects appropriate jar according to min api level for the selected runtime.
diff --git a/src/test/java/com/android/tools/r8/kotlin/AbstractR8KotlinTestBase.java b/src/test/java/com/android/tools/r8/kotlin/AbstractR8KotlinTestBase.java
index c175e04..f99934e 100644
--- a/src/test/java/com/android/tools/r8/kotlin/AbstractR8KotlinTestBase.java
+++ b/src/test/java/com/android/tools/r8/kotlin/AbstractR8KotlinTestBase.java
@@ -223,7 +223,7 @@
protected void runTest(String folder, String mainClass, String extraProguardRules,
Consumer<InternalOptions> optionsConsumer, AndroidAppInspector inspector) throws Exception {
- Assume.assumeTrue(ToolHelper.artSupported());
+ Assume.assumeTrue(ToolHelper.artSupported() || ToolHelper.compareAgaintsGoldenFiles());
String proguardRules = buildProguardRules(mainClass);
if (extraProguardRules != null) {
diff --git a/src/test/java/com/android/tools/r8/kotlin/R8KotlinDataClassTest.java b/src/test/java/com/android/tools/r8/kotlin/R8KotlinDataClassTest.java
index c8c8aec..73d1e50 100644
--- a/src/test/java/com/android/tools/r8/kotlin/R8KotlinDataClassTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/R8KotlinDataClassTest.java
@@ -10,7 +10,9 @@
import com.android.tools.r8.utils.DexInspector;
import com.android.tools.r8.utils.DexInspector.ClassSubject;
import com.android.tools.r8.utils.DexInspector.MethodSubject;
+import com.android.tools.r8.utils.InternalOptions;
import java.util.Collections;
+import java.util.function.Consumer;
import org.junit.Test;
public class R8KotlinDataClassTest extends AbstractR8KotlinTestBase {
@@ -33,13 +35,15 @@
private static final MethodSignature COPY_DEFAULT_METHOD =
TEST_DATA_CLASS.getCopyDefaultSignature();
+ private Consumer<InternalOptions> disableClassInliner = o -> o.enableClassInlining = false;
+
@Test
public void test_dataclass_gettersOnly() throws Exception {
final String mainClassName = "dataclass.MainGettersOnlyKt";
final MethodSignature testMethodSignature =
new MethodSignature("testDataClassGetters", "void", Collections.emptyList());
final String extraRules = keepClassMethod(mainClassName, testMethodSignature);
- runTest("dataclass", mainClassName, extraRules, (app) -> {
+ runTest("dataclass", mainClassName, extraRules, disableClassInliner, (app) -> {
DexInspector dexInspector = new DexInspector(app);
ClassSubject dataClass = checkClassIsKept(dexInspector, TEST_DATA_CLASS.getClassName());
@@ -74,7 +78,7 @@
final MethodSignature testMethodSignature =
new MethodSignature("testAllDataClassComponentFunctions", "void", Collections.emptyList());
final String extraRules = keepClassMethod(mainClassName, testMethodSignature);
- runTest("dataclass", mainClassName, extraRules, (app) -> {
+ runTest("dataclass", mainClassName, extraRules, disableClassInliner, (app) -> {
DexInspector dexInspector = new DexInspector(app);
ClassSubject dataClass = checkClassIsKept(dexInspector, TEST_DATA_CLASS.getClassName());
@@ -109,7 +113,7 @@
final MethodSignature testMethodSignature =
new MethodSignature("testSomeDataClassComponentFunctions", "void", Collections.emptyList());
final String extraRules = keepClassMethod(mainClassName, testMethodSignature);
- runTest("dataclass", mainClassName, extraRules, (app) -> {
+ runTest("dataclass", mainClassName, extraRules, disableClassInliner, (app) -> {
DexInspector dexInspector = new DexInspector(app);
ClassSubject dataClass = checkClassIsKept(dexInspector, TEST_DATA_CLASS.getClassName());
@@ -144,7 +148,7 @@
final MethodSignature testMethodSignature =
new MethodSignature("testDataClassCopy", "void", Collections.emptyList());
final String extraRules = keepClassMethod(mainClassName, testMethodSignature);
- runTest("dataclass", mainClassName, extraRules, (app) -> {
+ runTest("dataclass", mainClassName, extraRules, disableClassInliner, (app) -> {
DexInspector dexInspector = new DexInspector(app);
ClassSubject dataClass = checkClassIsKept(dexInspector, TEST_DATA_CLASS.getClassName());
@@ -159,7 +163,7 @@
final MethodSignature testMethodSignature =
new MethodSignature("testDataClassCopyWithDefault", "void", Collections.emptyList());
final String extraRules = keepClassMethod(mainClassName, testMethodSignature);
- runTest("dataclass", mainClassName, extraRules, (app) -> {
+ runTest("dataclass", mainClassName, extraRules, disableClassInliner, (app) -> {
DexInspector dexInspector = new DexInspector(app);
ClassSubject dataClass = checkClassIsKept(dexInspector, TEST_DATA_CLASS.getClassName());
diff --git a/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingTest.java b/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingTest.java
index e2dfb6d..46a439d 100644
--- a/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingTest.java
+++ b/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingTest.java
@@ -16,6 +16,7 @@
import com.android.tools.r8.utils.DexInspector.InstructionSubject;
import com.android.tools.r8.utils.DexInspector.InvokeInstructionSubject;
import com.android.tools.r8.utils.DexInspector.MethodSubject;
+import com.android.tools.r8.utils.TestDescriptionWatcher;
import com.google.common.collect.ImmutableList;
import java.io.IOException;
import java.nio.file.Path;
@@ -26,6 +27,7 @@
import java.util.concurrent.ExecutionException;
import java.util.function.Consumer;
import org.junit.Assert;
+import org.junit.Assume;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -60,6 +62,9 @@
@Rule
public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
+ @Rule
+ public TestDescriptionWatcher watcher = new TestDescriptionWatcher();
+
public MemberRebindingTest(TestConfiguration configuration) {
this.kind = configuration.kind;
originalDex = configuration.getDexPath();
@@ -319,9 +324,8 @@
@Test
public void memberRebindingTest() throws IOException, InterruptedException, ExecutionException {
- if (!ToolHelper.artSupported()) {
- return;
- }
+ Assume.assumeTrue(ToolHelper.artSupported() || ToolHelper.compareAgaintsGoldenFiles());
+
String out = temp.getRoot().getCanonicalPath();
Path processed = Paths.get(out, "classes.dex");
diff --git a/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232.java b/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232.java
index 2e4d6cf..fa4e3e1 100644
--- a/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232.java
+++ b/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232.java
@@ -7,11 +7,9 @@
import com.android.tools.r8.ClassFileConsumer.ArchiveConsumer;
import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.ToolHelper;
-import com.android.tools.r8.ToolHelper.DexVm;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
-import org.junit.Assume;
import org.junit.Test;
public class Regress78493232 extends AsmTestBase {
@@ -20,7 +18,6 @@
public void test() throws Exception {
// Run test on JVM and ART(x86) to ensure expected behavior.
// Running the same test on an ARM JIT causes errors.
- Assume.assumeTrue(ToolHelper.getDexVm() != DexVm.ART_7_0_0_HOST); // b/78866151
ensureSameOutput(
Regress78493232Dump.CLASS_NAME,
Regress78493232Dump.dump(),
diff --git a/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232Dump_WithPhi.java b/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232Dump_WithPhi.java
new file mode 100644
index 0000000..bfb0674
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232Dump_WithPhi.java
@@ -0,0 +1,341 @@
+// 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.b78493232;
+
+import com.android.tools.r8.utils.DescriptorUtils;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.FieldVisitor;
+import org.objectweb.asm.Label;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
+
+public class Regress78493232Dump_WithPhi implements Opcodes {
+
+ public static final String CLASS_NAME = "regress78493232.Test";
+ public static final String CLASS_DESC = DescriptorUtils.javaTypeToDescriptor(CLASS_NAME);
+ public static final String CLASS_INTERNAL = DescriptorUtils.descriptorToInternalName(CLASS_DESC);
+
+ public static final String UTILS_CLASS_NAME = Regress78493232Utils.class.getCanonicalName();
+ public static final String UTILS_CLASS_DESC =
+ DescriptorUtils.javaTypeToDescriptor(UTILS_CLASS_NAME);
+ public static final String UTILS_CLASS_INTERNAL =
+ DescriptorUtils.descriptorToInternalName(UTILS_CLASS_DESC);
+
+ static final int iterations = 1000;
+
+ // Arguments that have been seen when issue occurred.
+ static byte arg0 = 13;
+ static short arg1 = 25;
+ static int arg2 = 312;
+
+ // Static state of the class seen when the issue occurred.
+ static int staticIntA = 0;
+ static int staticIntB = 29;
+ static byte[] staticByteArray =
+ new byte[] {
+ 63, 101, 52, -33, 9, -21, 21, 51, -59, -6, 65, -27, -37, -2, -5, 1, 33, -33, 2, 13, 4, -12,
+ -53, 54, 2, -15, 46, 2, 13, 4, -3, 30, -47, 9, 0, -13, 3, -11, -10, 13, -2, 61, -69, -6, 6,
+ -1, 15, -8, 63, -22, -33, -19, 50, -35, -3, 7, 8, 2, -7, 9, -21, 21, 51, -62, 11, -13, 7,
+ 57, -37, -38, 6, -1, 15, -8, -53, 3, -19, 19, 50, -53, 3, -19, 19, 50, 9, -21, 21, 51, -59,
+ -6, 65, -27, -6, 10, -51, 21, -2, -11, -4, 11, -6, 1, 1, 11, -3, 61, -50, 50, -75, 75, -52,
+ 0, 52, -53, -9, -3, -4, 14, 2, -15, 49, -41, 11, -18, 0, 39, -35, 14, -3, -1, -13, 9, -21,
+ 21, 51, -59, -6, 65, -70, 7, -3, 12, -5, -9, 2, -13, 23, -27, 9, -11, 15, -6, 11, 11, 11, 7,
+ 16, -31, -2, 4, 7, 9, -21, 21, 51, -69, 14, 2, -18, 3, 9, -11, -5, 75, -37, -18, 2, -18, 3,
+ 13, 19, -15, -13, 10, -11, 2, -1, -7, 7, -15, 15, 5, 9, -11, 15, 13, 4, -3, -18, 3, 0, 13,
+ -9, -6, 32, -21, -4, 8, 24, -28, -3, 0, 3, -10, 9, -21, 21, 51, -59, -6, 65, -20, -55, 5,
+ 15, 36, -49, 0, 17, -24, 48, -37, -2, -5, 1, 33, -33, 2, 13, 4, -12, 9, -21, 21, 51, -59,
+ -6, 65, -24, -35, -3, 7, 22, -38, 1, 4, -5, 1, 33, -33, 2, 13, 4, -12, 1, 11, -3, 61, -50,
+ 50, -75, 75, -52, 0, 52, -52, 63, -77, 2, -15, 47, -51, 4, 15, -13, 4, 13, -11, 25, -33, 5,
+ -3, 17, -6, 2, 33, -37, -9, 13, 2, -17, 5, -3, -7, -3, 14, -3, 33, -41, 11, -18, 0, 2, -15,
+ 43, -37, -5, -1, 19, -13, 11, -2, -13, 10, -14, 3, 6, 5, 54, -65, -4, 69, -23, -41, -8, 13,
+ -9, 3, 1, 1, 8, -9, -6, 21, -4, 20, -8, 9, -21, 21, 51, -62, 11, -13, 7, 57, -21, -41, 11,
+ -18, 0, 39, -35, 14, -3, -1, -13, -3, 14, -3, 32, -33, -19, 1, 11, -3, 62, -51, 51, -76, 76,
+ -53, 0, 53, -54, 14, -15, 33, -18, 0, 1, 9, -21, 21, 51, -59, -6, 65, -22, -29, -19, 19, 24,
+ -37, -2, -5, 1, 33, -33, 2, 13, 4, -12, 2, -15, 36, -34, 3, -1, 11, -13, -2, -5, 2, -15, 51,
+ -33, -17, 4, 3, -9, 1, 15, 21, -17, -19, 12, 9, -21, 21, 51, -59, -6, 65, -24, -35, -3, 7,
+ 9, -21, 21, 51, -59, -6, 65, -24, -35, -3, 7, 33, -33, -14, 16, -15, 9, -7, -4, 5, -3, 21,
+ -3, 19, -8, 9, -19, 4, 43, -37, -6
+ };
+
+ public static byte[] dump() {
+ return dump(arg0, arg1, arg2, staticIntA, staticIntB, staticByteArray);
+ }
+
+ public static byte[] dump(
+ byte arg0, short arg1, int arg2, int staticIntA, int staticIntB, byte[] staticByteArray) {
+ ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES);
+ FieldVisitor fv;
+ MethodVisitor mv;
+
+ cw.visit(V1_6, ACC_PUBLIC + ACC_SUPER, CLASS_INTERNAL, null, "java/lang/Object", null);
+
+ {
+ fv = cw.visitField(ACC_PRIVATE + ACC_FINAL + ACC_STATIC, "staticByteArray", "[B", null, null);
+ fv.visitEnd();
+ }
+ {
+ fv = cw.visitField(ACC_PRIVATE + ACC_STATIC, "staticIntA", "I", null, null);
+ fv.visitEnd();
+ }
+ {
+ fv = cw.visitField(ACC_PRIVATE + ACC_STATIC, "staticIntB", "I", null, null);
+ fv.visitEnd();
+ }
+
+ {
+ mv = cw.visitMethod(ACC_PUBLIC + ACC_STATIC, "run", "()Ljava/lang/String;", null, null);
+ mv.visitCode();
+ mv.visitIntInsn(BIPUSH, arg0);
+ mv.visitIntInsn(SIPUSH, arg1);
+ mv.visitIntInsn(SIPUSH, arg2);
+ mv.visitMethodInsn(
+ INVOKESTATIC, CLASS_INTERNAL, "methodCausingIssue", "(BSI)Ljava/lang/String;", false);
+ mv.visitInsn(ARETURN);
+ mv.visitMaxs(-1, -1);
+ mv.visitEnd();
+ }
+
+ {
+ mv = cw.visitMethod(ACC_PUBLIC + ACC_STATIC, "getHash", "()I", null, null);
+ mv.visitCode();
+ getHash(mv);
+ mv.visitInsn(IRETURN);
+ mv.visitMaxs(-1, -1);
+ mv.visitEnd();
+ }
+
+ {
+ mv = cw.visitMethod(ACC_PUBLIC + ACC_STATIC, "main", "([Ljava/lang/String;)V", null, null);
+ mv.visitCode();
+
+ mv.visitInsn(ICONST_0);
+ mv.visitVarInsn(ISTORE, 1);
+ Label l0 = new Label();
+ mv.visitLabel(l0);
+ mv.visitVarInsn(ILOAD, 1);
+ mv.visitLdcInsn(new Integer(iterations));
+ Label l1 = new Label();
+ mv.visitJumpInsn(IF_ICMPGE, l1);
+ mv.visitMethodInsn(INVOKESTATIC, CLASS_INTERNAL, "run", "()Ljava/lang/String;", false);
+ mv.visitVarInsn(ILOAD, 1);
+ mv.visitMethodInsn(
+ INVOKESTATIC, UTILS_CLASS_INTERNAL, "compare", "(Ljava/lang/String;I)V", false);
+ mv.visitFieldInsn(GETSTATIC, CLASS_INTERNAL, "staticIntA", "I");
+ mv.visitFieldInsn(GETSTATIC, CLASS_INTERNAL, "staticIntB", "I");
+ mv.visitFieldInsn(GETSTATIC, CLASS_INTERNAL, "staticByteArray", "[B");
+ mv.visitVarInsn(ILOAD, 1);
+ mv.visitMethodInsn(INVOKESTATIC, UTILS_CLASS_INTERNAL, "compareHash", "(II[BI)V", false);
+ mv.visitIincInsn(1, 1);
+ mv.visitJumpInsn(GOTO, l0);
+ mv.visitLabel(l1);
+ mv.visitLdcInsn("Completed successfully after " + iterations + " iterations");
+ mv.visitMethodInsn(
+ INVOKESTATIC, UTILS_CLASS_INTERNAL, "println", "(Ljava/lang/String;)V", false);
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(-1, -1);
+ mv.visitEnd();
+ }
+
+ {
+ mv = cw.visitMethod(ACC_STATIC, "<clinit>", "()V", null, null);
+ mv.visitCode();
+ mv.visitIntInsn(BIPUSH, staticIntA);
+ mv.visitFieldInsn(PUTSTATIC, CLASS_INTERNAL, "staticIntA", "I");
+ mv.visitIntInsn(BIPUSH, staticIntB);
+ mv.visitFieldInsn(PUTSTATIC, CLASS_INTERNAL, "staticIntB", "I");
+ mv.visitLdcInsn(staticByteArray.length);
+ mv.visitIntInsn(NEWARRAY, T_BYTE);
+ for (int i = 0; i < staticByteArray.length; i++) {
+ mv.visitInsn(DUP);
+ mv.visitIntInsn(SIPUSH, i);
+ mv.visitIntInsn(BIPUSH, staticByteArray[i]);
+ mv.visitInsn(BASTORE);
+ }
+ mv.visitFieldInsn(PUTSTATIC, CLASS_INTERNAL, "staticByteArray", "[B");
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(-1, -1);
+ mv.visitEnd();
+ }
+
+ {
+ mv =
+ cw.visitMethod(
+ ACC_PRIVATE + ACC_STATIC,
+ "methodCausingIssue",
+ "(BSI)Ljava/lang/String;",
+ null,
+ null);
+ mv.visitCode();
+ Label l0 = new Label();
+ mv.visitJumpInsn(GOTO, l0);
+ mv.visitLabel(l0);
+ mv.visitIntInsn(SIPUSH, 472);
+ mv.visitVarInsn(ILOAD, 2);
+ mv.visitInsn(ISUB);
+ mv.visitVarInsn(ISTORE, 2);
+ mv.visitInsn(ICONST_0);
+ mv.visitVarInsn(ISTORE, 3);
+ mv.visitTypeInsn(NEW, "java/lang/String");
+ mv.visitVarInsn(ASTORE, 10);
+ mv.visitIntInsn(BIPUSH, 119);
+ mv.visitVarInsn(ILOAD, 0);
+ mv.visitInsn(ISUB);
+ mv.visitVarInsn(ISTORE, 0);
+ mv.visitIincInsn(1, 1);
+ mv.visitFieldInsn(GETSTATIC, CLASS_INTERNAL, "staticByteArray", "[B");
+ mv.visitVarInsn(ASTORE, 4);
+ mv.visitVarInsn(ILOAD, 1);
+ mv.visitIntInsn(NEWARRAY, T_BYTE);
+ mv.visitVarInsn(ALOAD, 4);
+ Label l1 = new Label();
+ mv.visitJumpInsn(IFNONNULL, l1);
+ Label l2 = new Label();
+ mv.visitJumpInsn(GOTO, l2);
+ mv.visitLabel(l1);
+ Label l3 = new Label();
+ mv.visitJumpInsn(GOTO, l3);
+ mv.visitLabel(l2);
+ mv.visitVarInsn(ILOAD, 1);
+ mv.visitVarInsn(ILOAD, 2);
+ Label l4 = new Label();
+ mv.visitJumpInsn(GOTO, l4);
+ Label l5 = new Label();
+ mv.visitLabel(l5);
+ mv.visitInsn(INEG);
+ mv.visitInsn(IADD);
+ mv.visitVarInsn(ISTORE, 0);
+ mv.visitJumpInsn(GOTO, l3);
+ mv.visitLabel(l3);
+ mv.visitInsn(DUP);
+ mv.visitVarInsn(ILOAD, 3);
+ mv.visitIincInsn(3, 1);
+ mv.visitVarInsn(ILOAD, 0);
+ mv.visitInsn(I2B);
+ mv.visitInsn(BASTORE);
+ mv.visitVarInsn(ILOAD, 3);
+ mv.visitVarInsn(ILOAD, 1);
+ Label l6 = new Label();
+ mv.visitJumpInsn(IF_ICMPNE, l6);
+ Label l7 = new Label();
+ Label l7_b0 = new Label();
+ Label l7_b1 = new Label();
+ Label l7_join = new Label();
+ mv.visitJumpInsn(GOTO, l7);
+ mv.visitLabel(l6);
+ Label l8 = new Label();
+ mv.visitJumpInsn(GOTO, l8);
+ // Depending on the argument either load the new-instance string or null on the stack.
+ mv.visitLabel(l7);
+ mv.visitVarInsn(ILOAD, 0);
+ mv.visitJumpInsn(IFEQ, l7_b1);
+ mv.visitLabel(l7_b0);
+ mv.visitInsn(ACONST_NULL);
+ // At this point, NULL flows to l7_join.
+ mv.visitJumpInsn(GOTO, l7_join);
+ mv.visitLabel(l7_b1);
+ mv.visitVarInsn(ALOAD, 10);
+ // At this point, an uninitialized String flows to l7_join.
+ mv.visitLabel(l7_join);
+ // At this point, stack contains [staticByteArray, NULL/uninitialized]
+ mv.visitInsn(SWAP);
+ // At this point, stack contains [NULL/uninitialized, staticByteArray]
+ mv.visitVarInsn(ALOAD, 10);
+ // At this point, stack contains [NULL/uninitialized, staticByteArray, uninitialized]
+ mv.visitInsn(SWAP);
+ // At this point, stack contains [NULL/uninitialized, uninitialized, staticByteArray]
+ mv.visitInsn(ICONST_0);
+ // At this point, stack contains [NULL/uninitialized, uninitialized, staticByteArray, 0]
+ mv.visitMethodInsn(INVOKESPECIAL, "java/lang/String", "<init>", "([BI)V", false);
+ // Just before ARETURN, stack contains [NULL/String].
+ // This will force a non-trivial phi of the new-instance on this block, ie, prior to <init>.
+ // This phi must remain since it is not trivial.
+ mv.visitInsn(ARETURN);
+ mv.visitLabel(l8);
+ mv.visitVarInsn(ILOAD, 0);
+ mv.visitVarInsn(ALOAD, 4);
+ mv.visitIincInsn(2, 1);
+ mv.visitVarInsn(ILOAD, 2);
+ mv.visitInsn(BALOAD);
+ Label l9 = new Label();
+ mv.visitJumpInsn(GOTO, l9);
+ mv.visitLabel(l4);
+ mv.visitFieldInsn(GETSTATIC, CLASS_INTERNAL, "staticIntB", "I");
+ mv.visitIntInsn(BIPUSH, 125);
+ mv.visitInsn(IADD);
+ mv.visitInsn(DUP);
+ mv.visitIntInsn(SIPUSH, 128);
+ mv.visitInsn(IREM);
+ mv.visitFieldInsn(PUTSTATIC, CLASS_INTERNAL, "staticIntA", "I");
+ mv.visitInsn(ICONST_2);
+ mv.visitInsn(IREM);
+ Label l10 = new Label();
+ mv.visitJumpInsn(IFEQ, l10);
+ Label l11 = new Label();
+ mv.visitJumpInsn(GOTO, l11);
+ mv.visitLabel(l10);
+ Label l12 = new Label();
+ mv.visitJumpInsn(GOTO, l12);
+ Label l13 = new Label();
+ mv.visitLabel(l13);
+ mv.visitInsn(INEG);
+ mv.visitInsn(IADD);
+ mv.visitVarInsn(ISTORE, 0);
+ mv.visitJumpInsn(GOTO, l3);
+ mv.visitLabel(l9);
+ mv.visitFieldInsn(GETSTATIC, CLASS_INTERNAL, "staticIntA", "I");
+ mv.visitIntInsn(BIPUSH, 29);
+ mv.visitInsn(IADD);
+ mv.visitInsn(DUP);
+ mv.visitIntInsn(SIPUSH, 128);
+ mv.visitInsn(IREM);
+ mv.visitFieldInsn(PUTSTATIC, CLASS_INTERNAL, "staticIntB", "I");
+ mv.visitInsn(ICONST_2);
+ mv.visitInsn(IREM);
+ Label l14 = new Label();
+ mv.visitJumpInsn(IFNE, l14);
+ Label l15 = new Label();
+ mv.visitJumpInsn(GOTO, l15);
+ mv.visitLabel(l14);
+ Label l16 = new Label();
+ mv.visitJumpInsn(GOTO, l16);
+ Label l17 = new Label();
+ mv.visitLabel(l17);
+ mv.visitInsn(INEG);
+ mv.visitInsn(IADD);
+ mv.visitVarInsn(ISTORE, 0);
+ mv.visitJumpInsn(GOTO, l3);
+ Label l18 = new Label();
+ mv.visitLabel(l18);
+ mv.visitTableSwitchInsn(0, 1, l11, new Label[] {l13, l5});
+ mv.visitLabel(l12);
+ mv.visitInsn(ICONST_1);
+ mv.visitJumpInsn(GOTO, l18);
+ mv.visitLabel(l11);
+ mv.visitInsn(ICONST_0);
+ mv.visitJumpInsn(GOTO, l18);
+ Label l19 = new Label();
+ mv.visitLabel(l19);
+ mv.visitTableSwitchInsn(0, 1, l15, new Label[] {l5, l17});
+ mv.visitLabel(l16);
+ mv.visitInsn(ICONST_0);
+ mv.visitJumpInsn(GOTO, l19);
+ mv.visitLabel(l15);
+ mv.visitInsn(ICONST_1);
+ mv.visitJumpInsn(GOTO, l19);
+ mv.visitMaxs(8, 5);
+ mv.visitEnd();
+ }
+
+ cw.visitEnd();
+
+ return cw.toByteArray();
+ }
+
+ private static void getHash(MethodVisitor mv) {
+ mv.visitFieldInsn(GETSTATIC, CLASS_INTERNAL, "staticIntA", "I");
+ mv.visitFieldInsn(GETSTATIC, CLASS_INTERNAL, "staticIntB", "I");
+ mv.visitFieldInsn(GETSTATIC, CLASS_INTERNAL, "staticByteArray", "[B");
+ mv.visitMethodInsn(INVOKESTATIC, UTILS_CLASS_INTERNAL, "getHash", "(II[B)I", false);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232Utils.java b/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232Utils.java
index ff7da8f..ccf6467 100644
--- a/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232Utils.java
+++ b/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232Utils.java
@@ -31,13 +31,14 @@
public static void compare(String output, int iterations) {
String expected = "java.security.SecureRandom";
- if (output.equals(expected)) {
+ if (expected.equals(output)) {
return;
}
System.out.println(
"After " + iterations + " iterations, expected \"" +
expected + "\", but got \"" + output + "\"");
- System.exit(1);
+ // Exit with code 0 to allow test to use ensureSameOutput().
+ System.exit(0);
}
public static void compareHash(int a, int b, byte[] c, int iterations) {
@@ -53,6 +54,7 @@
System.out.println("staticIntB: " + b);
System.out.print("staticIntByteArray: ");
printByteArray(c);
- System.exit(1);
+ // Exit with code 0 to allow test to use ensureSameOutput().
+ System.exit(0);
}
}
diff --git a/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232_WithPhi.java b/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232_WithPhi.java
new file mode 100644
index 0000000..3a56004
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232_WithPhi.java
@@ -0,0 +1,81 @@
+// 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.b78493232;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
+import com.android.tools.r8.AsmTestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.DexVm.Version;
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.utils.AndroidApp;
+import org.junit.Test;
+
+// Variant of Regress78493232, but where the new-instance is forced to flow to a non-trivial phi
+// function prior to the call to <init>.
+public class Regress78493232_WithPhi extends AsmTestBase {
+
+ @Test
+ public void test() throws Exception {
+ // Run test on JVM and ART(x86) to ensure expected behavior.
+ // Running the same test on an ARM JIT causes errors.
+
+ // TODO(b/80118070): Remove this if-statement when fixed.
+ if (ToolHelper.getDexVm().getVersion() != Version.V5_1_1) {
+ AndroidApp app =
+ buildAndroidApp(
+ Regress78493232Dump_WithPhi.dump(),
+ ToolHelper.getClassAsBytes(Regress78493232Utils.class));
+ ProcessResult javaResult =
+ runOnJava(
+ Regress78493232Dump_WithPhi.CLASS_NAME,
+ Regress78493232Dump_WithPhi.dump(),
+ ToolHelper.getClassAsBytes(Regress78493232Utils.class));
+ ProcessResult d8Result =
+ runOnArtRaw(compileWithD8(app), Regress78493232Dump_WithPhi.CLASS_NAME);
+ ProcessResult r8Result =
+ runOnArtRaw(compileWithR8(app), Regress78493232Dump_WithPhi.CLASS_NAME);
+ String proguardConfig =
+ keepMainProguardConfiguration(Regress78493232Dump_WithPhi.CLASS_NAME)
+ + "-dontobfuscate\n";
+ ProcessResult r8ShakenResult =
+ runOnArtRaw(compileWithR8(app, proguardConfig), Regress78493232Dump_WithPhi.CLASS_NAME);
+ assertEquals(
+ "After 0 iterations, expected \"java.security.SecureRandom\", but got \"null\"\n",
+ javaResult.stdout);
+ assertEquals(0, javaResult.exitCode);
+ switch (ToolHelper.getDexVm().getVersion()) {
+ case V4_0_4:
+ case V4_4_4:
+ case V7_0_0:
+ case DEFAULT:
+ assertNotEquals(-1, d8Result.stderr.indexOf("java.lang.VerifyError"));
+ assertNotEquals(-1, r8Result.stderr.indexOf("java.lang.VerifyError"));
+ assertNotEquals(-1, r8ShakenResult.stderr.indexOf("java.lang.VerifyError"));
+ assertEquals(1, d8Result.exitCode);
+ assertEquals(1, r8Result.exitCode);
+ assertEquals(1, r8ShakenResult.exitCode);
+ break;
+ case V6_0_1:
+ assertEquals("Completed successfully after 1000 iterations\n", d8Result.stdout);
+ assertEquals("Completed successfully after 1000 iterations\n", r8Result.stdout);
+ assertEquals("Completed successfully after 1000 iterations\n", r8ShakenResult.stdout);
+ assertEquals(0, d8Result.exitCode);
+ assertEquals(0, r8Result.exitCode);
+ assertEquals(0, r8ShakenResult.exitCode);
+ break;
+ case V5_1_1:
+ default:
+ throw new Unreachable();
+ }
+ return;
+ }
+ ensureSameOutput(
+ Regress78493232Dump_WithPhi.CLASS_NAME,
+ Regress78493232Dump_WithPhi.dump(),
+ ToolHelper.getClassAsBytes(Regress78493232Utils.class));
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/resource/DataResourceTest.java b/src/test/java/com/android/tools/r8/resource/DataResourceTest.java
index 2df0612..66161e1 100644
--- a/src/test/java/com/android/tools/r8/resource/DataResourceTest.java
+++ b/src/test/java/com/android/tools/r8/resource/DataResourceTest.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.ToolHelper.ProcessResult;
import com.android.tools.r8.utils.FileUtils;
+import com.android.tools.r8.utils.TestDescriptionWatcher;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
@@ -23,6 +24,9 @@
@Rule
public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
+ @Rule
+ public TestDescriptionWatcher watcher = new TestDescriptionWatcher();
+
@Test
public void dataResourceTest() throws IOException, CompilationFailedException {
String packageName = "dataresource";
diff --git a/src/test/java/com/android/tools/r8/shaking/TreeShakingTest.java b/src/test/java/com/android/tools/r8/shaking/TreeShakingTest.java
index f341969..c9488b4 100644
--- a/src/test/java/com/android/tools/r8/shaking/TreeShakingTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/TreeShakingTest.java
@@ -19,6 +19,7 @@
import com.android.tools.r8.utils.DexInspector.FoundFieldSubject;
import com.android.tools.r8.utils.DexInspector.FoundMethodSubject;
import com.android.tools.r8.utils.ListUtils;
+import com.android.tools.r8.utils.TestDescriptionWatcher;
import com.google.common.collect.ImmutableList;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -76,6 +77,9 @@
@Rule
public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
+ @Rule
+ public TestDescriptionWatcher watcher = new TestDescriptionWatcher();
+
protected TreeShakingTest(
String name, String mainClass, Frontend frontend, Backend backend, MinifyMode minify) {
this.name = name;
@@ -204,7 +208,7 @@
}
return;
}
- if (!ToolHelper.artSupported()) {
+ if (!ToolHelper.artSupported() && !ToolHelper.compareAgaintsGoldenFiles()) {
return;
}
Consumer<ArtCommandBuilder> extraArtArgs = builder -> {
diff --git a/src/test/java/com/android/tools/r8/utils/TestDescriptionWatcher.java b/src/test/java/com/android/tools/r8/utils/TestDescriptionWatcher.java
new file mode 100644
index 0000000..5295c0e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/utils/TestDescriptionWatcher.java
@@ -0,0 +1,29 @@
+// 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.utils;
+
+import org.junit.rules.TestWatcher;
+import org.junit.runner.Description;
+
+/**
+ * This {@link TestWatcher} passes test description information
+ * (namely test class and method names) to the test framework
+ * via system properties.
+ */
+public class TestDescriptionWatcher extends TestWatcher {
+
+ @Override
+ protected void starting(Description description) {
+ System.setProperty("test_class_name", description.getClassName());
+ System.setProperty("test_name", description.getMethodName());
+ System.setProperty("reset_output_index", "true");
+ }
+
+ @Override
+ protected void finished(Description description) {
+ System.clearProperty("test_class_name");
+ System.clearProperty("test_name");
+ }
+
+}
diff --git a/tools/download_from_x20.py b/tools/download_from_x20.py
index 01ba4e1..5f64684 100755
--- a/tools/download_from_x20.py
+++ b/tools/download_from_x20.py
@@ -19,17 +19,10 @@
def parse_options():
return optparse.OptionParser().parse_args()
-def extract_dir(filename):
- return filename[0:len(filename) - len('.tar.gz')]
-
-def unpack_archive(filename):
- dest_dir = extract_dir(filename)
- if os.path.exists(dest_dir):
- print 'Deleting existing dir %s' % dest_dir
- shutil.rmtree(dest_dir)
- dirname = os.path.dirname(os.path.abspath(filename))
- with tarfile.open(filename, 'r:gz') as tar:
- tar.extractall(path=dirname)
+def download(src, dest):
+ print 'Downloading %s to %s' % (src, dest)
+ shutil.copyfile(src, dest)
+ utils.unpack_archive(dest)
def Main():
(options, args) = parse_options()
@@ -41,20 +34,18 @@
sha1 = input_sha.readline()
if os.path.exists(dest) and utils.get_sha1(dest) == sha1:
print 'sha1 matches, not downloading'
- dest_dir = extract_dir(dest)
+ dest_dir = utils.extract_dir(dest)
if os.path.exists(dest_dir):
print 'destination directory exists, no extraction'
else:
- unpack_archive(dest)
+ utils.unpack_archive(dest)
return
src = os.path.join(GMSCORE_DEPS, sha1)
if not os.path.exists(src):
print 'File (%s) does not exist on x20' % src
print 'Maybe pass -Pno_internal to your gradle invocation'
return 42
- print 'Downloading %s to %s' % (src, dest)
- shutil.copyfile(src, dest)
- unpack_archive(dest)
+ download(src, dest)
if __name__ == '__main__':
sys.exit(Main())
diff --git a/tools/test.py b/tools/test.py
index ed99c1d..b92c8dc 100755
--- a/tools/test.py
+++ b/tools/test.py
@@ -15,6 +15,7 @@
import utils
import uuid
import notify
+import upload_to_x20
ALL_ART_VMS = ["default", "7.0.0", "6.0.1", "5.1.1", "4.4.4", "4.0.4"]
@@ -78,6 +79,13 @@
' Note that the directory will not be cleared before the test.')
result.add_option('--java_home',
help='Use a custom java version to run tests.')
+ result.add_option('--generate_golden_files_to',
+ help='Store dex files produced by tests in the specified directory.'
+ ' It is aimed to be read on platforms with no host runtime available'
+ ' for comparison.')
+ result.add_option('--use_golden_files_in',
+ help='Download golden files hierarchy for this commit in the specified'
+ ' location and use them instead of executing on host runtime.')
return result.parse_args()
@@ -93,7 +101,7 @@
def Main():
(options, args) = ParseOptions()
- gradle_args = []
+ gradle_args = ['--stacktrace']
# Set all necessary Gradle properties and options first.
if options.verbose:
gradle_args.append('-Pprint_test_stdout')
@@ -134,7 +142,16 @@
os.makedirs(options.test_dir)
if options.java_home:
gradle_args.append('-Dorg.gradle.java.home=' + options.java_home)
-
+ if options.generate_golden_files_to:
+ gradle_args.append('-Pgenerate_golden_files_to=' + options.generate_golden_files_to)
+ if not os.path.exists(options.generate_golden_files_to):
+ os.makedirs(options.generate_golden_files_to)
+ gradle_args.append('-PHEAD_sha1=' + utils.get_HEAD_sha1())
+ if options.use_golden_files_in:
+ gradle_args.append('-Puse_golden_files_in=' + options.use_golden_files_in)
+ if not os.path.exists(options.use_golden_files_in):
+ os.makedirs(options.use_golden_files_in)
+ gradle_args.append('-PHEAD_sha1=' + utils.get_HEAD_sha1())
# Add Gradle tasks
gradle_args.append('cleanTest')
gradle_args.append('test')
@@ -146,12 +163,29 @@
# Create Jacoco report after tests.
gradle_args.append('jacocoTestReport')
+ if options.use_golden_files_in:
+ sha1 = '%s' % utils.get_HEAD_sha1()
+ with utils.ChangedWorkingDirectory(options.use_golden_files_in):
+ utils.download_file_from_cloud_storage(
+ 'gs://r8-test-results/golden-files/%s.tar.gz' % sha1,
+ '%s.tar.gz' % sha1)
+ utils.unpack_archive('%s.tar.gz' % sha1)
+
+
# Now run tests on selected runtime(s).
vms_to_test = [options.dex_vm] if options.dex_vm != "all" else ALL_ART_VMS
for art_vm in vms_to_test:
vm_kind_to_test = "_" + options.dex_vm_kind if art_vm != "default" else ""
return_code = gradle.RunGradle(gradle_args + ['-Pdex_vm=%s' % (art_vm + vm_kind_to_test)],
throw_on_failure=False)
+
+ if options.generate_golden_files_to:
+ sha1 = '%s' % utils.get_HEAD_sha1()
+ with utils.ChangedWorkingDirectory(options.generate_golden_files_to):
+ archive = utils.create_archive(sha1)
+ utils.upload_file_to_cloud_storage(archive,
+ 'gs://r8-test-results/golden-files/' + archive)
+
if return_code != 0:
if options.archive_failures and os.name != 'nt':
archive_failures()
diff --git a/tools/upload_to_x20.py b/tools/upload_to_x20.py
index 1efacf5..2d08362 100755
--- a/tools/upload_to_x20.py
+++ b/tools/upload_to_x20.py
@@ -20,11 +20,10 @@
def parse_options():
return optparse.OptionParser().parse_args()
-def create_archive(name):
- tarname = '%s.tar.gz' % name
- with tarfile.open(tarname, 'w:gz') as tar:
- tar.add(name)
- return tarname
+def uploadFile(filename, dest):
+ print 'Uploading to %s' % dest
+ shutil.copyfile(filename, dest)
+ subprocess.check_call(['chmod', '664', dest])
def Main():
(options, args) = parse_options()
@@ -34,12 +33,10 @@
if not name in os.listdir('.'):
print 'You must be standing directly below the directory you are uploading'
return 1
- filename = create_archive(name)
+ filename = utils.create_archive(name)
sha1 = utils.get_sha1(filename)
dest = os.path.join(GMSCORE_DEPS, sha1)
- print 'Uploading to %s' % dest
- shutil.copyfile(filename, dest)
- subprocess.check_call(['chmod', '664', dest])
+ uploadFile(filename, dest)
sha1_file = '%s.sha1' % filename
with open(sha1_file, 'w') as output:
output.write(sha1)
diff --git a/tools/utils.py b/tools/utils.py
index 3aab572..859da97 100644
--- a/tools/utils.py
+++ b/tools/utils.py
@@ -10,6 +10,7 @@
import shutil
import subprocess
import sys
+import tarfile
import tempfile
TOOLS_DIR = os.path.abspath(os.path.normpath(os.path.join(__file__, '..')))
@@ -71,6 +72,12 @@
sha1.update(chunk)
return sha1.hexdigest()
+def get_HEAD_sha1():
+ cmd = ['git', 'rev-parse', 'HEAD']
+ PrintCmd(cmd)
+ with ChangedWorkingDirectory(REPO_ROOT):
+ return subprocess.check_output(cmd).strip()
+
def makedirs_if_needed(path):
try:
os.makedirs(path)
@@ -92,6 +99,29 @@
PrintCmd(cmd)
subprocess.check_call(cmd)
+def download_file_from_cloud_storage(source, destination):
+ cmd = ['gsutil.py', 'cp', source, destination]
+ PrintCmd(cmd)
+ subprocess.check_call(cmd)
+
+def create_archive(name):
+ tarname = '%s.tar.gz' % name
+ with tarfile.open(tarname, 'w:gz') as tar:
+ tar.add(name)
+ return tarname
+
+def extract_dir(filename):
+ return filename[0:len(filename) - len('.tar.gz')]
+
+def unpack_archive(filename):
+ dest_dir = extract_dir(filename)
+ if os.path.exists(dest_dir):
+ print 'Deleting existing dir %s' % dest_dir
+ shutil.rmtree(dest_dir)
+ dirname = os.path.dirname(os.path.abspath(filename))
+ with tarfile.open(filename, 'r:gz') as tar:
+ tar.extractall(path=dirname)
+
# Note that gcs is eventually consistent with regards to list operations.
# This is not a problem in our case, but don't ever use this method
# for synchronization.
