Merge "Assert against removal of instructions with debug users/values."
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 3d68ace..7020f4b 100644
--- a/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
+++ b/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
@@ -91,7 +91,7 @@
// If this is the end of the block clear out the pending state.
pendingLocals = null;
pendingLocalChanges = false;
- } else if (pc != emittedPc) {
+ } else if (pc != emittedPc && !instruction.isNop()) {
// For non-exit / pc-advancing instructions emit any pending changes once possible.
emitLocalChanges(pc);
}
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 40a4776..8a2de9b 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
@@ -108,6 +108,7 @@
throw new IllegalStateException();
}
assert current.outValue() == null || !current.outValue().isUsed();
+ assert current.getDebugValues().isEmpty();
for (int i = 0; i < current.inValues().size(); i++) {
Value value = current.inValues().get(i);
value.removeUser(current);
@@ -115,11 +116,28 @@
for (Value value : current.getDebugValues()) {
value.removeDebugUser(current);
}
+ if (current.getLocalInfo() != null) {
+ for (Instruction user : current.outValue().debugUsers()) {
+ user.removeDebugValue(current.outValue());
+ }
+ }
listIterator.remove();
current = null;
}
@Override
+ public void removeOrReplaceByNop() {
+ if (current == null) {
+ throw new IllegalStateException();
+ }
+ if (current.getDebugValues().isEmpty()) {
+ remove();
+ } else {
+ replaceCurrentInstruction(new Nop());
+ }
+ }
+
+ @Override
public void detach() {
if (current == null) {
throw new IllegalStateException();
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlockIterator.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlockIterator.java
index d9ce088..74c2768 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlockIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlockIterator.java
@@ -81,7 +81,8 @@
// Remove all instructions from the block before removing the block.
Iterator<Instruction> iterator = current.iterator();
while (iterator.hasNext()) {
- iterator.next();
+ Instruction instruction = iterator.next();
+ instruction.clearDebugValues();
iterator.remove();
}
listIterator.remove();
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 f14d66a..de198c9 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
@@ -131,9 +131,19 @@
}
public void removeDebugValue(Value value) {
- assert debugValues.contains(value);
- value.removeDebugUser(this);
- debugValues.remove(value);
+ assert value.getLocalInfo() != null;
+ if (debugValues != null) {
+ assert debugValues.contains(value);
+ debugValues.remove(value);
+ return;
+ }
+ assert false;
+ }
+
+ public void clearDebugValues() {
+ if (debugValues != null) {
+ debugValues.clear();
+ }
}
/**
@@ -824,6 +834,14 @@
return null;
}
+ public boolean isNop() {
+ return false;
+ }
+
+ public Nop asNop() {
+ return null;
+ }
+
public boolean canBeFolded() {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
index 97f1885..ef49910 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
@@ -60,6 +60,12 @@
}
/**
+ * Safe removal function that will insert a Nop to take over the debug values if any are assocated
+ * with the current instruction.
+ */
+ void removeOrReplaceByNop();
+
+ /**
* Remove the current instruction (aka the {@link Instruction} returned by the previous call to
* {@link #next}) without updating its def/use chains.
* <p>
diff --git a/src/main/java/com/android/tools/r8/ir/code/Nop.java b/src/main/java/com/android/tools/r8/ir/code/Nop.java
new file mode 100644
index 0000000..dc378de
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/code/Nop.java
@@ -0,0 +1,63 @@
+// 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.ir.code;
+
+import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.graph.AppInfoWithSubtyping;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.ir.conversion.DexBuilder;
+import com.android.tools.r8.ir.optimize.Inliner.Constraint;
+import com.android.tools.r8.utils.InternalOptions;
+
+public class Nop extends Instruction {
+
+ public Nop() {
+ super(null);
+ }
+
+ @Override
+ public boolean isNop() {
+ return true;
+ }
+
+ @Override
+ public Nop asNop() {
+ return this;
+ }
+
+ @Override
+ public void buildDex(DexBuilder builder) {
+ builder.addNop(this);
+ }
+
+ @Override
+ public boolean identicalNonValueParts(Instruction other) {
+ return true;
+ }
+
+ @Override
+ public int compareNonValueParts(Instruction other) {
+ return 0;
+ }
+
+ @Override
+ public int maxInValueRegister() {
+ throw new Unreachable();
+ }
+
+ @Override
+ public int maxOutValueRegister() {
+ throw new Unreachable();
+ }
+
+ @Override
+ public Constraint inliningConstraint(AppInfoWithSubtyping info, DexType holder) {
+ return Constraint.ALWAYS;
+ }
+
+ @Override
+ public boolean canBeDeadCode(IRCode code, InternalOptions options) {
+ return getDebugValues().isEmpty();
+ }
+}
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 2880c87..0af8f63 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
@@ -260,9 +260,7 @@
}
public boolean isUsed() {
- return !users.isEmpty()
- || !phiUsers.isEmpty()
- || ((debugData != null) && !debugData.users.isEmpty());
+ return !users.isEmpty() || !phiUsers.isEmpty() || numberOfAllDebugUsers() > 0;
}
public boolean usedInMonitorOperation() {
@@ -291,6 +289,7 @@
uniquePhiUsers = null;
if (debugData != null) {
debugData.users.clear();
+ debugData.phiUsers.clear();
}
}
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 da067d8..b35c1a0 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
@@ -317,7 +317,8 @@
}
private boolean isNopInstruction(com.android.tools.r8.ir.code.Instruction instruction) {
- return instruction.isDebugLocalsChange()
+ return instruction.isNop()
+ || instruction.isDebugLocalsChange()
|| (instruction.isConstNumber() && !instruction.outValue().needsRegister());
}
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 b698fc5..52c9eb4 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
@@ -50,9 +50,7 @@
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.InvokeNewArray;
import com.android.tools.r8.ir.code.InvokeVirtual;
-import com.android.tools.r8.ir.code.JumpInstruction;
import com.android.tools.r8.ir.code.MemberType;
-import com.android.tools.r8.ir.code.Move;
import com.android.tools.r8.ir.code.MoveType;
import com.android.tools.r8.ir.code.NewArrayEmpty;
import com.android.tools.r8.ir.code.NewArrayFilledData;
@@ -72,7 +70,6 @@
import com.google.common.base.Equivalence.Wrapper;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Maps;
@@ -423,7 +420,6 @@
private int right;
private BasicBlock target;
private BasicBlock fallthrough;
- private int blockNumber;
public IfBuilder(IRCode code) {
this.code = code;
@@ -483,7 +479,6 @@
// Extract the information from the switch before removing it.
Int2ReferenceSortedMap<BasicBlock> keyToTarget = theSwitch.getKeyToTargetMap();
- Set<Value> originalDebugValues = ImmutableSet.copyOf(theSwitch.getDebugValues());
// Keep track of the current fallthrough, starting with the original.
BasicBlock fallthroughBlock = theSwitch.fallthroughBlock();
@@ -493,6 +488,8 @@
BasicBlock originalSwitchBlock = iterator.split(code, blocksIterator);
assert !originalSwitchBlock.hasCatchHandlers();
assert originalSwitchBlock.getInstructions().size() == 1;
+ assert originalBlock.exit().isGoto();
+ theSwitch.moveDebugValues(originalBlock.exit());
blocksIterator.remove();
theSwitch.getBlock().detachAllSuccessors();
BasicBlock block = theSwitch.getBlock().unlinkSinglePredecessor();
@@ -537,9 +534,6 @@
fallthroughBlock = ifBlock;
}
- // Attach the debug values from the original switch to the first of the new instructions,
- originalDebugValues.forEach(fallthroughBlock.exit()::addDebugValue);
-
// Finally link the block before the original switch to the new block sequence.
originalBlock.link(fallthroughBlock);
@@ -1770,16 +1764,15 @@
Instruction current = iterator.next();
if (current.isInvokeMethod()) {
DexMethod invokedMethod = current.asInvokeMethod().getInvokedMethod();
-
if (matchesMethodOfThrowable(invokedMethod, throwableMethods.addSuppressed)) {
// Remove Throwable::addSuppressed(Throwable) call.
- iterator.remove();
+ iterator.removeOrReplaceByNop();
} else if (matchesMethodOfThrowable(invokedMethod, throwableMethods.getSuppressed)) {
Value destValue = current.outValue();
if (destValue == null) {
// If the result of the call was not used we don't create
// an empty array and just remove the call.
- iterator.remove();
+ iterator.removeOrReplaceByNop();
continue;
}
diff --git a/src/test/java/com/android/tools/r8/ir/regalloc/RegisterMoveSchedulerTest.java b/src/test/java/com/android/tools/r8/ir/regalloc/RegisterMoveSchedulerTest.java
index 0e24dbd..8ab2de5 100644
--- a/src/test/java/com/android/tools/r8/ir/regalloc/RegisterMoveSchedulerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/regalloc/RegisterMoveSchedulerTest.java
@@ -74,6 +74,11 @@
}
@Override
+ public void removeOrReplaceByNop() {
+ remove();
+ }
+
+ @Override
public void detach() {
remove();
}
