Merge "Update run_on_as_app.py"
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/type/TypeLatticeElement.java b/src/main/java/com/android/tools/r8/ir/analysis/type/TypeLatticeElement.java
index 1544570..9c3bcc3 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/type/TypeLatticeElement.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/type/TypeLatticeElement.java
@@ -107,7 +107,6 @@
throw new Unreachable("unless a new type lattice is introduced.");
}
-
public static TypeLatticeElement join(
Iterable<TypeLatticeElement> typeLattices, AppInfo appInfo) {
TypeLatticeElement result = BOTTOM;
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 77f7fb8..9b8a00e 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
@@ -27,17 +27,6 @@
assert !ending.isEmpty() || !starting.isEmpty();
this.ending = ending;
this.starting = starting;
- super.setPosition(Position.none());
- }
-
- @Override
- public void setPosition(Position position) {
- throw new Unreachable();
- }
-
- @Override
- public boolean verifyValidPositionInfo(boolean debug) {
- return true;
}
public Int2ReferenceMap<DebugLocalInfo> getEnding() {
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 8e0d572..cb93a5e 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
@@ -662,13 +662,9 @@
}
// After the throwing instruction only debug instructions and the final jump
// instruction is allowed.
- // TODO(ager): For now allow const instructions due to the way consts are pushed
- // towards their use
if (seenThrowing) {
assert instruction.isDebugInstruction()
|| instruction.isJumpInstruction()
- || instruction.isConstInstruction()
- || instruction.isNewArrayFilledData()
|| instruction.isStore()
|| instruction.isPop();
}
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 b98662c..cdb40bf 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
@@ -19,10 +19,7 @@
@Override
public boolean hasNext() {
- if (instructionIterator.hasNext()) {
- return true;
- }
- return blockIterator.hasNext();
+ return instructionIterator.hasNext() || blockIterator.hasNext();
}
@Override
@@ -39,6 +36,35 @@
}
@Override
+ public boolean hasPrevious() {
+ return instructionIterator.hasPrevious() || blockIterator.hasPrevious();
+ }
+
+ @Override
+ public Instruction previous() {
+ if (instructionIterator.hasPrevious()) {
+ return instructionIterator.previous();
+ }
+ if (!blockIterator.hasPrevious()) {
+ throw new NoSuchElementException();
+ }
+ BasicBlock block = blockIterator.previous();
+ instructionIterator = block.listIterator(block.getInstructions().size());
+ assert instructionIterator.hasPrevious();
+ return instructionIterator.previous();
+ }
+
+ @Override
+ public int nextIndex() {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public int previousIndex() {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
public void add(Instruction instruction) {
instructionIterator.add(instruction);
}
@@ -49,6 +75,11 @@
}
@Override
+ public void set(Instruction instruction) {
+ instructionIterator.set(instruction);
+ }
+
+ @Override
public void replaceCurrentInstruction(Instruction newInstruction) {
instructionIterator.replaceCurrentInstruction(newInstruction);
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/If.java b/src/main/java/com/android/tools/r8/ir/code/If.java
index 2979039..25d1fc4 100644
--- a/src/main/java/com/android/tools/r8/ir/code/If.java
+++ b/src/main/java/com/android/tools/r8/ir/code/If.java
@@ -142,6 +142,7 @@
return super.toString()
+ " "
+ type
+ + (isZeroTest() ? "Z" : " ")
+ " block "
+ getTrueTarget().getNumberAsString()
+ " (fallthrough "
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 2f2b3f1..72aa3ee 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
@@ -4,7 +4,10 @@
package com.android.tools.r8.ir.code;
-public interface InstructionIterator extends NextUntilIterator<Instruction> {
+import java.util.ListIterator;
+
+public interface InstructionIterator
+ extends ListIterator<Instruction>, NextUntilIterator<Instruction> {
/**
* Replace the current instruction (aka the {@link Instruction} returned by the previous call to
* {@link #next} with the passed in <code>newInstruction</code>.
@@ -22,15 +25,6 @@
*/
void replaceCurrentInstruction(Instruction newInstruction);
- /**
- * Adds an instruction. The instruction will be added just before the current
- * cursor position.
- *
- * The instruction will be assigned to the block it is added to.
- *
- * @param instruction The instruction to add.
- */
- void add(Instruction instruction);
/**
* Safe removal function that will insert a DebugLocalRead to take over the debug values if any
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 a5e5d78..c232d3f 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
@@ -11,9 +11,7 @@
import java.util.ListIterator;
public interface InstructionListIterator
- extends ListIterator<Instruction>,
- NextUntilIterator<Instruction>,
- PreviousUntilIterator<Instruction> {
+ extends InstructionIterator, PreviousUntilIterator<Instruction> {
/**
* Peek the previous instruction.
@@ -50,27 +48,6 @@
}
/**
- * Safe removal function that will insert a DebugLocalRead to take over the debug values if any
- * are associated with the current instruction.
- */
- void removeOrReplaceByDebugLocalRead();
-
- /**
- * Replace the current instruction (aka the {@link Instruction} returned by the previous call to
- * {@link #next} with the passed in <code>newInstruction</code>.
- *
- * The current instruction will be completely detached from the instruction stream with uses
- * of its in-values removed.
- *
- * If the current instruction produces an out-value the new instruction must also produce
- * an out-value, and all uses of the current instructions out-value will be replaced by the
- * new instructions out-value.
- *
- * @param newInstruction the instruction to insert instead of the current.
- */
- void replaceCurrentInstruction(Instruction newInstruction);
-
- /**
* Split the block into two blocks at the point of the {@link ListIterator} cursor. The existing
* block will have all the instructions before the cursor, and the new block all the
* instructions after the cursor.
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 f95b988..374acd5 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
@@ -797,7 +797,17 @@
}
public boolean knownToBeBoolean() {
- return knownToBeBoolean;
+ if (knownToBeBoolean) {
+ return true;
+ }
+ if (getTypeLattice().isInt()) {
+ Value aliasedValue = getAliasedValue();
+ if (!aliasedValue.isPhi() && aliasedValue.definition.isConstNumber()) {
+ ConstNumber definition = aliasedValue.definition.asConstNumber();
+ return definition.isZero() || definition.getRawValue() == 1;
+ }
+ }
+ return false;
}
public void markAsThis(boolean receiverCanBeNull) {
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 4b2fabb..ec2fe4c 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
@@ -377,8 +377,11 @@
unresolvedPosition = instruction.asDebugPosition();
localsAtUnresolvedPosition = new Int2ReferenceOpenHashMap<>(locals);
}
- } else if (instruction.getPosition().isSome()) {
- if (!isNopInstruction(instruction, nextBlock)) {
+ } else {
+ assert instruction.getPosition().isSome();
+ if (instruction.isDebugLocalsChange()) {
+ instruction.asDebugLocalsChange().apply(locals);
+ } else if (!isNopInstruction(instruction, nextBlock)) {
if (unresolvedPosition != null) {
if (unresolvedPosition.getPosition() == instruction.getPosition()
&& locals.equals(localsAtUnresolvedPosition)) {
@@ -389,12 +392,6 @@
}
currentMaterializedPosition = instruction.getPosition();
}
- } else {
- // Only local-change instructions don't have positions in debug mode but fail gracefully.
- assert instruction.isDebugLocalsChange();
- if (instruction.isDebugLocalsChange()) {
- instruction.asDebugLocalsChange().apply(locals);
- }
}
}
}
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 b1ea3f9..7b55543 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
@@ -997,6 +997,7 @@
codeRewriter.rewriteSwitch(code);
codeRewriter.processMethodsNeverReturningNormally(code);
codeRewriter.simplifyIf(code);
+ codeRewriter.redundantConstNumberRemoval(code);
new RedundantFieldLoadElimination(appInfo, code, enableWholeProgramOptimizations).run();
if (options.testing.invertConditionals) {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
index 62cddc9..cf88f0f 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
@@ -210,7 +210,15 @@
iterator.replaceCurrentInstruction(newInvoke);
if (constantReturnMaterializingInstruction != null) {
- iterator.add(constantReturnMaterializingInstruction);
+ if (block.hasCatchHandlers()) {
+ // Split the block to ensure no instructions after throwing instructions.
+ iterator
+ .split(code, blocks)
+ .listIterator()
+ .add(constantReturnMaterializingInstruction);
+ } else {
+ iterator.add(constantReturnMaterializingInstruction);
+ }
}
DexType actualReturnType = actualTarget.proto.returnType;
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 7447f76..3227d45 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
@@ -26,6 +26,7 @@
import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer.TrivialInstanceInitializer;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexItemFactory.ThrowableMethods;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProto;
import com.android.tools.r8.graph.DexString;
@@ -120,6 +121,8 @@
import it.unimi.dsi.fastutil.ints.IntArrayList;
import it.unimi.dsi.fastutil.ints.IntIterator;
import it.unimi.dsi.fastutil.ints.IntList;
+import it.unimi.dsi.fastutil.longs.Long2ReferenceMap;
+import it.unimi.dsi.fastutil.longs.Long2ReferenceOpenHashMap;
import it.unimi.dsi.fastutil.objects.Object2IntLinkedOpenHashMap;
import it.unimi.dsi.fastutil.objects.Object2IntMap;
import it.unimi.dsi.fastutil.objects.Reference2IntMap;
@@ -3144,6 +3147,210 @@
simplifyIfWithKnownCondition(code, block, theIf, theIf.targetFromCondition(cond));
}
+ /**
+ * This optimization exploits that we can sometimes learn the constant value of an SSA value that
+ * flows into an if-eq of if-neq instruction.
+ *
+ * <p>Consider the following example:
+ *
+ * <pre>
+ * 1. if (obj != null) {
+ * 2. return doStuff();
+ * 3. }
+ * 4. return null;
+ * </pre>
+ *
+ * <p>Since we know that `obj` is null in all blocks that are dominated by the false-target of the
+ * if-instruction in line 1, we can safely replace the null-constant in line 4 by `obj`, and
+ * thereby save a const-number instruction.
+ */
+ public void redundantConstNumberRemoval(IRCode code) {
+ Supplier<Long2ReferenceMap<List<ConstNumber>>> constantsByValue =
+ Suppliers.memoize(() -> getConstantsByValue(code));
+ Supplier<DominatorTree> dominatorTree = Suppliers.memoize(() -> new DominatorTree(code));
+
+ boolean changed = false;
+ for (BasicBlock block : code.blocks) {
+ Instruction lastInstruction = block.getInstructions().getLast();
+ if (!lastInstruction.isIf()) {
+ continue;
+ }
+
+ If ifInstruction = lastInstruction.asIf();
+ Type type = ifInstruction.getType();
+
+ Value lhs = ifInstruction.inValues().get(0);
+ Value rhs = !ifInstruction.isZeroTest() ? ifInstruction.inValues().get(1) : null;
+
+ if (!ifInstruction.isZeroTest() && !lhs.isConstNumber() && !rhs.isConstNumber()) {
+ // We can only conclude anything from an if-instruction if it is a zero-test or if one of
+ // the two operands is a constant.
+ continue;
+ }
+
+ // If the type is neither EQ nor NE, we cannot conclude anything about any of the in-values
+ // of the if-instruction from the outcome of the if-instruction.
+ if (type != Type.EQ && type != Type.NE) {
+ continue;
+ }
+
+ BasicBlock trueTarget, falseTarget;
+ if (type == Type.EQ) {
+ trueTarget = ifInstruction.getTrueTarget();
+ falseTarget = ifInstruction.fallthroughBlock();
+ } else {
+ falseTarget = ifInstruction.getTrueTarget();
+ trueTarget = ifInstruction.fallthroughBlock();
+ }
+
+ if (ifInstruction.isZeroTest()) {
+ changed |=
+ replaceDominatedConstNumbers(0, lhs, trueTarget, constantsByValue, dominatorTree);
+ if (lhs.knownToBeBoolean()) {
+ changed |=
+ replaceDominatedConstNumbers(1, lhs, falseTarget, constantsByValue, dominatorTree);
+ }
+ } else {
+ assert rhs != null;
+ if (lhs.isConstNumber()) {
+ ConstNumber lhsAsNumber = lhs.getConstInstruction().asConstNumber();
+ changed |=
+ replaceDominatedConstNumbers(
+ lhsAsNumber.getRawValue(), rhs, trueTarget, constantsByValue, dominatorTree);
+ if (lhs.knownToBeBoolean() && rhs.knownToBeBoolean()) {
+ changed |=
+ replaceDominatedConstNumbers(
+ negateBoolean(lhsAsNumber), rhs, falseTarget, constantsByValue, dominatorTree);
+ }
+ } else {
+ assert rhs.isConstNumber();
+ ConstNumber rhsAsNumber = rhs.getConstInstruction().asConstNumber();
+ changed |=
+ replaceDominatedConstNumbers(
+ rhsAsNumber.getRawValue(), lhs, trueTarget, constantsByValue, dominatorTree);
+ if (lhs.knownToBeBoolean() && rhs.knownToBeBoolean()) {
+ changed |=
+ replaceDominatedConstNumbers(
+ negateBoolean(rhsAsNumber), lhs, falseTarget, constantsByValue, dominatorTree);
+ }
+ }
+ }
+
+ if (constantsByValue.get().isEmpty()) {
+ break;
+ }
+ }
+
+ if (changed) {
+ code.removeAllTrivialPhis();
+ }
+ assert code.isConsistentSSA();
+ }
+
+ private static Long2ReferenceMap<List<ConstNumber>> getConstantsByValue(IRCode code) {
+ // A map from the raw value of constants in `code` to the const number instructions that define
+ // the given raw value (irrespective of the type of the raw value).
+ Long2ReferenceMap<List<ConstNumber>> constantsByValue = new Long2ReferenceOpenHashMap<>();
+
+ // Initialize `constantsByValue`.
+ Iterable<Instruction> instructions = code::instructionIterator;
+ for (Instruction instruction : instructions) {
+ if (instruction.isConstNumber()) {
+ ConstNumber constNumber = instruction.asConstNumber();
+ if (constNumber.outValue().hasLocalInfo()) {
+ // Not necessarily constant, because it could be changed in the debugger.
+ continue;
+ }
+ long rawValue = constNumber.getRawValue();
+ if (constantsByValue.containsKey(rawValue)) {
+ constantsByValue.get(rawValue).add(constNumber);
+ } else {
+ List<ConstNumber> list = new ArrayList<>();
+ list.add(constNumber);
+ constantsByValue.put(rawValue, list);
+ }
+ }
+ }
+ return constantsByValue;
+ }
+
+ private static int negateBoolean(ConstNumber number) {
+ assert number.outValue().knownToBeBoolean();
+ return number.getRawValue() == 0 ? 1 : 0;
+ }
+
+ private boolean replaceDominatedConstNumbers(
+ long withValue,
+ Value newValue,
+ BasicBlock dominator,
+ Supplier<Long2ReferenceMap<List<ConstNumber>>> constantsByValueSupplier,
+ Supplier<DominatorTree> dominatorTree) {
+ if (newValue.hasLocalInfo()) {
+ // We cannot replace a constant with a value that has local info, because that could change
+ // debugging behavior.
+ return false;
+ }
+
+ Long2ReferenceMap<List<ConstNumber>> constantsByValue = constantsByValueSupplier.get();
+ List<ConstNumber> constantsWithValue = constantsByValue.get(withValue);
+ if (constantsWithValue == null || constantsWithValue.isEmpty()) {
+ return false;
+ }
+
+ boolean changed = false;
+
+ ListIterator<ConstNumber> constantWithValueIterator = constantsWithValue.listIterator();
+ while (constantWithValueIterator.hasNext()) {
+ ConstNumber constNumber = constantWithValueIterator.next();
+ Value value = constNumber.outValue();
+ assert !value.hasLocalInfo();
+ assert constNumber.getRawValue() == withValue;
+
+ BasicBlock block = constNumber.getBlock();
+
+ // If the following condition does not hold, then the if-instruction does not dominate the
+ // block containing the constant, although the true or false target does.
+ if (block == dominator && block.getPredecessors().size() != 1) {
+ // This should generally not happen, but it is possible to write bytecode where it does.
+ assert false;
+ continue;
+ }
+
+ if (value.knownToBeBoolean() && !newValue.knownToBeBoolean()) {
+ // We cannot replace a boolean by a none-boolean since that can lead to verification
+ // errors. For example, the following code fails with "register v1 has type Imprecise
+ // Constant: 127 but expected Boolean return-1nr".
+ //
+ // public boolean convertIntToBoolean(int v1) {
+ // const/4 v0, 0x1
+ // if-eq v1, v0, :eq_true
+ // const/4 v1, 0x0
+ // :eq_true
+ // return v1
+ // }
+ continue;
+ }
+
+ if (dominatorTree.get().dominatedBy(block, dominator)) {
+ if (newValue.getTypeLattice().lessThanOrEqual(value.getTypeLattice(), appInfo)) {
+ value.replaceUsers(newValue);
+ block.listIterator(constNumber).removeOrReplaceByDebugLocalRead();
+ constantWithValueIterator.remove();
+ changed = true;
+ } else if (value.getTypeLattice().isNullType()) {
+ // TODO(b/120257211): Need a mechanism to determine if `newValue` can be used at all of
+ // the use sites of `value` without introducing a type error.
+ }
+ }
+ }
+
+ if (constantsWithValue.isEmpty()) {
+ constantsByValue.remove(withValue);
+ }
+
+ return changed;
+ }
+
// Find all method invocations that never returns normally, split the block
// after each such invoke instruction and follow it with a block throwing a
// null value (which should result in NPE). Note that this throw is not
@@ -3439,7 +3646,7 @@
// Note that addSuppressed() and getSuppressed() methods are final in
// Throwable, so these changes don't have to worry about overrides.
public void rewriteThrowableAddAndGetSuppressed(IRCode code) {
- DexItemFactory.ThrowableMethods throwableMethods = dexItemFactory.throwableMethods;
+ ThrowableMethods throwableMethods = dexItemFactory.throwableMethods;
for (BasicBlock block : code.blocks) {
InstructionListIterator iterator = block.listIterator();
@@ -3579,7 +3786,7 @@
} else {
// Insert "if (argument != null) ...".
successor = block.unlinkSingleSuccessor();
- If theIf = new If(If.Type.NE, argument);
+ If theIf = new If(Type.NE, argument);
theIf.setPosition(position);
BasicBlock ifBlock = BasicBlock.createIfBlock(code.blocks.size(), theIf);
code.blocks.add(ifBlock);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
index 4da9f8d..99f9731 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
@@ -17,11 +17,13 @@
import com.android.tools.r8.ir.analysis.type.Nullability;
import com.android.tools.r8.ir.analysis.type.TypeAnalysis;
import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
+import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.ConstNumber;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.InstancePut;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionIterator;
+import com.android.tools.r8.ir.code.InstructionListIterator;
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.StaticGet;
import com.android.tools.r8.ir.code.StaticPut;
@@ -29,6 +31,7 @@
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import com.android.tools.r8.shaking.ProguardMemberRule;
import com.google.common.collect.Sets;
+import java.util.ListIterator;
import java.util.Set;
import java.util.function.Predicate;
@@ -133,151 +136,160 @@
public void rewriteWithConstantValues(
IRCode code, DexType callingContext, Predicate<DexEncodedMethod> isProcessedConcurrently) {
Set<Value> affectedValues = Sets.newIdentityHashSet();
- InstructionIterator iterator = code.instructionIterator();
- while (iterator.hasNext()) {
- Instruction current = iterator.next();
- if (current.isInvokeMethod()) {
- InvokeMethod invoke = current.asInvokeMethod();
- DexMethod invokedMethod = invoke.getInvokedMethod();
- DexType invokedHolder = invokedMethod.getHolder();
- if (!invokedHolder.isClassType()) {
- continue;
- }
- // TODO(70550443): Maybe check all methods here.
- DexEncodedMethod definition = appInfo
- .lookup(invoke.getType(), invokedMethod, callingContext);
+ ListIterator<BasicBlock> blocks = code.blocks.listIterator();
+ while (blocks.hasNext()) {
+ BasicBlock block = blocks.next();
+ InstructionListIterator iterator = block.listIterator();
+ while (iterator.hasNext()) {
+ Instruction current = iterator.next();
+ if (current.isInvokeMethod()) {
+ InvokeMethod invoke = current.asInvokeMethod();
+ DexMethod invokedMethod = invoke.getInvokedMethod();
+ DexType invokedHolder = invokedMethod.getHolder();
+ if (!invokedHolder.isClassType()) {
+ continue;
+ }
+ // TODO(70550443): Maybe check all methods here.
+ DexEncodedMethod definition = appInfo
+ .lookup(invoke.getType(), invokedMethod, callingContext);
- boolean invokeReplaced = false;
- ProguardMemberRuleLookup lookup = lookupMemberRule(definition);
- if (lookup != null) {
- if (lookup.type == RuleType.ASSUME_NO_SIDE_EFFECTS
- && (invoke.outValue() == null || !invoke.outValue().isUsed())) {
- // Remove invoke if marked as having no side effects and the return value is not used.
- iterator.remove();
- invokeReplaced = true;
- } else if (invoke.outValue() != null && invoke.outValue().isUsed()) {
- // Check to see if a constant value can be assumed.
- Instruction replacement =
- constantReplacementFromProguardRule(lookup.rule, code, invoke);
- if (replacement != null) {
- affectedValues.add(replacement.outValue());
- replaceInstructionFromProguardRule(lookup.type, iterator, current, replacement);
+ boolean invokeReplaced = false;
+ ProguardMemberRuleLookup lookup = lookupMemberRule(definition);
+ if (lookup != null) {
+ if (lookup.type == RuleType.ASSUME_NO_SIDE_EFFECTS
+ && (invoke.outValue() == null || !invoke.outValue().isUsed())) {
+ // Remove invoke if marked as having no side effects and the return value is not used.
+ iterator.remove();
invokeReplaced = true;
- } else {
- // Check to see if a value range can be assumed.
- setValueRangeFromProguardRule(lookup.rule, current.outValue());
+ } else if (invoke.outValue() != null && invoke.outValue().isUsed()) {
+ // Check to see if a constant value can be assumed.
+ Instruction replacement =
+ constantReplacementFromProguardRule(lookup.rule, code, invoke);
+ if (replacement != null) {
+ affectedValues.add(replacement.outValue());
+ replaceInstructionFromProguardRule(lookup.type, iterator, current, replacement);
+ invokeReplaced = true;
+ } else {
+ // Check to see if a value range can be assumed.
+ setValueRangeFromProguardRule(lookup.rule, current.outValue());
+ }
}
}
- }
- // If no Proguard rule could replace the instruction check for knowledge about the
- // return value.
- if (!invokeReplaced && invoke.outValue() != null) {
- DexEncodedMethod target = invoke.lookupSingleTarget(appInfo, callingContext);
- if (target != null) {
- if (target.getOptimizationInfo().neverReturnsNull() && invoke.outValue().canBeNull()) {
- Value knownToBeNonNullValue = invoke.outValue();
- knownToBeNonNullValue.markNeverNull();
- TypeLatticeElement typeLattice = knownToBeNonNullValue.getTypeLattice();
- assert typeLattice.isNullable() && typeLattice.isReference();
- knownToBeNonNullValue.narrowing(appInfo, typeLattice.asNonNullable());
- affectedValues.addAll(knownToBeNonNullValue.affectedValues());
- }
- if (target.getOptimizationInfo().returnsConstant()) {
- long constant = target.getOptimizationInfo().getReturnedConstant();
- ConstNumber replacement = createConstNumberReplacement(
- code, constant, invoke.outValue().getTypeLattice(), invoke.getLocalInfo());
- affectedValues.add(replacement.outValue());
- invoke.outValue().replaceUsers(replacement.outValue());
- invoke.setOutValue(null);
- replacement.setPosition(invoke.getPosition());
- invoke.moveDebugValues(replacement);
- iterator.add(replacement);
- }
- }
- }
- } else if (current.isInstancePut()) {
- InstancePut instancePut = current.asInstancePut();
- DexField field = instancePut.getField();
- DexEncodedField target = appInfo.lookupInstanceTarget(field.getHolder(), field);
- if (target != null) {
- // Remove writes to dead (i.e. never read) fields.
- if (!isFieldRead(target, false) && instancePut.object().isNeverNull()) {
- iterator.remove();
- }
- }
- } else if (current.isStaticGet()) {
- StaticGet staticGet = current.asStaticGet();
- DexField field = staticGet.getField();
- DexEncodedField target = appInfo.lookupStaticTarget(field.getHolder(), field);
- ProguardMemberRuleLookup lookup = null;
- if (target != null) {
- // Check if a this value is known const.
- Instruction replacement = target.valueAsConstInstruction(appInfo, staticGet.dest());
- if (replacement == null) {
- lookup = lookupMemberRule(target);
- if (lookup != null) {
- replacement = constantReplacementFromProguardRule(lookup.rule, code, staticGet);
- }
- }
- if (replacement == null) {
- // If no const replacement was found, at least store the range information.
- if (lookup != null) {
- setValueRangeFromProguardRule(lookup.rule, staticGet.dest());
- }
- }
- if (replacement != null) {
- affectedValues.add(replacement.outValue());
- // Ignore assumenosideeffects for fields.
- if (lookup != null && lookup.type == RuleType.ASSUME_VALUES) {
- replaceInstructionFromProguardRule(lookup.type, iterator, current, replacement);
- } else {
- iterator.replaceCurrentInstruction(replacement);
- }
- } else if (staticGet.dest() != null) {
- // In case the class holder of this static field satisfying following criteria:
- // -- cannot trigger other static initializer except for its own
- // -- is final
- // -- has a class initializer which is classified as trivial
- // (see CodeRewriter::computeClassInitializerInfo) and
- // initializes the field being accessed
- //
- // ... and the field itself is not pinned by keep rules (in which case it might
- // be updated outside the class constructor, e.g. via reflections), it is safe
- // to assume that the static-get instruction reads the value it initialized value
- // in class initializer and is never null.
- //
- DexClass holderDefinition = appInfo.definitionFor(field.getHolder());
- if (holderDefinition != null
- && holderDefinition.accessFlags.isFinal()
- && !appInfo.canTriggerStaticInitializer(field.getHolder(), true)) {
- Value outValue = staticGet.dest();
- DexEncodedMethod classInitializer = holderDefinition.getClassInitializer();
- if (classInitializer != null && !isProcessedConcurrently.test(classInitializer)) {
- TrivialInitializer info =
- classInitializer.getOptimizationInfo().getTrivialInitializerInfo();
- if (info != null
- && ((TrivialClassInitializer) info).field == field
- && !appInfo.isPinned(field)
- && outValue.canBeNull()) {
- outValue.markNeverNull();
- TypeLatticeElement typeLattice = outValue.getTypeLattice();
- assert typeLattice.isNullable() && typeLattice.isReference();
- outValue.narrowing(appInfo, typeLattice.asNonNullable());
- affectedValues.addAll(outValue.affectedValues());
+ // If no Proguard rule could replace the instruction check for knowledge about the
+ // return value.
+ if (!invokeReplaced && invoke.outValue() != null) {
+ DexEncodedMethod target = invoke.lookupSingleTarget(appInfo, callingContext);
+ if (target != null) {
+ if (target.getOptimizationInfo().neverReturnsNull() && invoke.outValue()
+ .canBeNull()) {
+ Value knownToBeNonNullValue = invoke.outValue();
+ knownToBeNonNullValue.markNeverNull();
+ TypeLatticeElement typeLattice = knownToBeNonNullValue.getTypeLattice();
+ assert typeLattice.isNullable() && typeLattice.isReference();
+ knownToBeNonNullValue.narrowing(appInfo, typeLattice.asNonNullable());
+ affectedValues.addAll(knownToBeNonNullValue.affectedValues());
+ }
+ if (target.getOptimizationInfo().returnsConstant()) {
+ long constant = target.getOptimizationInfo().getReturnedConstant();
+ ConstNumber replacement = createConstNumberReplacement(
+ code, constant, invoke.outValue().getTypeLattice(), invoke.getLocalInfo());
+ affectedValues.add(replacement.outValue());
+ invoke.outValue().replaceUsers(replacement.outValue());
+ invoke.setOutValue(null);
+ replacement.setPosition(invoke.getPosition());
+ invoke.moveDebugValues(replacement);
+ if (current.getBlock().hasCatchHandlers()) {
+ iterator.split(code, blocks).listIterator().add(replacement);
+ } else {
+ iterator.add(replacement);
}
}
}
}
- }
- } else if (current.isStaticPut()) {
- StaticPut staticPut = current.asStaticPut();
- DexField field = staticPut.getField();
- DexEncodedField target = appInfo.lookupStaticTarget(field.getHolder(), field);
- if (target != null) {
- // Remove writes to dead (i.e. never read) fields.
- if (!isFieldRead(target, true)) {
- iterator.removeOrReplaceByDebugLocalRead();
+ } else if (current.isInstancePut()) {
+ InstancePut instancePut = current.asInstancePut();
+ DexField field = instancePut.getField();
+ DexEncodedField target = appInfo.lookupInstanceTarget(field.getHolder(), field);
+ if (target != null) {
+ // Remove writes to dead (i.e. never read) fields.
+ if (!isFieldRead(target, false) && instancePut.object().isNeverNull()) {
+ iterator.remove();
+ }
+ }
+ } else if (current.isStaticGet()) {
+ StaticGet staticGet = current.asStaticGet();
+ DexField field = staticGet.getField();
+ DexEncodedField target = appInfo.lookupStaticTarget(field.getHolder(), field);
+ ProguardMemberRuleLookup lookup = null;
+ if (target != null) {
+ // Check if a this value is known const.
+ Instruction replacement = target.valueAsConstInstruction(appInfo, staticGet.dest());
+ if (replacement == null) {
+ lookup = lookupMemberRule(target);
+ if (lookup != null) {
+ replacement = constantReplacementFromProguardRule(lookup.rule, code, staticGet);
+ }
+ }
+ if (replacement == null) {
+ // If no const replacement was found, at least store the range information.
+ if (lookup != null) {
+ setValueRangeFromProguardRule(lookup.rule, staticGet.dest());
+ }
+ }
+ if (replacement != null) {
+ affectedValues.add(replacement.outValue());
+ // Ignore assumenosideeffects for fields.
+ if (lookup != null && lookup.type == RuleType.ASSUME_VALUES) {
+ replaceInstructionFromProguardRule(lookup.type, iterator, current, replacement);
+ } else {
+ iterator.replaceCurrentInstruction(replacement);
+ }
+ } else if (staticGet.dest() != null) {
+ // In case the class holder of this static field satisfying following criteria:
+ // -- cannot trigger other static initializer except for its own
+ // -- is final
+ // -- has a class initializer which is classified as trivial
+ // (see CodeRewriter::computeClassInitializerInfo) and
+ // initializes the field being accessed
+ //
+ // ... and the field itself is not pinned by keep rules (in which case it might
+ // be updated outside the class constructor, e.g. via reflections), it is safe
+ // to assume that the static-get instruction reads the value it initialized value
+ // in class initializer and is never null.
+ //
+ DexClass holderDefinition = appInfo.definitionFor(field.getHolder());
+ if (holderDefinition != null
+ && holderDefinition.accessFlags.isFinal()
+ && !appInfo.canTriggerStaticInitializer(field.getHolder(), true)) {
+ Value outValue = staticGet.dest();
+ DexEncodedMethod classInitializer = holderDefinition.getClassInitializer();
+ if (classInitializer != null && !isProcessedConcurrently.test(classInitializer)) {
+ TrivialInitializer info =
+ classInitializer.getOptimizationInfo().getTrivialInitializerInfo();
+ if (info != null
+ && ((TrivialClassInitializer) info).field == field
+ && !appInfo.isPinned(field)
+ && outValue.canBeNull()) {
+ outValue.markNeverNull();
+ TypeLatticeElement typeLattice = outValue.getTypeLattice();
+ assert typeLattice.isNullable() && typeLattice.isReference();
+ outValue.narrowing(appInfo, typeLattice.asNonNullable());
+ affectedValues.addAll(outValue.affectedValues());
+ }
+ }
+ }
+ }
+ }
+ } else if (current.isStaticPut()) {
+ StaticPut staticPut = current.asStaticPut();
+ DexField field = staticPut.getField();
+ DexEncodedField target = appInfo.lookupStaticTarget(field.getHolder(), field);
+ if (target != null) {
+ // Remove writes to dead (i.e. never read) fields.
+ if (!isFieldRead(target, true)) {
+ iterator.removeOrReplaceByDebugLocalRead();
+ }
}
}
}
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 c46504c..5e32629 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,6 +6,7 @@
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;
@@ -22,12 +23,14 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.Objects;
+import java.util.Set;
public class PeepholeOptimizer {
/**
@@ -36,10 +39,159 @@
public static void optimize(IRCode code, LinearScanRegisterAllocator allocator) {
removeIdenticalPredecessorBlocks(code, allocator);
removeRedundantInstructions(code, allocator);
+ shareIdenticalBlockPrefix(code, allocator);
shareIdenticalBlockSuffix(code, allocator, 0);
assert code.isConsistentGraph();
}
+ /** Identify common prefixes in successor blocks and share them. */
+ private static void shareIdenticalBlockPrefix(IRCode code, RegisterAllocator allocator) {
+ InstructionEquivalence equivalence = new InstructionEquivalence(allocator);
+ Set<BasicBlock> blocksToBeRemoved = new HashSet<>();
+ for (BasicBlock block : code.blocks) {
+ if (blocksToBeRemoved.contains(block)) {
+ // This block has already been removed entirely.
+ continue;
+ }
+
+ List<BasicBlock> normalSuccessors = block.getNormalSuccessors();
+ if (normalSuccessors.size() != 2) {
+ continue;
+ }
+
+ // Exactly two normal successors.
+ BasicBlock normalSuccessor = normalSuccessors.get(0);
+ BasicBlock otherNormalSuccessor = normalSuccessors.get(1);
+
+ // Ensure that the current block is on all paths to the two successor blocks.
+ if (normalSuccessor.getPredecessors().size() != 1
+ || otherNormalSuccessor.getPredecessors().size() != 1) {
+ continue;
+ }
+
+ // Only try to share instructions if the two successors have the same locals state on entry.
+ if (!Objects.equals(
+ normalSuccessor.getLocalsAtEntry(), otherNormalSuccessor.getLocalsAtEntry())) {
+ continue;
+ }
+
+ // Share instructions from the two normal successors one-by-one.
+ while (true) {
+ // Check if all instructions were already removed from the two successors.
+ if (normalSuccessor.isEmpty() || otherNormalSuccessor.isEmpty()) {
+ assert blocksToBeRemoved.contains(normalSuccessor);
+ assert blocksToBeRemoved.contains(otherNormalSuccessor);
+ break;
+ }
+
+ Instruction instruction = normalSuccessor.entry();
+ Instruction otherInstruction = otherNormalSuccessor.entry();
+
+ // If the two instructions are not the same, we cannot merge them into the predecessor.
+ if (!equivalence.doEquivalent(instruction, otherInstruction)) {
+ break;
+ }
+
+ // Each block with one or more catch handlers may have at most one throwing instruction.
+ if (instruction.instructionTypeCanThrow() && block.hasCatchHandlers()) {
+ break;
+ }
+
+ // If the instruction can throw and one of the normal successor blocks has a catch handler,
+ // then we cannot merge the instruction into the predecessor, since this would change the
+ // exceptional control flow.
+ if (instruction.instructionInstanceCanThrow()
+ && (normalSuccessor.hasCatchHandlers() || otherNormalSuccessor.hasCatchHandlers())) {
+ break;
+ }
+
+ // Check for commutativity (semantics). If the instruction writes to a register, then we
+ // need to make sure that the instruction commutes with the last instruction in the
+ // predecessor. Consider the following example.
+ //
+ // <Block A>
+ // if-eqz r0 then goto Block B else goto Block C
+ // / \
+ // <Block B> <Block C>
+ // const r0, 1 const r0, 1
+ // ... ...
+ //
+ // In the example, it is not possible to change the order of "if-eqz r0" and
+ // "const r0, 1" without changing the semantics.
+ if (instruction.outValue() != null && instruction.outValue().needsRegister()) {
+ int destinationRegister =
+ allocator.getRegisterForValue(instruction.outValue(), instruction.getNumber());
+ boolean commutative =
+ block.exit().inValues().stream()
+ .allMatch(
+ inValue -> {
+ int operandRegister =
+ allocator.getRegisterForValue(inValue, block.exit().getNumber());
+ for (int i = 0; i < instruction.outValue().requiredRegisters(); i++) {
+ for (int j = 0; j < inValue.requiredRegisters(); j++) {
+ if (destinationRegister + i == operandRegister + j) {
+ // Overlap detected, the two instructions do not commute.
+ return false;
+ }
+ }
+ }
+ return true;
+ });
+ if (!commutative) {
+ break;
+ }
+ }
+
+ // Check for commutativity (debug info).
+ if (!instruction.getPosition().equals(block.exit().getPosition())
+ && !(block.exit().getPosition().isNone() && !block.exit().getDebugValues().isEmpty())) {
+ break;
+ }
+
+ // Remove the instruction from the two normal successors.
+ normalSuccessor.getInstructions().removeFirst();
+ otherNormalSuccessor.getInstructions().removeFirst();
+
+ // Move the instruction into the predecessor.
+ if (instruction.isJumpInstruction()) {
+ // Replace jump instruction in predecessor with the jump instruction from the two normal
+ // successors.
+ LinkedList<Instruction> instructions = block.getInstructions();
+ instructions.removeLast();
+ instructions.add(instruction);
+ instruction.setBlock(block);
+
+ // Update successors of predecessor block.
+ block.detachAllSuccessors();
+ for (BasicBlock newNormalSuccessor : normalSuccessor.getNormalSuccessors()) {
+ block.link(newNormalSuccessor);
+ }
+
+ // Detach the two normal successors from the rest of the graph.
+ normalSuccessor.detachAllSuccessors();
+ otherNormalSuccessor.detachAllSuccessors();
+
+ // Record that the two normal successors should be removed entirely.
+ blocksToBeRemoved.add(normalSuccessor);
+ blocksToBeRemoved.add(otherNormalSuccessor);
+ } else {
+ // Insert instruction before the jump instruction in the predecessor.
+ block.getInstructions().listIterator(block.getInstructions().size() - 1).add(instruction);
+ instruction.setBlock(block);
+
+ // Update locals-at-entry if needed.
+ if (instruction.isDebugLocalsChange()) {
+ DebugLocalsChange localsChange = instruction.asDebugLocalsChange();
+ localsChange.apply(normalSuccessor.getLocalsAtEntry());
+ localsChange.apply(otherNormalSuccessor.getLocalsAtEntry());
+ }
+ }
+ }
+ }
+
+ code.blocks.removeAll(blocksToBeRemoved);
+ }
+
/** Identify common suffixes in predecessor blocks and share them. */
public static void shareIdenticalBlockSuffix(
IRCode code, RegisterAllocator allocator, int overhead) {
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 1ac8555..a453346 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
@@ -28,6 +28,7 @@
import com.android.tools.r8.ir.code.NumericType;
import com.android.tools.r8.ir.code.Or;
import com.android.tools.r8.ir.code.Phi;
+import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.ir.code.StackValue;
import com.android.tools.r8.ir.code.StackValues;
import com.android.tools.r8.ir.code.Sub;
@@ -423,7 +424,8 @@
// Compute the final change in locals and insert it before nextInstruction.
boolean localsChanged = !ending.isEmpty() || !starting.isEmpty();
if (localsChanged) {
- DebugLocalsChange change = createLocalsChange(ending, starting);
+ DebugLocalsChange change =
+ createLocalsChange(ending, starting, instruction.getPosition());
if (change != null) {
// Insert the DebugLocalsChange instruction before nextInstruction.
instructionIterator.add(change);
@@ -506,36 +508,43 @@
starting.put(finalLocal.getIntKey(), finalLocal.getValue());
}
}
- DebugLocalsChange change = createLocalsChange(ending, starting);
+ DebugLocalsChange change = createLocalsChange(ending, starting, block.getPosition());
if (change != null) {
instructionIterator.add(change);
}
}
private static DebugLocalsChange createLocalsChange(
- Int2ReferenceMap<DebugLocalInfo> ending, Int2ReferenceMap<DebugLocalInfo> starting) {
+ Int2ReferenceMap<DebugLocalInfo> ending,
+ Int2ReferenceMap<DebugLocalInfo> starting,
+ Position position) {
+ assert position.isSome();
if (ending.isEmpty() && starting.isEmpty()) {
return null;
}
+ DebugLocalsChange localsChange;
if (ending.isEmpty() || starting.isEmpty()) {
- return new DebugLocalsChange(ending, starting);
- }
- IntSet unneeded = new IntArraySet(Math.min(ending.size(), starting.size()));
- for (Entry<DebugLocalInfo> entry : ending.int2ReferenceEntrySet()) {
- if (starting.get(entry.getIntKey()) == entry.getValue()) {
- unneeded.add(entry.getIntKey());
+ localsChange = new DebugLocalsChange(ending, starting);
+ } else {
+ IntSet unneeded = new IntArraySet(Math.min(ending.size(), starting.size()));
+ for (Entry<DebugLocalInfo> entry : ending.int2ReferenceEntrySet()) {
+ if (starting.get(entry.getIntKey()) == entry.getValue()) {
+ unneeded.add(entry.getIntKey());
+ }
}
+ if (unneeded.size() == ending.size() && unneeded.size() == starting.size()) {
+ return null;
+ }
+ IntIterator iterator = unneeded.iterator();
+ while (iterator.hasNext()) {
+ int key = iterator.nextInt();
+ ending.remove(key);
+ starting.remove(key);
+ }
+ localsChange = new DebugLocalsChange(ending, starting);
}
- if (unneeded.size() == ending.size() && unneeded.size() == starting.size()) {
- return null;
- }
- IntIterator iterator = unneeded.iterator();
- while (iterator.hasNext()) {
- int key = iterator.nextInt();
- ending.remove(key);
- starting.remove(key);
- }
- return new DebugLocalsChange(ending, starting);
+ localsChange.setPosition(position);
+ return localsChange;
}
private void clearState() {
diff --git a/src/test/java/com/android/tools/r8/TestRunResult.java b/src/test/java/com/android/tools/r8/TestRunResult.java
index 65a57e6..a032fd6 100644
--- a/src/test/java/com/android/tools/r8/TestRunResult.java
+++ b/src/test/java/com/android/tools/r8/TestRunResult.java
@@ -29,6 +29,10 @@
abstract RR self();
+ public AndroidApp app() {
+ return app;
+ }
+
public String getStdOut() {
return result.stdout;
}
diff --git a/src/test/java/com/android/tools/r8/ir/analysis/type/ConstrainedPrimitiveTypeTest.java b/src/test/java/com/android/tools/r8/ir/analysis/type/ConstrainedPrimitiveTypeTest.java
index c140610..798f1b0 100644
--- a/src/test/java/com/android/tools/r8/ir/analysis/type/ConstrainedPrimitiveTypeTest.java
+++ b/src/test/java/com/android/tools/r8/ir/analysis/type/ConstrainedPrimitiveTypeTest.java
@@ -10,10 +10,12 @@
import static com.android.tools.r8.ir.analysis.type.TypeLatticeElement.LONG;
import static org.junit.Assert.assertEquals;
+import com.android.tools.r8.NeverInline;
import com.android.tools.r8.ir.analysis.AnalysisTestBase;
import com.android.tools.r8.ir.code.ConstNumber;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.utils.StringUtils;
import com.google.common.collect.Streams;
import java.util.function.Consumer;
import org.junit.Test;
@@ -25,6 +27,21 @@
}
@Test
+ public void testOutput() throws Exception {
+ String expectedOutput =
+ StringUtils.lines("1", "2", "3", "1.0", "1.0", "2.0", "1", "1", "2", "1.0", "1.0", "2.0");
+
+ testForJvm().addTestClasspath().run(TestClass.class).assertSuccessWithOutput(expectedOutput);
+
+ testForR8(Backend.DEX)
+ .addInnerClasses(ConstrainedPrimitiveTypeTest.class)
+ .addKeepMainRule(TestClass.class)
+ .enableInliningAnnotations()
+ .run(TestClass.class)
+ .assertSuccessWithOutput(expectedOutput);
+ }
+
+ @Test
public void testIntWithInvokeUser() throws Exception {
buildAndCheckIR("intWithInvokeUserTest", testInspector(INT, 1));
}
@@ -83,26 +100,47 @@
static class TestClass {
- public static void intWithInvokeUserTest() {
- int x = 1;
- Integer.toString(x);
+ public static void main(String[] args) {
+ boolean unknownButTrue = args.length >= 0;
+ boolean unknownButFalse = args.length < 0;
+ intWithInvokeUserTest();
+ intWithIndirectInvokeUserTest(unknownButTrue);
+ intWithIndirectInvokeUserTest(unknownButFalse);
+ floatWithInvokeUserTest();
+ floatWithIndirectInvokeUserTest(unknownButTrue);
+ floatWithIndirectInvokeUserTest(unknownButFalse);
+ longWithInvokeUserTest();
+ longWithIndirectInvokeUserTest(unknownButTrue);
+ longWithIndirectInvokeUserTest(unknownButFalse);
+ doubleWithInvokeUserTest();
+ doubleWithIndirectInvokeUserTest(unknownButTrue);
+ doubleWithIndirectInvokeUserTest(unknownButFalse);
}
+ @NeverInline
+ public static void intWithInvokeUserTest() {
+ int x = 1;
+ System.out.println(Integer.toString(x));
+ }
+
+ @NeverInline
public static void intWithIndirectInvokeUserTest(boolean unknown) {
int x;
if (unknown) {
- x = 1;
- } else {
x = 2;
+ } else {
+ x = 3;
}
- Integer.toString(x);
+ System.out.println(Integer.toString(x));
}
+ @NeverInline
public static void floatWithInvokeUserTest() {
float x = 1f;
- Float.toString(x);
+ System.out.println(Float.toString(x));
}
+ @NeverInline
public static void floatWithIndirectInvokeUserTest(boolean unknown) {
float x;
if (unknown) {
@@ -110,14 +148,16 @@
} else {
x = 2f;
}
- Float.toString(x);
+ System.out.println(Float.toString(x));
}
+ @NeverInline
public static void longWithInvokeUserTest() {
long x = 1L;
- Long.toString(x);
+ System.out.println(Long.toString(x));
}
+ @NeverInline
public static void longWithIndirectInvokeUserTest(boolean unknown) {
long x;
if (unknown) {
@@ -125,14 +165,16 @@
} else {
x = 2L;
}
- Long.toString(x);
+ System.out.println(Long.toString(x));
}
+ @NeverInline
public static void doubleWithInvokeUserTest() {
double x = 1.0;
- Double.toString(x);
+ System.out.println(Double.toString(x));
}
+ @NeverInline
public static void doubleWithIndirectInvokeUserTest(boolean unknown) {
double x;
if (unknown) {
@@ -140,7 +182,7 @@
} else {
x = 2f;
}
- Double.toString(x);
+ System.out.println(Double.toString(x));
}
}
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/RedundantConstNumberRemovalTest.java b/src/test/java/com/android/tools/r8/ir/optimize/RedundantConstNumberRemovalTest.java
new file mode 100644
index 0000000..fc382ee
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/RedundantConstNumberRemovalTest.java
@@ -0,0 +1,248 @@
+// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.ir.optimize;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.R8TestRunResult;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ir.code.BasicBlock;
+import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.Streams;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class RedundantConstNumberRemovalTest extends TestBase {
+
+ private final Backend backend;
+
+ @Parameters(name = "Backend: {0}")
+ public static Backend[] data() {
+ return Backend.values();
+ }
+
+ public RedundantConstNumberRemovalTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Test
+ public void test() throws Exception {
+ String expectedOutput =
+ StringUtils.lines(
+ "true", "true", "true", "true", "true", "true", "true", "true", "true", "true", "true",
+ "true", "true", "true", "true", "true");
+
+ if (backend == Backend.CF) {
+ testForJvm().addTestClasspath().run(TestClass.class).assertSuccessWithOutput(expectedOutput);
+ }
+
+ R8TestRunResult result =
+ testForR8(backend)
+ .addInnerClasses(RedundantConstNumberRemovalTest.class)
+ .addKeepMainRule(TestClass.class)
+ .enableInliningAnnotations()
+ .run(TestClass.class)
+ .assertSuccessWithOutput(expectedOutput);
+
+ ClassSubject classSubject = result.inspector().clazz(TestClass.class);
+ verifyBooleanCheckTest(classSubject.uniqueMethodWithName("booleanCheckTest"));
+ verifyBooleanCheckTest(classSubject.uniqueMethodWithName("negateBooleanCheckTest"));
+ verifyIntCheckTest(classSubject.uniqueMethodWithName("intCheckTest"));
+ verifyIntCheckTest(classSubject.uniqueMethodWithName("negateIntCheckTest"));
+ verifyNullCheckTest(classSubject.uniqueMethodWithName("nullCheckTest"));
+ verifyNullCheckTest(classSubject.uniqueMethodWithName("invertedNullCheckTest"));
+ verifyNullCheckTest(classSubject.uniqueMethodWithName("nonNullCheckTest"));
+ verifyNullCheckWithWrongTypeTest(
+ classSubject.uniqueMethodWithName("nullCheckWithWrongTypeTest"));
+ }
+
+ private void verifyBooleanCheckTest(MethodSubject methodSubject) {
+ assertThat(methodSubject, isPresent());
+
+ if (backend == Backend.DEX) {
+ // Check that the generated code for booleanCheckTest() only has a return instruction.
+ assertEquals(1, methodSubject.streamInstructions().count());
+ assertTrue(methodSubject.iterateInstructions().next().isReturn());
+ } else {
+ assert backend == Backend.CF;
+ // Check that the generated code for booleanCheckTest() only has return instructions that
+ // return the argument.
+ // TODO(christofferqa): CF backend does not share identical prefix of successors.
+ IRCode code = methodSubject.buildIR();
+ assertTrue(
+ Streams.stream(code.instructionIterator())
+ .filter(Instruction::isReturn)
+ .allMatch(
+ instruction -> instruction.asReturn().returnValue().definition.isArgument()));
+ }
+ }
+
+ private void verifyIntCheckTest(MethodSubject methodSubject) {
+ assertThat(methodSubject, isPresent());
+ IRCode code = methodSubject.buildIR();
+
+ if (backend == Backend.DEX) {
+ // Only a single basic block.
+ assertEquals(1, code.blocks.size());
+ // The block only has three instructions.
+ BasicBlock entryBlock = code.blocks.get(0);
+ assertEquals(3, entryBlock.getInstructions().size());
+ // The first one is the `argument` instruction.
+ Instruction argument = entryBlock.getInstructions().getFirst();
+ assertTrue(argument.isArgument());
+ // The next one is a `const-number` instruction is not used for anything.
+ // TODO(christofferqa): D8 should be able to get rid of the unused const-number instruction.
+ Instruction unused = entryBlock.getInstructions().get(1);
+ assertTrue(unused.isConstNumber());
+ assertEquals(0, unused.outValue().numberOfAllUsers());
+ // The `return` instruction returns the argument.
+ Instruction ret = entryBlock.getInstructions().getLast();
+ assertTrue(ret.isReturn());
+ assertTrue(ret.asReturn().returnValue().definition.isArgument());
+ } else {
+ // Check that the generated code for intCheckTest() only has return instructions that
+ // return the argument.
+ // TODO(christofferqa): CF backend does not share identical prefix of successors.
+ assertTrue(
+ Streams.stream(code.instructionIterator())
+ .filter(Instruction::isReturn)
+ .allMatch(
+ instruction -> instruction.asReturn().returnValue().definition.isArgument()));
+ }
+ }
+
+ private void verifyNullCheckTest(MethodSubject methodSubject) {
+ // Check that the generated code for nullCheckTest() only has a single `const-null` instruction.
+ assertThat(methodSubject, isPresent());
+ assertEquals(
+ 1, methodSubject.streamInstructions().filter(InstructionSubject::isConstNull).count());
+
+ // Also check that one of the return instructions actually returns the argument.
+ IRCode code = methodSubject.buildIR();
+ assertEquals(1, code.collectArguments().size());
+ // TODO(b/120257211): D8 should replace `return null` by `return arg`.
+ assertFalse(
+ code.collectArguments().get(0).uniqueUsers().stream().anyMatch(Instruction::isReturn));
+ }
+
+ private void verifyNullCheckWithWrongTypeTest(MethodSubject methodSubject) {
+ // Check that the generated code for nullCheckWithWrongTypeTest() still has a `return null`
+ // instruction.
+ assertThat(methodSubject, isPresent());
+ IRCode code = methodSubject.buildIR();
+
+ // Check that the code returns null.
+ assertTrue(
+ Streams.stream(code.instructionIterator())
+ .anyMatch(
+ instruction ->
+ instruction.isReturn()
+ && instruction.asReturn().returnValue().definition.isConstNumber()));
+
+ // Also check that none of the return instructions actually returns the argument.
+ assertEquals(1, code.collectArguments().size());
+ assertTrue(
+ code.collectArguments().get(0).uniqueUsers().stream().noneMatch(Instruction::isReturn));
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ System.out.println(booleanCheckTest(true) == true);
+ System.out.println(booleanCheckTest(false) == false);
+ System.out.println(negateBooleanCheckTest(true) == true);
+ System.out.println(negateBooleanCheckTest(false) == false);
+ System.out.println(intCheckTest(0) == 0);
+ System.out.println(intCheckTest(42) == 42);
+ System.out.println(negateIntCheckTest(0) == 0);
+ System.out.println(negateIntCheckTest(42) == 42);
+ System.out.println(nullCheckTest(new Object()) != null);
+ System.out.println(nullCheckTest(null) == null);
+ System.out.println(invertedNullCheckTest(new Object()) != null);
+ System.out.println(invertedNullCheckTest(null) == null);
+ System.out.println(nonNullCheckTest(new Object()) != null);
+ System.out.println(nonNullCheckTest(null) == null);
+ System.out.println(nullCheckWithWrongTypeTest(new Object()) != null);
+ System.out.println(nullCheckWithWrongTypeTest(null) == null);
+ }
+
+ @NeverInline
+ private static boolean booleanCheckTest(boolean x) {
+ if (x) {
+ return true; // should be replaced by `x`.
+ }
+ return false; // should be replaced by `x`.
+ }
+
+ @NeverInline
+ private static boolean negateBooleanCheckTest(boolean x) {
+ if (!x) {
+ return false; // should be replaced by `x`
+ }
+ return true; // should be replaced by `x`
+ }
+
+ @NeverInline
+ private static int intCheckTest(int x) {
+ if (x == 42) {
+ return 42; // should be replaced by `x`.
+ }
+ return x;
+ }
+
+ @NeverInline
+ private static int negateIntCheckTest(int x) {
+ if (x != 42) {
+ return x;
+ }
+ return 42; // should be replaced by `x`
+ }
+
+ @NeverInline
+ private static Object nullCheckTest(Object x) {
+ if (x != null) {
+ return new Object();
+ }
+ return null; // should be replaced by `x`.
+ }
+
+ @NeverInline
+ private static Object invertedNullCheckTest(Object x) {
+ if (null == x) {
+ return null; // should be replaced by `x`.
+ }
+ return new Object();
+ }
+
+ @NeverInline
+ private static Object nonNullCheckTest(Object x) {
+ if (x == null) {
+ return null; // should be replaced by `x`
+ }
+ return new Object();
+ }
+
+ @NeverInline
+ private static Throwable nullCheckWithWrongTypeTest(Object x) {
+ if (x != null) {
+ return new Throwable();
+ }
+ return null; // cannot be replaced by `x` because Object is not a subtype of Throwable.
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java
index c2ced01..8e24088 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java
@@ -6,12 +6,18 @@
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.naming.MemberNaming.Signature;
public class AbsentMethodSubject extends MethodSubject {
@Override
+ public IRCode buildIR() {
+ throw new Unreachable("Cannot build IR for an absent method");
+ }
+
+ @Override
public boolean isPresent() {
return false;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
index 924ec2d..ee2155e 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
@@ -4,7 +4,6 @@
package com.android.tools.r8.utils.codeinspector;
-import static org.junit.Assert.assertTrue;
import com.android.tools.r8.cf.code.CfArithmeticBinop;
import com.android.tools.r8.cf.code.CfCheckCast;
@@ -31,7 +30,6 @@
import com.android.tools.r8.cf.code.CfStackInstruction;
import com.android.tools.r8.cf.code.CfSwitch;
import com.android.tools.r8.cf.code.CfThrow;
-import com.android.tools.r8.graph.CfCode;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.ir.code.Monitor.Type;
@@ -167,6 +165,11 @@
}
@Override
+ public boolean isReturn() {
+ return instruction instanceof CfReturn;
+ }
+
+ @Override
public boolean isReturnVoid() {
return instruction instanceof CfReturnVoid;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java
index 1f520ff..9983320 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java
@@ -58,7 +58,7 @@
public class CodeInspector {
- private final DexApplication application;
+ final DexApplication application;
final DexItemFactory dexItemFactory;
private final ClassNameMapper mapping;
final Map<String, String> originalToObfuscatedMapping;
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
index 6d463da..7051226 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
@@ -70,6 +70,7 @@
import com.android.tools.r8.code.NewInstance;
import com.android.tools.r8.code.Nop;
import com.android.tools.r8.code.PackedSwitch;
+import com.android.tools.r8.code.Return;
import com.android.tools.r8.code.ReturnObject;
import com.android.tools.r8.code.ReturnVoid;
import com.android.tools.r8.code.Sget;
@@ -275,6 +276,11 @@
}
@Override
+ public boolean isReturn() {
+ return instruction instanceof Return;
+ }
+
+ @Override
public boolean isReturnVoid() {
return instruction instanceof ReturnVoid;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
index 6c23162..796532d 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
@@ -9,6 +9,7 @@
import com.android.tools.r8.code.Instruction;
import com.android.tools.r8.errors.Unimplemented;
import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.CfCode;
import com.android.tools.r8.graph.Code;
import com.android.tools.r8.graph.DexCode;
@@ -17,10 +18,14 @@
import com.android.tools.r8.graph.DexDebugPositionState;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexString;
+import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.graph.JarCode;
+import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.naming.MemberNaming;
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.naming.signature.GenericSignatureParser;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.InternalOptions;
import it.unimi.dsi.fastutil.objects.Reference2IntMap;
import it.unimi.dsi.fastutil.objects.Reference2IntOpenHashMap;
import java.util.Arrays;
@@ -41,6 +46,19 @@
}
@Override
+ public IRCode buildIR() {
+ DexEncodedMethod method = getMethod();
+ return method
+ .getCode()
+ .buildIR(
+ method,
+ new AppInfo(codeInspector.application),
+ GraphLense.getIdentityLense(),
+ new InternalOptions(),
+ Origin.unknown());
+ }
+
+ @Override
public boolean isPresent() {
return true;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
index 6abeacf..294cc8c 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
@@ -58,6 +58,8 @@
boolean isIfEqz();
+ boolean isReturn();
+
boolean isReturnVoid();
boolean isReturnObject();
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
index b0d2c9b..8fb4001 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
@@ -5,6 +5,7 @@
package com.android.tools.r8.utils.codeinspector;
import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.google.common.collect.Streams;
import java.util.Iterator;
@@ -13,6 +14,8 @@
public abstract class MethodSubject extends MemberSubject {
+ public abstract IRCode buildIR();
+
public abstract boolean isAbstract();
public abstract boolean isBridge();
