Merge "Use ART default options"
diff --git a/src/main/java/com/android/tools/r8/D8.java b/src/main/java/com/android/tools/r8/D8.java
index 6598ae4..0f0b38f 100644
--- a/src/main/java/com/android/tools/r8/D8.java
+++ b/src/main/java/com/android/tools/r8/D8.java
@@ -55,7 +55,6 @@
*/
public final class D8 {
- private static final String VERSION = "v0.2.0";
private static final int STATUS_ERROR = 1;
private D8() {}
@@ -111,7 +110,7 @@
return;
}
if (command.isPrintVersion()) {
- System.out.println("D8 " + VERSION);
+ System.out.println("D8 " + Version.LABEL);
return;
}
run(command);
@@ -161,7 +160,7 @@
return options.getMarker();
}
return new Marker(Tool.D8)
- .put("version", VERSION)
+ .put("version", Version.LABEL)
.put("min-api", options.minApiLevel);
}
diff --git a/src/main/java/com/android/tools/r8/Disassemble.java b/src/main/java/com/android/tools/r8/Disassemble.java
index bb72d5d..a0770bc 100644
--- a/src/main/java/com/android/tools/r8/Disassemble.java
+++ b/src/main/java/com/android/tools/r8/Disassemble.java
@@ -144,7 +144,7 @@
return;
}
if (command.isPrintVersion()) {
- System.out.println("R8 v0.0.1");
+ System.out.println("R8 " + Version.LABEL);
return;
}
R8.disassemble(command);
diff --git a/src/main/java/com/android/tools/r8/GenerateMainDexList.java b/src/main/java/com/android/tools/r8/GenerateMainDexList.java
index 44d523e..5a18b41 100644
--- a/src/main/java/com/android/tools/r8/GenerateMainDexList.java
+++ b/src/main/java/com/android/tools/r8/GenerateMainDexList.java
@@ -28,7 +28,6 @@
import java.util.stream.Collectors;
public class GenerateMainDexList {
- private static final String VERSION = "v0.2.0";
private final Timing timing = new Timing("maindex");
private final InternalOptions options;
@@ -113,7 +112,7 @@
return;
}
if (command.isPrintVersion()) {
- System.out.println("MainDexListGenerator " + VERSION);
+ System.out.println("MainDexListGenerator " + Version.LABEL);
return;
}
List<String> result = run(command);
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index b4c19ea..e8990f3 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -71,7 +71,6 @@
public class R8 {
- private static final String VERSION = "v0.2.0";
private final Timing timing = new Timing("R8");
private final InternalOptions options;
@@ -86,7 +85,7 @@
return options.getMarker();
}
return new Marker(Tool.R8)
- .put("version", VERSION)
+ .put("version", Version.LABEL)
.put("min-api", options.minApiLevel);
}
@@ -535,7 +534,7 @@
return;
}
if (command.isPrintVersion()) {
- System.out.println("R8 " + VERSION);
+ System.out.println("R8 " + Version.LABEL);
return;
}
run(command);
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
new file mode 100644
index 0000000..ed19d71
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -0,0 +1,13 @@
+// Copyright (c) 2017, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8;
+
+public final class Version {
+
+ public static final String LABEL = "v0.2.0-dev";
+
+ private Version() {
+ }
+}
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 2134ccf..b6fbdee 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
@@ -638,6 +638,12 @@
return currentDefinitions.get(register);
}
+ public void replaceCurrentDefinitions(Value oldValue, Value newValue) {
+ assert oldValue.definition.getBlock() == this;
+ assert !oldValue.isUsed();
+ currentDefinitions.replaceAll((index, value) -> value == oldValue ? newValue : value);
+ }
+
public void updateCurrentDefinition(int register, Value value, EdgeType readingEdge) {
// If the reading/writing block is a catch successor, possibly update the on-throw value.
if (isOnThrowValue(register, readingEdge)) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/DebugPosition.java b/src/main/java/com/android/tools/r8/ir/code/DebugPosition.java
index 9a8f8a5..d6063f3 100644
--- a/src/main/java/com/android/tools/r8/ir/code/DebugPosition.java
+++ b/src/main/java/com/android/tools/r8/ir/code/DebugPosition.java
@@ -50,6 +50,15 @@
}
@Override
+ public boolean equals(Object other) {
+ if (other instanceof DebugPosition) {
+ DebugPosition o = (DebugPosition) other;
+ return line == o.line && file == o.file;
+ }
+ return false;
+ }
+
+ @Override
public int maxInValueRegister() {
throw new Unreachable();
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionsIterator.java b/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionsIterator.java
index 760a7ed..1390620 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionsIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionsIterator.java
@@ -51,4 +51,9 @@
public void replaceCurrentInstruction(Instruction newInstruction) {
instructionIterator.replaceCurrentInstruction(newInstruction);
}
+
+ @Override
+ public void removeOrReplaceByNop() {
+ instructionIterator.removeOrReplaceByNop();
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Instruction.java b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
index de198c9..c110f2f 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Instruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
@@ -86,6 +86,7 @@
instruction.inValues.forEach(Value::clearUsersInfo);
if (instruction.debugValues != null) {
instruction.debugValues.forEach(Value::clearUsersInfo);
+ instruction.debugValues = null;
}
}
@@ -134,7 +135,9 @@
assert value.getLocalInfo() != null;
if (debugValues != null) {
assert debugValues.contains(value);
- debugValues.remove(value);
+ if (debugValues.remove(value)) {
+ value.removeDebugUser(this);
+ }
return;
}
assert false;
@@ -142,6 +145,9 @@
public void clearDebugValues() {
if (debugValues != null) {
+ for (Value debugValue : debugValues) {
+ debugValue.removeDebugUser(this);
+ }
debugValues.clear();
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstructionIterator.java b/src/main/java/com/android/tools/r8/ir/code/InstructionIterator.java
index 2cb8185..56d81a2 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstructionIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstructionIterator.java
@@ -31,4 +31,10 @@
* @param instruction The instruction to add.
*/
void add(Instruction instruction);
+
+ /**
+ * Safe removal function that will insert a Nop to take over the debug values if any are
+ * associated with the current instruction.
+ */
+ void removeOrReplaceByNop();
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Value.java b/src/main/java/com/android/tools/r8/ir/code/Value.java
index ffe3181..605eac3 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Value.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Value.java
@@ -115,10 +115,17 @@
}
public void setLocalInfo(DebugLocalInfo local) {
+ assert local != null;
assert debugData == null;
debugData = new DebugData(local);
}
+ public void clearLocalInfo() {
+ assert debugData.users.isEmpty();
+ assert debugData.phiUsers.isEmpty();
+ debugData = null;
+ }
+
public List<Instruction> getDebugLocalStarts() {
if (debugData == null) {
return Collections.emptyList();
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 b35c1a0..cbd8941 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
@@ -48,8 +48,10 @@
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.If;
import com.android.tools.r8.ir.code.InstructionIterator;
+import com.android.tools.r8.ir.code.InstructionListIterator;
import com.android.tools.r8.ir.code.Move;
import com.android.tools.r8.ir.code.NewArrayFilledData;
+import com.android.tools.r8.ir.code.Return;
import com.android.tools.r8.ir.code.Switch;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.regalloc.LinearScanRegisterAllocator;
@@ -148,6 +150,11 @@
// Populate the builder info objects.
numberOfInstructions = 0;
+
+ // Remove redundant debug position instructions. They would otherwise materialize as
+ // unnecessary nops.
+ removeRedundantDebugPositions();
+
ListIterator<BasicBlock> iterator = ir.listIterator();
assert iterator.hasNext();
BasicBlock block = iterator.next();
@@ -235,6 +242,26 @@
return code;
}
+ private void removeRedundantDebugPositions() {
+ ListIterator<BasicBlock> iterator = ir.listIterator();
+ DebugPosition lastMoveExceptionPosition = null;
+ while (iterator.hasNext()) {
+ BasicBlock next = iterator.next();
+ InstructionListIterator it = next.listIterator();
+ while (it.hasNext()) {
+ com.android.tools.r8.ir.code.Instruction instruction = it.next();
+ if (instruction.isDebugPosition()) {
+ if (instruction.asDebugPosition().equals(lastMoveExceptionPosition)) {
+ it.remove();
+ }
+ lastMoveExceptionPosition = null;
+ } else if (instruction.isMoveException()) {
+ lastMoveExceptionPosition = instruction.asMoveException().getPosition();
+ }
+ }
+ }
+ }
+
// Rewrite ifs with offsets that are too large for the if encoding. The rewriting transforms:
//
//
@@ -316,7 +343,7 @@
add(instruction, new FallThroughInfo(instruction));
}
- private boolean isNopInstruction(com.android.tools.r8.ir.code.Instruction instruction) {
+ private static boolean isNopInstruction(com.android.tools.r8.ir.code.Instruction instruction) {
return instruction.isNop()
|| instruction.isDebugLocalsChange()
|| (instruction.isConstNumber() && !instruction.outValue().needsRegister());
@@ -813,7 +840,7 @@
} else {
size = 3;
}
- if (targetInfo.getIR().isReturn()) {
+ if (targetInfo.getIR().isReturn() && canTargetReturn(targetInfo.getIR().asReturn())) {
// Set the size to the min of the size of the return and the size of the goto. When
// adding instructions, we use the return if the computed size matches the size of the
// return.
@@ -834,8 +861,9 @@
Instruction dex;
// Emit a return if the target is a return and the size of the return is the computed
// size of this instruction.
- if (targetInfo.getIR().isReturn() && size == targetInfo.getSize()) {
- dex = targetInfo.getIR().asReturn().createDexInstruction(builder);
+ Return ret = targetInfo.getIR().asReturn();
+ if (ret != null && size == targetInfo.getSize() && canTargetReturn(ret)) {
+ dex = ret.createDexInstruction(builder);
} else {
switch (size) {
case 1:
@@ -861,6 +889,18 @@
dex.setOffset(getOffset()); // for better printing of the dex code.
instructions.add(dex);
}
+
+ private static boolean canTargetReturn(Return ret) {
+ InstructionListIterator it = ret.getBlock().listIterator(ret);
+ com.android.tools.r8.ir.code.Instruction prev = it.previous();
+ while (it.hasPrevious()) {
+ prev = it.previous();
+ if (!DexBuilder.isNopInstruction(prev)) {
+ break;
+ }
+ }
+ return !prev.isDebugPosition();
+ }
}
public static class IfInfo extends Info {
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 17b0e36..543f707 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
@@ -88,11 +88,6 @@
}
@Override
- public boolean needsPrelude() {
- return !accessFlags.isStatic() || !argumentTypes.isEmpty();
- }
-
- @Override
public int instructionCount() {
return code.instructions.length;
}
@@ -143,11 +138,9 @@
}
@Override
- public void closedCurrentBlockWithFallthrough(int fallthroughInstructionIndex) {
- }
-
- @Override
- public void closedCurrentBlock() {
+ public void closingCurrentBlockWithFallthrough(
+ int fallthroughInstructionIndex, IRBuilder builder) {
+ // Intentionally empty.
}
@Override
@@ -206,7 +199,7 @@
int offset = instructionOffset(instructionIndex);
for (DexDebugEntry entry : debugEntries) {
if (entry.address == offset) {
- builder.updateCurrentDebugPosition(entry.line, entry.sourceFile);
+ builder.addDebugPosition(entry.line, entry.sourceFile);
return;
}
if (entry.address > offset) {
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 245a75a..1b7a276 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
@@ -5,7 +5,6 @@
package com.android.tools.r8.ir.conversion;
import com.android.tools.r8.ApiLevelException;
-import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.InternalCompilerError;
import com.android.tools.r8.errors.InvalidDebugInfoException;
@@ -60,6 +59,7 @@
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.Nop;
import com.android.tools.r8.ir.code.Not;
import com.android.tools.r8.ir.code.NumberConversion;
import com.android.tools.r8.ir.code.NumericType;
@@ -269,8 +269,6 @@
final private ValueNumberGenerator valueNumberGenerator;
- private DebugPosition currentDebugPosition = null;
-
private DexEncodedMethod method;
// Source code to build IR from. Null if already built.
@@ -280,11 +278,9 @@
private final InternalOptions options;
- // Pending local changes.
+ // Pending local reads.
private Value previousLocalValue = null;
- private List<Value> debugLocalStarts = new ArrayList<>();
private List<Value> debugLocalReads = new ArrayList<>();
- private List<Value> debugLocalEnds = new ArrayList<>();
private int nextBlockNumber = 0;
@@ -470,13 +466,12 @@
// Build IR for each dex instruction in the block.
for (int i = item.firstInstructionIndex; i < source.instructionCount(); ++i) {
if (currentBlock == null) {
- source.closedCurrentBlock();
break;
}
BlockInfo info = targets.get(source.instructionOffset(i));
if (info != null && info.block != currentBlock) {
+ source.closingCurrentBlockWithFallthrough(i, this);
closeCurrentBlockWithFallThrough(info.block);
- source.closedCurrentBlockWithFallthrough(i);
addToWorklist(info.block, i);
break;
}
@@ -551,26 +546,13 @@
addInstruction(new DebugLocalUninitialized(type, value));
}
- public void addDebugLocalStart(int register, DebugLocalInfo local) {
- if (!options.debug) {
- return;
- }
- assert local != null;
- assert local == getCurrentLocal(register);
- MoveType moveType = MoveType.fromDexType(local.type);
- Value in = readRegisterIgnoreLocal(register, moveType);
- if (in.isPhi() || in.getLocalInfo() != local) {
- // We cannot shortcut if the local is defined by a phi as it could end up being trivial.
- addDebugLocalWrite(moveType, register, in);
- } else {
- Value value = getLocalValue(register, local);
- if (value != null) {
- debugLocalStarts.add(value);
- }
- }
+ public void addDebugPosition(int line, DexString file) {
+ // Always add positions as they influence release builds (ie, stack traces).
+ addInstruction(new DebugPosition(line, file));
}
private void addDebugLocalWrite(MoveType type, int dest, Value in) {
+ assert options.debug;
Value out = writeRegister(dest, type, ThrowingInfo.NO_THROW);
DebugLocalWrite write = new DebugLocalWrite(out, in);
assert !write.instructionTypeCanThrow();
@@ -578,17 +560,17 @@
}
private Value getLocalValue(int register, DebugLocalInfo local) {
+ assert options.debug;
assert local != null;
assert local == getCurrentLocal(register);
MoveType moveType = MoveType.fromDexType(local.type);
+ return readRegisterIgnoreLocal(register, moveType);
+ }
+
+ private static boolean isValidFor(Value value, DebugLocalInfo local) {
// Invalid debug-info may cause attempt to read a local that is not actually alive.
// See b/37722432 and regression test {@code jasmin.InvalidDebugInfoTests::testInvalidInfoThrow}
- Value in = readRegisterIgnoreLocal(register, moveType);
- if (in.isUninitializedLocal() || in.getLocalInfo() != local) {
- return null;
- }
- assert in.getLocalInfo() == local;
- return in;
+ return !value.isUninitializedLocal() && value.getLocalInfo() == local;
}
public void addDebugLocalRead(int register, DebugLocalInfo local) {
@@ -596,18 +578,72 @@
return;
}
Value value = getLocalValue(register, local);
- if (value != null) {
+ if (isValidFor(value, local)) {
debugLocalReads.add(value);
}
}
+ public void addDebugLocalStart(int register, DebugLocalInfo local) {
+ if (!options.debug) {
+ return;
+ }
+ Value value = getLocalValue(register, local);
+
+ // If the value is for a different local, introduce the new local. We cannot shortcut if the
+ // local is defined by a phi as it could end up being trivial.
+ if (value.isPhi() || value.getLocalInfo() != local) {
+ addDebugLocalWrite(MoveType.fromDexType(local.type), register, value);
+ return;
+ }
+
+ if (!isValidFor(value, local)) {
+ return;
+ }
+
+ // When inserting a start there are two possibilities:
+ // 1. The block is empty (eg, instructions from block entry until now materialized to nothing).
+ // 2. The block is non-empty (and the last instruction does not define the local to start).
+ if (currentBlock.getInstructions().isEmpty()) {
+ addInstruction(new Nop());
+ }
+ Instruction instruction = currentBlock.getInstructions().getLast();
+ assert instruction.outValue() != value;
+ instruction.addDebugValue(value);
+ value.addDebugLocalStart(instruction);
+ }
+
public void addDebugLocalEnd(int register, DebugLocalInfo local) {
if (!options.debug) {
return;
}
Value value = getLocalValue(register, local);
- if (value != null) {
- debugLocalEnds.add(value);
+ if (!isValidFor(value, local)) {
+ return;
+ }
+
+ // When inserting an end there are three possibilities:
+ // 1. The block is empty (eg, instructions from block entry until now materialized to nothing).
+ // 2. The block has an instruction not defining the local being ended.
+ // 3. The block has an instruction defining the local being ended.
+ if (currentBlock.getInstructions().isEmpty()) {
+ addInstruction(new Nop());
+ }
+ Instruction instruction = currentBlock.getInstructions().getLast();
+ if (instruction.outValue() != value) {
+ instruction.addDebugValue(value);
+ value.addDebugLocalEnd(instruction);
+ return;
+ }
+ // In case 3. there are two cases:
+ // a. The defining instruction is a debug-write, in which case it should be removed.
+ // b. The defining instruction is overwriting the local value, in which case we de-associate it.
+ assert !instruction.outValue().isUsed();
+ if (instruction.isDebugLocalWrite()) {
+ DebugLocalWrite write = instruction.asDebugLocalWrite();
+ currentBlock.replaceCurrentDefinitions(value, write.src());
+ currentBlock.listIterator(write).removeOrReplaceByNop();
+ } else {
+ instruction.outValue().clearLocalInfo();
}
}
@@ -1146,6 +1182,13 @@
assert out.getLocalInfo() == null;
MoveException instruction = new MoveException(out);
assert !instruction.instructionTypeCanThrow();
+ if (!currentBlock.getInstructions().isEmpty()
+ && currentBlock.getInstructions().getLast().isDebugPosition()) {
+ DebugPosition position = currentBlock.getInstructions().getLast().asDebugPosition();
+ position.moveDebugValues(instruction);
+ instruction.setPosition(position);
+ currentBlock.removeInstruction(position);
+ }
if (!currentBlock.getInstructions().isEmpty()) {
throw new CompilationError("Invalid MoveException instruction encountered. "
+ "The MoveException instruction is not the first instruction in the block in "
@@ -1621,8 +1664,7 @@
// Private instruction helpers.
private void addInstruction(Instruction ir) {
- attachLocalChanges(ir);
- flushCurrentDebugPosition(ir);
+ attachLocalValues(ir);
currentBlock.add(ir);
if (ir.instructionTypeCanThrow()) {
assert source.verifyCurrentInstructionCanThrow();
@@ -1660,8 +1702,10 @@
}
}
- private void attachLocalChanges(Instruction ir) {
+ private void attachLocalValues(Instruction ir) {
if (!options.debug) {
+ assert previousLocalValue == null;
+ assert debugLocalReads.isEmpty();
return;
}
// Add a use if this instruction is overwriting a previous value of the same local.
@@ -1669,24 +1713,12 @@
assert ir.outValue() != null;
ir.addDebugValue(previousLocalValue);
}
+ // Add reads of locals if any are pending.
+ for (Value value : debugLocalReads) {
+ ir.addDebugValue(value);
+ }
previousLocalValue = null;
- if (debugLocalStarts.isEmpty() && debugLocalReads.isEmpty() && debugLocalEnds.isEmpty()) {
- return;
- }
- for (Value debugLocalStart : debugLocalStarts) {
- ir.addDebugValue(debugLocalStart);
- debugLocalStart.addDebugLocalStart(ir);
- }
- for (Value debugLocalRead : debugLocalReads) {
- ir.addDebugValue(debugLocalRead);
- }
- for (Value debugLocalEnd : debugLocalEnds) {
- ir.addDebugValue(debugLocalEnd);
- debugLocalEnd.addDebugLocalEnd(ir);
- }
- debugLocalStarts.clear();
debugLocalReads.clear();
- debugLocalEnds.clear();
}
// Package (ie, SourceCode accessed) helpers.
@@ -1787,7 +1819,6 @@
// insert reads before closing the block. It is unclear if we can rely on a local-end to ensure
// liveness in all blocks where the local should be live.
assert currentBlock != null;
- assert currentDebugPosition == null;
currentBlock.close(this);
setCurrentBlock(null);
throwingInstructionInCurrentBlock = false;
@@ -2061,30 +2092,6 @@
code.blocks = tracedBlocks;
}
- // Debug info helpers.
-
- public void updateCurrentDebugPosition(int line, DexString file) {
- // Stack-trace support requires position information in both debug and release mode.
- flushCurrentDebugPosition(null);
- currentDebugPosition = new DebugPosition(line, file);
- attachLocalChanges(currentDebugPosition);
- }
-
- private void flushCurrentDebugPosition(Instruction instruction) {
- if (currentDebugPosition != null) {
- DebugPosition position = currentDebugPosition;
- currentDebugPosition = null;
- if (instruction != null && instruction.isMoveException()) {
- // Set the position on the move-exception instruction.
- // ART/DX associates the move-exception pc with the catch-declaration line.
- // See debug.ExceptionTest.testStepOnCatch().
- instruction.asMoveException().setPosition(position);
- } else {
- addInstruction(position);
- }
- }
- }
-
// Other stuff.
boolean isIntegerType(NumericType type) {
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 569e554..e44ee80 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
@@ -257,11 +257,6 @@
}
@Override
- public boolean needsPrelude() {
- return isSynchronized() || actualArgumentCount() > 0 || !node.localVariables.isEmpty();
- }
-
- @Override
public void buildPrelude(IRBuilder builder) {
// Record types for arguments.
Int2ReferenceMap<MoveType> argumentLocals = recordArgumentTypes();
@@ -418,26 +413,11 @@
AbstractInsnNode insn = getInstruction(i);
updateState(insn);
-
- if (!isControlFlowInstruction(insn)) {
- updateStateForLocalVariableEnd(insn);
- }
}
}
state.restoreState(0);
}
- private void updateStateForLocalVariableEnd(AbstractInsnNode insn) {
- assert !isControlFlowInstruction(insn);
- if (!(insn.getNext() instanceof LabelNode)) {
- return;
- }
- // If the label is the end of any local-variable scopes end the locals.
- LabelNode label = (LabelNode) insn.getNext();
- List<Local> locals = state.getLocalsToClose(label);
- state.closeLocals(locals);
- }
-
@Override
public void buildPostlude(IRBuilder builder) {
if (isSynchronized()) {
@@ -461,11 +441,14 @@
}
@Override
- public void closedCurrentBlockWithFallthrough(int fallthroughInstructionIndex) {
- }
-
- @Override
- public void closedCurrentBlock() {
+ public void closingCurrentBlockWithFallthrough(
+ int fallthroughInstructionIndex, IRBuilder builder) {
+ AbstractInsnNode insn = node.instructions.get(fallthroughInstructionIndex);
+ if (insn instanceof LabelNode) {
+ for (Local local : state.getLocalsToClose((LabelNode) insn)) {
+ builder.addDebugLocalEnd(local.slot.register, local.info);
+ }
+ }
}
@Override
@@ -488,12 +471,6 @@
preInstructionState = state.toString();
}
- // Process local-variable end scopes if this instruction is not a control-flow instruction.
- // For control-flow instructions, processing takes place before closing their respective blocks.
- if (!isControlFlowInstruction(insn)) {
- processLocalVariableEnd(insn, builder);
- }
-
build(insn, builder);
if (Log.ENABLED && !(insn instanceof LineNumberNode)) {
@@ -1727,6 +1704,9 @@
}
private void updateState(LabelNode insn) {
+ // Close scope of locals ending at this point.
+ List<Local> locals = state.getLocalsToClose(insn);
+ state.closeLocals(locals);
// Open the scope of locals starting at this point.
if (insn != initialLabel) {
state.openLocals(insn);
@@ -1834,20 +1814,6 @@
}
}
- private void processLocalVariableEnd(AbstractInsnNode insn, IRBuilder builder) {
- assert !isControlFlowInstruction(insn);
- if (!(insn.getNext() instanceof LabelNode)) {
- return;
- }
- // If the label is the end of any local-variable scopes end the locals.
- LabelNode label = (LabelNode) insn.getNext();
- List<Local> locals = state.getLocalsToClose(label);
- for (Local local : locals) {
- builder.addDebugLocalEnd(local.slot.register, local.info);
- }
- state.closeLocals(locals);
- }
-
private void processLocalVariablesAtControlEdge(AbstractInsnNode insn, IRBuilder builder) {
assert isControlFlowInstruction(insn) && !isReturn(insn);
int offset = getOffset(insn);
@@ -2743,6 +2709,13 @@
}
private void build(LabelNode insn, IRBuilder builder) {
+ // Close locals starting at this point.
+ List<Local> locals = state.getLocalsToClose(insn);
+ for (Local local : locals) {
+ builder.addDebugLocalEnd(local.slot.register, local.info);
+ }
+ state.closeLocals(locals);
+
// Open the scope of locals starting at this point.
if (insn != initialLabel) {
for (Local local : state.openLocals(insn)) {
@@ -2846,7 +2819,7 @@
}
private void build(LineNumberNode insn, IRBuilder builder) {
- builder.updateCurrentDebugPosition(insn.line, null);
+ builder.addDebugPosition(insn.line, null);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/JarState.java b/src/main/java/com/android/tools/r8/ir/conversion/JarState.java
index c6b83a2..828ff10 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/JarState.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/JarState.java
@@ -294,7 +294,9 @@
int register = getLocalRegister(node.index, type);
Local local = getLocalForRegister(register);
assert local != null;
- locals.add(local);
+ if (local.info != null) {
+ locals.add(local);
+ }
}
// Sort to ensure deterministic instruction ordering (correctness is unaffected).
locals.sort(Comparator.comparingInt(local -> local.slot.register));
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 16e124a..3ed90fb 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
@@ -22,10 +22,6 @@
int instructionIndex(int instructionOffset);
int instructionOffset(int instructionIndex);
- // True if the method needs a special entry block (prelude) that is not a targetable block.
- // This is the case for methods with arguments or that need additional synchronization support.
- boolean needsPrelude();
-
DebugLocalInfo getCurrentLocal(int register);
DebugPosition getDebugPositionAtOffset(int offset);
@@ -41,8 +37,7 @@
*/
int traceInstruction(int instructionIndex, IRBuilder builder);
- void closedCurrentBlockWithFallthrough(int fallthroughInstructionIndex);
- void closedCurrentBlock();
+ void closingCurrentBlockWithFallthrough(int fallthroughInstructionIndex, IRBuilder builder);
// Setup and release resources used temporarily during trace/build.
void setUp();
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 20f272a..fbde02b 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
@@ -91,7 +91,6 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.IdentityHashMap;
-import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.ListIterator;
@@ -1088,7 +1087,7 @@
user -> user.isCheckCast()
&& user.asCheckCast().getType().isSubtypeOf(checkCast.getType(), appInfo))) {
checkCast.outValue().replaceUsers(checkCast.inValues().get(0));
- it.remove();
+ it.removeOrReplaceByNop();
}
}
}
@@ -1537,7 +1536,7 @@
for (int i = 0; i < dominatorTree.getSortedBlocks().length; i++) {
BasicBlock block = dominatorTree.getSortedBlocks()[i];
- Iterator<Instruction> iterator = block.iterator();
+ InstructionListIterator iterator = block.listIterator();
while (iterator.hasNext()) {
Instruction instruction = iterator.next();
if (instruction.isBinop()
@@ -1552,7 +1551,7 @@
shareCatchHandlers(instruction, candidate.definition)) {
instruction.outValue().replaceUsers(candidate);
eliminated = true;
- iterator.remove();
+ iterator.removeOrReplaceByNop();
break; // Don't try any more candidates.
}
}
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 50f224c..ebf69b5 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
@@ -120,7 +120,7 @@
// 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();
- iterator.remove();
+ iterator.removeOrReplaceByNop();
}
}
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 ade7537..5bf6e8b 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
@@ -788,11 +788,6 @@
}
@Override
- public boolean needsPrelude() {
- return outline.argumentCount() > 0;
- }
-
- @Override
public int instructionCount() {
return outline.templateInstructions.size() + 1;
}
@@ -819,11 +814,8 @@
}
@Override
- public void closedCurrentBlockWithFallthrough(int fallthroughInstructionIndex) {
- }
-
- @Override
- public void closedCurrentBlock() {
+ public void closingCurrentBlockWithFallthrough(
+ int fallthroughInstructionIndex, IRBuilder builder) {
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/synthetic/SingleBlockSourceCode.java b/src/main/java/com/android/tools/r8/ir/synthetic/SingleBlockSourceCode.java
index 391b479..c8fc818 100644
--- a/src/main/java/com/android/tools/r8/ir/synthetic/SingleBlockSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/synthetic/SingleBlockSourceCode.java
@@ -100,11 +100,6 @@
protected abstract void prepareInstructions();
@Override
- public final boolean needsPrelude() {
- return receiver != null || paramRegisters.length > 0;
- }
-
- @Override
public final int instructionCount() {
return constructors.size();
}
@@ -130,11 +125,8 @@
}
@Override
- public final void closedCurrentBlockWithFallthrough(int fallthroughInstructionIndex) {
- }
-
- @Override
- public final void closedCurrentBlock() {
+ public final void closingCurrentBlockWithFallthrough(
+ int fallthroughInstructionIndex, IRBuilder builder) {
}
@Override
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 54ac455..91e0047 100644
--- a/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
+++ b/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
@@ -12,8 +12,10 @@
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.IteratorUtils;
import com.android.tools.r8.utils.OffOrAuto;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
import it.unimi.dsi.fastutil.longs.LongArrayList;
import it.unimi.dsi.fastutil.longs.LongList;
import java.io.File;
@@ -269,6 +271,12 @@
return step(StepKind.INTO, stepFilter);
}
+ protected static List<Variable> getVisibleKotlinInlineVariables(
+ JUnit3Wrapper.DebuggeeState debuggeeState) {
+ return debuggeeState.getVisibleVariables().stream()
+ .filter(v -> v.getName().matches("^\\$i\\$f\\$.*$")).collect(Collectors.toList());
+ }
+
public enum StepKind {
INTO(StepDepth.INTO),
OVER(StepDepth.OVER),
@@ -317,10 +325,7 @@
}
protected final JUnit3Wrapper.Command checkNoLocal(String localName) {
- return inspect(t -> {
- List<String> localNames = t.getLocalNames();
- Assert.assertFalse("Unexpected local: " + localName, localNames.contains(localName));
- });
+ return inspect(t -> t.checkNoLocal(localName));
}
protected final JUnit3Wrapper.Command checkNoLocal() {
@@ -508,8 +513,9 @@
.getMethodSignature(debuggeeState.getLocation().classID,
debuggeeState.getLocation().methodID);
System.out.println(String
- .format("Suspended in %s#%s%s@0x%x", classSig, methodName, methodSig,
- Long.valueOf(debuggeeState.getLocation().index)));
+ .format("Suspended in %s#%s%s@0x%x (line=%d)", classSig, methodName, methodSig,
+ Long.valueOf(debuggeeState.getLocation().index),
+ Integer.valueOf(debuggeeState.getLineNumber())));
}
// Handle event.
@@ -553,6 +559,16 @@
return new ArtTestOptions(debuggeePath);
}
+ public void enqueueCommandFirst(Command command) {
+ commandsQueue.addFirst(command);
+ }
+
+ public void enqueueCommandsFirst(List<Command> commands) {
+ for (int i = commands.size() - 1; i >= 0; --i) {
+ enqueueCommandFirst(commands.get(i));
+ }
+ }
+
//
// Inspection
//
@@ -570,6 +586,8 @@
// Locals
+ List<Variable> getVisibleVariables();
+
/**
* Returns the names of all local variables visible at the current location
*/
@@ -579,6 +597,7 @@
* Returns the values of all locals visible at the current location.
*/
Map<String, Value> getLocalValues();
+ void checkNoLocal(String localName);
void checkLocal(String localName);
void checkLocal(String localName, Value expectedValue);
}
@@ -622,7 +641,7 @@
// Code indices are in ascending order.
assert currentLineCodeIndex >= startCodeIndex;
assert currentLineCodeIndex <= endCodeIndex;
- assert currentLineCodeIndex > previousLineCodeIndex;
+ assert currentLineCodeIndex >= previousLineCodeIndex;
previousLineCodeIndex = currentLineCodeIndex;
if (location.index >= currentLineCodeIndex) {
@@ -650,13 +669,19 @@
}
}
- public List<String> getLocalNames() {
- Location location = getLocation();
- return JUnit3Wrapper.getVariablesAt(mirror, location).stream()
- .map(v -> v.getName())
+ @Override
+ public List<Variable> getVisibleVariables() {
+ // Get variable table and keep only variables visible at this location.
+ Location frameLocation = getLocation();
+ return getVariables(getMirror(), frameLocation.classID, frameLocation.methodID).stream()
+ .filter(v -> inScope(frameLocation.index, v))
.collect(Collectors.toList());
}
+ public List<String> getLocalNames() {
+ return getVisibleVariables().stream().map(v -> v.getName()).collect(Collectors.toList());
+ }
+
@Override
public Map<String, Value> getLocalValues() {
return JUnit3Wrapper.getVariablesAt(mirror, location).stream()
@@ -685,6 +710,13 @@
"line " + getLineNumber() + ": Expected local '" + localName + "' not present");
}
+ @Override
+ public void checkNoLocal(String localName) {
+ Optional<Variable> localVar = JUnit3Wrapper
+ .getVariableAt(mirror, getLocation(), localName);
+ Assert.assertFalse("Unexpected local: " + localName, localVar.isPresent());
+ }
+
public void checkLocal(String localName) {
Optional<Variable> localVar = JUnit3Wrapper
.getVariableAt(mirror, getLocation(), localName);
@@ -778,6 +810,10 @@
return threadId;
}
+ public int getFrameDepth() {
+ return frames.size();
+ }
+
public FrameInspector getFrame(int index) {
return frames.get(index);
}
@@ -797,6 +833,11 @@
}
@Override
+ public void checkNoLocal(String localName) {
+ getTopFrame().checkNoLocal(localName);
+ }
+
+ @Override
public void checkLocal(String localName) {
getTopFrame().checkLocal(localName);
}
@@ -846,6 +887,11 @@
return getTopFrame().getMethodSignature();
}
+ @Override
+ public List<Variable> getVisibleVariables() {
+ return getTopFrame().getVisibleVariables();
+ }
+
public Value getStaticField(String className, String fieldName, String fieldSignature) {
String classSignature = DescriptorUtils.javaTypeToDescriptor(className);
byte typeTag = TypeTag.CLASS;
@@ -918,19 +964,19 @@
}
}
- private static boolean inScope(long index, Variable var) {
- long varStart = var.getCodeIndex();
- long varEnd = varStart + var.getLength();
- return index >= varStart && index < varEnd;
- }
-
- private static Optional<Variable> getVariableAt(VmMirror mirror, Location location,
+ public static Optional<Variable> getVariableAt(VmMirror mirror, Location location,
String localName) {
return getVariablesAt(mirror, location).stream()
.filter(v -> localName.equals(v.getName()))
.findFirst();
}
+ protected static boolean inScope(long index, Variable var) {
+ long varStart = var.getCodeIndex();
+ long varEnd = varStart + var.getLength();
+ return index >= varStart && index < varEnd;
+ }
+
private static List<Variable> getVariablesAt(VmMirror mirror, Location location) {
// Get variable table and keep only variables visible at this location.
return getVariables(mirror, location.classID, location.methodID).stream()
@@ -1057,10 +1103,9 @@
wrapper.getMirror().clearEvent(JDWPConstants.EventKind.CLASS_PREPARE,
classPrepareRequestId);
- // Breakpoint then resume. Note: we add them at the beginning of the queue (to be the
- // next commands to be processed), thus they need to be pushed in reverse order.
- wrapper.commandsQueue.addFirst(new JUnit3Wrapper.Command.RunCommand());
- wrapper.commandsQueue.addFirst(BreakpointCommand.this);
+ // Breakpoint then resume.
+ wrapper.enqueueCommandsFirst(
+ Arrays.asList(BreakpointCommand.this, new JUnit3Wrapper.Command.RunCommand()));
// Set wrapper ready to process next command.
wrapper.setState(State.ProcessCommand);
@@ -1260,7 +1305,7 @@
}
if (repeatStep) {
// In order to repeat the step now, we need to add it at the beginning of the queue.
- testBase.commandsQueue.addFirst(stepCommand);
+ testBase.enqueueCommandFirst(stepCommand);
}
super.handle(testBase);
}
diff --git a/src/test/java/com/android/tools/r8/debug/KotlinDebugTestBase.java b/src/test/java/com/android/tools/r8/debug/KotlinDebugTestBase.java
new file mode 100644
index 0000000..a0a9c3f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/debug/KotlinDebugTestBase.java
@@ -0,0 +1,83 @@
+// Copyright (c) 2017, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.debug;
+
+import java.util.Arrays;
+import java.util.List;
+import org.apache.harmony.jpda.tests.framework.jdwp.Frame.Variable;
+import org.apache.harmony.jpda.tests.framework.jdwp.Location;
+
+/**
+ * A specialization for Kotlin-based tests which provides extra commands.
+ */
+public abstract class KotlinDebugTestBase extends DebugTestBase {
+
+ protected final JUnit3Wrapper.Command kotlinStepOver() {
+ return testBaseBeforeStep -> {
+ final JUnit3Wrapper.DebuggeeState debuggeeStateBeforeStep = testBaseBeforeStep
+ .getDebuggeeState();
+ final int frameDepthBeforeStep = debuggeeStateBeforeStep.getFrameDepth();
+ final Location locationBeforeStep = debuggeeStateBeforeStep.getLocation();
+ final List<Variable> kotlinLvsBeforeStep = getVisibleKotlinInlineVariables(
+ debuggeeStateBeforeStep);
+
+ // This is the command that will be executed after the initial (normal) step over. If we
+ // reach an inlined location, this command will step until reaching a non-inlined location.
+ JUnit3Wrapper.Command commandAfterStep = testBaseAfterStep -> {
+ // Get the new debuggee state (previous one is stale).
+ JUnit3Wrapper.DebuggeeState debuggeeStateAfterStep = testBaseBeforeStep.getDebuggeeState();
+
+ // Are we in the same frame ?
+ final int frameDepthAfterStep = debuggeeStateAfterStep.getFrameDepth();
+ final Location locationAfterStep = debuggeeStateAfterStep.getLocation();
+ if (frameDepthBeforeStep == frameDepthAfterStep
+ && locationBeforeStep.classID == locationAfterStep.classID
+ && locationBeforeStep.methodID == locationAfterStep.methodID) {
+ // We remain in the same method. Do we step into an inlined section ?
+ List<Variable> kotlinLvsAfterStep = getVisibleKotlinInlineVariables(
+ debuggeeStateAfterStep);
+ if (kotlinLvsBeforeStep.isEmpty() && !kotlinLvsAfterStep.isEmpty()) {
+ assert kotlinLvsAfterStep.size() == 1;
+
+ // We're located in an inlined section. Instead of doing a classic step out, we must
+ // jump out of the inlined section.
+ Variable inlinedSectionLv = kotlinLvsAfterStep.get(0);
+ testBaseAfterStep.enqueueCommandFirst(stepUntilOutOfInlineScope(inlinedSectionLv));
+ }
+ }
+ };
+
+ // Step over then check whether we need to continue stepping.
+ testBaseBeforeStep.enqueueCommandsFirst(Arrays.asList(stepOver(), commandAfterStep));
+ };
+ }
+
+ protected final JUnit3Wrapper.Command kotlinStepOut() {
+ return wrapper -> {
+ final List<Variable> kotlinLvsBeforeStep = getVisibleKotlinInlineVariables(
+ wrapper.getDebuggeeState());
+
+ JUnit3Wrapper.Command nextCommand;
+ if (!kotlinLvsBeforeStep.isEmpty()) {
+ // We are in an inline section. We need to step until being out of inline scope.
+ assert kotlinLvsBeforeStep.size() == 1;
+ final Variable inlinedSectionLv = kotlinLvsBeforeStep.get(0);
+ nextCommand = stepUntilOutOfInlineScope(inlinedSectionLv);
+ } else {
+ nextCommand = stepOut();
+ }
+ wrapper.enqueueCommandFirst(nextCommand);
+ };
+ }
+
+ private JUnit3Wrapper.Command stepUntilOutOfInlineScope(Variable inlineScopeLv) {
+ return stepUntil(StepKind.OVER, StepLevel.LINE, debuggeeState -> {
+ boolean inInlineScope = JUnit3Wrapper
+ .inScope(debuggeeState.getLocation().index, inlineScopeLv);
+ return !inInlineScope;
+ });
+ }
+
+}
diff --git a/src/test/java/com/android/tools/r8/debug/KotlinInlineTest.java b/src/test/java/com/android/tools/r8/debug/KotlinInlineTest.java
index e62a17b..8e9d145 100644
--- a/src/test/java/com/android/tools/r8/debug/KotlinInlineTest.java
+++ b/src/test/java/com/android/tools/r8/debug/KotlinInlineTest.java
@@ -10,9 +10,9 @@
import org.junit.Ignore;
import org.junit.Test;
-public class KotlinInlineTest extends DebugTestBase {
+// TODO check double-depth inline (an inline in another inline)
+public class KotlinInlineTest extends KotlinDebugTestBase {
- @Ignore("Requires kotlin-specific stepping behavior")
@Test
public void testStepOverInline() throws Throwable {
String methodName = "singleInline";
@@ -26,7 +26,6 @@
assertEquals(41, s.getLineNumber());
s.checkLocal("this");
}),
- // TODO(shertz) stepping over must take kotlin inline range into account.
stepOver(),
inspect(s -> {
assertEquals("KotlinInline", s.getClassName());
@@ -35,7 +34,7 @@
assertEquals(42, s.getLineNumber());
s.checkLocal("this");
}),
- stepOver(),
+ kotlinStepOver(),
inspect(s -> {
assertEquals("KotlinInline", s.getClassName());
assertEquals(methodName, s.getMethodName());
@@ -46,7 +45,6 @@
run());
}
- @Ignore("Requires kotlin-specific stepping behavior")
@Test
public void testStepIntoInline() throws Throwable {
String methodName = "singleInline";
@@ -60,7 +58,14 @@
assertEquals(41, s.getLineNumber());
s.checkLocal("this");
}),
- // TODO(shertz) stepping over must take kotlin inline range into account.
+ stepOver(),
+ inspect(s -> {
+ assertEquals("KotlinInline", s.getClassName());
+ assertEquals(methodName, s.getMethodName());
+ assertEquals("KotlinInline.kt", s.getSourceFile());
+ assertEquals(42, s.getLineNumber());
+ s.checkLocal("this");
+ }),
stepInto(),
inspect(s -> {
assertEquals("KotlinInline", s.getClassName());
@@ -76,7 +81,6 @@
run());
}
- @Ignore("Requires kotlin-specific stepping behavior")
@Test
public void testStepOutInline() throws Throwable {
String methodName = "singleInline";
@@ -90,13 +94,20 @@
assertEquals(41, s.getLineNumber());
s.checkLocal("this");
}),
- // TODO(shertz) stepping out must take kotlin inline range into account.
+ stepOver(),
+ inspect(s -> {
+ assertEquals("KotlinInline", s.getClassName());
+ assertEquals(methodName, s.getMethodName());
+ assertEquals("KotlinInline.kt", s.getSourceFile());
+ assertEquals(42, s.getLineNumber());
+ s.checkLocal("this");
+ }),
stepInto(),
inspect(s -> {
assertEquals("KotlinInline", s.getClassName());
assertEquals(methodName, s.getMethodName());
}),
- stepOut(),
+ kotlinStepOut(),
inspect(s -> {
assertEquals("KotlinInline", s.getClassName());
assertEquals(methodName, s.getMethodName());
diff --git a/src/test/java/com/android/tools/r8/debug/KotlinTest.java b/src/test/java/com/android/tools/r8/debug/KotlinTest.java
index a6b57b8..2baa426 100644
--- a/src/test/java/com/android/tools/r8/debug/KotlinTest.java
+++ b/src/test/java/com/android/tools/r8/debug/KotlinTest.java
@@ -8,7 +8,7 @@
import org.apache.harmony.jpda.tests.framework.jdwp.Value;
import org.junit.Test;
-public class KotlinTest extends DebugTestBase {
+public class KotlinTest extends KotlinDebugTestBase {
// TODO(shertz) simplify test
// TODO(shertz) add more variables ?
diff --git a/src/test/java/com/android/tools/r8/debug/LocalsTest.java b/src/test/java/com/android/tools/r8/debug/LocalsTest.java
index 35aeab2..9b0a888 100644
--- a/src/test/java/com/android/tools/r8/debug/LocalsTest.java
+++ b/src/test/java/com/android/tools/r8/debug/LocalsTest.java
@@ -20,50 +20,6 @@
public static final String SOURCE_FILE = "Locals.java";
@Test
- // TODO(b/65402086) Remove @ignore when debug behavior will be fixed.
- @Ignore
- public void testLocalConstantBis() throws Throwable {
- final String className = "Locals";
- final String methodName = "localConstantBis";
- runDebugTest(className,
- breakpoint(className, methodName),
- run(),
- checkLine(SOURCE_FILE, 332),
- checkNoLocal("result"),
- stepOver(),
- checkLine(SOURCE_FILE, 333),
- checkLocal("result", Value.createInt(0)),
- stepOver(),
- checkLine(SOURCE_FILE, 334),
- checkLocal("result", Value.createInt(0)),
- stepOver(),
- checkLine(SOURCE_FILE, 338),
- checkLocal("result", Value.createInt(1)),
- run());
- }
-
- @Test
- public void testLocalConstant() throws Throwable {
- final String className = "Locals";
- final String methodName = "localConstant";
- runDebugTest(className,
- breakpoint(className, methodName),
- run(),
- checkLine(SOURCE_FILE, 322),
- checkNoLocal("result1"),
- checkNoLocal("result2"),
- stepOver(),
- checkLine(SOURCE_FILE, 323),
- checkNoLocal("result1"),
- checkNoLocal("result2"),
- stepOver(),
- checkLine(SOURCE_FILE, 324),
- checkLocal("result1"),
- checkNoLocal("result2"),
- run());
- }
-
- @Test
public void testNoLocal() throws Throwable {
final String className = "Locals";
final String methodName = "noLocals";
@@ -610,4 +566,46 @@
checkLine(SOURCE_FILE, 319),
run());
}
+
+ @Test
+ public void testLocalConstantBis() throws Throwable {
+ final String className = "Locals";
+ final String methodName = "localConstantBis";
+ runDebugTest(className,
+ breakpoint(className, methodName),
+ run(),
+ checkLine(SOURCE_FILE, 332),
+ checkNoLocal("result"),
+ stepOver(),
+ checkLine(SOURCE_FILE, 333),
+ checkLocal("result", Value.createInt(0)),
+ stepOver(),
+ checkLine(SOURCE_FILE, 334),
+ checkLocal("result", Value.createInt(0)),
+ stepOver(),
+ checkLine(SOURCE_FILE, 338),
+ checkLocal("result", Value.createInt(1)),
+ run());
+ }
+
+ @Test
+ public void testLocalConstant() throws Throwable {
+ final String className = "Locals";
+ final String methodName = "localConstant";
+ runDebugTest(className,
+ breakpoint(className, methodName),
+ run(),
+ checkLine(SOURCE_FILE, 322),
+ checkNoLocal("result1"),
+ checkNoLocal("result2"),
+ stepOver(),
+ checkLine(SOURCE_FILE, 323),
+ checkNoLocal("result1"),
+ checkNoLocal("result2"),
+ stepOver(),
+ checkLine(SOURCE_FILE, 324),
+ checkLocal("result1"),
+ checkNoLocal("result2"),
+ run());
+ }
}
diff --git a/src/test/java/com/android/tools/r8/jasmin/DebugLocalTests.java b/src/test/java/com/android/tools/r8/jasmin/DebugLocalTests.java
index 835474b..8c93a8a 100644
--- a/src/test/java/com/android/tools/r8/jasmin/DebugLocalTests.java
+++ b/src/test/java/com/android/tools/r8/jasmin/DebugLocalTests.java
@@ -15,7 +15,6 @@
import com.android.tools.r8.utils.DexInspector.ClassSubject;
import com.android.tools.r8.utils.DexInspector.MethodSubject;
import com.google.common.collect.ImmutableList;
-import org.junit.Ignore;
import org.junit.Test;
public class DebugLocalTests extends JasminTestBase {
@@ -117,7 +116,6 @@
}
@Test
- @Ignore("b/65430598")
public void testNoLocalInfoOnStack() throws Exception {
JasminBuilder builder = new JasminBuilder();
JasminBuilder.ClassBuilder clazz = builder.addClass("Test");
@@ -489,11 +487,11 @@
"L3_ORIGINAL:", // This is where javac normally ends t.
".line 7",
" iload 0",
- "L3:", // Moved L3 here to end t on the switch instruction.
"lookupswitch",
" 0: L4",
" 100: L5",
" default: L6",
+ "L3:", // Moved L3 here to end t on the switch instruction.
"L4:",
".line 9",
" iconst_0",
@@ -595,7 +593,6 @@
"L3_ORIGINAL:", // This is where javac normally ends t.
".line 7",
" iload 0",
- "L3:", // Moved L3 here to end t on the switch instruction.
"lookupswitch",
" 0: L4",
" 1: L5",
@@ -604,6 +601,7 @@
" 101: L8",
" 102: L9",
" default: L10",
+ "L3:", // Moved L3 here to end t on the switch instruction.
"L4:",
".line 9",
" iconst_0",
@@ -663,4 +661,64 @@
info.checkLineHasExactLocals(11, "x", "int");
info.checkLineHasExactLocals(13, "x", "int");
}
+
+ @Test
+ public void testLocalEndAfterLine() throws Exception {
+ JasminBuilder builder = new JasminBuilder();
+ JasminBuilder.ClassBuilder clazz = builder.addClass("Test");
+ MethodSignature foo = clazz.addStaticMethod("foo", ImmutableList.of("I"), "I",
+ ".limit stack 2",
+ ".limit locals 2",
+ ".var 0 is x I from L0 to L4",
+ ".var 1 is t I from L1 to L3",
+ "L0:",
+ ".line 1",
+ " iload 0",
+ " iconst_1",
+ " iadd",
+ " istore 1",
+ "L1:",
+ ".line 2",
+ " iload 1",
+ " istore 0",
+ "L2:",
+ ".line 3",
+ " iload 0",
+ " iload 0",
+ " iadd",
+ " istore 0",
+ ".line 4",
+ " iload 0",
+ "L3:", // This should not end t on line 4!
+ ".line 5",
+ " ireturn",
+ "L4:"
+ );
+
+ clazz.addMainMethod(
+ ".limit stack 2",
+ ".limit locals 1",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " ldc 1",
+ " invokestatic Test/foo(I)I",
+ " invokevirtual java/io/PrintStream/print(I)V",
+ " return");
+
+ String expected = "4";
+ String javaResult = runOnJava(builder, clazz.name);
+ assertEquals(expected, javaResult);
+
+ AndroidApp jasminApp = builder.build();
+ AndroidApp d8App = ToolHelper.runD8(jasminApp);
+ String artResult = runOnArt(d8App, clazz.name);
+ assertEquals(expected, artResult);
+
+ DebugInfoInspector info = new DebugInfoInspector(d8App, clazz.name, foo);
+ info.checkStartLine(1);
+ info.checkLineHasExactLocals(1, "x", "int");
+ info.checkLineHasExactLocals(2, "x", "int", "t", "int");
+ info.checkLineHasExactLocals(3, "x", "int", "t", "int");
+ info.checkLineHasExactLocals(4, "x", "int", "t", "int");
+ info.checkLineHasExactLocals(5, "x", "int");
+ }
}
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 4122416..a316969 100644
--- a/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java
+++ b/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java
@@ -622,11 +622,6 @@
}
@Override
- public boolean needsPrelude() {
- return false;
- }
-
- @Override
public DebugLocalInfo getCurrentLocal(int register) {
return null;
}
@@ -637,16 +632,12 @@
}
@Override
- public void closedCurrentBlockWithFallthrough(int fallthroughInstructionIndex) {
+ public void closingCurrentBlockWithFallthrough(
+ int fallthroughInstructionIndex, IRBuilder builder) {
throw new Unreachable();
}
@Override
- public void closedCurrentBlock() {
- // Intentionally empty.
- }
-
- @Override
public void setUp() {
// Intentionally empty.
}
diff --git a/tools/archive.py b/tools/archive.py
index ee1053d..3d4523b 100755
--- a/tools/archive.py
+++ b/tools/archive.py
@@ -6,6 +6,7 @@
import d8
import os
import r8
+import subprocess
import sys
import utils
@@ -21,27 +22,58 @@
'Version mismatch: \n%s\n%s' % (d8_version, r8_version))
return d8_version.split()[1]
-def GetStorageDestination(storage_prefix, version, file_name):
- return '%s%s/raw/%s/%s' % (storage_prefix, ARCHIVE_BUCKET, version, file_name)
+def GetGitBranches():
+ return subprocess.check_output(['git', 'show', '-s', '--pretty=%d', 'HEAD'])
-def GetUploadDestination(version, file_name):
- return GetStorageDestination('gs://', version, file_name)
+def GetGitHash():
+ return subprocess.check_output(['git', 'rev-parse', 'HEAD']).strip()
-def GetUrl(version, file_name):
+def IsMaster(version):
+ branches = subprocess.check_output(['git', 'show', '-s', '--pretty=%d',
+ 'HEAD'])
+ if not version.endswith('-dev'):
+ # Sanity check, we don't want to archive on top of release builds EVER
+ # Note that even though we branch, we never push the bots to build the same
+ # commit as master on a branch since we always change the version to
+ # not have dev (or we crash here :-)).
+ if 'origin/master' in branches:
+ raise Exception('We are seeing origin/master in a commit that '
+ 'don\'t have -dev in version')
+ return False;
+ if not 'origin/master' in branches:
+ raise Exception('We are not seeing origin/master '
+ 'in a commit that have -dev in version')
+ return True;
+
+def GetStorageDestination(storage_prefix, version, file_name, is_master):
+ # We archive master commits under raw/master instead of directly under raw
+ archive_dir = 'raw/master' if is_master else 'raw'
+ return '%s%s/%s/%s/%s' % (storage_prefix, ARCHIVE_BUCKET, archive_dir,
+ version, file_name)
+
+def GetUploadDestination(version, file_name, is_master):
+ return GetStorageDestination('gs://', version, file_name, is_master)
+
+def GetUrl(version, file_name, is_master):
return GetStorageDestination('http://storage.googleapis.com/',
- version,
- file_name)
+ version, file_name, is_master)
def Main():
if not 'BUILDBOT_BUILDERNAME' in os.environ:
raise Exception('You are not a bot, don\'t archive builds')
version = GetVersion()
+ is_master = IsMaster(version)
+ if is_master:
+ # On master we use the git hash to archive with
+ print 'On master, using git hash for archiving'
+ version = GetGitHash()
+
for jar in [utils.D8_JAR, utils.R8_JAR]:
file_name = os.path.basename(jar)
- destination = GetUploadDestination(version, file_name)
+ destination = GetUploadDestination(version, file_name, is_master)
print('Uploading %s to %s' % (jar, destination))
utils.upload_file_to_cloud_storage(jar, destination)
- print('File available at: %s' % GetUrl(version, file_name))
+ print('File available at: %s' % GetUrl(version, file_name, is_master))
if __name__ == '__main__':
sys.exit(Main())
diff --git a/tools/checkout_aosp.py b/tools/checkout_aosp.py
new file mode 100755
index 0000000..d7208ab
--- /dev/null
+++ b/tools/checkout_aosp.py
@@ -0,0 +1,55 @@
+#!/usr/bin/env python
+# Copyright (c) 2017, the R8 project authors. Please see the AUTHORS file
+# for details. All rights reserved. Use of this source code is governed by a
+# BSD-style license that can be found in the LICENSE file.
+
+from os.path import basename, join
+from shutil import copy2
+from subprocess import check_call
+import argparse
+import multiprocessing
+import os
+import sys
+
+import utils
+
+AOSP_MANIFEST_XML = join(utils.REPO_ROOT, 'third_party',
+ 'aosp_manifest.xml')
+AOSP_MANIFEST_URL = 'https://android.googlesource.com/platform/manifest'
+
+J_DEFAULT = multiprocessing.cpu_count() - 2
+
+# Checkout AOSP source to the specified direcotry using the speficied manifest.
+def checkout_aosp(aosp_root, manifest_xml, concurrency):
+ manifests_dir = join(aosp_root, '.repo', 'manifests')
+ utils.makedirs_if_needed(manifests_dir)
+
+ copy2(manifest_xml, manifests_dir)
+ check_call(['repo', 'init', '-u', AOSP_MANIFEST_URL, '-m',
+ basename(manifest_xml), '--depth=1'], cwd = aosp_root)
+
+ check_call(['repo', 'sync', '-dq', '-j' + concurrency], cwd = aosp_root)
+
+def parse_arguments():
+ parser = argparse.ArgumentParser(
+ description = 'Checkout the AOSP source tree.')
+ parser.add_argument('--aosp-root',
+ help='Root of the AOSP checkout. ' +
+ 'Defaults to current working directory.',
+ default=os.getcwd())
+ parser.add_argument('--manifest',
+ help='Manifest to use for the checkout. ' +
+ 'Defaults to ' + AOSP_MANIFEST_XML + '.',
+ default=AOSP_MANIFEST_XML)
+ parser.add_argument('-j',
+ help='Projects to fetch simultaneously. ' +
+ 'Defaults to ' + str(J_DEFAULT) + '.',
+ default=str(J_DEFAULT))
+ return parser.parse_args()
+
+def Main():
+ args = parse_arguments()
+ checkout_aosp(args.aosp_root, args.manifest, args.j)
+
+if __name__ == '__main__':
+ sys.exit(Main())
diff --git a/tools/test_android_cts.py b/tools/test_android_cts.py
index 887e3ae..ccb762b 100755
--- a/tools/test_android_cts.py
+++ b/tools/test_android_cts.py
@@ -30,6 +30,7 @@
import sys
import time
+import checkout_aosp
import gradle
import utils
@@ -53,7 +54,8 @@
CTS_TRADEFED = join(OUT_CTS,
'host/linux-x86/cts/android-cts/tools/cts-tradefed')
-J_OPTION = '-j8'
+J_DEFAULT = '8'
+J_OPTION = '-j' + J_DEFAULT
EXIT_FAILURE = 1
@@ -142,17 +144,6 @@
if counter > 0:
print('Removed {} dex files.'.format(counter))
-def checkout_aosp():
- # checkout AOSP source
- manifests_dir = join(AOSP_ROOT, '.repo', 'manifests')
- utils.makedirs_if_needed(manifests_dir)
-
- copy2(AOSP_MANIFEST_XML, manifests_dir)
- check_call(['repo', 'init', '-u', AOSP_MANIFEST_URL, '-m',
- 'aosp_manifest.xml', '--depth=1'], cwd = AOSP_ROOT)
-
- check_call(['repo', 'sync', '-dq', J_OPTION], cwd = AOSP_ROOT)
-
def Main():
args = parse_arguments()
@@ -179,7 +170,7 @@
setup_and_clean(args.tool == 'd8', args.clean_dex)
- checkout_aosp()
+ checkout_aosp.checkout_aosp(AOSP_ROOT, AOSP_MANIFEST_XML, J_DEFAULT)
# activate OUT_CTS and build Android CTS
# AOSP has no clean way to set the output directory.
