Associate a non-none position to all instructions in debug mode.
Bug: 78619571, 110873173
Change-Id: I1a3d4a8b7da2408cc67664a2c3a6ecc884245bb6
diff --git a/src/main/java/com/android/tools/r8/cf/LoadStoreHelper.java b/src/main/java/com/android/tools/r8/cf/LoadStoreHelper.java
index 12c1321..aa3457b 100644
--- a/src/main/java/com/android/tools/r8/cf/LoadStoreHelper.java
+++ b/src/main/java/com/android/tools/r8/cf/LoadStoreHelper.java
@@ -63,8 +63,9 @@
}
}
InstructionListIterator it = pred.listIterator(pred.getInstructions().size());
- it.previous();
- movePhis(moves, it);
+ Instruction exit = it.previous();
+ assert pred.exit() == exit;
+ movePhis(moves, it, exit.getPosition());
}
allocator.addToLiveAtEntrySet(block, block.getPhis());
}
@@ -137,13 +138,13 @@
}
}
- private void movePhis(List<PhiMove> moves, InstructionListIterator it) {
+ private void movePhis(List<PhiMove> moves, InstructionListIterator it, Position position) {
// TODO(zerny): Accounting for non-interfering phis would lower the max stack size.
int topOfStack = 0;
List<StackValue> temps = new ArrayList<>(moves.size());
for (PhiMove move : moves) {
StackValue tmp = createStackValue(move.phi, topOfStack++);
- add(load(tmp, move.operand), move.phi.getBlock(), Position.none(), it);
+ add(load(tmp, move.operand), move.phi.getBlock(), position, it);
temps.add(tmp);
move.operand.removePhiUser(move.phi);
}
@@ -151,7 +152,7 @@
PhiMove move = moves.get(i);
StackValue tmp = temps.get(i);
FixedLocalValue out = new FixedLocalValue(move.phi);
- add(new Store(out, tmp), move.phi.getBlock(), Position.none(), it);
+ add(new Store(out, tmp), move.phi.getBlock(), position, it);
move.phi.replaceUsers(out);
}
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java b/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
index bfc0a9f..18f23e3 100644
--- a/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
+++ b/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
@@ -82,16 +82,8 @@
Position position = instruction.getPosition();
boolean pcAdvancing = pc != postPc;
- // In release mode we can only check that all throwing instructions have positions.
- // See IRCode's isConsistentGraph and computeAllThrowingInstructionsHavePositions.
-
// In debug mode check that all non-nop instructions have positions.
- assert startLine == NO_LINE_INFO || !hasDebugPositions || !pcAdvancing || position.isSome()
- : "PC-advancing instruction " + instruction + " expected to have an associated position.";
-
- // In any mode check that nop instructions have no position info.
- assert pcAdvancing || position.isNone()
- : "Nop instruction " + instruction + " must never have an associated position.";
+ assert instruction.verifyValidPositionInfo(options.debug);
if (instruction.isArgument()) {
startArgument(instruction.asArgument());
diff --git a/src/main/java/com/android/tools/r8/ir/code/AlwaysMaterializingUser.java b/src/main/java/com/android/tools/r8/ir/code/AlwaysMaterializingUser.java
index cd6a731..91be1a4 100644
--- a/src/main/java/com/android/tools/r8/ir/code/AlwaysMaterializingUser.java
+++ b/src/main/java/com/android/tools/r8/ir/code/AlwaysMaterializingUser.java
@@ -16,8 +16,6 @@
public AlwaysMaterializingUser(Value src) {
super(null, src);
- // The user instruction never materializes so ensure it has position none.
- setPosition(Position.none());
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/code/Argument.java b/src/main/java/com/android/tools/r8/ir/code/Argument.java
index ac1ed7a..b5fea0b 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Argument.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Argument.java
@@ -24,12 +24,6 @@
public Argument(Value outValue) {
super(outValue);
outValue.markAsArgument();
- super.setPosition(Position.none());
- }
-
- @Override
- public void setPosition(Position position) {
- // Arguments never have positional information as they never materialize to actual instructions.
}
@Override
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 a2962ab..51ed473 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
@@ -47,9 +47,9 @@
private Int2ReferenceMap<DebugLocalInfo> localsAtEntry;
- public boolean consistentBlockInstructions(boolean argumentsAllowed) {
+ public boolean consistentBlockInstructions(boolean argumentsAllowed, boolean debug) {
for (Instruction instruction : getInstructions()) {
- assert instruction.getPosition() != null;
+ assert instruction.verifyValidPositionInfo(debug);
assert instruction.getBlock() == this;
assert !instruction.isArgument() || argumentsAllowed;
assert !instruction.isDebugLocalRead() || !instruction.getDebugValues().isEmpty();
@@ -290,6 +290,7 @@
}
Instruction exit = new Goto();
exit.setBlock(this);
+ exit.setPosition(instruction.getPosition());
getInstructions().addLast(exit);
} else if (indexOfOldBlock >= successors.size() - 2) {
// Old is either true or fallthrough and we need to swap the new block into the right
@@ -1099,14 +1100,14 @@
/**
* Create a new basic block with a single goto instruction.
*
- * <p>The constructed basic block has no predecessors and has one
- * successors which is the target block.
+ * <p>The constructed basic block has no predecessors and has one successor which is the target
+ * block.
*
* @param blockNumber the block number of the goto block
* @param target the target of the goto block
*/
- public static BasicBlock createGotoBlock(int blockNumber, BasicBlock target) {
- BasicBlock block = createGotoBlock(blockNumber);
+ public static BasicBlock createGotoBlock(int blockNumber, Position position, BasicBlock target) {
+ BasicBlock block = createGotoBlock(blockNumber, position);
block.getSuccessors().add(target);
return block;
}
@@ -1118,11 +1119,12 @@
*
* @param blockNumber the block number of the goto block
*/
- public static BasicBlock createGotoBlock(int blockNumber) {
+ public static BasicBlock createGotoBlock(int blockNumber, Position position) {
BasicBlock block = new BasicBlock();
block.add(new Goto());
block.close(null);
block.setNumber(blockNumber);
+ block.entry().setPosition(position);
return block;
}
@@ -1228,8 +1230,7 @@
}
public Position getPosition() {
- BasicBlock block = endOfGotoChain();
- return block != null ? block.entry().getPosition() : Position.none();
+ return entry().getPosition();
}
public boolean hasOneNormalExit() {
@@ -1459,7 +1460,9 @@
newBlock.add(newMove);
newMove.setPosition(position);
}
- newBlock.add(new Goto());
+ Goto next = new Goto();
+ next.setPosition(position);
+ newBlock.add(next);
newBlock.close(null);
newBlock.getSuccessors().add(this);
newBlock.getPredecessors().add(predecessor);
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionIterator.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionIterator.java
index 2fcbed1..08d7a07 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionIterator.java
@@ -193,6 +193,9 @@
// Don't allow splitting after the last instruction.
assert hasNext();
+ // Get the position at which the block is being split.
+ Position position = current != null ? current.getPosition() : block.getPosition();
+
// Prepare the new block, placing the exception handlers on the block with the throwing
// instruction.
boolean keepCatchHandlers = hasPrevious() && peekPrevious().instructionTypeCanThrow();
@@ -201,6 +204,7 @@
// Add a goto instruction.
Goto newGoto = new Goto(block);
listIterator.add(newGoto);
+ newGoto.setPosition(position);
// Move all remaining instructions to the new block.
while (listIterator.hasNext()) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/ConstNumber.java b/src/main/java/com/android/tools/r8/ir/code/ConstNumber.java
index 6f5fa47..73cf1c2 100644
--- a/src/main/java/com/android/tools/r8/ir/code/ConstNumber.java
+++ b/src/main/java/com/android/tools/r8/ir/code/ConstNumber.java
@@ -118,7 +118,6 @@
@Override
public void buildDex(DexBuilder builder) {
if (!dest().needsRegister()) {
- forceSetPosition(Position.none());
builder.addNothing(this);
return;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/DebugLocalsChange.java b/src/main/java/com/android/tools/r8/ir/code/DebugLocalsChange.java
index 5de4b91..c956593 100644
--- a/src/main/java/com/android/tools/r8/ir/code/DebugLocalsChange.java
+++ b/src/main/java/com/android/tools/r8/ir/code/DebugLocalsChange.java
@@ -35,6 +35,11 @@
throw new Unreachable();
}
+ @Override
+ public boolean verifyValidPositionInfo(boolean debug) {
+ return true;
+ }
+
public Int2ReferenceMap<DebugLocalInfo> getEnding() {
return ending;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Goto.java b/src/main/java/com/android/tools/r8/ir/code/Goto.java
index 80a76fe..972d2d2 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Goto.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Goto.java
@@ -15,7 +15,6 @@
public Goto() {
super(null);
- super.setPosition(Position.none());
}
public Goto(BasicBlock block) {
@@ -23,12 +22,6 @@
setBlock(block);
}
- @Override
- public void setPosition(Position position) {
- // In general goto's do not signify program points only transitions, so we avoid
- // associating them with positional information.
- }
-
public BasicBlock getTarget() {
assert getBlock().exit() == this;
List<BasicBlock> successors = getBlock().getSuccessors();
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 96cc33a..1cafe2a 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
@@ -95,6 +95,7 @@
ValueNumberGenerator valueNumberGenerator,
boolean hasDebugPositions,
boolean hasConstString) {
+ assert options != null;
this.options = options;
this.method = method;
this.blocks = blocks;
@@ -212,7 +213,8 @@
// correct predecessor and successor structure. It is inserted
// at the end of the list of blocks disregarding branching
// structure.
- BasicBlock newBlock = BasicBlock.createGotoBlock(nextBlockNumber++, block);
+ BasicBlock newBlock =
+ BasicBlock.createGotoBlock(nextBlockNumber++, pred.exit().getPosition(), block);
newBlocks.add(newBlock);
pred.replaceSuccessor(block, newBlock);
newBlock.getPredecessors().add(pred);
@@ -268,7 +270,9 @@
fallthrough = fallthrough.exit().fallthroughBlock();
}
if (fallthrough != null) {
- BasicBlock newFallthrough = BasicBlock.createGotoBlock(nextBlockNumber++, fallthrough);
+ BasicBlock newFallthrough =
+ BasicBlock.createGotoBlock(
+ nextBlockNumber++, block.exit().getPosition(), fallthrough);
current.exit().setFallthroughBlock(newFallthrough);
newFallthrough.getPredecessors().add(current);
fallthrough.replacePredecessor(current, newFallthrough);
@@ -563,7 +567,7 @@
private boolean consistentBlockInstructions() {
boolean argumentsAllowed = true;
for (BasicBlock block : blocks) {
- block.consistentBlockInstructions(argumentsAllowed);
+ block.consistentBlockInstructions(argumentsAllowed, options.debug);
argumentsAllowed = false;
}
return true;
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 bae912d..6f62d21 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
@@ -1116,4 +1116,11 @@
public boolean triggersInitializationOfClass(DexType klass) {
return false;
}
+
+ public boolean verifyValidPositionInfo(boolean debug) {
+ assert position != null;
+ assert !debug || getPosition().isSome();
+ assert !instructionTypeCanThrow() || getPosition().isSome() || getPosition().isSyntheticNone();
+ return true;
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Position.java b/src/main/java/com/android/tools/r8/ir/code/Position.java
index 74aaadf..ec923ee 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Position.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Position.java
@@ -5,12 +5,22 @@
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexString;
+import com.google.common.annotations.VisibleForTesting;
import java.util.Objects;
public class Position {
+ // A no-position marker. Not having a position means the position is implicitly defined by the
+ // context, e.g., the marker does not materialize anything concrete.
private static final Position NO_POSITION = new Position(-1, null, null, null, false);
+ // A synthetic marker position that should never materialize.
+ // This is used specifically to mark exceptional exit blocks from synchronized methods in release.
+ private static final Position NO_POSITION_SYNTHETIC = new Position(-1, null, null, null, true);
+
+ // Fake position to use for representing an actual position in testing code.
+ private static final Position TESTING_POSITION = new Position(0, null, null, null, true);
+
public final int line;
public final DexString file;
public final boolean synthetic;
@@ -26,6 +36,7 @@
public Position(int line, DexString file, DexMethod method, Position callerPosition) {
this(line, file, method, callerPosition, false);
assert line >= 0;
+ assert method != null;
}
private Position(
@@ -36,11 +47,11 @@
this.method = method;
this.callerPosition = callerPosition;
assert callerPosition == null || callerPosition.method != null;
- assert line == -1 || method != null; // It's NO_POSITION or must have valid method.
}
public static Position synthetic(int line, DexMethod method, Position callerPosition) {
assert line >= 0;
+ assert method != null;
return new Position(line, null, method, callerPosition, true);
}
@@ -48,6 +59,15 @@
return NO_POSITION;
}
+ public static Position syntheticNone() {
+ return NO_POSITION_SYNTHETIC;
+ }
+
+ @VisibleForTesting
+ public static Position testingPosition() {
+ return TESTING_POSITION;
+ }
+
// This factory method is used by the Inliner to create Positions when the caller has no valid
// positions. Since the callee still may have valid positions we need a non-null Position to set
// it as the caller of the inlined Positions.
@@ -60,6 +80,10 @@
return line == -1;
}
+ public boolean isSyntheticNone() {
+ return this == NO_POSITION_SYNTHETIC;
+ }
+
public boolean isSome() {
return !isNone();
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
index 4d0a5ba..37a0150 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
@@ -139,9 +139,7 @@
registerAllocator.allocateRegisters();
loadStoreHelper.insertPhiMoves(registerAllocator);
CodeRewriter.collapsTrivialGotos(method, code);
- int instructionTableCount =
- DexBuilder.instructionNumberToIndex(code.numberRemainingInstructions());
- DexBuilder.removeRedundantDebugPositions(code, instructionTableCount);
+ DexBuilder.removeRedundantDebugPositions(code);
CfCode code = buildCfCode();
return code;
}
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 07107a5..0084a74 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
@@ -50,7 +50,6 @@
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.Position;
@@ -65,7 +64,6 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
-import it.unimi.dsi.fastutil.ints.Int2ReferenceMaps;
import it.unimi.dsi.fastutil.ints.Int2ReferenceOpenHashMap;
import java.util.ArrayList;
import java.util.List;
@@ -158,7 +156,7 @@
// Remove redundant debug position instructions. They would otherwise materialize as
// unnecessary nops.
- removeRedundantDebugPositions(ir, instructionToInfo.length);
+ removeRedundantDebugPositions(ir);
// Populate the builder info objects.
numberOfInstructions = 0;
@@ -251,114 +249,95 @@
return code;
}
- private static boolean verifyNopHasNoPosition(
- com.android.tools.r8.ir.code.Instruction instruction, ListIterator<BasicBlock> blocks) {
- BasicBlock nextBlock = null;
- if (blocks.hasNext()) {
- nextBlock = blocks.next();
- blocks.previous();
- }
- return verifyNopHasNoPosition(instruction, nextBlock);
- }
-
- private static boolean verifyNopHasNoPosition(
- com.android.tools.r8.ir.code.Instruction instruction, BasicBlock nextBlock) {
- if (isNopInstruction(instruction, nextBlock)) {
- assert instruction.getPosition().isNone();
- }
- return true;
- }
-
// Eliminates unneeded debug positions.
//
- // After this pass all instructions that don't materialize to an actual DEX/CF instruction will
- // have Position.none(). If any other instruction has a non-none position then all other
- // instructions that do materialize to a DEX/CF instruction (eg, non-fallthrough gotos) will have
- // a non-none position.
- //
- // Remaining debug positions indicate two successive lines without intermediate instructions.
- // For these we must emit a nop instruction to ensure they don't share the same pc.
- public static void removeRedundantDebugPositions(IRCode code, int instructionTableSize) {
+ // After this pass all remaining debug positions mark places where we must ensure a materializing
+ // instruction, eg, for two successive lines without intermediate instructions.
+ public static void removeRedundantDebugPositions(IRCode code) {
if (!code.hasDebugPositions) {
return;
}
- Int2ReferenceMap[] localsMap = new Int2ReferenceMap[instructionTableSize];
- // Scan forwards removing debug positions equal to the previous instruction position.
- {
- Int2ReferenceMap<DebugLocalInfo> locals = Int2ReferenceMaps.emptyMap();
- Position previous = Position.none();
- ListIterator<BasicBlock> blockIterator = code.listIterator();
- BasicBlock previousBlock = null;
- while (blockIterator.hasNext()) {
- BasicBlock block = blockIterator.next();
- if (previousBlock != null
- && previousBlock.exit().isGoto()
- && !isNopInstruction(previousBlock.exit(), block)) {
- assert previousBlock.exit().getPosition().isNone()
- || previousBlock.exit().getPosition().equals(previous);
- previousBlock.exit().forceSetPosition(previous);
+ // Current position known to have a materializing instruction associated with it.
+ Position currentMaterializedPosition = Position.none();
+
+ // Current debug-position marker that is not yet known to have another instruction materializing
+ // to the same position.
+ DebugPosition unresolvedPosition = null;
+
+ // Locals live at the debug-position marker. These must also be the same at a possible
+ // materializing instruction with the same position for it to be sound to remove the marker.
+ Int2ReferenceMap<DebugLocalInfo> localsAtUnresolvedPosition = null;
+
+ // Compute the set of all positions that can be removed.
+ // (Delaying removal to avoid ConcurrentModificationException).
+ List<DebugPosition> toRemove = new ArrayList<>();
+
+ for (int blockIndex = 0; blockIndex < code.blocks.size(); blockIndex++) {
+ BasicBlock currentBlock = code.blocks.get(blockIndex);
+ BasicBlock nextBlock =
+ blockIndex + 1 < code.blocks.size() ? code.blocks.get(blockIndex + 1) : null;
+
+ // Current materialized position can remain on block entry only if it is also the exit of
+ // the blocks predecessors. If not, we cannot ensure the jumps will hit this line unless
+ // another instruction materialized the position.
+ for (BasicBlock pred : currentBlock.getPredecessors()) {
+ if (pred.exit().getPosition() != currentMaterializedPosition) {
+ currentMaterializedPosition = Position.none();
+ break;
}
- InstructionListIterator instructionIterator = block.listIterator();
- if (block.getLocalsAtEntry() != null && !locals.equals(block.getLocalsAtEntry())) {
- locals = new Int2ReferenceOpenHashMap<>(block.getLocalsAtEntry());
- }
- while (instructionIterator.hasNext()) {
- com.android.tools.r8.ir.code.Instruction instruction = instructionIterator.next();
- if (instruction.isDebugPosition() && previous.equals(instruction.getPosition())) {
- instructionIterator.remove();
- } else if (isNonMaterializingConstNumber(instruction)) {
- instruction.forceSetPosition(Position.none());
- } else if (instruction.getPosition().isSome()) {
- assert verifyNopHasNoPosition(instruction, blockIterator);
- previous = instruction.getPosition();
+ }
+
+ // Current locals.
+ Int2ReferenceMap<DebugLocalInfo> locals =
+ currentBlock.getLocalsAtEntry() != null
+ ? new Int2ReferenceOpenHashMap<>(currentBlock.getLocalsAtEntry())
+ : new Int2ReferenceOpenHashMap<>();
+
+ InstructionIterator iterator = currentBlock.iterator();
+ while (iterator.hasNext()) {
+ com.android.tools.r8.ir.code.Instruction instruction = iterator.next();
+ if (instruction.isDebugPosition()) {
+ if (unresolvedPosition == null
+ && currentMaterializedPosition == instruction.getPosition()) {
+ // Here we don't need to check locals state as the line is already active.
+ toRemove.add(instruction.asDebugPosition());
+ } else if (unresolvedPosition != null
+ && unresolvedPosition.getPosition() == instruction.getPosition()
+ && locals.equals(localsAtUnresolvedPosition)) {
+ toRemove.add(instruction.asDebugPosition());
+ } else {
+ unresolvedPosition = instruction.asDebugPosition();
+ localsAtUnresolvedPosition = new Int2ReferenceOpenHashMap<>(locals);
}
+ } else if (instruction.getPosition().isSome()) {
+ if (!isNopInstruction(instruction, nextBlock)) {
+ if (unresolvedPosition != null) {
+ if (unresolvedPosition.getPosition() == instruction.getPosition()
+ && locals.equals(localsAtUnresolvedPosition)) {
+ toRemove.add(unresolvedPosition);
+ }
+ unresolvedPosition = null;
+ localsAtUnresolvedPosition = null;
+ }
+ currentMaterializedPosition = instruction.getPosition();
+ }
+ } else {
+ // Only local-change instructions don't have positions in debug mode but fail gracefully.
+ assert instruction.isDebugLocalsChange();
if (instruction.isDebugLocalsChange()) {
- locals = new Int2ReferenceOpenHashMap<>(locals);
instruction.asDebugLocalsChange().apply(locals);
}
- localsMap[instructionNumberToIndex(instruction.getNumber())] = locals;
}
- previousBlock = block;
- }
- if (previousBlock != null && previousBlock.exit().isGoto()) {
- // If the last block ends in a goto it cannot be a fallthrough/nop.
- assert previousBlock.exit().getPosition().isNone();
- previousBlock.exit().forceSetPosition(previous);
}
}
- // Scan backwards removing debug positions equal to the following instruction position.
- {
- ListIterator<BasicBlock> blocks = code.blocks.listIterator(code.blocks.size());
- BasicBlock block = null;
- BasicBlock nextBlock;
- com.android.tools.r8.ir.code.Instruction next = null;
- Int2ReferenceMap nextLocals = null;
- while (blocks.hasPrevious()) {
- nextBlock = block;
- block = blocks.previous();
- InstructionListIterator instructions = block.listIterator(block.getInstructions().size());
- while (instructions.hasPrevious()) {
- com.android.tools.r8.ir.code.Instruction instruction = instructions.previous();
- int index = instructionNumberToIndex(instruction.getNumber());
- if (instruction.isDebugPosition() && localsMap[index].equals(nextLocals)) {
- Position nextPosition = next.getPosition();
- Position thisPosition = instruction.getPosition();
- if (nextPosition.isNone()) {
- next.forceSetPosition(thisPosition);
- instructions.remove();
- } else if (nextPosition.equals(thisPosition)) {
- instructions.remove();
- } else {
- next = instruction;
- }
- } else {
- assert verifyNopHasNoPosition(instruction, nextBlock);
- if (!isNopInstruction(instruction, nextBlock)) {
- next = instruction;
- nextLocals = localsMap[index];
- assert nextLocals != null;
- }
- }
+ // Remove all unneeded positions.
+ if (!toRemove.isEmpty()) {
+ InstructionIterator it = code.instructionIterator();
+ int i = 0;
+ while (it.hasNext() && i < toRemove.size()) {
+ if (it.next() == toRemove.get(i)) {
+ it.removeOrReplaceByDebugLocalRead();
+ ++i;
}
}
}
@@ -385,7 +364,8 @@
if (ifsNeedingRewrite.contains(block)) {
If theIf = block.exit().asIf();
BasicBlock trueTarget = theIf.getTrueTarget();
- BasicBlock newBlock = BasicBlock.createGotoBlock(ir.blocks.size(), trueTarget);
+ BasicBlock newBlock =
+ BasicBlock.createGotoBlock(ir.blocks.size(), theIf.getPosition(), trueTarget);
theIf.setTrueTarget(newBlock);
theIf.invert();
it.add(newBlock);
@@ -438,7 +418,6 @@
}
public void addNothing(com.android.tools.r8.ir.code.Instruction instruction) {
- assert instruction.getPosition().isNone();
add(instruction, new FallThroughInfo(instruction));
}
@@ -496,7 +475,6 @@
public void addReturn(Return ret, Instruction dex) {
if (nextBlock != null
&& ret.identicalAfterRegisterAllocation(nextBlock.entry(), registerAllocator)) {
- ret.forceSetPosition(Position.none());
addNothing(ret);
} else {
add(ret, dex);
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 d14273e..d855e0e 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
@@ -153,10 +153,16 @@
private static class SplitBlockWorklistItem extends WorklistItem {
private final int sourceOffset;
private final int targetOffset;
+ private final Position position;
public SplitBlockWorklistItem(
- int firstInstructionIndex, BasicBlock block, int sourceOffset, int targetOffset) {
+ int firstInstructionIndex,
+ BasicBlock block,
+ Position position,
+ int sourceOffset,
+ int targetOffset) {
super(block, firstInstructionIndex);
+ this.position = position;
this.sourceOffset = sourceOffset;
this.targetOffset = targetOffset;
}
@@ -473,6 +479,7 @@
// Insert definitions for all uninitialized local values.
if (uninitializedDebugLocalValues != null) {
+ Position position = entryBlock.getPosition();
InstructionListIterator it = entryBlock.listIterator();
it.nextUntil(i -> !i.isArgument());
it.previous();
@@ -481,7 +488,7 @@
if (value.isUsed()) {
Instruction def = new DebugLocalUninitialized(value);
def.setBlock(entryBlock);
- def.setPosition(Position.none());
+ def.setPosition(position);
it.add(def);
}
}
@@ -515,11 +522,11 @@
resolver.resolve(ir, this);
}
+ assert ir.isConsistentSSA();
+
// Clear the code so we don't build multiple times.
source.clear();
source = null;
-
- assert ir.isConsistentSSA();
return ir;
}
@@ -631,7 +638,7 @@
this, splitEdgeItem.sourceOffset, splitEdgeItem.targetOffset, false);
if (item.firstInstructionIndex == -1) {
// If the block is a pure split-edge block emit goto (picks up local ends) and close.
- addInstruction(new Goto(), Position.none());
+ addInstruction(new Goto(), splitEdgeItem.position);
closeCurrentBlockGuaranteedNotToNeedEdgeSplitting();
continue;
} else if (!debugLocalEnds.isEmpty()) {
@@ -768,10 +775,15 @@
public void addDebugPosition(Position position) {
if (options.debug) {
+ assert previousLocalValue == null;
assert source.getCurrentPosition().equals(position);
if (!debugLocalEnds.isEmpty()) {
// If there are pending local ends, end them before changing the line.
- addInstruction(new DebugLocalRead(), Position.none());
+ if (currentBlock.getInstructions().isEmpty()) {
+ addInstruction(new DebugLocalRead());
+ } else {
+ attachLocalValues(currentBlock.getInstructions().getLast());
+ }
}
addInstruction(new DebugPosition());
}
@@ -2126,6 +2138,7 @@
assert currentBlock != null;
assert currentBlock.isEmpty() || !currentBlock.getInstructions().getLast().isJumpInstruction();
BlockInfo info = getBlockInfo(currentBlock);
+ Position position = source.getCurrentPosition();
if (info.hasMoreThanASingleNormalExit()) {
// Exceptional edges are always split on construction, so no need to split edges to them.
// Introduce split-edge blocks for all normal edges and push them on the work list.
@@ -2145,13 +2158,14 @@
new SplitBlockWorklistItem(
oldItem.firstInstructionIndex,
oldItem.block,
+ position,
currentInstructionOffset,
successorOffset));
} else {
BasicBlock splitBlock = createSplitEdgeBlock(currentBlock, successorInfo.block);
ssaWorklist.add(
new SplitBlockWorklistItem(
- -1, splitBlock, currentInstructionOffset, successorOffset));
+ -1, splitBlock, position, currentInstructionOffset, successorOffset));
}
}
} else if (info.normalSuccessors.size() == 1) {
@@ -2230,7 +2244,9 @@
int otherPredecessorIndex = values.get(v);
BasicBlock joinBlock = joinBlocks.get(otherPredecessorIndex);
if (joinBlock == null) {
- joinBlock = BasicBlock.createGotoBlock(blocks.size() + blocksToAdd.size(), block);
+ joinBlock =
+ BasicBlock.createGotoBlock(
+ blocks.size() + blocksToAdd.size(), block.getPosition(), block);
joinBlocks.put(otherPredecessorIndex, joinBlock);
blocksToAdd.add(joinBlock);
BasicBlock otherPredecessor = block.getPredecessors().get(otherPredecessorIndex);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index f4a1e4e..6890595 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -1076,6 +1076,7 @@
// Forced user of the forced definition to ensure it has a user and thus live range.
Instruction fixitUser = new AlwaysMaterializingUser(fixitValue);
fixitUser.setBlock(addBefore.getBlock());
+ fixitUser.setPosition(addBefore.getPosition());
it.add(fixitUser);
}
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 0c62134..faa3aae 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
@@ -487,8 +487,10 @@
|| successorOffset == EXCEPTIONAL_SYNC_EXIT_OFFSET) {
return;
}
- // The transfer has not yet taken place, so the current position is that of the predecessor.
- currentPosition = getCanonicalDebugPositionAtOffset(predecessorOffset);
+ // The transfer has not yet taken place, so the current position is that of the predecessor,
+ // except for exceptional edges where the transfer has already taken place.
+ currentPosition =
+ getCanonicalDebugPositionAtOffset(isExceptional ? successorOffset : predecessorOffset);
LocalChangeAtOffset localChange = state.getLocalChange(predecessorOffset, successorOffset);
if (!isExceptional) {
@@ -2874,7 +2876,7 @@
int index = instructionIndex(offset);
if (index < 0 || instructionCount() <= index) {
assert false;
- return Position.none();
+ return getPreamblePosition();
}
AbstractInsnNode insn = node.instructions.get(index);
if (insn instanceof LabelNode) {
@@ -2927,10 +2929,14 @@
}
syntheticPosition =
(min == Integer.MAX_VALUE)
- ? Position.noneWithMethod(originalMethod, callerPosition)
+ ? getPreamblePosition()
: Position.synthetic(min < max ? min - 1 : min, originalMethod, callerPosition);
} else {
- syntheticPosition = Position.noneWithMethod(originalMethod, callerPosition);
+ // If in release mode we explicitly associate a synthetic none position with monitor exit.
+ // This is safe as the runtime must never throw at this position because the monitor cannot
+ // be null and the thread calling exit can only be the same thread that entered the monitor
+ // at method entry.
+ syntheticPosition = Position.syntheticNone();
}
}
return syntheticPosition;
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 194558e..c20da53 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
@@ -1982,8 +1982,7 @@
|| (hasCatchHandlers && i.instructionTypeCanThrow())
|| (options.canHaveCmpIfFloatBug() && i.isCmp()));
Instruction next = insertAt.previous();
- instruction.forceSetPosition(
- next.isGoto() ? next.asGoto().getTarget().getPosition() : next.getPosition());
+ instruction.forceSetPosition(next.getPosition());
insertAt.add(instruction);
}
@@ -3002,7 +3001,7 @@
iterator.add(new InvokeVirtual(print, null, ImmutableList.of(out, indent)));
// Add a block for end-of-line printing.
- BasicBlock eol = BasicBlock.createGotoBlock(code.blocks.size());
+ BasicBlock eol = BasicBlock.createGotoBlock(code.blocks.size(), position);
code.blocks.add(eol);
BasicBlock successor = block.unlinkSingleSuccessor();
@@ -3020,9 +3019,9 @@
BasicBlock ifBlock = BasicBlock.createIfBlock(code.blocks.size(), theIf);
code.blocks.add(ifBlock);
// Fallthrough block must be added right after the if.
- BasicBlock isNullBlock = BasicBlock.createGotoBlock(code.blocks.size());
+ BasicBlock isNullBlock = BasicBlock.createGotoBlock(code.blocks.size(), position);
code.blocks.add(isNullBlock);
- BasicBlock isNotNullBlock = BasicBlock.createGotoBlock(code.blocks.size());
+ BasicBlock isNotNullBlock = BasicBlock.createGotoBlock(code.blocks.size(), position);
code.blocks.add(isNotNullBlock);
// Link the added blocks together.
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
index 92bda1d..da96c4c 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
@@ -8,7 +8,6 @@
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionListIterator;
-import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.ir.code.Value;
import it.unimi.dsi.fastutil.Hash.Strategy;
import it.unimi.dsi.fastutil.objects.Object2ObjectLinkedOpenCustomHashMap;
@@ -79,7 +78,6 @@
ConstNumber canonicalizedConstant = entry.getKey().asConstNumber();
ConstNumber newConst = ConstNumber.copyOf(code, canonicalizedConstant);
insertCanonicalizedConstant(code, newConst);
- newConst.setPosition(Position.none());
for (Value outValue : entry.getValue()) {
outValue.replaceUsers(newConst.outValue());
}
@@ -96,6 +94,7 @@
// instructions. It is important that the const instruction is put before any instruction
// that can throw exceptions (since the value could be used on the exceptional edge).
InstructionListIterator it = entryBlock.listIterator();
+ canonicalizedConstant.setPosition(entryBlock.getPosition());
while (it.hasNext()) {
if (!it.next().isArgument()) {
it.previous();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/PeepholeOptimizer.java b/src/main/java/com/android/tools/r8/ir/optimize/PeepholeOptimizer.java
index 8bb4de7..eb140c4 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/PeepholeOptimizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/PeepholeOptimizer.java
@@ -6,11 +6,12 @@
import com.android.tools.r8.graph.DebugLocalInfo;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.ConstNumber;
-import com.android.tools.r8.ir.code.DebugLocalsChange;
import com.android.tools.r8.ir.code.Goto;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.InstructionIterator;
import com.android.tools.r8.ir.code.InstructionListIterator;
+import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.regalloc.LinearScanRegisterAllocator;
import com.android.tools.r8.ir.regalloc.LiveIntervals;
@@ -18,7 +19,6 @@
import com.google.common.base.Equivalence.Wrapper;
import com.google.common.collect.ImmutableList;
import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
-import it.unimi.dsi.fastutil.ints.Int2ReferenceMap.Entry;
import it.unimi.dsi.fastutil.ints.Int2ReferenceOpenHashMap;
import java.util.ArrayList;
import java.util.Collection;
@@ -28,6 +28,7 @@
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
+import java.util.Objects;
public class PeepholeOptimizer {
@@ -128,47 +129,48 @@
private static BasicBlock createAndInsertBlockForSuffix(
int blockNumber, int suffixSize, List<BasicBlock> preds, BasicBlock successorBlock) {
- BasicBlock newBlock = BasicBlock.createGotoBlock(blockNumber);
BasicBlock first = preds.get(0);
assert (successorBlock != null && first.exit().isGoto())
|| (successorBlock == null && first.exit().isReturn());
- int offsetFromEnd = successorBlock == null ? 0 : 1;
- if (successorBlock == null) {
- newBlock.getInstructions().removeLast();
+ BasicBlock newBlock = new BasicBlock();
+ newBlock.setNumber(blockNumber);
+ InstructionListIterator from = first.listIterator(first.getInstructions().size());
+ Int2ReferenceMap<DebugLocalInfo> newBlockEntryLocals = null;
+ if (first.getLocalsAtEntry() != null) {
+ newBlockEntryLocals = new Int2ReferenceOpenHashMap<>(first.getLocalsAtEntry());
+ int prefixSize = first.getInstructions().size() - suffixSize;
+ InstructionIterator it = first.iterator();
+ for (int i = 0; i < prefixSize; i++) {
+ Instruction instruction = it.next();
+ if (instruction.isDebugLocalsChange()) {
+ instruction.asDebugLocalsChange().apply(newBlockEntryLocals);
+ }
+ }
}
- InstructionListIterator from =
- first.listIterator(first.getInstructions().size() - offsetFromEnd);
- Int2ReferenceMap<DebugLocalInfo> newBlockEntryLocals =
- (successorBlock == null || successorBlock.getLocalsAtEntry() == null)
- ? new Int2ReferenceOpenHashMap<>()
- : new Int2ReferenceOpenHashMap<>(successorBlock.getLocalsAtEntry());
boolean movedThrowingInstruction = false;
- for (int i = offsetFromEnd; i < suffixSize; i++) {
+ for (int i = 0; i < suffixSize; i++) {
Instruction instruction = from.previous();
movedThrowingInstruction = movedThrowingInstruction || instruction.instructionTypeCanThrow();
newBlock.getInstructions().addFirst(instruction);
instruction.setBlock(newBlock);
- if (instruction.isDebugLocalsChange()) {
- // Replay the debug local changes backwards to compute the entry state.
- DebugLocalsChange change = instruction.asDebugLocalsChange();
- for (int starting : change.getStarting().keySet()) {
- newBlockEntryLocals.remove(starting);
- }
- for (Entry<DebugLocalInfo> ending : change.getEnding().int2ReferenceEntrySet()) {
- newBlockEntryLocals.put(ending.getIntKey(), ending.getValue());
- }
- }
}
if (movedThrowingInstruction && first.hasCatchHandlers()) {
newBlock.transferCatchHandlers(first);
}
for (BasicBlock pred : preds) {
+ Position lastPosition = pred.getPosition();
LinkedList<Instruction> instructions = pred.getInstructions();
for (int i = 0; i < suffixSize; i++) {
instructions.removeLast();
}
+ for (Instruction instruction : pred.getInstructions()) {
+ if (instruction.getPosition().isSome()) {
+ lastPosition = instruction.getPosition();
+ }
+ }
Goto jump = new Goto();
jump.setBlock(pred);
+ jump.setPosition(lastPosition);
instructions.add(jump);
newBlock.getPredecessors().add(pred);
if (successorBlock != null) {
@@ -181,16 +183,37 @@
pred.clearCatchHandlers();
}
}
- newBlock.setLocalsAtEntry(newBlockEntryLocals);
+ newBlock.close(null);
+ if (newBlockEntryLocals != null) {
+ newBlock.setLocalsAtEntry(newBlockEntryLocals);
+ }
if (successorBlock != null) {
newBlock.link(successorBlock);
}
return newBlock;
}
+ private static Int2ReferenceMap<DebugLocalInfo> localsAtBlockExit(BasicBlock block) {
+ if (block.getLocalsAtEntry() == null) {
+ return null;
+ }
+ Int2ReferenceMap<DebugLocalInfo> locals =
+ new Int2ReferenceOpenHashMap<>(block.getLocalsAtEntry());
+ for (Instruction instruction : block.getInstructions()) {
+ if (instruction.isDebugLocalsChange()) {
+ instruction.asDebugLocalsChange().apply(locals);
+ }
+ }
+ return locals;
+ }
+
private static int sharedSuffixSize(
BasicBlock block0, BasicBlock block1, RegisterAllocator allocator) {
assert block0.exit().isGoto() || block0.exit().isReturn();
+ // If the blocks do not agree on locals at exit then they don't have any shared suffix.
+ if (!Objects.equals(localsAtBlockExit(block0), localsAtBlockExit(block1))) {
+ return 0;
+ }
InstructionListIterator it0 = block0.listIterator(block0.getInstructions().size());
InstructionListIterator it1 = block1.listIterator(block1.getInstructions().size());
int suffixSize = 0;
@@ -230,6 +253,7 @@
changed = true;
int otherPredIndex = blockToIndex.get(wrapper);
BasicBlock otherPred = block.getPredecessors().get(otherPredIndex);
+ assert Objects.equals(pred.getPosition(), otherPred.getPosition());
pred.clearCatchHandlers();
pred.getInstructions().clear();
equivalence.clearComputedHash(pred);
@@ -242,6 +266,7 @@
otherPred.getPredecessors().add(pred);
Goto exit = new Goto();
exit.setBlock(pred);
+ exit.setPosition(otherPred.getPosition());
pred.getInstructions().add(exit);
} else {
blockToIndex.put(wrapper, predIndex);
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 c381944..75bf9c0 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
@@ -331,7 +331,9 @@
Int2ReferenceMap<DebugLocalInfo> currentLocals =
new Int2ReferenceOpenHashMap<>(openRanges.size());
for (LocalRange openRange : openRanges) {
- currentLocals.put(openRange.register, openRange.local);
+ if (liveLocalValues.contains(openRange.value)) {
+ currentLocals.put(openRange.register, openRange.local);
+ }
}
// Set locals at entry. This will adjust initial local registers in case of spilling.
@@ -350,7 +352,7 @@
while (it.hasNext()) {
LocalRange openRange = it.next();
if (openRange.value == endAnnotation) {
- it.remove();
+ // Don't remove the local from open-ranges as it is still technically open.
assert currentLocals.get(openRange.register) == openRange.local;
currentLocals.remove(openRange.register);
ending.put(openRange.register, openRange.local);
@@ -382,8 +384,9 @@
it.remove();
// It may be that currentLocals does not contain this local because an explicit end
// already closed the local.
- currentLocals.remove(openRange.register);
- ending.put(openRange.register, openRange.local);
+ if (currentLocals.remove(openRange.register) != null) {
+ ending.put(openRange.register, openRange.local);
+ }
}
}
}
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 8af8922..17e49a5 100644
--- a/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
+++ b/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
@@ -32,6 +32,7 @@
import java.util.Deque;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Optional;
import java.util.TreeMap;
import java.util.function.Consumer;
@@ -442,10 +443,7 @@
}
protected final JUnit3Wrapper.Command checkLine(String sourceFile, int line) {
- return inspect(t -> {
- Assert.assertEquals(sourceFile, t.getSourceFile());
- Assert.assertEquals(line, t.getLineNumber());
- });
+ return inspect(t -> t.checkLine(sourceFile, line));
}
protected final JUnit3Wrapper.Command checkInlineFrames(List<SignatureAndLine> expectedFrames) {
@@ -1043,6 +1041,8 @@
void checkNoLocal(String localName);
void checkLocal(String localName);
void checkLocal(String localName, Value expectedValue);
+
+ void checkLine(String sourceFile, int line);
}
public static class DebuggeeState implements FrameInspector {
@@ -1226,6 +1226,15 @@
checkIncorrectLocal(localName, expectedValue, localValue);
}
+ @Override
+ public void checkLine(String sourceFile, int line) {
+ if (!Objects.equals(sourceFile, getSourceFile()) || line != getLineNumber()) {
+ String locationString = convertCurrentLocationToString();
+ Assert.fail(
+ "Incorrect line at " + locationString + ", expected " + sourceFile + ":" + line);
+ }
+ }
+
/**
* Return class name, as found in the binary. If it has not been obfuscated (minified) it's
* identical to the original class name. Otherwise, it's the obfuscated one.
@@ -1355,6 +1364,11 @@
}
@Override
+ public void checkLine(String sourceFile, int line) {
+ getTopFrame().checkLine(sourceFile, line);
+ }
+
+ @Override
public int getLineNumber() {
return getTopFrame().getLineNumber();
}
diff --git a/src/test/java/com/android/tools/r8/debug/EmptyLineAfterJoinTestRunner.java b/src/test/java/com/android/tools/r8/debug/EmptyLineAfterJoinTestRunner.java
index 5e20a21..37782dd 100644
--- a/src/test/java/com/android/tools/r8/debug/EmptyLineAfterJoinTestRunner.java
+++ b/src/test/java/com/android/tools/r8/debug/EmptyLineAfterJoinTestRunner.java
@@ -10,7 +10,6 @@
import com.google.common.collect.ImmutableList;
import java.nio.file.Path;
import java.util.Collection;
-import org.junit.Assume;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
@@ -59,8 +58,6 @@
@Test
public void test() throws Throwable {
- Assume.assumeFalse(
- "b/110873173 Fails because of missing line 16", (config instanceof D8DebugTestConfig));
runDebugTest(
config,
NAME,
diff --git a/src/test/java/com/android/tools/r8/debug/LocalEndTest.java b/src/test/java/com/android/tools/r8/debug/LocalEndTest.java
index 036e590..f4a3413 100644
--- a/src/test/java/com/android/tools/r8/debug/LocalEndTest.java
+++ b/src/test/java/com/android/tools/r8/debug/LocalEndTest.java
@@ -17,11 +17,22 @@
System.out.println(y);
}
- private void bar() {
- // nothing to do
+ public void bar() {
+ if (raise) {
+ System.out.print("throwing ");
+ throw new RuntimeException();
+ }
+ System.out.print("not-throwing ");
+ }
+
+ public final boolean raise;
+
+ public LocalEndTest(boolean raise) {
+ this.raise = raise;
}
public static void main(String[] args) {
- new LocalEndTest().foo();
+ new LocalEndTest(false).foo();
+ new LocalEndTest(true).foo();
}
}
diff --git a/src/test/java/com/android/tools/r8/debug/LocalEndTestDump.java b/src/test/java/com/android/tools/r8/debug/LocalEndTestDump.java
index ad6737c..3111779 100644
--- a/src/test/java/com/android/tools/r8/debug/LocalEndTestDump.java
+++ b/src/test/java/com/android/tools/r8/debug/LocalEndTestDump.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.debug;
import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.FieldVisitor;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;
@@ -13,6 +14,7 @@
public static byte[] dump() {
ClassWriter cw = new ClassWriter(0);
+ FieldVisitor fv;
MethodVisitor mv;
cw.visit(
@@ -26,19 +28,8 @@
cw.visitSource("LocalEndTest.java", null);
{
- mv = cw.visitMethod(ACC_PUBLIC, "<init>", "()V", null, null);
- mv.visitCode();
- Label l0 = new Label();
- mv.visitLabel(l0);
- mv.visitLineNumber(6, l0);
- mv.visitVarInsn(ALOAD, 0);
- mv.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
- mv.visitInsn(RETURN);
- Label l1 = new Label();
- mv.visitLabel(l1);
- mv.visitLocalVariable("this", "Lcom/android/tools/r8/debug/LocalEndTest;", null, l0, l1, 0);
- mv.visitMaxs(1, 1);
- mv.visitEnd();
+ fv = cw.visitField(ACC_PUBLIC + ACC_FINAL, "raise", "Z", null, null);
+ fv.visitEnd();
}
{
mv = cw.visitMethod(ACC_PUBLIC, "foo", "()V", null, null);
@@ -57,7 +48,7 @@
mv.visitVarInsn(ILOAD, 1); // push x on the stack for later use in the join block.
mv.visitVarInsn(ALOAD, 0);
mv.visitMethodInsn(
- INVOKESPECIAL, "com/android/tools/r8/debug/LocalEndTest", "bar", "()V", false);
+ INVOKEVIRTUAL, "com/android/tools/r8/debug/LocalEndTest", "bar", "()V", false);
Label l4 = new Label();
mv.visitLabel(l4);
mv.visitLineNumber(13, l4);
@@ -76,10 +67,8 @@
new Object[] {"java/lang/Throwable"});
mv.visitVarInsn(ASTORE, 2);
mv.visitVarInsn(ILOAD, 1); // push x on the stack again (should be same as above).
- // mv.visitInsn(ICONST_0);
mv.visitLabel(l5);
mv.visitLineNumber(16, l5);
- // mv.visitFrame(Opcodes.F_CHOP, 1, null, 0, null);
mv.visitFrame(
Opcodes.F_FULL,
1,
@@ -107,38 +96,61 @@
mv.visitEnd();
}
{
- mv = cw.visitMethod(ACC_PRIVATE, "bar", "()V", null, null);
+ mv = cw.visitMethod(ACC_PUBLIC, "bar", "()V", null, null);
mv.visitCode();
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitFieldInsn(GETFIELD, "com/android/tools/r8/debug/LocalEndTest", "raise", "Z");
Label l0 = new Label();
+ mv.visitJumpInsn(IFEQ, l0);
+ mv.visitFieldInsn(GETSTATIC, "java/lang/System", "out", "Ljava/io/PrintStream;");
+ mv.visitLdcInsn("throwing ");
+ mv.visitMethodInsn(
+ INVOKEVIRTUAL, "java/io/PrintStream", "print", "(Ljava/lang/String;)V", false);
+ mv.visitTypeInsn(NEW, "java/lang/RuntimeException");
+ mv.visitInsn(DUP);
+ mv.visitMethodInsn(INVOKESPECIAL, "java/lang/RuntimeException", "<init>", "()V", false);
+ mv.visitInsn(ATHROW);
mv.visitLabel(l0);
- mv.visitLineNumber(22, l0);
+ mv.visitFrame(Opcodes.F_SAME, 0, null, 0, null);
+ mv.visitFieldInsn(GETSTATIC, "java/lang/System", "out", "Ljava/io/PrintStream;");
+ mv.visitLdcInsn("not-throwing ");
+ mv.visitMethodInsn(
+ INVOKEVIRTUAL, "java/io/PrintStream", "print", "(Ljava/lang/String;)V", false);
mv.visitInsn(RETURN);
- Label l1 = new Label();
- mv.visitLabel(l1);
- mv.visitLocalVariable("this", "Lcom/android/tools/r8/debug/LocalEndTest;", null, l0, l1, 0);
- mv.visitMaxs(0, 1);
+ mv.visitMaxs(2, 1);
+ mv.visitEnd();
+ }
+ {
+ mv = cw.visitMethod(ACC_PUBLIC, "<init>", "(Z)V", null, null);
+ mv.visitCode();
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitVarInsn(ILOAD, 1);
+ mv.visitFieldInsn(PUTFIELD, "com/android/tools/r8/debug/LocalEndTest", "raise", "Z");
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(2, 2);
mv.visitEnd();
}
{
mv = cw.visitMethod(ACC_PUBLIC + ACC_STATIC, "main", "([Ljava/lang/String;)V", null, null);
mv.visitCode();
- Label l0 = new Label();
- mv.visitLabel(l0);
- mv.visitLineNumber(25, l0);
mv.visitTypeInsn(NEW, "com/android/tools/r8/debug/LocalEndTest");
mv.visitInsn(DUP);
+ mv.visitInsn(ICONST_0);
mv.visitMethodInsn(
- INVOKESPECIAL, "com/android/tools/r8/debug/LocalEndTest", "<init>", "()V", false);
+ INVOKESPECIAL, "com/android/tools/r8/debug/LocalEndTest", "<init>", "(Z)V", false);
mv.visitMethodInsn(
INVOKEVIRTUAL, "com/android/tools/r8/debug/LocalEndTest", "foo", "()V", false);
- Label l1 = new Label();
- mv.visitLabel(l1);
- mv.visitLineNumber(26, l1);
+ mv.visitTypeInsn(NEW, "com/android/tools/r8/debug/LocalEndTest");
+ mv.visitInsn(DUP);
+ mv.visitInsn(ICONST_1);
+ mv.visitMethodInsn(
+ INVOKESPECIAL, "com/android/tools/r8/debug/LocalEndTest", "<init>", "(Z)V", false);
+ mv.visitMethodInsn(
+ INVOKEVIRTUAL, "com/android/tools/r8/debug/LocalEndTest", "foo", "()V", false);
mv.visitInsn(RETURN);
- Label l2 = new Label();
- mv.visitLabel(l2);
- mv.visitLocalVariable("args", "[Ljava/lang/String;", null, l0, l2, 0);
- mv.visitMaxs(2, 1);
+ mv.visitMaxs(3, 1);
mv.visitEnd();
}
cw.visitEnd();
diff --git a/src/test/java/com/android/tools/r8/debug/LocalEndTestRunner.java b/src/test/java/com/android/tools/r8/debug/LocalEndTestRunner.java
index 3f86969..af89b16 100644
--- a/src/test/java/com/android/tools/r8/debug/LocalEndTestRunner.java
+++ b/src/test/java/com/android/tools/r8/debug/LocalEndTestRunner.java
@@ -10,6 +10,7 @@
import com.google.common.collect.ImmutableList;
import java.nio.file.Path;
import java.util.Collection;
+import org.apache.harmony.jpda.tests.framework.jdwp.Value;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
@@ -54,16 +55,33 @@
@Test
public void test() throws Throwable {
+ Value xInitial = Value.createInt(42);
+ Value xNormal = Value.createInt(7);
+ Value xExceptional = xInitial;
runDebugTest(
config,
NAME,
- breakpoint(NAME, "foo", 13),
+ breakpoint(NAME, "foo", 12),
+ // Run to breakpoint for the non-throwing case.
run(),
+ checkLine(FILE, 12),
+ checkLocal("x", xInitial),
+ stepOver(),
checkLine(FILE, 13),
- checkLocal("x"),
+ checkLocal("x", xInitial),
stepOver(),
checkLine(FILE, 14),
- checkLocal("x"),
+ checkLocal("x", xNormal),
+ stepOver(),
+ checkLine(FILE, 16),
+ checkNoLocal("x"),
+ // Run to breakpoint for the throwing case.
+ run(),
+ checkLine(FILE, 12),
+ checkLocal("x", xInitial),
+ stepOver(),
+ checkLine(FILE, 14),
+ checkLocal("x", xExceptional),
stepOver(),
checkLine(FILE, 16),
checkNoLocal("x"),
diff --git a/src/test/java/com/android/tools/r8/debuginfo/SynchronizedMethodTestRunner.java b/src/test/java/com/android/tools/r8/debuginfo/SynchronizedMethodTestRunner.java
index c8e6e41..cd9e19d 100644
--- a/src/test/java/com/android/tools/r8/debuginfo/SynchronizedMethodTestRunner.java
+++ b/src/test/java/com/android/tools/r8/debuginfo/SynchronizedMethodTestRunner.java
@@ -103,5 +103,7 @@
// have an associated line.
inspector.checkStartLine(9);
inspector.checkNoLine(8);
+ // Also ensure we did not emit a preamble position at line zero.
+ inspector.checkNoLine(0);
}
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/ConstantRemovalTest.java b/src/test/java/com/android/tools/r8/ir/optimize/ConstantRemovalTest.java
index 9fb4702..2b92ab1 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/ConstantRemovalTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/ConstantRemovalTest.java
@@ -67,56 +67,57 @@
// is needed and the value 10 is *not* still in register 0 at that point.
BasicBlock block = new BasicBlock();
block.setNumber(0);
+ Position position = Position.testingPosition();
Value v3 = new Value(3, ValueType.LONG, null);
v3.setNeedsRegister(true);
new MockLiveIntervals(v3);
Instruction instruction = new ConstNumber(v3, 0);
- instruction.setPosition(Position.none());
+ instruction.setPosition(position);
block.add(instruction);
Value v0 = new Value(0, ValueType.LONG, null);
v0.setNeedsRegister(true);
new MockLiveIntervals(v0);
instruction = new ConstNumber(v0, 10);
- instruction.setPosition(Position.none());
+ instruction.setPosition(position);
block.add(instruction);
instruction = new Div(NumericType.LONG, v3, v3, v0);
- instruction.setPosition(Position.none());
+ instruction.setPosition(position);
block.add(instruction);
Value v2 = new Value(2, ValueType.INT, null);
v2.setNeedsRegister(true);
new MockLiveIntervals(v2);
instruction = new ConstNumber(v2, 10);
- instruction.setPosition(Position.none());
+ instruction.setPosition(position);
block.add(instruction);
Value v1 = new Value(1, ValueType.INT, null);
v1.setNeedsRegister(true);
new MockLiveIntervals(v1);
instruction = new Move(v1 ,v2);
- instruction.setPosition(Position.none());
+ instruction.setPosition(position);
block.add(instruction);
instruction = new Div(NumericType.INT, v1, v1, v1);
- instruction.setPosition(Position.none());
+ instruction.setPosition(position);
block.add(instruction);
Value v0_2 = new Value(0, ValueType.LONG, null);
v0_2.setNeedsRegister(true);
new MockLiveIntervals(v0_2);
instruction = new ConstNumber(v0_2, 10);
- instruction.setPosition(Position.none());
+ instruction.setPosition(position);
block.add(instruction);
instruction = new Div(NumericType.LONG, v3, v3, v0_2);
- instruction.setPosition(Position.none());
+ instruction.setPosition(position);
block.add(instruction);
Instruction ret = new Return();
- ret.setPosition(Position.none());
+ ret.setPosition(position);
block.add(ret);
block.setFilledForTesting();
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/TrivialGotoEliminationTest.java b/src/test/java/com/android/tools/r8/ir/optimize/TrivialGotoEliminationTest.java
index a095615..6470876 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/TrivialGotoEliminationTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/TrivialGotoEliminationTest.java
@@ -20,6 +20,7 @@
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.code.ValueNumberGenerator;
import com.android.tools.r8.ir.code.ValueType;
+import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.ImmutableList;
import java.util.LinkedList;
import org.junit.Test;
@@ -36,23 +37,24 @@
// throw v0
// block2:
// return
+ Position position = Position.testingPosition();
BasicBlock block2 = new BasicBlock();
block2.setNumber(2);
- BasicBlock block0 = BasicBlock.createGotoBlock(0, block2);
+ BasicBlock block0 = BasicBlock.createGotoBlock(0, position, block2);
block0.setFilledForTesting();
block2.getPredecessors().add(block0);
Instruction ret = new Return();
- ret.setPosition(Position.none());
+ ret.setPosition(position);
block2.add(ret);
block2.setFilledForTesting();
BasicBlock block1 = new BasicBlock();
block1.setNumber(1);
Value value = new Value(0, ValueType.INT, null);
Instruction number = new ConstNumber(value, 0);
- number.setPosition(Position.none());
+ number.setPosition(position);
block1.add(number);
Instruction throwing = new Throw(value);
- throwing.setPosition(Position.none());
+ throwing.setPosition(position);
block1.add(throwing);
block1.setFilledForTesting();
LinkedList<BasicBlock> blocks = new LinkedList<>();
@@ -62,7 +64,8 @@
// Check that the goto in block0 remains. There was a bug in the trivial goto elimination
// that ended up removing that goto changing the code to start with the unreachable
// throw.
- IRCode code = new IRCode(null, null, blocks, new ValueNumberGenerator(), false, false);
+ IRCode code =
+ new IRCode(new InternalOptions(), null, blocks, new ValueNumberGenerator(), false, false);
CodeRewriter.collapsTrivialGotos(null, code);
assertTrue(code.blocks.get(0).isTrivialGoto());
assertTrue(blocks.contains(block0));
@@ -85,22 +88,23 @@
//
// block3:
// goto block3
+ Position position = Position.testingPosition();
BasicBlock block2 = new BasicBlock();
block2.setNumber(2);
Instruction ret = new Return();
- ret.setPosition(Position.none());
+ ret.setPosition(position);
block2.add(ret);
block2.setFilledForTesting();
BasicBlock block3 = new BasicBlock();
block3.setNumber(3);
Instruction instruction = new Goto();
- instruction.setPosition(Position.none());
+ instruction.setPosition(position);
block3.add(instruction);
block3.setFilledForTesting();
block3.getSuccessors().add(block3);
- BasicBlock block1 = BasicBlock.createGotoBlock(1);
+ BasicBlock block1 = BasicBlock.createGotoBlock(1, position);
block1.getSuccessors().add(block3);
block1.setFilledForTesting();
@@ -108,10 +112,10 @@
block0.setNumber(0);
Value value = new Value(0, ValueType.OBJECT, null);
instruction = new Argument(value);
- instruction.setPosition(Position.none());
+ instruction.setPosition(position);
block0.add(instruction);
instruction = new If(Type.EQ, value);
- instruction.setPosition(Position.none());
+ instruction.setPosition(position);
block0.add(instruction);
block0.getSuccessors().add(block2);
block0.getSuccessors().add(block1);
@@ -130,7 +134,8 @@
// Check that the goto in block0 remains. There was a bug in the trivial goto elimination
// that ended up removing that goto changing the code to start with the unreachable
// throw.
- IRCode code = new IRCode(null, null, blocks, new ValueNumberGenerator(), false, false);
+ IRCode code =
+ new IRCode(new InternalOptions(), null, blocks, new ValueNumberGenerator(), false, false);
CodeRewriter.collapsTrivialGotos(null, code);
assertTrue(block0.getInstructions().get(1).isIf());
assertEquals(block1, block0.getInstructions().get(1).asIf().fallthroughBlock());
