Merge "Update FinallyBlockTest to handle differences in Java compilers"
diff --git a/build.gradle b/build.gradle
index 8237781..ff22c66 100644
--- a/build.gradle
+++ b/build.gradle
@@ -348,7 +348,6 @@
def x20Dependencies = [
"third_party": [
- "benchmarks/santa-tracker",
"gmail/gmail_android_170604.16",
"gmscore/v4",
"gmscore/v5",
@@ -358,7 +357,6 @@
"gmscore/gmscore_v9",
"gmscore/gmscore_v10",
"gmscore/latest",
- "gradle-plugin",
"photos/2017-06-06",
"youtube/youtube.android_12.10",
"youtube/youtube.android_12.17",
diff --git a/src/main/java/com/android/tools/r8/DexSplitterHelper.java b/src/main/java/com/android/tools/r8/DexSplitterHelper.java
index d28f93f..8573e0f 100644
--- a/src/main/java/com/android/tools/r8/DexSplitterHelper.java
+++ b/src/main/java/com/android/tools/r8/DexSplitterHelper.java
@@ -4,7 +4,7 @@
package com.android.tools.r8;
-import com.android.tools.r8.DexIndexedConsumer.ArchiveConsumer;
+import com.android.tools.r8.DexIndexedConsumer.DirectoryConsumer;
import com.android.tools.r8.dex.ApplicationReader;
import com.android.tools.r8.dex.ApplicationWriter;
import com.android.tools.r8.graph.AppInfo;
@@ -19,6 +19,8 @@
import com.android.tools.r8.utils.ThreadUtils;
import com.android.tools.r8.utils.Timing;
import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.HashMap;
import java.util.Map;
@@ -31,7 +33,7 @@
public static void run(
D8Command command,
FeatureClassMapping featureClassMapping,
- String outputArchive,
+ String output,
String proguardMap)
throws IOException, CompilationException, ExecutionException {
InternalOptions options = command.getInternalOptions();
@@ -65,8 +67,12 @@
AppInfo appInfo = new AppInfo(featureApp);
featureApp = D8.optimize(featureApp, appInfo, options, timing, executor);
// We create a specific consumer for each split.
- DexIndexedConsumer consumer =
- new ArchiveConsumer(Paths.get(outputArchive + "." + entry.getKey() + ".zip"));
+ Path outputDir = Paths.get(output).resolve(entry.getKey());
+ if (!Files.exists(outputDir)) {
+ Files.createDirectory(outputDir);
+ }
+ DexIndexedConsumer consumer = new DirectoryConsumer(outputDir);
+
try {
new ApplicationWriter(
featureApp,
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 5a0d2c4..eb6718d 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 = "v1.2.6-dev";
+ public static final String LABEL = "v1.2.7-dev";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/dexsplitter/DexSplitter.java b/src/main/java/com/android/tools/r8/dexsplitter/DexSplitter.java
index e3753a7..5e8d54d 100644
--- a/src/main/java/com/android/tools/r8/dexsplitter/DexSplitter.java
+++ b/src/main/java/com/android/tools/r8/dexsplitter/DexSplitter.java
@@ -16,6 +16,7 @@
import com.android.tools.r8.utils.OptionsParsing.ParseContext;
import com.google.common.collect.ImmutableList;
import java.io.IOException;
+import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
@@ -23,23 +24,55 @@
public class DexSplitter {
- private static final String DEFAULT_OUTPUT_ARCHIVE_FILENAME = "split";
+ private static final String DEFAULT_OUTPUT_DIR = "output";
private static final boolean PRINT_ARGS = false;
+ public static class FeatureJar {
+ private String jar;
+ private String outputName;
+
+ public FeatureJar(String jar, String outputName) {
+ this.jar = jar;
+ this.outputName = outputName;
+ }
+
+ public FeatureJar(String jar) {
+ this(jar, featureNameFromJar(jar));
+ }
+
+ public String getJar() {
+ return jar;
+ }
+
+ public String getOutputName() {
+ return outputName;
+ }
+
+ private static String featureNameFromJar(String jar) {
+ Path jarPath = Paths.get(jar);
+ String featureName = jarPath.getFileName().toString();
+ if (featureName.endsWith(".jar") || featureName.endsWith(".zip")) {
+ featureName = featureName.substring(0, featureName.length() - 4);
+ }
+ return featureName;
+ }
+
+ }
+
public static class Options {
private List<String> inputArchives = new ArrayList<>();
- private List<String> featureJars = new ArrayList<>();
- private String splitBaseName = DEFAULT_OUTPUT_ARCHIVE_FILENAME;
+ private List<FeatureJar> featureJars = new ArrayList<>();
+ private String output = DEFAULT_OUTPUT_DIR;
private String featureSplitMapping;
private String proguardMap;
- public String getSplitBaseName() {
- return splitBaseName;
+ public String getOutput() {
+ return output;
}
- public void setSplitBaseName(String splitBaseName) {
- this.splitBaseName = splitBaseName;
+ public void setOutput(String output) {
+ this.output = output;
}
public String getFeatureSplitMapping() {
@@ -62,19 +95,45 @@
inputArchives.add(inputArchive);
}
- public void addFeatureJar(String featureJar) {
+ protected void addFeatureJar(FeatureJar featureJar) {
featureJars.add(featureJar);
}
+ public void addFeatureJar(String jar) {
+ featureJars.add(new FeatureJar(jar));
+ }
+
+ public void addFeatureJar(String jar, String outputName) {
+ featureJars.add(new FeatureJar(jar, outputName));
+ }
+
public ImmutableList<String> getInputArchives() {
return ImmutableList.copyOf(inputArchives);
}
- public ImmutableList<String> getFeatureJars() {
+ public ImmutableList<FeatureJar> getFeatureJars() {
return ImmutableList.copyOf(featureJars);
}
}
+ /**
+ * Parse a feature jar argument and return the corresponding FeatureJar representation.
+ * Default to use the name of the jar file if the argument contains no ':', if the argument
+ * contains ':', then use the value after the ':' as the name.
+ * @param argument
+ * @return
+ */
+ private static FeatureJar parseFeatureJarArgument(String argument) {
+ if (argument.contains(":")) {
+ String[] parts = argument.split(":");
+ if (parts.length > 2) {
+ throw new RuntimeException("--feature-jar argument contains more than one :");
+ }
+ return new FeatureJar(parts[0], parts[1]);
+ }
+ return new FeatureJar(argument);
+ }
+
private static Options parseArguments(String[] args) throws IOException {
Options options = new Options();
ParseContext context = new ParseContext(args);
@@ -86,12 +145,13 @@
}
List<String> featureJars = OptionsParsing.tryParseMulti(context, "--feature-jar");
if (featureJars != null) {
- featureJars.stream().forEach(options::addFeatureJar);
+ featureJars.stream().forEach(
+ (feature) -> options.addFeatureJar(parseFeatureJarArgument(feature)));
continue;
}
String output = OptionsParsing.tryParseSingle(context, "--output", "-o");
if (output != null) {
- options.setSplitBaseName(output);
+ options.setOutput(output);
continue;
}
String proguardMap = OptionsParsing.tryParseSingle(context, "--proguard-map", null);
@@ -114,8 +174,8 @@
if (options.getFeatureSplitMapping() != null) {
return FeatureClassMapping.fromSpecification(Paths.get(options.getFeatureSplitMapping()));
}
- assert !options.featureJars.isEmpty();
- return FeatureClassMapping.fromJarFiles(options.featureJars);
+ assert !options.getFeatureJars().isEmpty();
+ return FeatureClassMapping.fromJarFiles(options.getFeatureJars());
}
private static void run(String[] args)
@@ -149,7 +209,7 @@
FeatureClassMapping featureClassMapping = createFeatureClassMapping(options);
DexSplitterHelper.run(
- builder.build(), featureClassMapping, options.getSplitBaseName(), options.getProguardMap());
+ builder.build(), featureClassMapping, options.getOutput(), options.getProguardMap());
}
public static void main(String[] args) {
diff --git a/src/main/java/com/android/tools/r8/graph/DexMethod.java b/src/main/java/com/android/tools/r8/graph/DexMethod.java
index c0c2e27..9c3e9a5 100644
--- a/src/main/java/com/android/tools/r8/graph/DexMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexMethod.java
@@ -71,6 +71,14 @@
return false;
}
+ /**
+ * Returns true if the other method has the same name and prototype (including signature and
+ * return type), false otherwise.
+ */
+ public boolean hasSameProtoAndName(DexMethod other) {
+ return name == other.name && proto == other.proto;
+ }
+
@Override
public int compareTo(DexMethod other) {
return sortedCompareTo(other.getSortedIndex());
diff --git a/src/main/java/com/android/tools/r8/graph/JarCode.java b/src/main/java/com/android/tools/r8/graph/JarCode.java
index 8c1a6c5..55ae757 100644
--- a/src/main/java/com/android/tools/r8/graph/JarCode.java
+++ b/src/main/java/com/android/tools/r8/graph/JarCode.java
@@ -199,14 +199,8 @@
if (context != null) {
// The SecondVistor is in charge of setting the context to null.
DexProgramClass owner = context.owner;
- int flags = ClassReader.SKIP_FRAMES;
- if (application.options.isGeneratingClassFiles()) {
- // TODO(mathiasr): Keep frames in JarCode until IR->CF construction is complete.
- // When we throw Unimplemented in IR->CF construction, the original JarCode is output
- // instead. In this case we must output frames as well, or we will fail verification.
- flags = 0;
- }
- new ClassReader(context.classCache).accept(new SecondVisitor(context, application), flags);
+ new ClassReader(context.classCache).accept(new SecondVisitor(context, application),
+ ClassReader.SKIP_FRAMES);
assert verifyNoReparseContext(owner);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/constant/SparseConditionalConstantPropagation.java b/src/main/java/com/android/tools/r8/ir/analysis/constant/SparseConditionalConstantPropagation.java
index 3379864..6c9f075 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/constant/SparseConditionalConstantPropagation.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/constant/SparseConditionalConstantPropagation.java
@@ -129,16 +129,24 @@
BasicBlock phiBlock = phi.getBlock();
int phiBlockNumber = phiBlock.getNumber();
LatticeElement element = Top.getInstance();
- for (int i = 0; i < phiBlock.getPredecessors().size(); i++) {
- BasicBlock predecessor = phiBlock.getPredecessors().get(i);
+ List<BasicBlock> predecessors = phiBlock.getPredecessors();
+ int size = predecessors.size();
+ for (int i = 0; i < size; i++) {
+ BasicBlock predecessor = predecessors.get(i);
if (isExecutableEdge(predecessor.getNumber(), phiBlockNumber)) {
element = element.meet(getLatticeElement(phi.getOperand(i)));
+ // bottom lattice can no longer be changed, thus no need to continue
+ if (element.isBottom()) {
+ break;
+ }
}
}
- LatticeElement currentPhiElement = getLatticeElement(phi);
- if (!element.isTop() && currentPhiElement.meet(element) != currentPhiElement) {
- ssaEdges.add(phi);
- setLatticeElement(phi, element);
+ if (!element.isTop()) {
+ LatticeElement currentPhiElement = getLatticeElement(phi);
+ if (currentPhiElement.meet(element) != currentPhiElement) {
+ ssaEdges.add(phi);
+ setLatticeElement(phi, element);
+ }
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
index ee2c6ab..163c20b 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
@@ -569,13 +569,15 @@
}
/**
- * Unlinks this block from a single predecessor and successor.
+ * Unlinks the current block based on the assumption that it is a catch handler.
*
- * @return Returns the unlinked successor
+ * Catch handlers always have only one predecessor and at most one successor.
+ * That is because we have edge-split form for all exceptional flow.
*/
- public BasicBlock unlinkSingle() {
- unlinkSinglePredecessor();
- return unlinkSingleSuccessor();
+ public void unlinkCatchHandler() {
+ assert predecessors.size() == 1;
+ predecessors.get(0).removeSuccessor(this);
+ predecessors.clear();
}
public void detachAllSuccessors() {
@@ -590,34 +592,41 @@
assert successor.predecessors.contains(this);
List<BasicBlock> removedBlocks = new ArrayList<>();
for (BasicBlock dominated : dominator.dominatedBlocks(successor)) {
+ dominated.cleanForRemoval();
removedBlocks.add(dominated);
- for (BasicBlock block : dominated.successors) {
- block.removePredecessor(dominated);
- }
- dominated.successors.clear();
- for (BasicBlock block : dominated.predecessors) {
- block.removeSuccessor(dominated);
- }
- dominated.predecessors.clear();
- for (Phi phi : dominated.getPhis()) {
- for (Value operand : phi.getOperands()) {
- operand.removePhiUser(phi);
- }
- }
- dominated.getPhis().clear();
- for (Instruction instruction : dominated.getInstructions()) {
- for (Value value : instruction.inValues) {
- value.removeUser(instruction);
- }
- for (Value value : instruction.getDebugValues()) {
- value.removeDebugUser(instruction);
- }
- }
}
assert blocksClean(removedBlocks);
return removedBlocks;
}
+ public void cleanForRemoval() {
+ for (BasicBlock block : successors) {
+ block.removePredecessor(this);
+ }
+ successors.clear();
+ for (BasicBlock block : predecessors) {
+ block.removeSuccessor(this);
+ }
+ predecessors.clear();
+ for (Phi phi : getPhis()) {
+ for (Value operand : phi.getOperands()) {
+ operand.removePhiUser(phi);
+ }
+ }
+ getPhis().clear();
+ for (Instruction instruction : getInstructions()) {
+ if (instruction.outValue != null) {
+ instruction.outValue.clearUsers();
+ }
+ for (Value value : instruction.inValues) {
+ value.removeUser(instruction);
+ }
+ for (Value value : instruction.getDebugValues()) {
+ value.removeDebugUser(instruction);
+ }
+ }
+ }
+
public void linkCatchSuccessors(List<DexType> guards, List<BasicBlock> targets) {
List<Integer> successorIndexes = new ArrayList<>(targets.size());
for (BasicBlock target : targets) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCode.java b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
index bf62fed..2613aae 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCode.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
@@ -663,4 +663,31 @@
public boolean verifyNoColorsInUse() {
return usedMarkingColors == 0;
}
+
+ public void removeUnreachableBlocks() {
+ int color = reserveMarkingColor();
+ Queue<BasicBlock> worklist = new ArrayDeque<>();
+ worklist.add(blocks.getFirst());
+ while (!worklist.isEmpty()) {
+ BasicBlock block = worklist.poll();
+ if (block.isMarked(color)) {
+ continue;
+ }
+ block.mark(color);
+ for (BasicBlock successor : block.getSuccessors()) {
+ if (!successor.isMarked(color)) {
+ worklist.add(successor);
+ }
+ }
+ }
+ ListIterator<BasicBlock> blockIterator = listIterator();
+ while (blockIterator.hasNext()) {
+ BasicBlock current = blockIterator.next();
+ if (!current.isMarked(color)) {
+ current.cleanForRemoval();
+ blockIterator.remove();
+ }
+ }
+ returnMarkingColor(color);
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
index 96b2122..182bb49 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
@@ -1165,7 +1165,7 @@
@Override
public int computeSize(DexBuilder builder) {
Move move = getMove();
- int srcRegister = builder.allocatedRegister(move.src(), move.getNumber());
+ int srcRegister = builder.argumentOrAllocateRegister(move.src(), move.getNumber());
int destRegister = builder.allocatedRegister(move.dest(), move.getNumber());
if (srcRegister == destRegister) {
size = 1;
@@ -1183,8 +1183,8 @@
public void addInstructions(DexBuilder builder, List<Instruction> instructions) {
Move move = getMove();
MoveType moveType = MoveType.fromValueType(move.outType());
+ int src = builder.argumentOrAllocateRegister(move.src(), move.getNumber());
int dest = builder.allocatedRegister(move.dest(), move.getNumber());
- int src = builder.allocatedRegister(move.src(), move.getNumber());
Instruction instruction = null;
switch (size) {
case 1:
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
index 228f594..9c6bb86 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
@@ -172,7 +172,9 @@
}
@Override
- public void buildInstruction(IRBuilder builder, int instructionIndex) throws ApiLevelException {
+ public void buildInstruction(
+ IRBuilder builder, int instructionIndex, boolean firstBlockInstruction)
+ throws ApiLevelException {
updateCurrentCatchHandlers(instructionIndex);
updateDebugPosition(instructionIndex, builder);
currentDexInstruction = code.instructions[instructionIndex];
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
index 5bea64e..13d66c2 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
@@ -504,7 +504,7 @@
addToWorklist(info.block, i);
break;
}
- source.buildInstruction(this, i);
+ source.buildInstruction(this, i, i == item.firstInstructionIndex);
}
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
index 2298bc6..b5dfdbb 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
@@ -49,7 +49,6 @@
import org.objectweb.asm.Type;
import org.objectweb.asm.tree.AbstractInsnNode;
import org.objectweb.asm.tree.FieldInsnNode;
-import org.objectweb.asm.tree.FrameNode;
import org.objectweb.asm.tree.IincInsnNode;
import org.objectweb.asm.tree.InsnNode;
import org.objectweb.asm.tree.IntInsnNode;
@@ -489,7 +488,9 @@
}
@Override
- public void buildInstruction(IRBuilder builder, int instructionIndex) throws ApiLevelException {
+ public void buildInstruction(
+ IRBuilder builder, int instructionIndex, boolean firstBlockInstruction)
+ throws ApiLevelException {
if (instructionIndex == EXCEPTIONAL_SYNC_EXIT_OFFSET) {
buildExceptionalPostlude(builder);
return;
@@ -499,8 +500,17 @@
assert verifyExceptionEdgesAreRecorded(insn);
// If a new block is starting here, we restore the computed JarState.
- if (builder.getCFG().containsKey(instructionIndex) || instructionIndex == 0) {
+ // current position needs to be compute only for the first instruction of a block, thereafter
+ // current position will be updated by LineNumberNode into this block.
+ if (firstBlockInstruction || instructionIndex == 0) {
state.restoreState(instructionIndex);
+ // Don't include line changes when processing a label. Doing so will end up emitting local
+ // writes after the line has changed and thus causing locals to become visible too late.
+ currentPosition =
+ getDebugPositionAtOffset(
+ ((instructionIndex > 0) && (insn instanceof LabelNode))
+ ? instructionIndex - 1
+ : instructionIndex);
}
String preInstructionState;
@@ -508,14 +518,6 @@
preInstructionState = state.toString();
}
- // Don't include line changes when processing a label. Doing so will end up emitting local
- // writes after the line has changed and thus causing locals to become visible too late.
- currentPosition =
- getDebugPositionAtOffset(
- ((instructionIndex > 0) && (insn instanceof LabelNode))
- ? instructionIndex - 1
- : instructionIndex);
-
build(insn, builder);
if (Log.ENABLED && !(insn instanceof LineNumberNode)) {
@@ -1171,9 +1173,6 @@
case AbstractInsnNode.LINE:
updateState((LineNumberNode) insn);
break;
- case AbstractInsnNode.FRAME:
- updateState((FrameNode) insn);
- break;
default:
throw new Unreachable("Unexpected instruction " + insn);
}
@@ -1795,11 +1794,6 @@
// Intentionally empty.
}
- private void updateState(FrameNode insn) {
- assert application.options.isGeneratingClassFiles();
- // Intentionally empty.
- }
-
private void updateStateForConversion(Type from, Type to) {
state.pop();
state.push(to);
@@ -1854,9 +1848,6 @@
case AbstractInsnNode.LINE:
build((LineNumberNode) insn, builder);
break;
- case AbstractInsnNode.FRAME:
- build((FrameNode) insn, builder);
- break;
default:
throw new Unreachable("Unexpected instruction " + insn);
}
@@ -2836,11 +2827,6 @@
builder.addDebugPosition(currentPosition);
}
- private void build(FrameNode insn, IRBuilder builder) {
- assert application.options.isGeneratingClassFiles();
- // Intentionally empty.
- }
-
@Override
public Position getDebugPositionAtOffset(int offset) {
if (offset == EXCEPTIONAL_SYNC_EXIT_OFFSET) {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/SourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/SourceCode.java
index 13d1551..be1c670 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/SourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/SourceCode.java
@@ -47,7 +47,9 @@
// Delegates for IR building.
void buildPrelude(IRBuilder builder);
- void buildInstruction(IRBuilder builder, int instructionIndex) throws ApiLevelException;
+
+ void buildInstruction(IRBuilder builder, int instructionIndex, boolean firstBlockInstruction)
+ throws ApiLevelException;
void buildPostlude(IRBuilder builder);
// Helper to resolve switch payloads and build switch instructions (dex code only).
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java b/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java
index f767e1b..531989e 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java
@@ -5,7 +5,6 @@
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.CatchHandlers;
-import com.android.tools.r8.ir.code.DominatorTree;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionListIterator;
@@ -20,50 +19,43 @@
public static void removeDeadCode(
IRCode code, CodeRewriter codeRewriter, InternalOptions options) {
+ removeUnneededCatchHandlers(code);
Queue<BasicBlock> worklist = new LinkedList<>();
- int color = code.reserveMarkingColor();
worklist.addAll(code.blocks);
for (BasicBlock block = worklist.poll(); block != null; block = worklist.poll()) {
- if (block.isMarked(color)) {
- // Ignore marked blocks, as they are scheduled for removal.
- continue;
- }
- removeDeadInstructions(worklist, code, block, options, color);
- removeDeadPhis(worklist, block, options, color);
- removeUnneededCatchHandlers(worklist, block, code, color);
+ removeDeadInstructions(worklist, code, block, options);
+ removeDeadPhis(worklist, block, options);
}
- code.removeMarkedBlocks(color);
- code.returnMarkingColor(color);
assert code.isConsistentSSA();
codeRewriter.rewriteMoveResult(code);
}
// Add the block from where the value originates to the worklist.
- private static void updateWorklist(Queue<BasicBlock> worklist, Value value, int color) {
+ private static void updateWorklist(Queue<BasicBlock> worklist, Value value) {
BasicBlock block = null;
if (value.isPhi()) {
block = value.asPhi().getBlock();
} else if (value.definition.hasBlock()) {
block = value.definition.getBlock();
}
- if (block != null && !block.isMarked(color)) {
+ if (block != null) {
worklist.add(block);
}
}
// Add all blocks from where the in/debug-values to the instruction originates.
private static void updateWorklist(
- Queue<BasicBlock> worklist, Instruction instruction, int color) {
+ Queue<BasicBlock> worklist, Instruction instruction) {
for (Value inValue : instruction.inValues()) {
- updateWorklist(worklist, inValue, color);
+ updateWorklist(worklist, inValue);
}
for (Value debugValue : instruction.getDebugValues()) {
- updateWorklist(worklist, debugValue, color);
+ updateWorklist(worklist, debugValue);
}
}
private static void removeDeadPhis(Queue<BasicBlock> worklist, BasicBlock block,
- InternalOptions options, int color) {
+ InternalOptions options) {
Iterator<Phi> phiIt = block.getPhis().iterator();
while (phiIt.hasNext()) {
Phi phi = phiIt.next();
@@ -71,15 +63,14 @@
phiIt.remove();
for (Value operand : phi.getOperands()) {
operand.removePhiUser(phi);
- updateWorklist(worklist, operand, color);
+ updateWorklist(worklist, operand);
}
}
}
}
private static void removeDeadInstructions(
- Queue<BasicBlock> worklist, IRCode code, BasicBlock block, InternalOptions options,
- int color) {
+ Queue<BasicBlock> worklist, IRCode code, BasicBlock block, InternalOptions options) {
InstructionListIterator iterator = block.listIterator(block.getInstructions().size());
while (iterator.hasPrevious()) {
Instruction current = iterator.previous();
@@ -99,7 +90,7 @@
if (!outValue.isDead(options)) {
continue;
}
- updateWorklist(worklist, current, color);
+ updateWorklist(worklist, current);
// All users will be removed for this instruction. Eagerly clear them so further inspection
// of this instruction during dead code elimination will terminate here.
outValue.clearUsers();
@@ -107,22 +98,15 @@
}
}
- private static void removeUnneededCatchHandlers(Queue<BasicBlock> worklist, BasicBlock block,
- IRCode code, int color) {
- if (block.hasCatchHandlers() && !block.canThrow()) {
- CatchHandlers<BasicBlock> handlers = block.getCatchHandlers();
- for (BasicBlock target : handlers.getUniqueTargets()) {
- DominatorTree dominatorTree = new DominatorTree(code);
- for (BasicBlock unlinked : block.unlink(target, dominatorTree)) {
- if (!unlinked.isMarked(color)) {
- Iterator<Instruction> iterator = unlinked.iterator();
- while (iterator.hasNext()) {
- updateWorklist(worklist, iterator.next(), color);
- }
- unlinked.mark(color);
- }
+ private static void removeUnneededCatchHandlers(IRCode code) {
+ for (BasicBlock block : code.blocks) {
+ if (block.hasCatchHandlers() && !block.canThrow()) {
+ CatchHandlers<BasicBlock> handlers = block.getCatchHandlers();
+ for (BasicBlock target : handlers.getUniqueTargets()) {
+ target.unlinkCatchHandler();
}
}
}
+ code.removeUnreachableBlocks();
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java
index dba6183..126b127 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java
@@ -859,7 +859,8 @@
}
@Override
- public void buildInstruction(IRBuilder builder, int instructionIndex) {
+ public void buildInstruction(
+ IRBuilder builder, int instructionIndex, boolean firstBlockInstruction) {
if (instructionIndex == outline.templateInstructions.size()) {
if (outline.returnType == dexItemFactory.voidType) {
builder.addReturn();
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 85e0060..6803c75 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
@@ -513,9 +513,12 @@
// Compute the set of registers that is used based on all live intervals.
Set<Integer> usedRegisters = new HashSet<>();
for (LiveIntervals intervals : liveIntervals) {
- addRegisterIfUsed(usedRegisters, intervals);
- for (LiveIntervals childIntervals : intervals.getSplitChildren()) {
- addRegisterIfUsed(usedRegisters, childIntervals);
+ // Argument sentinel values do not occupy any registers.
+ if (!isArgumentSentinelIntervals(intervals)) {
+ addRegisterIfUsed(usedRegisters, intervals);
+ for (LiveIntervals childIntervals : intervals.getSplitChildren()) {
+ addRegisterIfUsed(usedRegisters, childIntervals);
+ }
}
}
// Additionally, we have used temporary registers for parallel move scheduling, those
@@ -535,6 +538,11 @@
unusedRegisters = computed;
}
+ private boolean isArgumentSentinelIntervals(LiveIntervals intervals) {
+ return intervals.isArgumentInterval() &&
+ (intervals.getPreviousConsecutive() == null || intervals.getNextConsecutive() == null);
+ }
+
private void addRegisterIfUsed(Set<Integer> used, LiveIntervals intervals) {
boolean unused = intervals.isSpilledAndRematerializable(this);
if (!unused) {
diff --git a/src/main/java/com/android/tools/r8/ir/synthetic/SyntheticSourceCode.java b/src/main/java/com/android/tools/r8/ir/synthetic/SyntheticSourceCode.java
index 5d03dfd..5a6cc2d 100644
--- a/src/main/java/com/android/tools/r8/ir/synthetic/SyntheticSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/synthetic/SyntheticSourceCode.java
@@ -188,7 +188,8 @@
}
@Override
- public final void buildInstruction(IRBuilder builder, int instructionIndex)
+ public final void buildInstruction(
+ IRBuilder builder, int instructionIndex, boolean firstBlockInstruction)
throws ApiLevelException {
constructors.get(instructionIndex).accept(builder);
}
diff --git a/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java b/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java
index c815cf8..5847a80 100644
--- a/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java
+++ b/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java
@@ -33,8 +33,9 @@
method.getCode().registerCodeReferences(targetExtractor);
DexMethod target = targetExtractor.getTarget();
InvokeKind kind = targetExtractor.getKind();
- if (target != null &&
- target.proto == method.method.proto) {
+ // javac-generated visibility forward bridge method has same descriptor (name, signature and
+ // return type).
+ if (target != null && target.hasSameProtoAndName(method.method)) {
assert !method.accessFlags.isPrivate() && !method.accessFlags.isConstructor();
if (kind == InvokeKind.SUPER) {
// This is a visibility forward, so check for the direct target.
diff --git a/src/main/java/com/android/tools/r8/utils/FeatureClassMapping.java b/src/main/java/com/android/tools/r8/utils/FeatureClassMapping.java
index 5b96201..6893eb1 100644
--- a/src/main/java/com/android/tools/r8/utils/FeatureClassMapping.java
+++ b/src/main/java/com/android/tools/r8/utils/FeatureClassMapping.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.utils;
import com.android.tools.r8.ArchiveClassFileProvider;
+import com.android.tools.r8.dexsplitter.DexSplitter.FeatureJar;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
@@ -58,20 +59,15 @@
return mapping;
}
- public static FeatureClassMapping fromJarFiles(List<String> jarFiles)
+ public static FeatureClassMapping fromJarFiles(List<FeatureJar> featureJars)
throws FeatureMappingException, IOException {
FeatureClassMapping mapping = new FeatureClassMapping();
- for (String jar : jarFiles) {
- Path jarPath = Paths.get(jar);
- String featureName = jarPath.getFileName().toString();
- if (featureName.endsWith(".jar") || featureName.endsWith(".zip")) {
- featureName = featureName.substring(0, featureName.length() - 4);
- }
-
+ for (FeatureJar featureJar : featureJars) {
+ Path jarPath = Paths.get(featureJar.getJar());
ArchiveClassFileProvider provider = new ArchiveClassFileProvider(jarPath);
for (String javaDescriptor : provider.getClassDescriptors()) {
String javaType = DescriptorUtils.descriptorToJavaType(javaDescriptor);
- mapping.addMapping(javaType, featureName);
+ mapping.addMapping(javaType, featureJar.getOutputName());
}
}
return mapping;
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 dc0cd0f..42775d2 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -9,6 +9,7 @@
import com.android.tools.r8.ProgramConsumer;
import com.android.tools.r8.StringConsumer;
import com.android.tools.r8.dex.Marker;
+import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.InvalidDebugInfoException;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
@@ -100,6 +101,8 @@
public boolean verbose = false;
// Silencing output.
public boolean quiet = false;
+ // Throw exception if there is a warning about invalid debug info.
+ public boolean invalidDebugInfoFatal = false;
// Hidden marker for classes.dex
private boolean hasMarker = false;
@@ -268,6 +271,9 @@
public void warningInvalidDebugInfo(
DexEncodedMethod method, Origin origin, InvalidDebugInfoException e) {
+ if (invalidDebugInfoFatal) {
+ throw new CompilationError("Fatal warning: Invalid debug info", e);
+ }
synchronized (warningInvalidDebugInfo) {
warningInvalidDebugInfo.computeIfAbsent(
origin, k -> new ArrayList<>()).add(new Pair<>(method, e.getMessage()));
diff --git a/src/test/java/com/android/tools/r8/TestBase.java b/src/test/java/com/android/tools/r8/TestBase.java
index d7d8b99..a29d673 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -398,6 +398,13 @@
+ (obfuscate ? "-printmapping\n" : "-dontobfuscate\n");
}
+ public static String keepMainProguardConfiguration(
+ String clazz, boolean allowaccessmodification, boolean obfuscate) {
+ return keepMainProguardConfiguration(clazz)
+ + (allowaccessmodification ? "-allowaccessmodification\n" : "")
+ + (obfuscate ? "-printmapping\n" : "-dontobfuscate\n");
+ }
+
/**
* Run application on the specified version of Art with the specified main class.
*/
diff --git a/src/test/java/com/android/tools/r8/bridgeremoval/RemoveVisibilityBridgeMethodsTest.java b/src/test/java/com/android/tools/r8/bridgeremoval/RemoveVisibilityBridgeMethodsTest.java
index 6e3c114..35a7613 100644
--- a/src/test/java/com/android/tools/r8/bridgeremoval/RemoveVisibilityBridgeMethodsTest.java
+++ b/src/test/java/com/android/tools/r8/bridgeremoval/RemoveVisibilityBridgeMethodsTest.java
@@ -4,14 +4,26 @@
package com.android.tools.r8.bridgeremoval;
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.ProcessResult;
import com.android.tools.r8.bridgeremoval.bridgestoremove.Main;
import com.android.tools.r8.bridgeremoval.bridgestoremove.Outer;
+import com.android.tools.r8.jasmin.JasminBuilder;
+import com.android.tools.r8.jasmin.JasminBuilder.ClassBuilder;
+import com.android.tools.r8.utils.AndroidApp;
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.google.common.collect.ImmutableList;
import java.lang.reflect.Method;
+import java.nio.file.Path;
+import java.util.Collections;
import java.util.List;
import org.junit.Test;
@@ -40,4 +52,81 @@
public void testWithoutObfuscation() throws Exception {
run(false);
}
+
+ @Test
+ public void regressionTest_b76383728_WithObfuscation() throws Exception {
+ runRegressionTest_b76383728(true);
+ }
+
+ @Test
+ public void regressionTest_b76383728_WithoutObfuscation() throws Exception {
+ runRegressionTest_b76383728(false);
+ }
+
+ /**
+ * Regression test for b76383728 to make sure we correctly identify and remove real visibility
+ * forward bridge methods synthesized by javac.
+ */
+ private void runRegressionTest_b76383728(boolean obfuscate) throws Exception {
+ JasminBuilder jasminBuilder = new JasminBuilder();
+
+ ClassBuilder superClass = jasminBuilder.addClass("SuperClass");
+ superClass.addDefaultConstructor();
+ superClass.addVirtualMethod("method", Collections.emptyList(), "Ljava/lang/String;",
+ ".limit stack 1",
+ "ldc \"Hello World\"",
+ "areturn");
+
+ // Generate a subclass with a method with same
+ ClassBuilder subclass = jasminBuilder.addClass("SubClass", superClass.name);
+ subclass.addBridgeMethod("getMethod", Collections.emptyList(), "Ljava/lang/String;",
+ ".limit stack 1",
+ "aload_0",
+ "invokespecial " + superClass.name + "/method()Ljava/lang/String;",
+ "areturn");
+
+ ClassBuilder mainClass = jasminBuilder.addClass("Main");
+ mainClass.addMainMethod(
+ ".limit stack 3",
+ ".limit locals 2",
+ "getstatic java/lang/System/out Ljava/io/PrintStream;",
+ "new " + subclass.name,
+ "dup",
+ "invokespecial " + subclass.name + "/<init>()V",
+ "invokevirtual " + subclass.name + "/getMethod()Ljava/lang/String;",
+ "invokevirtual java/io/PrintStream/print(Ljava/lang/String;)V",
+ "return"
+ );
+
+ final String mainClassName = mainClass.name;
+
+ String proguardConfig = keepMainProguardConfiguration(mainClass.name, true, obfuscate);
+
+ // Run input program on java.
+ Path outputDirectory = temp.newFolder().toPath();
+ jasminBuilder.writeClassFiles(outputDirectory);
+ ProcessResult javaResult = ToolHelper.runJava(outputDirectory, mainClassName);
+ assertEquals(0, javaResult.exitCode);
+
+ AndroidApp optimizedApp = compileWithR8(jasminBuilder.build(), proguardConfig,
+ // Disable inlining to avoid the (short) tested method from being inlined then removed.
+ internalOptions -> internalOptions.enableInlining = false);
+
+ // Run optimized (output) program on ART
+ String artResult = runOnArt(optimizedApp, mainClassName);
+ assertEquals(javaResult.stdout, artResult);
+
+ DexInspector inspector = new DexInspector(optimizedApp);
+
+ ClassSubject classSubject = inspector.clazz(superClass.name);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject
+ .method("java.lang.String", "method", Collections.emptyList());
+ assertThat(methodSubject, isPresent());
+
+ classSubject = inspector.clazz(subclass.name);
+ assertThat(classSubject, isPresent());
+ methodSubject = classSubject.method("java.lang.String", "getMethod", Collections.emptyList());
+ assertThat(methodSubject, isPresent());
+ }
}
diff --git a/src/test/java/com/android/tools/r8/cf/DebugInfoTest.java b/src/test/java/com/android/tools/r8/cf/DebugInfoTest.java
new file mode 100644
index 0000000..6e009e8
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/cf/DebugInfoTest.java
@@ -0,0 +1,27 @@
+// 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.cf;
+
+public class DebugInfoTest {
+
+ public static void main(String[] args) {
+ if (args.length > 0) {
+ arg = args.length % 2 == 0;
+ DebugInfoTest.method();
+ }
+ }
+
+ private static boolean arg;
+
+ private static void method() {
+ int intVar;
+ if (arg) {
+ float floatVar1 = 0f;
+ intVar = (int) floatVar1;
+ } else {
+ float floatVar2 = 0f;
+ intVar = (int) floatVar2;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/cf/DebugInfoTestRunner.java b/src/test/java/com/android/tools/r8/cf/DebugInfoTestRunner.java
new file mode 100644
index 0000000..fed7637
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/cf/DebugInfoTestRunner.java
@@ -0,0 +1,68 @@
+// 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.cf;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.ClassFileConsumer;
+import com.android.tools.r8.CompilationMode;
+import com.android.tools.r8.ProgramConsumer;
+import com.android.tools.r8.R8Command;
+import com.android.tools.r8.R8Command.Builder;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.errors.CompilationError;
+import com.android.tools.r8.errors.InvalidDebugInfoException;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.ThrowingConsumer;
+import java.nio.file.Path;
+import org.junit.Test;
+
+public class DebugInfoTestRunner extends TestBase {
+ static final Class CLASS = DebugInfoTest.class;
+
+ @Test
+ public void test() throws Exception {
+ ProcessResult runInput =
+ ToolHelper.runJava(ToolHelper.getClassPathForTests(), CLASS.getCanonicalName());
+ assertEquals(0, runInput.exitCode);
+ Path out1 = temp.getRoot().toPath().resolve("out1.zip");
+ build(
+ builder -> builder.addClassProgramData(ToolHelper.getClassAsBytes(CLASS), Origin.unknown()),
+ new ClassFileConsumer.ArchiveConsumer(out1));
+ ProcessResult run1 = ToolHelper.runJava(out1, CLASS.getCanonicalName());
+ assertEquals(runInput.toString(), run1.toString());
+ Path out2 = temp.getRoot().toPath().resolve("out2.zip");
+ boolean invalidDebugInfo = false;
+ try {
+ build(builder -> builder.addProgramFiles(out1), new ClassFileConsumer.ArchiveConsumer(out2));
+ } catch (CompilationError e) {
+ invalidDebugInfo = e.getCause() instanceof InvalidDebugInfoException;
+ }
+ // TODO(b/77522100): Change to assertFalse when fixed.
+ assertTrue(invalidDebugInfo);
+ if (!invalidDebugInfo) {
+ ProcessResult run2 = ToolHelper.runJava(out2, CLASS.getCanonicalName());
+ assertEquals(runInput.toString(), run2.toString());
+ }
+ }
+
+ private void build(ThrowingConsumer<Builder, Exception> input, ProgramConsumer consumer)
+ throws Exception {
+ Builder builder =
+ R8Command.builder()
+ .setMode(CompilationMode.DEBUG)
+ .addLibraryFiles(ToolHelper.getAndroidJar(ToolHelper.getMinApiLevelForDexVm()))
+ .setProgramConsumer(consumer);
+ input.accept(builder);
+ ToolHelper.runR8(
+ builder.build(),
+ o -> {
+ o.invalidDebugInfoFatal = true;
+ o.enableInlining = false;
+ });
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterTests.java b/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterTests.java
index cf87d46..2db8fe0 100644
--- a/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterTests.java
+++ b/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterTests.java
@@ -84,14 +84,15 @@
.addProgramFiles(Paths.get(CLASS4_LAMBDA_INTERFACE))
.build());
- Path output = temp.getRoot().toPath().resolve("output");
+ Path output = temp.newFolder().toPath().resolve("output");
+ Files.createDirectory(output);
Path splitSpec = createSplitSpec();
if (useOptions) {
Options options = new Options();
options.addInputArchive(inputZip.toString());
options.setFeatureSplitMapping(splitSpec.toString());
- options.setSplitBaseName(output.toString());
+ options.setOutput(output.toString());
DexSplitter.run(options);
} else {
DexSplitter.main(
@@ -102,8 +103,8 @@
});
}
- Path base = output.getParent().resolve("output.base.zip");
- Path feature = output.getParent().resolve("output.feature1.zip");
+ Path base = output.resolve("base").resolve("classes.dex");
+ Path feature = output.resolve("feature1").resolve("classes.dex");
validateUnobfuscatedOutput(base, feature);
}
@@ -201,7 +202,8 @@
.addProgramFiles(Paths.get(CLASS4_LAMBDA_INTERFACE))
.build());
- Path output = temp.getRoot().toPath().resolve("output");
+ Path output = temp.newFolder().toPath().resolve("output");
+ Files.createDirectory(output);
Path baseJar = temp.getRoot().toPath().resolve("base.jar");
Path featureJar = temp.getRoot().toPath().resolve("feature1.jar");
ZipOutputStream baseStream = new ZipOutputStream(Files.newOutputStream(baseJar));
@@ -233,14 +235,16 @@
featureStream.write(Files.readAllBytes(Paths.get(CLASS4_LAMBDA_INTERFACE)));
featureStream.closeEntry();
featureStream.close();
+ // Make sure that we can pass in a name for the output.
+ String specificOutputName = "renamed";
if (useOptions) {
Options options = new Options();
options.addInputArchive(inputZip.toString());
- options.setSplitBaseName(output.toString());
+ options.setOutput(output.toString());
if (explicitBase) {
options.addFeatureJar(baseJar.toString());
}
- options.addFeatureJar(featureJar.toString());
+ options.addFeatureJar(featureJar.toString(), specificOutputName);
DexSplitter.run(options);
} else {
List<String> args = Lists.newArrayList(
@@ -249,7 +253,7 @@
"--output",
output.toString(),
"--feature-jar",
- featureJar.toString());
+ featureJar.toString().concat(":").concat(specificOutputName));
if (explicitBase) {
args.add("--feature-jar");
args.add(baseJar.toString());
@@ -257,8 +261,8 @@
DexSplitter.main(args.toArray(new String[0]));
}
- Path base = output.getParent().resolve("output.base.zip");
- Path feature = output.getParent().resolve("output.feature1.zip");
+ Path base = output.resolve("base").resolve("classes.dex");
+ Path feature = output.resolve(specificOutputName).resolve("classes.dex");;
validateUnobfuscatedOutput(base, feature);
}
@@ -284,7 +288,8 @@
.addProguardConfiguration(getProguardConf(), null)
.build());
- Path outputDex = temp.getRoot().toPath().resolve("output");
+ Path outputDex = temp.newFolder().toPath().resolve("output");
+ Files.createDirectory(outputDex);
Path splitSpec = createSplitSpec();
DexSplitter.main(
@@ -295,8 +300,8 @@
"--proguard-map", proguardMap.toString()
});
- Path base = outputDex.getParent().resolve("output.base.zip");
- Path feature = outputDex.getParent().resolve("output.feature1.zip");
+ Path base = outputDex.resolve("base").resolve("classes.dex");
+ Path feature = outputDex.resolve("feature1").resolve("classes.dex");
String class3 = "dexsplitsample.Class3";
// We should still be able to run the Class3 which we kept, it has a call to the obfuscated
// class1 which is in base.
diff --git a/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java b/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java
index 64d092b..ff3f81a 100644
--- a/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java
+++ b/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java
@@ -120,6 +120,15 @@
return addMethod("public", name, argumentTypes, returnType, lines);
}
+ public MethodSignature addBridgeMethod(
+ String name,
+ List<String> argumentTypes,
+ String returnType,
+ String... lines) {
+ makeInit = true;
+ return addMethod("public bridge", name, argumentTypes, returnType, lines);
+ }
+
public MethodSignature addPrivateVirtualMethod(
String name,
List<String> argumentTypes,
diff --git a/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java b/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java
index 35df33e..a6d0177 100644
--- a/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java
+++ b/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java
@@ -707,7 +707,8 @@
}
@Override
- public void buildInstruction(IRBuilder builder, int instructionIndex) {
+ public void buildInstruction(
+ IRBuilder builder, int instructionIndex, boolean firstBlockInstruction) {
assert instructionIndex == 0;
builder.addReturn();
}
diff --git a/third_party/benchmarks/android-sdk.tar.gz.sha1 b/third_party/benchmarks/android-sdk.tar.gz.sha1
new file mode 100644
index 0000000..efc177e
--- /dev/null
+++ b/third_party/benchmarks/android-sdk.tar.gz.sha1
@@ -0,0 +1 @@
+d033ea956b548b5fb87a2b37c584e87cfca33717
\ No newline at end of file
diff --git a/third_party/benchmarks/antenna-pod.tar.gz.sha1 b/third_party/benchmarks/antenna-pod.tar.gz.sha1
new file mode 100644
index 0000000..c3703e0
--- /dev/null
+++ b/third_party/benchmarks/antenna-pod.tar.gz.sha1
@@ -0,0 +1 @@
+f6f15c4efb79772ce8af147e0e2639db1c585fc7
\ No newline at end of file
diff --git a/third_party/benchmarks/gradle-java-1.6.tar.gz.sha1 b/third_party/benchmarks/gradle-java-1.6.tar.gz.sha1
new file mode 100644
index 0000000..e30dbfc
--- /dev/null
+++ b/third_party/benchmarks/gradle-java-1.6.tar.gz.sha1
@@ -0,0 +1 @@
+e87ccb7a5cb95555f46a6c8adbbcec99ec14b578
\ No newline at end of file
diff --git a/third_party/benchmarks/init-script.gradle b/third_party/benchmarks/init-script.gradle
new file mode 100644
index 0000000..3434a4a
--- /dev/null
+++ b/third_party/benchmarks/init-script.gradle
@@ -0,0 +1,74 @@
+def r8RootDir = System.properties['r8.root.dir']
+
+allprojects {
+ buildscript {
+ repositories {
+ maven { url r8RootDir+ '/third_party/gradle-plugin' }
+ // We don't use 'google()' in order to support projects using gradle
+ // lower to 4.1 version.
+ maven { url 'https://maven.google.com' }
+ jcenter()
+ }
+ dependencies {
+ classpath files(r8RootDir + '/build/libs/r8.jar')
+ classpath 'com.android.tools.build:gradle:3.2.0-dev'
+ }
+ }
+ repositories {
+ maven { url r8RootDir+ '/third_party/gradle-plugin' }
+ maven { url 'https://maven.google.com' }
+ jcenter()
+ mavenCentral()
+ }
+}
+
+//
+// Dump detailed timings per subtask
+//
+import java.util.concurrent.TimeUnit;
+class TimingsListener implements TaskExecutionListener, BuildListener {
+ private long startTimeInNs;
+ private timings = []
+
+ @Override
+ void beforeExecute(Task task) {
+ startTimeInNs = System.nanoTime();
+ }
+
+ @Override
+ void afterExecute(Task task, TaskState taskState) {
+ def ms = TimeUnit.MILLISECONDS.convert(
+ System.nanoTime() - startTimeInNs, TimeUnit.NANOSECONDS);
+ timings.add([task.path,ms])
+ }
+
+ @Override
+ void buildStarted(Gradle gradle) {
+ }
+
+ @Override
+ void buildFinished(BuildResult result) {
+ def total=0
+ for (timing in timings) {
+ total += timing[1]
+ }
+
+ for (timing in timings) {
+ printf "BENCH,%s,%s\n", timing
+ }
+
+ printf "BENCH,totalGradleTasks,%s\n", total
+
+ }
+
+ @Override
+ void projectsEvaluated(Gradle gradle) {}
+
+ @Override
+ void projectsLoaded(Gradle gradle) {}
+
+ @Override
+ void settingsEvaluated(Settings settings) {}
+}
+
+gradle.addListener new TimingsListener()
\ No newline at end of file
diff --git a/third_party/benchmarks/santa-tracker.tar.gz.sha1 b/third_party/benchmarks/santa-tracker.tar.gz.sha1
index d37a000..93b21bd 100644
--- a/third_party/benchmarks/santa-tracker.tar.gz.sha1
+++ b/third_party/benchmarks/santa-tracker.tar.gz.sha1
@@ -1 +1 @@
-88150a9a2215f1f821ea9321e61b5fc8276dffd3
\ No newline at end of file
+c0985a5e801ca74dad932dcd734b297e9fabe374
\ No newline at end of file
diff --git a/third_party/benchmarks/tachiyomi.tar.gz.sha1 b/third_party/benchmarks/tachiyomi.tar.gz.sha1
new file mode 100644
index 0000000..b3cf670
--- /dev/null
+++ b/third_party/benchmarks/tachiyomi.tar.gz.sha1
@@ -0,0 +1 @@
+9101f02e39bb938d44794d22b9813628f63f87af
\ No newline at end of file
diff --git a/third_party/benchmarks/wordpress.tar.gz.sha1 b/third_party/benchmarks/wordpress.tar.gz.sha1
new file mode 100644
index 0000000..dea2249
--- /dev/null
+++ b/third_party/benchmarks/wordpress.tar.gz.sha1
@@ -0,0 +1 @@
+f06cd747d2e2880cd450e67a4a21c28cd24a8dd5
\ No newline at end of file
diff --git a/tools/build_sample_apk.py b/tools/build_sample_apk.py
index 5d691d9..e1b2deb 100755
--- a/tools/build_sample_apk.py
+++ b/tools/build_sample_apk.py
@@ -82,7 +82,8 @@
'-S', 'res',
'-m',
'-J', get_gen_path(app),
- '-F', os.path.join(get_bin_path(app), 'resources.ap_')]
+ '-F', os.path.join(get_bin_path(app), 'resources.ap_'),
+ '-G', os.path.join(get_build_dir(app), 'proguard_options')]
run_aapt(aapt, args)
def compile_with_javac(api, app):
diff --git a/tools/gradle.py b/tools/gradle.py
index 2303bce..862dd57 100755
--- a/tools/gradle.py
+++ b/tools/gradle.py
@@ -58,29 +58,41 @@
EnsureGradle()
EnsureShadow()
-def RunGradle(args, throw_on_failure=True):
+def RunGradleIn(gradleCmd, args, cwd, throw_on_failure=True, env=None):
EnsureDeps()
- cmd = [GRADLE]
+ cmd = [gradleCmd]
cmd.extend(args)
utils.PrintCmd(cmd)
- with utils.ChangedWorkingDirectory(utils.REPO_ROOT):
- return_value = subprocess.call(cmd)
+ with utils.ChangedWorkingDirectory(cwd):
+ return_value = subprocess.call(cmd, env=env)
if throw_on_failure and return_value != 0:
raise Exception('Failed to execute gradle')
return return_value
-def RunGradleExcludeDeps(args, throw_on_failure=True):
+def RunGradleWrapperIn(args, cwd, throw_on_failure=True, env=None):
+ return RunGradleIn('./gradlew', args, cwd, throw_on_failure, env=env)
+
+def RunGradle(args, throw_on_failure=True, env=None):
+ return RunGradleIn(GRADLE, args, utils.REPO_ROOT, throw_on_failure, env=env)
+
+def RunGradleExcludeDeps(args, throw_on_failure=True, env=None):
EnsureDeps()
args.append('-Pexclude_deps')
- RunGradle(args, throw_on_failure)
+ return RunGradle(args, throw_on_failure, env=env)
-def RunGradleGetOutput(args):
+def RunGradleInGetOutput(gradleCmd, args, cwd, env=None):
EnsureDeps()
- cmd = [GRADLE]
+ cmd = [gradleCmd]
cmd.extend(args)
utils.PrintCmd(cmd)
- with utils.ChangedWorkingDirectory(utils.REPO_ROOT):
- return subprocess.check_output(cmd)
+ with utils.ChangedWorkingDirectory(cwd):
+ return subprocess.check_output(cmd, env=env)
+
+def RunGradleWrapperInGetOutput(args, cwd, env=None):
+ return RunGradleInGetOutput('./gradlew', args, cwd, env=env)
+
+def RunGradleGetOutput(args, env=None):
+ return RunGradleInGetOutput(GRADLE, args, utils.REPO_ROOT, env=env)
def Main():
RunGradle(sys.argv[1:])
diff --git a/tools/run_on_app.py b/tools/run_on_app.py
index d8fac83..b9bcc60 100755
--- a/tools/run_on_app.py
+++ b/tools/run_on_app.py
@@ -88,15 +88,17 @@
'\'<BENCHMARKNAME>-<segment>(CodeSize): <bytes>\'')
return result.parse_args(argv)
-# Most apps have the -printmapping and -printseeds in the Proguard
-# configuration. However we don't want to write these files in these
-# locations. Instead generate an auxiliary Proguard configuration
-# placing these two output files together with the dex output.
+# Most apps have the -printmapping, -printseeds and -printusage in the
+# Proguard configuration. However we don't want to write these files
+# in the locations specified. Instead generate an auxiliary Proguard
+# configuration placing these two output files together with the dex
+# output.
def GenerateAdditionalProguardConfiguration(temp, outdir):
name = "output.config"
with open(os.path.join(temp, name), 'w') as f:
f.write('-printmapping ' + os.path.join(outdir, 'proguard.map') + "\n")
f.write('-printseeds ' + os.path.join(outdir, 'proguard.seeds') + "\n")
+ f.write('-printusage ' + os.path.join(outdir, 'proguard.usage') + "\n")
return os.path.abspath(f.name)
def main(argv):
diff --git a/tools/test_gradle_benchmarks.py b/tools/test_gradle_benchmarks.py
new file mode 100755
index 0000000..6720c4a
--- /dev/null
+++ b/tools/test_gradle_benchmarks.py
@@ -0,0 +1,205 @@
+#!/usr/bin/env python
+# 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.
+
+
+from __future__ import print_function
+import argparse
+import os
+import sys
+import utils
+import gradle
+from enum import Enum
+
+
+BENCHMARKS_ROOT_DIR = os.path.join(utils.REPO_ROOT, 'third_party', 'benchmarks')
+
+def parse_arguments():
+ parser = argparse.ArgumentParser(
+ description='Run D8 or DX on gradle apps located in'
+ ' third_party/benchmarks/.'
+ ' Report Golem-compatible RunTimeRaw values.')
+ parser.add_argument('--tool',
+ choices=['dx', 'd8'],
+ required=True,
+ help='Compiler tool to use.')
+ return parser.parse_args()
+
+
+class Benchmark:
+ class Tools(Enum):
+ D8 = 1
+ DX = 2
+
+ class DesugarMode(Enum):
+ D8_DESUGARING = 1
+ DESUGAR_TOOL = 2
+
+ displayName = ""
+ rootDirPath = ""
+ appPath = ""
+ moduleName = ""
+ buildCommand = ""
+ cleanCommand = ""
+ env = {}
+
+ def __init__(self, displayName, benchmarkDir, moduleName, buildCommand, cleanCommand):
+ self.displayName = displayName
+ self.rootDirPath = os.path.join(BENCHMARKS_ROOT_DIR, benchmarkDir.split(os.sep)[0])
+ self.appPath = os.path.join(BENCHMARKS_ROOT_DIR, benchmarkDir)
+ self.moduleName = moduleName
+ self.buildCommand = buildCommand
+ self.cleanCommand = cleanCommand
+ self.env = os.environ.copy()
+ self.env["ANDROID_HOME"] = os.path.join(utils.REPO_ROOT, 'third_party', 'benchmarks',
+ 'android-sdk')
+
+ def RunGradle(self, command, tool, desugarMode):
+
+ args = ['-Dr8.root.dir=' + utils.REPO_ROOT, '--init-script',
+ os.path.join(BENCHMARKS_ROOT_DIR, 'init-script.gradle')]
+
+ if tool == self.Tools.D8:
+ args.append('-Dandroid.enableD8=true')
+ elif tool == self.Tools.DX:
+ args.append('-Dandroid.enableD8=false')
+ else:
+ raise AssertionError("Unknown tool: " + repr(tool))
+
+ if desugarMode == self.DesugarMode.D8_DESUGARING:
+ args.append('-Dandroid.enableDesugar=false')
+ elif desugarMode == self.DesugarMode.DESUGAR_TOOL:
+ args.append('-Dandroid.enableDesugar=true')
+ else:
+ raise AssertionError("Unknown desugar mode: " + repr(desugarMode))
+
+ args.extend(command)
+
+ return gradle.RunGradleWrapperInGetOutput(args, self.appPath, env=self.env)
+
+ def Build(self, tool, desugarMode):
+ return self.RunGradle(self.buildCommand, tool, desugarMode)
+
+ def Clean(self):
+ # tools and desugar mode not relevant for clean
+ return self.RunGradle(self.cleanCommand, self.Tools.D8, self.DesugarMode.D8_DESUGARING)
+
+ def EnsurePresence(self):
+ EnsurePresence(self.rootDirPath, self.displayName)
+
+
+def EnsurePresence(dir, displayName):
+ if not os.path.exists(dir) or os.path.getmtime(dir + '.tar.gz')\
+ < os.path.getmtime(dir + '.tar.gz.sha1'):
+ utils.DownloadFromX20(dir + '.tar.gz.sha1')
+ # Update the mtime of the tar file to make sure we do not run again unless
+ # there is an update.
+ os.utime(dir + '.tar.gz', None)
+ else:
+ print('test_gradle_benchmarks.py: benchmark {} is present'.format(displayName))
+
+def TaskFilter(taskname):
+ acceptedGradleTasks = [
+ 'dex',
+ 'Dex',
+ 'proguard',
+ 'Proguard',
+ 'kotlin',
+ 'Kotlin',
+ ]
+
+ return any(namePattern in taskname for namePattern in acceptedGradleTasks)
+
+
+def PrintBuildTimeForGolem(benchmark, stdOut):
+ for line in stdOut.splitlines():
+ if 'BENCH' in line and benchmark.moduleName in line:
+ commaSplit = line.split(',')
+ assert len(commaSplit) == 3
+
+ # Keep only module that have been configured to use R8
+ if benchmark.moduleName + ':' not in commaSplit[1]:
+ continue
+
+ # remove <module-name> + ':'
+ taskName = commaSplit[1][(len(benchmark.moduleName) + 1):]
+
+ # Just a temporary assumption.
+ # This means we have submodules, so we'll need to check their configuration
+ # so that the right r8/d8 is taken. For now it shouldn't be the case.
+ assert taskName.find(':') == -1, taskName
+
+ # Output example:
+ # SantaTracker-transformClassesWithDexBuilderForDevelopmentDebug(RunTimeRaw): 748 ms
+
+ golemBenchmarkValue = benchmark.displayName + '-' + taskName + '(RunTimeRaw): '
+ if TaskFilter(taskName):
+ print('{}(RunTimeRaw): {} ms'
+ .format(benchmark.displayName + '-' + taskName, commaSplit[2]))
+
+
+def Main():
+ args = parse_arguments()
+
+ if args.tool == 'd8':
+ tool = Benchmark.Tools.D8
+ desugarMode = Benchmark.DesugarMode.D8_DESUGARING
+ else:
+ tool = Benchmark.Tools.DX
+ desugarMode = Benchmark.DesugarMode.DESUGAR_TOOL
+
+ buildTimeBenchmarks = [
+ Benchmark('AntennaPod',
+ os.path.join('antenna-pod', 'AntennaPod'),
+ ':app',
+ [':app:assembleDebug'],
+ ['clean']),
+ Benchmark('Maps',
+ 'gradle-java-1.6',
+ ':maps',
+ [':maps:assembleDebug', '--settings-file', 'settings.gradle.maps'],
+ ['clean']),
+ Benchmark('Music2',
+ 'gradle-java-1.6',
+ ':music2Old',
+ [':music2Old:assembleDebug', '--settings-file', 'settings.gradle.music2Old'],
+ ['clean']),
+ Benchmark('Velvet',
+ 'gradle-java-1.6',
+ ':velvet',
+ [':velvet:assembleDebug', '--settings-file', 'settings.gradle.velvet'],
+ ['clean']),
+ Benchmark('SantaTracker',
+ 'santa-tracker',
+ ':santa-tracker',
+ [':santa-tracker:assembleDebug'],
+ ['clean']),
+
+ # disabled for now, apparently because of b/74227571
+ # Benchmark('Tachiyomi',
+ # 'tachiyomi',
+ # ':app',
+ # ['assembleStandardDebug'],
+ # ['clean']),
+
+ Benchmark('WordPress',
+ 'wordpress',
+ ':WordPress',
+ ['assembleVanillaDebug'],
+ ['clean']),
+
+ ]
+
+ EnsurePresence(os.path.join('third_party', 'benchmarks', 'android-sdk'), 'android SDK')
+ EnsurePresence(os.path.join('third_party', 'gradle-plugin'), 'Android Gradle plugin')
+
+ for benchmark in buildTimeBenchmarks:
+ benchmark.EnsurePresence()
+ benchmark.Clean()
+ stdOut = benchmark.Build(tool, desugarMode)
+ PrintBuildTimeForGolem(benchmark, stdOut)
+
+
+if __name__ == '__main__':
+ sys.exit(Main())
diff --git a/tools/utils.py b/tools/utils.py
index 8716f65..a138461 100644
--- a/tools/utils.py
+++ b/tools/utils.py
@@ -48,6 +48,12 @@
def IsWindows():
return os.name == 'nt'
+def DownloadFromX20(sha1_file):
+ download_script = os.path.join(REPO_ROOT, 'tools', 'download_from_x20.py')
+ cmd = [download_script, sha1_file]
+ PrintCmd(cmd)
+ subprocess.check_call(cmd)
+
def DownloadFromGoogleCloudStorage(sha1_file, bucket='r8-deps'):
suffix = '.bat' if IsWindows() else ''
download_script = 'download_from_google_storage%s' % suffix
