Extend new-array-filled to support phis
This also changes the pass to be more conservative about
applying the transformation to arrays of classes that might
result in NoClassDefFoundError (mostly affects D8).
Bug: b/246971330
Change-Id: I8c57a1c86907ba8799e767fda27091c940b1b991
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 3570828..d784fa3 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
@@ -438,6 +438,17 @@
}
}
+ public void removeAllExceptionalSuccessors() {
+ assert hasCatchHandlers();
+ IntList successorsToRemove = new IntArrayList();
+ int numberOfExceptionalSuccessors = numberOfExceptionalSuccessors();
+ for (int i = 0; i < numberOfExceptionalSuccessors; i++) {
+ successorsToRemove.add(i);
+ successors.get(i).getMutablePredecessors().remove(this);
+ }
+ removeSuccessorsByIndex(successorsToRemove);
+ }
+
public void swapSuccessors(BasicBlock a, BasicBlock b) {
assert a != b;
int aIndex = successors.indexOf(a);
@@ -1153,6 +1164,36 @@
return true;
}
+ private boolean isExceptionTrampoline() {
+ boolean ret = instructions.size() == 2 && entry().isMoveException() && exit().isGoto();
+ assert !ret || !hasCatchHandlers() : "Trampoline should not have catch handlers";
+ return ret;
+ }
+
+ /** Returns whether the given blocks are in the same try block. */
+ public boolean hasEquivalentCatchHandlers(BasicBlock other) {
+ if (this == other) {
+ return true;
+ }
+ List<Integer> targets1 = catchHandlers.getAllTargets();
+ List<Integer> targets2 = other.catchHandlers.getAllTargets();
+ int numHandlers = targets1.size();
+ if (numHandlers != targets2.size()) {
+ return false;
+ }
+ // If all catch handlers are trampolines to the same block, then they are from the same try.
+ for (int i = 0; i < numHandlers; ++i) {
+ BasicBlock catchBlock1 = successors.get(targets1.get(i));
+ BasicBlock catchBlock2 = other.successors.get(targets2.get(i));
+ if (!catchBlock1.isExceptionTrampoline()
+ || !catchBlock2.isExceptionTrampoline()
+ || catchBlock1.getUniqueSuccessor() != catchBlock2.getUniqueSuccessor()) {
+ return false;
+ }
+ }
+ return true;
+ }
+
public void clearCurrentDefinitions() {
currentDefinitions = null;
for (Phi phi : getPhis()) {
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 ba73a12..365708f 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
@@ -406,6 +406,14 @@
return uniquePhiUsers = ImmutableSet.copyOf(phiUsers);
}
+ public Set<BasicBlock> uniquePhiUserBlocks() {
+ Set<BasicBlock> ret = Sets.newIdentityHashSet();
+ for (Phi phi : phiUsers) {
+ ret.add(phi.getBlock());
+ }
+ return ret;
+ }
+
public Set<Instruction> debugUsers() {
return debugData == null ? null : Collections.unmodifiableSet(debugData.users);
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/ArrayConstructionSimplifier.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/ArrayConstructionSimplifier.java
index 7d95d37..41379ad 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/passes/ArrayConstructionSimplifier.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/ArrayConstructionSimplifier.java
@@ -7,25 +7,28 @@
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.DexTypeUtils;
import com.android.tools.r8.ir.code.ArrayPut;
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.ir.code.InstructionListIterator;
-import com.android.tools.r8.ir.code.LinearFlowInstructionListIterator;
import com.android.tools.r8.ir.code.NewArrayEmpty;
import com.android.tools.r8.ir.code.NewArrayFilled;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.conversion.MethodProcessor;
import com.android.tools.r8.ir.conversion.passes.result.CodeRewriterResult;
-import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.ArrayUtils;
+import com.android.tools.r8.utils.DominatorChecker;
import com.android.tools.r8.utils.InternalOptions.RewriteArrayOptions;
-import com.android.tools.r8.utils.SetUtils;
+import com.android.tools.r8.utils.ValueUtils;
+import com.android.tools.r8.utils.ValueUtils.ArrayValues;
import com.android.tools.r8.utils.WorkList;
import com.google.common.collect.Sets;
import java.util.ArrayList;
-import java.util.Arrays;
+import java.util.IdentityHashMap;
import java.util.List;
+import java.util.Map;
import java.util.Set;
/**
@@ -80,8 +83,11 @@
*/
public class ArrayConstructionSimplifier extends CodeRewriterPass<AppInfo> {
+ private final RewriteArrayOptions rewriteArrayOptions;
+
public ArrayConstructionSimplifier(AppView<?> appView) {
super(appView);
+ rewriteArrayOptions = options.rewriteArrayOptions();
}
@Override
@@ -91,13 +97,13 @@
@Override
protected CodeRewriterResult rewriteCode(IRCode code) {
- boolean hasChanged = false;
- WorkList<BasicBlock> worklist = WorkList.newIdentityWorkList(code.blocks);
- while (worklist.hasNext()) {
- BasicBlock block = worklist.next();
- hasChanged |= simplifyArrayConstructionBlock(block, worklist, code, appView.options());
+ ArrayList<ArrayValues> candidates = findOptimizableArrays(code);
+
+ if (candidates.isEmpty()) {
+ return CodeRewriterResult.NO_CHANGE;
}
- return CodeRewriterResult.hasChanged(hasChanged);
+ applyChanges(code, candidates);
+ return CodeRewriterResult.HAS_CHANGED;
}
@Override
@@ -105,214 +111,257 @@
return code.metadata().mayHaveNewArrayEmpty();
}
- private boolean simplifyArrayConstructionBlock(
- BasicBlock block, WorkList<BasicBlock> worklist, IRCode code, InternalOptions options) {
- boolean hasChanged = false;
- RewriteArrayOptions rewriteOptions = options.rewriteArrayOptions();
- InstructionListIterator it = block.listIterator(code);
- while (it.hasNext()) {
- FilledArrayCandidate candidate = computeFilledArrayCandidate(it.next(), rewriteOptions);
- if (candidate == null) {
- continue;
- }
- FilledArrayConversionInfo info =
- computeConversionInfo(
- code, candidate, new LinearFlowInstructionListIterator(code, block, it.nextIndex()));
- if (info == null) {
- continue;
- }
-
- Instruction instructionAfterCandidate = it.peekNext();
- NewArrayEmpty newArrayEmpty = candidate.newArrayEmpty;
- DexType arrayType = newArrayEmpty.type;
- int size = candidate.size;
- Set<Instruction> instructionsToRemove = SetUtils.newIdentityHashSet(size + 1);
- assert newArrayEmpty.getLocalInfo() == null;
- Instruction lastArrayPut = info.lastArrayPutIterator.peekPrevious();
- Value invokeValue = code.createValue(newArrayEmpty.getOutType(), null);
- NewArrayFilled invoke =
- new NewArrayFilled(arrayType, invokeValue, Arrays.asList(info.values));
- invoke.setPosition(lastArrayPut.getPosition());
- for (Value value : newArrayEmpty.inValues()) {
- value.removeUser(newArrayEmpty);
- }
- newArrayEmpty.outValue().replaceUsers(invokeValue);
- instructionsToRemove.add(newArrayEmpty);
-
- boolean originalAllocationPointHasHandlers = block.hasCatchHandlers();
- boolean insertionPointHasHandlers = lastArrayPut.getBlock().hasCatchHandlers();
-
- if (!insertionPointHasHandlers && !originalAllocationPointHasHandlers) {
- info.lastArrayPutIterator.add(invoke);
- } else {
- BasicBlock insertionBlock = info.lastArrayPutIterator.split(code);
- if (originalAllocationPointHasHandlers) {
- if (!insertionBlock.isTrivialGoto()) {
- BasicBlock blockAfterInsertion = insertionBlock.listIterator(code).split(code);
- assert insertionBlock.isTrivialGoto();
- worklist.addIfNotSeen(blockAfterInsertion);
- }
- insertionBlock.moveCatchHandlers(block);
- } else {
- worklist.addIfNotSeen(insertionBlock);
- }
- insertionBlock.listIterator(code).add(invoke);
- }
-
- instructionsToRemove.addAll(info.arrayPutsToRemove);
- Set<BasicBlock> visitedBlocks = Sets.newIdentityHashSet();
- for (Instruction instruction : instructionsToRemove) {
- BasicBlock ownerBlock = instruction.getBlock();
- // If owner block is null, then the instruction has been removed already. We can't rely on
- // just having the block pointer nulled, so the visited blocks guards reprocessing.
- if (ownerBlock != null && visitedBlocks.add(ownerBlock)) {
- InstructionListIterator removeIt = ownerBlock.listIterator(code);
- while (removeIt.hasNext()) {
- if (instructionsToRemove.contains(removeIt.next())) {
- removeIt.removeOrReplaceByDebugLocalRead();
- }
- }
+ private ArrayList<ArrayValues> findOptimizableArrays(IRCode code) {
+ ArrayList<ArrayValues> candidates = new ArrayList<>();
+ for (Instruction instruction : code.instructions()) {
+ NewArrayEmpty newArrayEmpty = instruction.asNewArrayEmpty();
+ if (newArrayEmpty != null) {
+ ArrayValues arrayValues = analyzeCandidate(newArrayEmpty, code);
+ if (arrayValues != null) {
+ candidates.add(arrayValues);
}
}
-
- // The above has invalidated the block iterator so reset it and continue.
- it = block.listIterator(code, instructionAfterCandidate);
- hasChanged = true;
}
- if (hasChanged) {
- code.removeRedundantBlocks();
- }
-
- return hasChanged;
+ return candidates;
}
- private static class FilledArrayConversionInfo {
-
- Value[] values;
- List<ArrayPut> arrayPutsToRemove;
- LinearFlowInstructionListIterator lastArrayPutIterator;
-
- public FilledArrayConversionInfo(int size) {
- values = new Value[size];
- arrayPutsToRemove = new ArrayList<>(size);
+ private ArrayValues analyzeCandidate(NewArrayEmpty newArrayEmpty, IRCode code) {
+ if (newArrayEmpty.getLocalInfo() != null) {
+ return null;
}
+ if (!rewriteArrayOptions.isPotentialSize(newArrayEmpty.sizeIfConst())) {
+ return null;
+ }
+
+ ArrayValues arrayValues = ValueUtils.computeInitialArrayValues(newArrayEmpty);
+ // Holes (default-initialized entries) could be supported, but they are rare and would
+ // complicate the logic.
+ if (arrayValues == null || arrayValues.containsHoles()) {
+ return null;
+ }
+
+ // See if all instructions are in the same try/catch.
+ ArrayPut lastArrayPut = ArrayUtils.last(arrayValues.getArrayPutsByIndex());
+ if (!newArrayEmpty.getBlock().hasEquivalentCatchHandlers(lastArrayPut.getBlock())) {
+ // Possible improvements:
+ // 1) Ignore catch handlers that do not catch OutOfMemoryError / NoClassDefFoundError.
+ // 2) Use the catch handlers from the new-array-empty if all exception blocks exit without
+ // side effects (e.g. no method calls & no monitor instructions).
+ return null;
+ }
+
+ if (!checkTypesAreCompatible(arrayValues, code)) {
+ return null;
+ }
+ if (!checkDominance(arrayValues)) {
+ return null;
+ }
+ return arrayValues;
}
- @SuppressWarnings("ReferenceEquality")
- private FilledArrayConversionInfo computeConversionInfo(
- IRCode code, FilledArrayCandidate candidate, LinearFlowInstructionListIterator it) {
- NewArrayEmpty newArrayEmpty = candidate.newArrayEmpty;
- assert it.peekPrevious() == newArrayEmpty;
- Value arrayValue = newArrayEmpty.outValue();
- int size = candidate.size;
-
+ private boolean checkTypesAreCompatible(ArrayValues arrayValues, IRCode code) {
// aput-object allows any object for arrays of interfaces, but new-filled-array fails to verify
// if types require a cast.
// TODO(b/246971330): Check if adding a checked-cast would have the same observable result. E.g.
// if aput-object throws a ClassCastException if given an object that does not implement the
// desired interface, then we could add check-cast instructions for arguments we're not sure
// about.
+ NewArrayEmpty newArrayEmpty = arrayValues.getDefinition().asNewArrayEmpty();
DexType elementType = newArrayEmpty.type.toArrayElementType(dexItemFactory);
boolean needsTypeCheck =
- !elementType.isPrimitiveType() && elementType != dexItemFactory.objectType;
+ !elementType.isPrimitiveType() && elementType.isNotIdenticalTo(dexItemFactory.objectType);
+ if (!needsTypeCheck) {
+ return true;
+ }
- FilledArrayConversionInfo info = new FilledArrayConversionInfo(size);
- Value[] values = info.values;
- int remaining = size;
- Set<Instruction> users = newArrayEmpty.outValue().uniqueUsers();
- while (it.hasNext()) {
- Instruction instruction = it.next();
- BasicBlock block = instruction.getBlock();
- // If we encounter an instruction that can throw an exception we need to bail out of the
- // optimization so that we do not transform half-initialized arrays into fully initialized
- // arrays on exceptional edges. If the block has no handlers it is not observable so
- // we perform the rewriting.
- if (block.hasCatchHandlers()
- && instruction.instructionInstanceCanThrow(appView, code.context())) {
- return null;
- }
- if (!users.contains(instruction)) {
- // If any instruction can transfer control between the new-array and the last array put
- // then it is not safe to move the new array to the point of the last put.
- if (block.hasCatchHandlers() && instruction.instructionTypeCanThrow()) {
- return null;
- }
+ // Not safe to move allocation if NoClassDefError is possible.
+ // TODO(b/246971330): Make this work for D8 where it ~always returns false by checking that
+ // all instructions between new-array-empty and the last array-put report
+ // !instruction.instructionMayHaveSideEffects(). Alternatively, we could replace the
+ // new-array-empty with a const-class instruction in this case.
+ if (!DexTypeUtils.isTypeAccessibleInMethodContext(
+ appView, elementType.toBaseType(dexItemFactory), code.context())) {
+ return false;
+ }
+
+ for (ArrayPut arrayPut : arrayValues.getArrayPutsByIndex()) {
+ Value value = arrayPut.value();
+ if (value.isAlwaysNull(appView)) {
continue;
}
- ArrayPut arrayPut = instruction.asArrayPut();
- // If the initialization sequence is broken by another use we cannot use a fill-array-data
- // instruction.
+ DexType valueDexType = value.getType().asReferenceType().toDexType(dexItemFactory);
+ if (elementType.isArrayType()) {
+ if (elementType.isNotIdenticalTo(valueDexType)) {
+ return false;
+ }
+ } else if (valueDexType.isArrayType()) {
+ // isSubtype asserts for this case.
+ return false;
+ } else if (valueDexType.isNullValueType()) {
+ // Assume instructions can cause value.isAlwaysNull() == false while the DexType is
+ // null.
+ // TODO(b/246971330): Figure out how to write a test in SimplifyArrayConstructionTest
+ // that hits this case.
+ } else {
+ // TODO(b/246971330): When in d8 mode, we might still be able to see if this is true for
+ // library types (which this helper does not do).
+ if (appView.isSubtype(valueDexType, elementType).isPossiblyFalse()) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
+
+ private static boolean checkDominance(ArrayValues arrayValues) {
+ Value arrayValue = arrayValues.getArrayValue();
+
+ Set<BasicBlock> usageBlocks = Sets.newIdentityHashSet();
+ Set<Instruction> uniqueUsers = arrayValue.uniqueUsers();
+ for (Instruction user : uniqueUsers) {
+ ArrayPut arrayPut = user.asArrayPut();
if (arrayPut == null || arrayPut.array() != arrayValue) {
- return null;
+ usageBlocks.add(user.getBlock());
}
- int index = arrayPut.indexIfConstAndInBounds(values.length);
- if (index < 0 || values[index] != null) {
- return null;
+ }
+
+ // Ensure all blocks for users of the array are dominated by the last array-put's block.
+ ArrayPut lastArrayPut = ArrayUtils.last(arrayValues.getArrayPutsByIndex());
+ BasicBlock lastArrayPutBlock = lastArrayPut.getBlock();
+ BasicBlock subgraphEntryBlock = arrayValue.definition.getBlock();
+ for (BasicBlock usageBlock : usageBlocks) {
+ if (!DominatorChecker.check(subgraphEntryBlock, usageBlock, lastArrayPutBlock)) {
+ return false;
}
- if (arrayPut.instructionInstanceCanThrow(appView, code.context())) {
- return null;
+ }
+
+ // Ensure all array users in the same block appear after the last array-put
+ for (Instruction inst : lastArrayPutBlock.getInstructions()) {
+ if (inst == lastArrayPut) {
+ break;
}
- Value value = arrayPut.value();
- if (needsTypeCheck && !value.isAlwaysNull(appView)) {
- DexType valueDexType = value.getType().asReferenceType().toDexType(dexItemFactory);
- if (elementType.isArrayType()) {
- if (elementType != valueDexType) {
- return null;
- }
- } else if (valueDexType.isArrayType()) {
- // isSubtype asserts for this case.
- return null;
- } else if (valueDexType.isNullValueType()) {
- // Assume instructions can cause value.isAlwaysNull() == false while the DexType is null.
- // TODO(b/246971330): Figure out how to write a test in SimplifyArrayConstructionTest
- // that hits this case.
- } else {
- // TODO(b/246971330): When in d8 mode, we might still be able to see if this is true for
- // library types (which this helper does not do).
- if (appView.isSubtype(valueDexType, elementType).isPossiblyFalse()) {
- return null;
+ if (uniqueUsers.contains(inst)) {
+ ArrayPut arrayPut = inst.asArrayPut();
+ if (arrayPut == null || arrayPut.array() != arrayValue) {
+ return false;
+ }
+ }
+ }
+
+ // It will not be the case that the newArrayEmpty dominates the phi user (or else it would
+ // just be a normal user). It is safe to optimize if all paths from the new-array-empty to the
+ // phi user include the last array-put (where the filled-new-array will end up).
+ if (anyPhiUsersReachableWhenOptimized(arrayValues)) {
+ return false;
+ }
+ return true;
+ }
+
+ /**
+ * Determines if there are any paths from the new-array-empty to any of its phi users that do not
+ * go through the last array-put, and that do not go through exceptional edges for new-array-empty
+ * / array-puts that will be removed.
+ */
+ private static boolean anyPhiUsersReachableWhenOptimized(ArrayValues arrayValues) {
+ Value arrayValue = arrayValues.getArrayValue();
+ if (!arrayValue.hasPhiUsers()) {
+ return false;
+ }
+ Set<BasicBlock> phiUserBlocks = arrayValue.uniquePhiUserBlocks();
+ WorkList<BasicBlock> workList = WorkList.newIdentityWorkList();
+ // Mark the last array-put as seen in order to find paths that do not contain it.
+ workList.markAsSeen(ArrayUtils.last(arrayValues.getArrayPutsByIndex()).getBlock());
+ // Start with normal successors since if optimized, the new-array-empty block will have no
+ // throwing instructions.
+ workList.addIfNotSeen(arrayValue.definition.getBlock().getNormalSuccessors());
+ while (workList.hasNext()) {
+ BasicBlock current = workList.removeLast();
+ if (phiUserBlocks.contains(current)) {
+ return true;
+ }
+ if (current.hasCatchHandlers()) {
+ Instruction throwingInstruction = current.exceptionalExit();
+ if (throwingInstruction != null) {
+ ArrayPut arrayPut = throwingInstruction.asArrayPut();
+ // Ignore exceptional edges that will be remove if optimized.
+ if (arrayPut != null && arrayPut.array() == arrayValue) {
+ workList.addIfNotSeen(current.getNormalSuccessors());
+ continue;
}
}
}
- info.arrayPutsToRemove.add(arrayPut);
- values[index] = value;
- --remaining;
- if (remaining == 0) {
- info.lastArrayPutIterator = it;
- return info;
+ workList.addIfNotSeen(current.getSuccessors());
+ }
+ return false;
+ }
+
+ private void applyChanges(IRCode code, List<ArrayValues> candidates) {
+ Set<BasicBlock> relevantBlocks = Sets.newIdentityHashSet();
+ // All keys instructionsToChange are removed, and also maps lastArrayPut -> newArrayFilled.
+ Map<Instruction, Instruction> instructionsToChange = new IdentityHashMap<>();
+ boolean needToRemoveUnreachableBlocks = false;
+
+ for (ArrayValues arrayValues : candidates) {
+ NewArrayEmpty newArrayEmpty = arrayValues.getDefinition().asNewArrayEmpty();
+ instructionsToChange.put(newArrayEmpty, newArrayEmpty);
+ BasicBlock allocationBlock = newArrayEmpty.getBlock();
+ relevantBlocks.add(allocationBlock);
+
+ ArrayPut[] arrayPutsByIndex = arrayValues.getArrayPutsByIndex();
+ int lastArrayPutIndex = arrayPutsByIndex.length - 1;
+ for (int i = 0; i < lastArrayPutIndex; ++i) {
+ ArrayPut arrayPut = arrayPutsByIndex[i];
+ instructionsToChange.put(arrayPut, arrayPut);
+ relevantBlocks.add(arrayPut.getBlock());
+ }
+ ArrayPut lastArrayPut = arrayPutsByIndex[lastArrayPutIndex];
+ BasicBlock lastArrayPutBlock = lastArrayPut.getBlock();
+ relevantBlocks.add(lastArrayPutBlock);
+
+ // newArrayEmpty's outValue must be cleared before trying to remove newArrayEmpty. Rather than
+ // store the outValue for later, create and store newArrayFilled.
+ Value arrayValue = newArrayEmpty.clearOutValue();
+ NewArrayFilled newArrayFilled =
+ new NewArrayFilled(newArrayEmpty.type, arrayValue, arrayValues.getElementValues());
+ newArrayFilled.setPosition(lastArrayPut.getPosition());
+ instructionsToChange.put(lastArrayPut, newArrayFilled);
+
+ if (arrayValue.hasPhiUsers() && allocationBlock.hasCatchHandlers()) {
+ // When phi users exist, the phis belong to the exceptional successors of the allocation
+ // block. In order to preserve them, move them to the new allocation block.
+ // This is safe because we've already checked hasEquivalentCatchHandlers().
+ lastArrayPutBlock.removeAllExceptionalSuccessors();
+ lastArrayPutBlock.moveCatchHandlers(allocationBlock);
+ needToRemoveUnreachableBlocks = true;
}
}
- return null;
- }
- private static class FilledArrayCandidate {
+ for (BasicBlock block : relevantBlocks) {
+ boolean hasCatchHandlers = block.hasCatchHandlers();
+ InstructionListIterator it = block.listIterator(code);
+ while (it.hasNext()) {
+ Instruction possiblyNewArray = instructionsToChange.get(it.next());
+ if (possiblyNewArray != null) {
+ if (possiblyNewArray.isNewArrayFilled()) {
+ // Change the last array-put to the new-array-filled.
+ it.replaceCurrentInstruction(possiblyNewArray);
+ } else {
+ it.removeOrReplaceByDebugLocalRead();
+ if (hasCatchHandlers) {
+ // Removing these catch handlers shrinks their ranges to be only that where the
+ // allocation occurs.
+ needToRemoveUnreachableBlocks = true;
+ assert !block.canThrow();
+ block.removeAllExceptionalSuccessors();
+ }
+ }
+ }
+ }
+ }
- final NewArrayEmpty newArrayEmpty;
- final int size;
-
- public FilledArrayCandidate(NewArrayEmpty newArrayEmpty, int size) {
- assert size > 0;
- this.newArrayEmpty = newArrayEmpty;
- this.size = size;
+ if (needToRemoveUnreachableBlocks) {
+ code.removeUnreachableBlocks();
}
- }
-
- private FilledArrayCandidate computeFilledArrayCandidate(
- Instruction instruction, RewriteArrayOptions options) {
- NewArrayEmpty newArrayEmpty = instruction.asNewArrayEmpty();
- if (newArrayEmpty == null) {
- return null;
- }
- if (instruction.getLocalInfo() != null) {
- return null;
- }
- if (!newArrayEmpty.size().isConstant()) {
- return null;
- }
- int size = newArrayEmpty.size().getConstInstruction().asConstNumber().getIntValue();
- if (!options.isPotentialSize(size)) {
- return null;
- }
- return new FilledArrayCandidate(newArrayEmpty, size);
+ code.removeRedundantBlocks();
}
}
diff --git a/src/main/java/com/android/tools/r8/utils/DominatorChecker.java b/src/main/java/com/android/tools/r8/utils/DominatorChecker.java
index c2fd047..d1dc853 100644
--- a/src/main/java/com/android/tools/r8/utils/DominatorChecker.java
+++ b/src/main/java/com/android/tools/r8/utils/DominatorChecker.java
@@ -5,7 +5,6 @@
import com.android.tools.r8.ir.code.BasicBlock;
import com.google.common.collect.Sets;
-import java.util.ArrayDeque;
import java.util.Collections;
import java.util.Set;
@@ -32,20 +31,20 @@
}
class TraversingDominatorChecker implements DominatorChecker {
- private final BasicBlock sourceBlock;
- private final BasicBlock destBlock;
+ private final BasicBlock subgraphEntryBlock;
+ private final BasicBlock subgraphExitBlock;
private final Set<BasicBlock> knownDominators;
- private final ArrayDeque<BasicBlock> workQueue = new ArrayDeque<>();
- private final Set<BasicBlock> visited;
+ private final WorkList<BasicBlock> workList = WorkList.newIdentityWorkList();
private BasicBlock prevTargetBlock;
private TraversingDominatorChecker(
- BasicBlock sourceBlock, BasicBlock destBlock, Set<BasicBlock> knownDominators) {
- this.sourceBlock = sourceBlock;
- this.destBlock = destBlock;
+ BasicBlock subgraphEntryBlock,
+ BasicBlock subgraphExitBlock,
+ Set<BasicBlock> knownDominators) {
+ this.subgraphEntryBlock = subgraphEntryBlock;
+ this.subgraphExitBlock = subgraphExitBlock;
this.knownDominators = knownDominators;
- this.visited = Sets.newIdentityHashSet();
- prevTargetBlock = destBlock;
+ prevTargetBlock = subgraphExitBlock;
}
@Override
@@ -60,7 +59,7 @@
if (firstSplittingBlock.hasUniqueSuccessor()) {
do {
// knownDominators prevents firstSplittingBlock from being destBlock.
- assert firstSplittingBlock != destBlock;
+ assert firstSplittingBlock != subgraphExitBlock;
firstSplittingBlock = firstSplittingBlock.getUniqueSuccessor();
} while (firstSplittingBlock.hasUniqueSuccessor());
@@ -73,13 +72,12 @@
boolean ret;
// Since we know the previously checked block is a dominator, narrow the check by using it for
// either sourceBlock or destBlock.
- if (visited.contains(targetBlock)) {
- visited.clear();
- ret =
- checkWithTraversal(prevTargetBlock, destBlock, firstSplittingBlock, visited, workQueue);
+ if (workList.isSeen(targetBlock)) {
+ workList.clearSeen();
+ ret = checkWithTraversal(prevTargetBlock, subgraphExitBlock, firstSplittingBlock, workList);
prevTargetBlock = firstSplittingBlock;
} else {
- ret = checkWithTraversal(sourceBlock, prevTargetBlock, targetBlock, visited, workQueue);
+ ret = checkWithTraversal(subgraphEntryBlock, prevTargetBlock, targetBlock, workList);
prevTargetBlock = targetBlock;
}
if (ret) {
@@ -93,70 +91,71 @@
return ret;
}
+ /**
+ * Within the subgraph defined by the given entry/exit blocks, returns whether targetBlock
+ * dominates the exit block.
+ */
private static boolean checkWithTraversal(
- BasicBlock sourceBlock,
- BasicBlock destBlock,
+ BasicBlock subgraphEntryBlock,
+ BasicBlock subgraphExitBlock,
BasicBlock targetBlock,
- Set<BasicBlock> visited,
- ArrayDeque<BasicBlock> workQueue) {
- assert workQueue.isEmpty();
+ WorkList<BasicBlock> workList) {
+ assert workList.isEmpty();
- visited.add(targetBlock);
-
- workQueue.addAll(destBlock.getPredecessors());
- do {
- BasicBlock curBlock = workQueue.removeLast();
- if (!visited.add(curBlock)) {
- continue;
- }
- if (curBlock == sourceBlock) {
- // There is a path from sourceBlock -> destBlock that does not go through block.
+ workList.markAsSeen(targetBlock);
+ workList.addIfNotSeen(subgraphExitBlock.getPredecessors());
+ while (!workList.isEmpty()) {
+ BasicBlock curBlock = workList.removeLast();
+ if (curBlock == subgraphEntryBlock) {
+ // There is a path from subgraphExitBlock -> subgraphEntryBlock that does not go through
+ // targetBlock.
return false;
}
- assert !curBlock.isEntry() : "sourceBlock did not dominate destBlock";
- workQueue.addAll(curBlock.getPredecessors());
- } while (!workQueue.isEmpty());
+ assert !curBlock.isEntry() : "subgraphEntryBlock did not dominate subgraphExitBlock";
+ workList.addIfNotSeen(curBlock.getPredecessors());
+ }
return true;
}
}
- static DominatorChecker create(BasicBlock sourceBlock, BasicBlock destBlock) {
+ static DominatorChecker create(BasicBlock subgraphEntryBlock, BasicBlock subgraphExitBlock) {
// Fast-path: blocks are the same.
// As of Nov 2023: in Chrome for String.format() optimization, this covers 77% of cases.
- if (sourceBlock == destBlock) {
- return new PrecomputedDominatorChecker(Collections.singleton(sourceBlock));
+ if (subgraphEntryBlock == subgraphExitBlock) {
+ return new PrecomputedDominatorChecker(Collections.singleton(subgraphEntryBlock));
}
- // Shrink the subgraph by moving sourceBlock forward to the first block with multiple
+ // Shrink the subgraph by moving subgraphEntryBlock forward to the first block with multiple
// successors.
Set<BasicBlock> headAndTailDominators = Sets.newIdentityHashSet();
- headAndTailDominators.add(sourceBlock);
- while (sourceBlock.hasUniqueSuccessor()) {
- sourceBlock = sourceBlock.getUniqueSuccessor();
- if (!headAndTailDominators.add(sourceBlock)) {
+ headAndTailDominators.add(subgraphEntryBlock);
+ while (subgraphEntryBlock.hasUniqueSuccessor()) {
+ subgraphEntryBlock = subgraphEntryBlock.getUniqueSuccessor();
+ if (!headAndTailDominators.add(subgraphEntryBlock)) {
// Hit an infinite loop. Code would not verify in this case.
assert false;
return FALSE_CHECKER;
}
- if (sourceBlock == destBlock) {
+ if (subgraphEntryBlock == subgraphExitBlock) {
// As of Nov 2023: in Chrome for String.format() optimization, a linear path from
// source->dest was 14% of cases.
return new PrecomputedDominatorChecker(headAndTailDominators);
}
}
- if (sourceBlock.getSuccessors().isEmpty()) {
+ if (subgraphEntryBlock.getSuccessors().isEmpty()) {
return FALSE_CHECKER;
}
- // Shrink the subgraph by moving destBlock back to the first block with multiple predecessors.
- headAndTailDominators.add(destBlock);
- while (destBlock.hasUniquePredecessor()) {
- destBlock = destBlock.getUniquePredecessor();
- if (!headAndTailDominators.add(destBlock)) {
- if (sourceBlock == destBlock) {
- // This normally happens when moving sourceBlock forwards, but when moving destBlock
- // backwards when sourceBlock has multiple successors.
+ // Shrink the subgraph by moving subgraphExitBlock back to the first block with multiple
+ // predecessors.
+ headAndTailDominators.add(subgraphExitBlock);
+ while (subgraphExitBlock.hasUniquePredecessor()) {
+ subgraphExitBlock = subgraphExitBlock.getUniquePredecessor();
+ if (!headAndTailDominators.add(subgraphExitBlock)) {
+ if (subgraphEntryBlock == subgraphExitBlock) {
+ // This normally happens when moving subgraphEntryBlock forwards, but can also occur when
+ // moving subgraphExitBlock backwards when subgraphEntryBlock has multiple successors.
return new PrecomputedDominatorChecker(headAndTailDominators);
}
// Hit an infinite loop. Code would not verify in this case.
@@ -165,10 +164,28 @@
}
}
- if (destBlock.isEntry()) {
+ if (subgraphExitBlock.isEntry()) {
return FALSE_CHECKER;
}
- return new TraversingDominatorChecker(sourceBlock, destBlock, headAndTailDominators);
+ return new TraversingDominatorChecker(
+ subgraphEntryBlock, subgraphExitBlock, headAndTailDominators);
+ }
+
+ /**
+ * Returns whether targetBlock dominates subgraphExitBlock by performing a depth-first traversal
+ * from subgraphExitBlock to subgraphEntryBlock with targetBlock removed from the graph.
+ */
+ @SuppressWarnings("InconsistentOverloads")
+ static boolean check(
+ BasicBlock subgraphEntryBlock, BasicBlock subgraphExitBlock, BasicBlock targetBlock) {
+ if (targetBlock == subgraphExitBlock) {
+ return true;
+ }
+ if (subgraphEntryBlock == subgraphExitBlock) {
+ return false;
+ }
+ return TraversingDominatorChecker.checkWithTraversal(
+ subgraphEntryBlock, subgraphExitBlock, targetBlock, WorkList.newIdentityWorkList());
}
}
diff --git a/src/main/java/com/android/tools/r8/utils/ValueUtils.java b/src/main/java/com/android/tools/r8/utils/ValueUtils.java
index 68d3364..9082be5 100644
--- a/src/main/java/com/android/tools/r8/utils/ValueUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/ValueUtils.java
@@ -57,12 +57,15 @@
public static final class ArrayValues {
private List<Value> elementValues;
private ArrayPut[] arrayPutsByIndex;
+ private final Value arrayValue;
- private ArrayValues(List<Value> elementValues) {
+ private ArrayValues(Value arrayValue, List<Value> elementValues) {
+ this.arrayValue = arrayValue;
this.elementValues = elementValues;
}
- private ArrayValues(ArrayPut[] arrayPutsByIndex) {
+ private ArrayValues(Value arrayValue, ArrayPut[] arrayPutsByIndex) {
+ this.arrayValue = arrayValue;
this.arrayPutsByIndex = arrayPutsByIndex;
}
@@ -83,6 +86,28 @@
public int size() {
return elementValues != null ? elementValues.size() : arrayPutsByIndex.length;
}
+
+ public boolean containsHoles() {
+ for (ArrayPut arrayPut : arrayPutsByIndex) {
+ if (arrayPut == null) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ public ArrayPut[] getArrayPutsByIndex() {
+ assert arrayPutsByIndex != null;
+ return arrayPutsByIndex;
+ }
+
+ public Value getArrayValue() {
+ return arrayValue;
+ }
+
+ public Instruction getDefinition() {
+ return arrayValue.definition;
+ }
}
/**
@@ -92,12 +117,8 @@
* 1) The Array has a single users (other than array-puts)
* * This constraint is to ensure other users do not modify the array.
* * When users are in different blocks, their order is hard to know.
- * 2) The array size is a constant.
+ * 2) The array size is a constant and non-negative.
* 3) All array-put instructions have constant and unique indices.
- * * With the exception of array-puts that are in the same block as singleUser, in which case
- * non-unique puts are allowed.
- * * Indices must be unique in other blocks because order is hard to know when multiple blocks
- * are concerned (and reassignment is rare anyways).
* 4) The array-put instructions are guaranteed to be executed before singleUser.
* </pre>
*
@@ -106,7 +127,6 @@
* @return The computed array values, or null if they could not be determined.
*/
public static ArrayValues computeSingleUseArrayValues(Value arrayValue, Instruction singleUser) {
- assert singleUser == null || arrayValue.uniqueUsers().contains(singleUser);
TypeElement arrayType = arrayValue.getType();
if (!arrayType.isArrayType() || arrayValue.hasDebugUsers() || arrayValue.isPhi()) {
return null;
@@ -121,7 +141,7 @@
if (!arrayValue.hasSingleUniqueUser() || arrayValue.hasPhiUsers()) {
return null;
}
- return new ArrayValues(newArrayFilled.inValues());
+ return new ArrayValues(arrayValue, newArrayFilled.inValues());
} else if (newArrayEmpty == null) {
return null;
}
@@ -132,28 +152,83 @@
return null;
}
- if (singleUser == null) {
- for (Instruction user : arrayValue.uniqueUsers()) {
- ArrayPut arrayPut = user.asArrayPut();
- if (arrayPut == null || arrayPut.array() != arrayValue || arrayPut.value() == arrayValue) {
- if (singleUser == null) {
- singleUser = user;
- } else {
- return null;
- }
+ return computeArrayValuesInternal(newArrayEmpty, arraySize, singleUser, false);
+ }
+
+ /**
+ * Determines the values for the given array at the point the last element is assigned.
+ *
+ * <pre>
+ * Returns null under the following conditions:
+ * * The array has a non-const, negative, or abnormally large size.
+ * * An array-put with non-constant index exists.
+ * * An array-put with an out-of-bounds index exists.
+ * * An array-put for the last index is not found.
+ * * An array-put is found after the last-index array-put.
+ * * An array-put is found where the array and value are the same: arr[index] = arr;
+ * * There are multiple array-put instructions for the same index.
+ * </pre>
+ */
+ public static ArrayValues computeInitialArrayValues(NewArrayEmpty newArrayEmpty) {
+ int arraySize = newArrayEmpty.sizeIfConst();
+ if (arraySize < 0 || arraySize > MAX_ARRAY_SIZE) {
+ // Array is non-const size.
+ return null;
+ }
+
+ Value arrayValue = newArrayEmpty.outValue();
+ // Find array-put for the last element, as well as blocks for other array users.
+ ArrayPut lastArrayPut = null;
+ int lastIndex = arraySize - 1;
+ for (Instruction user : arrayValue.uniqueUsers()) {
+ ArrayPut arrayPut = user.asArrayPut();
+ if (arrayPut != null && arrayPut.array() == arrayValue) {
+ int index = arrayPut.indexOrDefault(-1);
+ if (index == lastIndex) {
+ lastArrayPut = arrayPut;
+ break;
}
}
}
+ if (lastArrayPut == null) {
+ return null;
+ }
+ // Find all array-puts up until the last one.
+ // Also checks that no array-puts appear after the last one.
+ ArrayValues ret = computeArrayValuesInternal(newArrayEmpty, arraySize, lastArrayPut, true);
+ if (ret == null) {
+ return null;
+ }
+ // Since the last array-put is used as firstUser, it will not already be in arrayPutsByIndex.
+ if (ret.arrayPutsByIndex[lastIndex] != null) {
+ return null;
+ }
+ ret.arrayPutsByIndex[lastIndex] = lastArrayPut;
+ return ret;
+ }
+
+ private static ArrayValues computeArrayValuesInternal(
+ NewArrayEmpty newArrayEmpty, int arraySize, Instruction firstUser, boolean allowOtherUsers) {
ArrayPut[] arrayPutsByIndex = new ArrayPut[arraySize];
- BasicBlock usageBlock = singleUser.getBlock();
+ Value arrayValue = newArrayEmpty.outValue();
+ BasicBlock usageBlock = firstUser.getBlock();
+
+ // Collect array-puts from non-usage blocks, and (optionally) check for multiple users.
for (Instruction user : arrayValue.uniqueUsers()) {
ArrayPut arrayPut = user.asArrayPut();
- if (arrayPut == null || arrayPut.array() != arrayValue || arrayPut.value() == arrayValue) {
- if (user == singleUser) {
+ if (arrayPut == null || arrayPut.array() != arrayValue) {
+ if (user == firstUser) {
continue;
}
// Found a second non-array-put user.
+ if (allowOtherUsers) {
+ continue;
+ }
+ return null;
+ } else if (arrayPut.value() == arrayValue) {
+ // An array that contains itself is uncommon and hard to reason about.
+ // e.g.: arr[0] = arr;
return null;
}
// Process same-block instructions later.
@@ -168,8 +243,10 @@
arrayPutsByIndex[index] = arrayPut;
}
- // Ensure that all paths from new-array-empty to |usage| contain all array-put instructions.
- DominatorChecker dominatorChecker = DominatorChecker.create(definition.getBlock(), usageBlock);
+ // Ensure that all paths from new-array-empty's block to |usage|'s block contain all array-put
+ // instructions.
+ DominatorChecker dominatorChecker =
+ DominatorChecker.create(newArrayEmpty.getBlock(), usageBlock);
// Visit in reverse order because array-puts generally appear in order, and DominatorChecker's
// cache is more effective when visiting in reverse order.
for (int i = arraySize - 1; i >= 0; --i) {
@@ -179,17 +256,18 @@
}
}
- boolean seenSingleUser = false;
+ // Collect array-puts from the usage block, and ensure no array-puts come after the first user.
+ boolean seenFirstUser = false;
for (Instruction inst : usageBlock.getInstructions()) {
- if (inst == singleUser) {
- seenSingleUser = true;
+ if (inst == firstUser) {
+ seenFirstUser = true;
continue;
}
ArrayPut arrayPut = inst.asArrayPut();
if (arrayPut == null || arrayPut.array() != arrayValue) {
continue;
}
- if (seenSingleUser) {
+ if (seenFirstUser) {
// Found an array-put after the array was used. This is too uncommon of a thing to support.
return null;
}
@@ -197,10 +275,14 @@
if (index < 0) {
return null;
}
- // We can allow reassignment at this point since we are visiting in order.
+ // Do not allow re-assignment so that we can use arrayPutsByIndex to find all array-put
+ // instructions.
+ if (arrayPutsByIndex[index] != null) {
+ return null;
+ }
arrayPutsByIndex[index] = arrayPut;
}
- return new ArrayValues(arrayPutsByIndex);
+ return new ArrayValues(arrayValue, arrayPutsByIndex);
}
}
diff --git a/src/main/java/com/android/tools/r8/utils/WorkList.java b/src/main/java/com/android/tools/r8/utils/WorkList.java
index cdfb34c..ed63185 100644
--- a/src/main/java/com/android/tools/r8/utils/WorkList.java
+++ b/src/main/java/com/android/tools/r8/utils/WorkList.java
@@ -145,8 +145,8 @@
return seen.contains(item);
}
- public void markAsSeen(T item) {
- seen.add(item);
+ public boolean markAsSeen(T item) {
+ return seen.add(item);
}
public void markAsSeen(Iterable<T> items) {
@@ -154,16 +154,23 @@
}
public T next() {
- assert hasNext();
return workingList.removeFirst();
}
+ public T removeLast() {
+ return workingList.removeLast();
+ }
+
public T removeSeen() {
T next = next();
seen.remove(next);
return next;
}
+ public void clearSeen() {
+ seen.clear();
+ }
+
public Set<T> getSeenSet() {
return SetUtils.unmodifiableForTesting(seen);
}
diff --git a/src/test/java/com/android/tools/r8/examples/filledarray/FilledArray.java b/src/test/java/com/android/tools/r8/examples/filledarray/FilledArray.java
index f25b76b..6551be6 100644
--- a/src/test/java/com/android/tools/r8/examples/filledarray/FilledArray.java
+++ b/src/test/java/com/android/tools/r8/examples/filledarray/FilledArray.java
@@ -115,8 +115,7 @@
}
try {
- // Array creation that cannot be turned into fill-array-data because an exception would
- // cause the initialization sequence to be interrupted.
+ // Exception does not prevent fill-array-data since only usage is within the try.
int[] ints = new int[5];
ints[0] = 0;
ints[1] = 1;
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/ArrayOfArraysTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/ArrayOfArraysTest.java
index 82ae38f..26a4674 100644
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/ArrayOfArraysTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/ArrayOfArraysTest.java
@@ -63,13 +63,16 @@
}
private void inspect(CodeInspector inspector) {
+ int canUseStrings = canUseFilledNewArrayOfStringObjects(parameters) ? 1 : 0;
+ int canUseObjects = canUseFilledNewArrayOfNonStringObjects(parameters) ? 1 : 0;
inspect(
inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"),
- 2
- + (canUseFilledNewArrayOfStringObjects(parameters) ? 2 : 0)
- + (canUseFilledNewArrayOfNonStringObjects(parameters) ? 2 : 0),
+ 2 + 2 * canUseStrings + 2 * canUseObjects,
2);
- inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"), 2, 2);
+ inspect(
+ inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"),
+ 2 + 2 * canUseStrings + 1 * canUseObjects,
+ compilationMode.isDebug() ? 10 : 11);
}
@Test
@@ -129,32 +132,32 @@
@NeverInline
public static void m2() {
+ Object[] array = null;
try {
- Object[] array = {
- new byte[] {(byte) 1},
- new short[] {(short) 1},
- new int[] {1},
- new long[] {1L},
- new char[] {(char) 1},
- new float[] {1.0f},
- new double[] {1.0d},
- new String[] {"one"},
- new Object[] {
- new byte[] {(byte) 2},
- new short[] {(short) 2},
- new int[] {2},
- new long[] {2L},
- new char[] {(char) 2},
- new float[] {2.0f},
- new double[] {2.0d},
- new String[] {"two"},
- }
- };
- printArray(array);
- System.out.println();
+ array = new Object[9];
+ array[0] = new byte[] {(byte) 1};
+ array[1] = new short[] {(short) 1};
+ array[2] = new int[] {1};
+ array[3] = new long[] {1L};
+ array[4] = new char[] {(char) 1};
+ array[5] = new float[] {1.0f};
+ array[6] = new double[] {1.0d};
+ array[7] = new String[] {"one"};
+ array[8] =
+ new Object[] {
+ new byte[] {(byte) 2},
+ new short[] {(short) 2},
+ new int[] {2},
+ new long[] {2L},
+ new char[] {(char) 2},
+ new float[] {2.0f},
+ new double[] {2.0d},
+ new String[] {"two"},
+ };
} catch (Exception e) {
- throw new RuntimeException();
}
+ printArray(array);
+ System.out.println();
}
@NeverInline
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/ArrayOfConstClassArraysTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/ArrayOfConstClassArraysTest.java
index 63fec32..b37c8a2 100644
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/ArrayOfConstClassArraysTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/ArrayOfConstClassArraysTest.java
@@ -62,10 +62,7 @@
private void inspect(CodeInspector inspector) {
inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), 5, 1);
- inspect(
- inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"),
- 0,
- compilationMode.isDebug() ? 1 : 2);
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"), 5, 4);
}
@Test
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/ArrayOfIntArraysTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/ArrayOfIntArraysTest.java
index edc54de..c429641 100644
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/ArrayOfIntArraysTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/ArrayOfIntArraysTest.java
@@ -59,15 +59,15 @@
private void inspect(CodeInspector inspector) {
// This test use smaller int arrays, where filled-new-array is preferred over filled-array-data.
+ int canUseObjects = canUseFilledNewArrayOfNonStringObjects(parameters) ? 1 : 0;
inspect(
inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"),
- 4 + (canUseFilledNewArrayOfNonStringObjects(parameters) ? 1 : 0),
+ 4 + 1 * canUseObjects,
1);
- // With catch handler the int[][] creation is not converted to filled-new-array.
inspect(
inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"),
- 4,
- compilationMode.isDebug() ? 1 : 2);
+ 4 + 1 * canUseObjects,
+ 4);
}
@Test
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/ArrayOfStringArraysTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/ArrayOfStringArraysTest.java
index 1934f24..d238b1a 100644
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/ArrayOfStringArraysTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/ArrayOfStringArraysTest.java
@@ -63,28 +63,27 @@
}
private void inspect(CodeInspector inspector) {
+ int canUseStrings = canUseFilledNewArrayOfStringObjects(parameters) ? 1 : 0;
+ int canUseObjects = canUseFilledNewArrayOfNonStringObjects(parameters) ? 1 : 0;
inspect(
inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"),
- (canUseFilledNewArrayOfStringObjects(parameters) ? 4 : 0)
- + (canUseFilledNewArrayOfNonStringObjects(parameters) ? 1 : 0),
+ 4 * canUseStrings + 1 * canUseObjects,
1);
inspect(
inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"),
- 0,
- compilationMode.isDebug() ? 2 : 1);
+ 4 * canUseStrings + 1 * canUseObjects,
+ 4);
inspect(
inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m3"),
- (canUseFilledNewArrayOfStringObjects(parameters) ? 4 : 0)
- + (canUseFilledNewArrayOfNonStringObjects(parameters) ? 5 : 0),
+ 4 * canUseStrings + 5 * canUseObjects,
5);
inspect(
inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m4"),
- 0,
- compilationMode.isDebug() ? 2 : 1);
+ 4 * canUseStrings + 5 * canUseObjects,
+ 5);
inspect(
inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m5"),
- (canUseFilledNewArrayOfStringObjects(parameters) ? 4 : 0)
- + (canUseFilledNewArrayOfNonStringObjects(parameters) ? 1 : 0),
+ 4 * canUseStrings + 1 * canUseObjects,
compilationMode.isDebug() ? 6 : 4);
}
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/ConstClassArrayTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/ConstClassArrayTest.java
index 4b6e64c..c27a171 100644
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/ConstClassArrayTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/ConstClassArrayTest.java
@@ -10,7 +10,6 @@
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.InstructionSubject;
@@ -46,9 +45,8 @@
EXPECTING_APUTOBJECT
}
- private void inspect(MethodSubject method, boolean insideCatchHandler) {
- boolean expectingFilledNewArray =
- canUseFilledNewArrayOfNonStringObjects(parameters) && !insideCatchHandler;
+ private void inspect(MethodSubject method) {
+ boolean expectingFilledNewArray = canUseFilledNewArrayOfNonStringObjects(parameters);
assertEquals(
expectingFilledNewArray ? 0 : 5,
method.streamInstructions().filter(InstructionSubject::isArrayPut).count());
@@ -78,8 +76,8 @@
}
private void inspect(CodeInspector inspector) {
- inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), false);
- inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"), true);
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"));
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"));
}
@Test
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/ConstClassArrayWithNonUniqueValuesTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/ConstClassArrayWithNonUniqueValuesTest.java
index 3a364dd..7d8bba7 100644
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/ConstClassArrayWithNonUniqueValuesTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/ConstClassArrayWithNonUniqueValuesTest.java
@@ -46,10 +46,8 @@
private static final String EXPECTED_OUTPUT = StringUtils.lines("100", "104");
- private void inspect(
- MethodSubject method, int constClasses, int puts, boolean insideCatchHandler) {
- boolean expectingFilledNewArray =
- canUseFilledNewArrayOfNonStringObjects(parameters) && !insideCatchHandler;
+ private void inspect(MethodSubject method, int constClasses, int puts) {
+ boolean expectingFilledNewArray = canUseFilledNewArrayOfNonStringObjects(parameters);
assertEquals(
expectingFilledNewArray ? 0 : puts,
method.streamInstructions().filter(InstructionSubject::isArrayPut).count());
@@ -68,12 +66,11 @@
}
private void inspectD8(CodeInspector inspector) {
- inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), 1, 100, false);
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), 1, 100);
inspect(
inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"),
maxMaterializingConstants == 2 ? 98 : 26,
- 104,
- false);
+ 104);
}
@Test
@@ -89,12 +86,11 @@
}
private void inspectR8(CodeInspector inspector) {
- inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), 1, 100, false);
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), 1, 100);
inspect(
inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"),
maxMaterializingConstants == 2 ? 32 : 26,
- 104,
- false);
+ 104);
}
@Test
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/ConstClassArrayWithUniqueValuesTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/ConstClassArrayWithUniqueValuesTest.java
index c7f847b..3c7976f 100644
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/ConstClassArrayWithUniqueValuesTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/ConstClassArrayWithUniqueValuesTest.java
@@ -52,9 +52,8 @@
EXPECTING_APUTOBJECT
}
- private void inspect(MethodSubject method, int puts, boolean insideCatchHandler) {
- boolean expectingFilledNewArray =
- canUseFilledNewArrayOfNonStringObjects(parameters) && !insideCatchHandler;
+ private void inspect(MethodSubject method, int puts) {
+ boolean expectingFilledNewArray = canUseFilledNewArrayOfNonStringObjects(parameters);
assertEquals(
expectingFilledNewArray ? 0 : puts,
method.streamInstructions().filter(InstructionSubject::isArrayPut).count());
@@ -86,9 +85,9 @@
}
private void inspect(CodeInspector inspector) {
- inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), 5, false);
- inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"), 5, true);
- inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m3"), 100, false);
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), 5);
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"), 5);
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m3"), 100);
}
@Test
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/FilledArrayDataWithCatchHandlerTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/FilledArrayDataWithCatchHandlerTest.java
deleted file mode 100644
index e28a4fc..0000000
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/FilledArrayDataWithCatchHandlerTest.java
+++ /dev/null
@@ -1,106 +0,0 @@
-// Copyright (c) 2022, 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.rewrite.arrays;
-
-import static com.android.tools.r8.cf.methodhandles.fields.ClassFieldMethodHandleTest.Main.assertEquals;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assume.assumeTrue;
-
-import com.android.tools.r8.TestBase;
-import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.TestParametersCollection;
-import com.android.tools.r8.dex.code.DexFillArrayData;
-import com.android.tools.r8.dex.code.DexNewArray;
-import com.android.tools.r8.utils.StringUtils;
-import com.android.tools.r8.utils.codeinspector.CodeInspector;
-import com.android.tools.r8.utils.codeinspector.InstructionOffsetSubject;
-import com.android.tools.r8.utils.codeinspector.InstructionSubject;
-import com.android.tools.r8.utils.codeinspector.MethodSubject;
-import java.util.List;
-import java.util.stream.Collectors;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-
-@RunWith(Parameterized.class)
-public class FilledArrayDataWithCatchHandlerTest extends TestBase {
-
- static final String EXPECTED = StringUtils.lines("1");
-
- private final TestParameters parameters;
-
- @Parameterized.Parameters(name = "{0}")
- public static TestParametersCollection data() {
- return getTestParameters().withAllRuntimes().withAllApiLevels().build();
- }
-
- public FilledArrayDataWithCatchHandlerTest(TestParameters parameters) {
- this.parameters = parameters;
- }
-
- @Test
- public void test() throws Exception {
- testForRuntime(parameters)
- .addInnerClasses(FilledArrayDataWithCatchHandlerTest.class)
- .run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutput(EXPECTED);
- }
-
- @Test
- public void testReleaseD8() throws Exception {
- assumeTrue(parameters.isDexRuntime());
- testForD8(parameters.getBackend())
- .release()
- .setMinApi(parameters)
- .addInnerClasses(FilledArrayDataWithCatchHandlerTest.class)
- .run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutput(EXPECTED)
- .inspect(this::checkInstructions);
- }
-
- private void checkInstructions(CodeInspector inspector) {
- MethodSubject foo = inspector.clazz(TestClass.class).uniqueMethodWithFinalName("foo");
- List<InstructionSubject> newArrays =
- foo.streamInstructions()
- .filter(i -> i.asDexInstruction().getInstruction() instanceof DexNewArray)
- .collect(Collectors.toList());
- assertEquals(1, newArrays.size());
-
- List<InstructionSubject> fillArrays =
- foo.streamInstructions()
- .filter(i -> i.asDexInstruction().getInstruction() instanceof DexFillArrayData)
- .collect(Collectors.toList());
- assertEquals(1, fillArrays.size());
-
- InstructionOffsetSubject offsetNew = newArrays.get(0).getOffset(foo);
- InstructionOffsetSubject offsetFill = newArrays.get(0).getOffset(foo);
- assertTrue(
- foo.streamTryCatches()
- .allMatch(r -> r.getRange().includes(offsetNew) && r.getRange().includes(offsetFill)));
- }
-
- static class TestClass {
-
- public static int foo() {
- int value = 1;
- int[] array = null;
- try {
- array = new int[6];
- } catch (RuntimeException e) {
- return array[0];
- }
- array[0] = value;
- array[1] = value;
- array[2] = value;
- array[3] = value;
- array[4] = value;
- array[5] = value;
- return array[5];
- }
-
- public static void main(String[] args) {
- System.out.println(foo());
- }
- }
-}
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/NewArrayInCatchRangeTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/NewArrayInCatchRangeTest.java
deleted file mode 100644
index f9e3ffe..0000000
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/NewArrayInCatchRangeTest.java
+++ /dev/null
@@ -1,90 +0,0 @@
-// Copyright (c) 2022, 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.rewrite.arrays;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assume.assumeTrue;
-
-import com.android.tools.r8.TestBase;
-import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.TestParametersCollection;
-import com.android.tools.r8.dex.code.DexFilledNewArray;
-import com.android.tools.r8.utils.StringUtils;
-import com.android.tools.r8.utils.codeinspector.CodeInspector;
-import com.android.tools.r8.utils.codeinspector.InstructionOffsetSubject;
-import com.android.tools.r8.utils.codeinspector.InstructionSubject;
-import com.android.tools.r8.utils.codeinspector.MethodSubject;
-import java.util.List;
-import java.util.stream.Collectors;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-
-@RunWith(Parameterized.class)
-public class NewArrayInCatchRangeTest extends TestBase {
-
- static final String EXPECTED = StringUtils.lines("1");
-
- private final TestParameters parameters;
-
- @Parameterized.Parameters(name = "{0}")
- public static TestParametersCollection data() {
- return getTestParameters().withAllRuntimes().withAllApiLevels().build();
- }
-
- public NewArrayInCatchRangeTest(TestParameters parameters) {
- this.parameters = parameters;
- }
-
- @Test
- public void test() throws Exception {
- testForRuntime(parameters)
- .addInnerClasses(NewArrayInCatchRangeTest.class)
- .run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutput(EXPECTED);
- }
-
- @Test
- public void testReleaseD8() throws Exception {
- assumeTrue(parameters.isDexRuntime());
- testForD8(parameters.getBackend())
- .release()
- .setMinApi(parameters)
- .addInnerClasses(NewArrayInCatchRangeTest.class)
- .run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutput(EXPECTED)
- .inspect(this::checkInstructions);
- }
-
- private void checkInstructions(CodeInspector inspector) {
- MethodSubject foo = inspector.clazz(TestClass.class).uniqueMethodWithFinalName("foo");
- List<InstructionSubject> filledArrayInstructions =
- foo.streamInstructions()
- .filter(i -> i.asDexInstruction().getInstruction() instanceof DexFilledNewArray)
- .collect(Collectors.toList());
- assertEquals(1, filledArrayInstructions.size());
- InstructionOffsetSubject offset = filledArrayInstructions.get(0).getOffset(foo);
- assertTrue(foo.streamTryCatches().allMatch(r -> r.getRange().includes(offset)));
- }
-
- static class TestClass {
-
- public static int foo() {
- int value = 1;
- int[] array = null;
- try {
- array = new int[1];
- } catch (Exception e) {
- return array == null ? -1 : array.length;
- }
- array[0] = value;
- return array[0];
- }
-
- public static void main(String[] args) {
- System.out.println(foo());
- }
- }
-}
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/NewArrayInTwoCatchRangesTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/NewArrayInTwoCatchRangesTest.java
deleted file mode 100644
index 3ecd9de..0000000
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/NewArrayInTwoCatchRangesTest.java
+++ /dev/null
@@ -1,89 +0,0 @@
-// Copyright (c) 2022, 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.rewrite.arrays;
-
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assume.assumeTrue;
-
-import com.android.tools.r8.TestBase;
-import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.TestParametersCollection;
-import com.android.tools.r8.dex.code.DexFilledNewArray;
-import com.android.tools.r8.utils.StringUtils;
-import com.android.tools.r8.utils.codeinspector.CodeInspector;
-import com.android.tools.r8.utils.codeinspector.MethodSubject;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-
-@RunWith(Parameterized.class)
-public class NewArrayInTwoCatchRangesTest extends TestBase {
-
- static final String EXPECTED = StringUtils.lines("1");
-
- private final TestParameters parameters;
-
- @Parameterized.Parameters(name = "{0}")
- public static TestParametersCollection data() {
- return getTestParameters().withAllRuntimes().withAllApiLevels().build();
- }
-
- public NewArrayInTwoCatchRangesTest(TestParameters parameters) {
- this.parameters = parameters;
- }
-
- @Test
- public void test() throws Exception {
- testForRuntime(parameters)
- .addInnerClasses(NewArrayInTwoCatchRangesTest.class)
- .run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutput(EXPECTED);
- }
-
- @Test
- public void testReleaseD8() throws Exception {
- assumeTrue(parameters.isDexRuntime());
- testForD8(parameters.getBackend())
- .release()
- .setMinApi(parameters)
- .addInnerClasses(NewArrayInTwoCatchRangesTest.class)
- .run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutput(EXPECTED)
- .inspect(this::checkHasFilledNewArray);
- }
-
- private void checkHasFilledNewArray(CodeInspector inspector) {
- MethodSubject foo = inspector.clazz(TestClass.class).uniqueMethodWithFinalName("foo");
- assertTrue(
- foo.streamInstructions()
- .anyMatch(i -> i.asDexInstruction().getInstruction() instanceof DexFilledNewArray));
- }
-
- static class TestClass {
-
- public static int foo() {
- int value = 1;
- try {
- int[] array = new int[2];
- try {
- array[0] = value;
- try {
- array[1] = value;
- } catch (RuntimeException e) {
- return array[1];
- }
- } catch (RuntimeException e) {
- return array[0];
- }
- return array[0];
- } catch (RuntimeException e) {
- return 42;
- }
- }
-
- public static void main(String[] args) {
- System.out.println(foo());
- }
- }
-}
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/NewArrayMonitorTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/NewArrayMonitorTest.java
deleted file mode 100644
index c7e3b2a..0000000
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/NewArrayMonitorTest.java
+++ /dev/null
@@ -1,89 +0,0 @@
-// Copyright (c) 2022, 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.rewrite.arrays;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assume.assumeTrue;
-
-import com.android.tools.r8.TestBase;
-import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.TestParametersCollection;
-import com.android.tools.r8.dex.code.DexFilledNewArray;
-import com.android.tools.r8.utils.StringUtils;
-import com.android.tools.r8.utils.codeinspector.CodeInspector;
-import com.android.tools.r8.utils.codeinspector.InstructionOffsetSubject;
-import com.android.tools.r8.utils.codeinspector.InstructionSubject;
-import com.android.tools.r8.utils.codeinspector.MethodSubject;
-import java.util.List;
-import java.util.stream.Collectors;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-
-@RunWith(Parameterized.class)
-public class NewArrayMonitorTest extends TestBase {
-
- static final String EXPECTED = StringUtils.lines("1");
-
- private final TestParameters parameters;
-
- @Parameterized.Parameters(name = "{0}")
- public static TestParametersCollection data() {
- return getTestParameters().withAllRuntimes().withAllApiLevels().build();
- }
-
- public NewArrayMonitorTest(TestParameters parameters) {
- this.parameters = parameters;
- }
-
- @Test
- public void test() throws Exception {
- testForRuntime(parameters)
- .addInnerClasses(NewArrayMonitorTest.class)
- .run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutput(EXPECTED);
- }
-
- @Test
- public void testReleaseD8() throws Exception {
- assumeTrue(parameters.isDexRuntime());
- testForD8(parameters.getBackend())
- .release()
- .setMinApi(parameters)
- .addInnerClasses(NewArrayMonitorTest.class)
- .run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutput(EXPECTED)
- .inspect(this::checkInstructions);
- }
-
- private void checkInstructions(CodeInspector inspector) {
- MethodSubject foo = inspector.clazz(TestClass.class).uniqueMethodWithFinalName("foo");
- List<InstructionSubject> filledArrayInstructions =
- foo.streamInstructions()
- .filter(i -> i.asDexInstruction().getInstruction() instanceof DexFilledNewArray)
- .collect(Collectors.toList());
- assertEquals(1, filledArrayInstructions.size());
- InstructionOffsetSubject offset = filledArrayInstructions.get(0).getOffset(foo);
- assertTrue(foo.streamTryCatches().allMatch(r -> r.getRange().includes(offset)));
- }
-
- static class TestClass {
-
- public static synchronized int foo() {
- int value = 1;
- int[] array = new int[1];
- try {
- array[0] = value;
- } catch (RuntimeException e) {
- return array[0];
- }
- return array[0];
- }
-
- public static void main(String[] args) {
- System.out.println(foo());
- }
- }
-}
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/NewArrayPutInCatchRangeTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/NewArrayPutInCatchRangeTest.java
deleted file mode 100644
index a3fd2da..0000000
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/NewArrayPutInCatchRangeTest.java
+++ /dev/null
@@ -1,85 +0,0 @@
-// Copyright (c) 2022, 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.rewrite.arrays;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assume.assumeTrue;
-
-import com.android.tools.r8.TestBase;
-import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.TestParametersCollection;
-import com.android.tools.r8.dex.code.DexFilledNewArray;
-import com.android.tools.r8.utils.StringUtils;
-import com.android.tools.r8.utils.codeinspector.CodeInspector;
-import com.android.tools.r8.utils.codeinspector.MethodSubject;
-import java.util.Collections;
-import java.util.stream.Collectors;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-
-// Regression test for issue found in b/259986613
-@RunWith(Parameterized.class)
-public class NewArrayPutInCatchRangeTest extends TestBase {
-
- static final String EXPECTED = StringUtils.lines("1");
-
- private final TestParameters parameters;
-
- @Parameterized.Parameters(name = "{0}")
- public static TestParametersCollection data() {
- return getTestParameters().withAllRuntimes().withAllApiLevels().build();
- }
-
- public NewArrayPutInCatchRangeTest(TestParameters parameters) {
- this.parameters = parameters;
- }
-
- @Test
- public void test() throws Exception {
- testForRuntime(parameters)
- .addInnerClasses(NewArrayPutInCatchRangeTest.class)
- .run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutput(EXPECTED);
- }
-
- @Test
- public void testReleaseD8() throws Exception {
- assumeTrue(parameters.isDexRuntime());
- testForD8(parameters.getBackend())
- .release()
- .setMinApi(parameters)
- .addInnerClasses(NewArrayPutInCatchRangeTest.class)
- .run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutput(EXPECTED)
- .inspect(this::checkHasFilledNewArray);
- }
-
- private void checkHasFilledNewArray(CodeInspector inspector) {
- MethodSubject foo = inspector.clazz(TestClass.class).uniqueMethodWithFinalName("foo");
- assertTrue(
- foo.streamInstructions()
- .anyMatch(i -> i.asDexInstruction().getInstruction() instanceof DexFilledNewArray));
- assertEquals(Collections.emptyList(), foo.streamTryCatches().collect(Collectors.toList()));
- }
-
- static class TestClass {
-
- public static int foo() {
- int value = 1;
- int[] array = new int[1];
- try {
- array[0] = value;
- } catch (RuntimeException e) {
- return array[0];
- }
- return array[0];
- }
-
- public static void main(String[] args) {
- System.out.println(foo());
- }
- }
-}
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/NewArraySynchronizedBlockTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/NewArraySynchronizedBlockTest.java
index c552566..5ed624e 100644
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/NewArraySynchronizedBlockTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/NewArraySynchronizedBlockTest.java
@@ -72,7 +72,7 @@
int[] array;
synchronized (TestClass.class) {
array = new int[1];
- } // monitor exit here prohibits optimization as its failure could observe the lack of init.
+ }
array[0] = value;
return array[0];
}
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java
index c80436c..7011411 100644
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java
@@ -63,7 +63,18 @@
"[1, null, 2]",
"[1, null, 2]",
"[1]",
- "[1, 2]",
+ "[1, 2, 3]",
+ "[2, 2, 2]",
+ "[6, 7]",
+ "[7]",
+ "[3, 4]",
+ "[99]",
+ "[0, 1]",
+ "[0, 1]",
+ "[0, 1]",
+ "[0, 1]",
+ "[0, 1]",
+ "[0, 1]",
"[1, 2, 3, 4, 5]",
"[1]",
"[a, 1, null, d, e, f]",
@@ -91,12 +102,16 @@
"[0, 1, 2, 3, 4]",
"[4, 0, 0, 0, 0]",
"[4, 1, 2, 3, 4]",
+ "[9]",
+ "[*]",
+ "[*]",
+ "finally: [1, 2]",
+ "[1, 2]",
+ "[1, 2]",
+ "[1, 2]",
+ "[1, 2]",
"[0, 1, 2]",
"[0]",
- "[0, 1, 2]",
- "[1, 2, 3]",
- "[1, 2, 3, 4, 5, 6]",
- "[0]",
"[null, null]",
};
@@ -176,6 +191,20 @@
mainClass.uniqueMethodWithOriginalName("interfaceArrayWithRawObject");
MethodSubject phiFilledNewArray = mainClass.uniqueMethodWithOriginalName("phiFilledNewArray");
+ MethodSubject phiFilledNewArrayBlocks =
+ mainClass.uniqueMethodWithOriginalName("phiFilledNewArrayBlocks");
+ MethodSubject arrayWithDominatingPhiUsers =
+ mainClass.uniqueMethodWithOriginalName("arrayWithDominatingPhiUsers");
+ MethodSubject arrayWithNonDominatingPhiUsers =
+ mainClass.uniqueMethodWithOriginalName("arrayWithNonDominatingPhiUsers");
+ MethodSubject phiWithExceptionalPhiUser =
+ mainClass.uniqueMethodWithOriginalName("phiWithExceptionalPhiUser");
+ MethodSubject phiWithNestedCatchHandler =
+ mainClass.uniqueMethodWithOriginalName("phiWithNestedCatchHandler");
+ MethodSubject multiUseArray = mainClass.uniqueMethodWithOriginalName("multiUseArray");
+ MethodSubject arrayWithHole = mainClass.uniqueMethodWithOriginalName("arrayWithHole");
+ MethodSubject reassignmentDoesNotOptimize =
+ mainClass.uniqueMethodWithOriginalName("reassignmentDoesNotOptimize");
MethodSubject intsThatUseFilledNewArray =
mainClass.uniqueMethodWithOriginalName("intsThatUseFilledNewArray");
MethodSubject twoDimensionalArrays =
@@ -191,26 +220,60 @@
mainClass.uniqueMethodWithOriginalName("arrayWithCorrectCountButIncompleteCoverage");
MethodSubject arrayWithExtraInitialPuts =
mainClass.uniqueMethodWithOriginalName("arrayWithExtraInitialPuts");
- MethodSubject catchHandlerThrowing =
- mainClass.uniqueMethodWithOriginalName("catchHandlerThrowing");
- MethodSubject catchHandlerNonThrowingFilledNewArray =
- mainClass.uniqueMethodWithOriginalName("catchHandlerNonThrowingFilledNewArray");
- MethodSubject catchHandlerNonThrowingFillArrayData =
- mainClass.uniqueMethodWithOriginalName("catchHandlerNonThrowingFillArrayData");
+ MethodSubject catchHandlerWithoutSideeffects =
+ mainClass.uniqueMethodWithOriginalName("catchHandlerWithoutSideeffects");
+ MethodSubject allocationWithCatchHandler =
+ mainClass.uniqueMethodWithOriginalName("allocationWithCatchHandler");
+ MethodSubject allocationWithoutCatchHandler =
+ mainClass.uniqueMethodWithOriginalName("allocationWithoutCatchHandler");
+ MethodSubject catchHandlerWithFinally =
+ mainClass.uniqueMethodWithOriginalName("catchHandlerWithFinally");
+ MethodSubject simpleSynchronized1 =
+ mainClass.uniqueMethodWithOriginalName("simpleSynchronized1");
+ MethodSubject simpleSynchronized2 =
+ mainClass.uniqueMethodWithOriginalName("simpleSynchronized2");
+ MethodSubject simpleSynchronized3 =
+ mainClass.uniqueMethodWithOriginalName("simpleSynchronized3");
+ MethodSubject simpleSynchronized4 =
+ mainClass.uniqueMethodWithOriginalName("simpleSynchronized4");
+ MethodSubject arrayInsideCatchHandler =
+ mainClass.uniqueMethodWithOriginalName("arrayInsideCatchHandler");
MethodSubject assumedValues = mainClass.uniqueMethodWithOriginalName("assumedValues");
+ // The explicit assignments can't be collapsed without breaking the debugger's ability to
+ // visit each line.
+ Class<?> filledNewArrayInRelease =
+ compilationMode == CompilationMode.DEBUG ? DexNewArray.class : DexFilledNewArray.class;
+
assertArrayTypes(arraysThatUseNewArrayEmpty, DexNewArray.class);
assertArrayTypes(intsThatUseFilledNewArray, DexFilledNewArray.class);
assertFilledArrayData(arraysThatUseFilledData);
- assertFilledArrayData(catchHandlerNonThrowingFillArrayData);
- if (compilationMode == CompilationMode.DEBUG) {
- // The explicit assignments can't be collapsed without breaking the debugger's ability to
- // visit each line.
- assertArrayTypes(reversedArray, DexNewArray.class);
- } else {
- assertArrayTypes(reversedArray, DexFilledNewArray.class);
- }
+ // Algorithm does not support out-of-order assignment.
+ assertArrayTypes(reversedArray, DexNewArray.class);
+ // Algorithm does not support assigning to array elements multiple times.
+ assertArrayTypes(reassignmentDoesNotOptimize, DexNewArray.class);
+ // Algorithm does not support default-initialized array elements.
+ assertArrayTypes(arrayWithHole, DexNewArray.class);
+ // ArrayPuts not dominated by return statement.
+ assertArrayTypes(phiFilledNewArrayBlocks, DexNewArray.class);
+ assertArrayTypes(arrayWithDominatingPhiUsers, filledNewArrayInRelease);
+ assertArrayTypes(arrayWithNonDominatingPhiUsers, DexNewArray.class);
+ assertArrayTypes(phiWithNestedCatchHandler, DexNewArray.class);
+ assertArrayTypes(phiWithExceptionalPhiUser, DexFilledNewArray.class, filledNewArrayInRelease);
+ // Not safe to change catch handlers.
+ assertArrayTypes(allocationWithoutCatchHandler, DexNewArray.class);
+ // Not safe to change catch handlers.
+ assertArrayTypes(allocationWithCatchHandler, DexNewArray.class);
+ assertArrayTypes(catchHandlerWithFinally, DexNewArray.class);
+ assertArrayTypes(simpleSynchronized1, DexFilledNewArray.class);
+ assertArrayTypes(simpleSynchronized2, DexFilledNewArray.class);
+ assertArrayTypes(simpleSynchronized3, DexNewArray.class);
+ assertArrayTypes(simpleSynchronized4, DexNewArray.class);
+ // Could be optimized if we had side-effect analysis of exceptional blocks.
+ assertArrayTypes(catchHandlerWithoutSideeffects, DexNewArray.class);
+ assertArrayTypes(arrayInsideCatchHandler, filledNewArrayInRelease);
+ assertArrayTypes(multiUseArray, filledNewArrayInRelease);
if (!canUseFilledNewArrayOfStringObjects(parameters)) {
assertArrayTypes(stringArrays, DexNewArray.class);
@@ -239,9 +302,7 @@
assertArrayTypes(referenceArraysWithInterfaceImplementations, DexNewArray.class);
}
- // TODO(b/246971330): Add support for arrays whose values have conditionals.
- // assertArrayTypes(phiFilledNewArray, DexFilledNewArray.class);
-
+ assertArrayTypes(phiFilledNewArray, DexFilledNewArray.class);
assertArrayTypes(
objectArraysFilledNewArrayRange, DexFilledNewArrayRange.class, DexNewArray.class);
@@ -261,9 +322,6 @@
// haven't bothered.
assertArrayTypes(arrayWithExtraInitialPuts, DexNewArray.class);
assertArrayTypes(arrayWithCorrectCountButIncompleteCoverage, DexNewArray.class);
-
- assertArrayTypes(catchHandlerThrowing, DexNewArray.class);
- assertArrayTypes(catchHandlerNonThrowingFilledNewArray, DexFilledNewArray.class);
}
private static Predicate<InstructionSubject> isInstruction(List<Class<?>> allowlist) {
@@ -310,6 +368,14 @@
referenceArraysWithInterfaceImplementations();
interfaceArrayWithRawObject();
phiFilledNewArray();
+ phiFilledNewArrayBlocks();
+ arrayWithDominatingPhiUsers();
+ arrayWithNonDominatingPhiUsers();
+ phiWithNestedCatchHandler();
+ phiWithExceptionalPhiUser();
+ multiUseArray();
+ arrayWithHole();
+ reassignmentDoesNotOptimize();
intsThatUseFilledNewArray();
twoDimensionalArrays();
objectArraysFilledNewArrayRange();
@@ -318,9 +384,15 @@
reversedArray();
arrayWithCorrectCountButIncompleteCoverage();
arrayWithExtraInitialPuts();
- catchHandlerThrowing();
- catchHandlerNonThrowingFilledNewArray();
- catchHandlerNonThrowingFillArrayData();
+ arrayInsideCatchHandler();
+ allocationWithCatchHandler();
+ allocationWithoutCatchHandler();
+ catchHandlerWithFinally();
+ simpleSynchronized1();
+ simpleSynchronized2();
+ simpleSynchronized3();
+ simpleSynchronized4();
+ catchHandlerWithoutSideeffects();
arrayIntoAnotherArray();
assumedValues();
}
@@ -411,21 +483,12 @@
}
@NeverInline
- private static void catchHandlerNonThrowingFilledNewArray() {
+ private static void arrayInsideCatchHandler() {
try {
- int[] arr1 = {1, 2, 3};
+ // Test filled-new-array with a throwing instruction before the last array-put.
+ int[] arr = new int[1];
System.currentTimeMillis();
- System.out.println(Arrays.toString(arr1));
- } catch (Throwable t) {
- throw new RuntimeException(t);
- }
- }
-
- @NeverInline
- private static void catchHandlerNonThrowingFillArrayData() {
- try {
- int[] arr = {1, 2, 3, 4, 5, 6};
- System.currentTimeMillis();
+ arr[0] = 9;
System.out.println(Arrays.toString(arr));
} catch (Throwable t) {
throw new RuntimeException(t);
@@ -433,37 +496,115 @@
}
@NeverInline
- private static void catchHandlerThrowing() {
- int[] arr1 = new int[3];
- arr1[0] = 0;
- arr1[1] = 1;
- // Since the array is used in only one spot, and that spot is not within the try/catch, it
- // should be safe to use filled-new-array, but we don't.
+ private static void allocationWithCatchHandler() {
+ Object[] arr;
try {
- System.currentTimeMillis();
+ arr = new Object[1];
+ } catch (NoClassDefFoundError | OutOfMemoryError t) {
+ throw new RuntimeException(t);
+ }
+
+ // new-array-empty dominates this, but catch handlers are relevant
+ arr[0] = "*";
+ System.out.println(Arrays.toString(arr));
+ }
+
+ @NeverInline
+ private static void allocationWithoutCatchHandler() {
+ Object[] arr = new Object[1];
+ try {
+ // new-array-empty dominates this, but catch handlers are relevant.
+ arr[0] = "*";
+ } catch (NoClassDefFoundError | OutOfMemoryError t) {
+ throw new RuntimeException(t);
+ }
+ System.out.println(Arrays.toString(arr));
+ }
+
+ @NeverInline
+ private static void catchHandlerWithFinally() {
+ Object[] arr = new Object[2];
+ try {
+ System.out.print("finally: ");
+ } finally {
+ // This will be duplicated into the throwing and non-throwing blocks.
+ arr[0] = "1";
+ }
+ arr[1] = "2";
+ System.out.println(Arrays.toString(arr));
+ }
+
+ @NeverInline
+ private static void simpleSynchronized1() {
+ // Should optimize since array is contained within a try block.
+ synchronized (Main.class) {
+ int[] arr = new int[] {1, 2};
+ System.out.println(Arrays.toString(arr));
+ }
+ }
+
+ @NeverInline
+ private static synchronized void simpleSynchronized2() {
+ // Should optimize since array is contained within a try block.
+ try {
+ try {
+ int[] arr = new int[] {1, 2};
+ System.out.println(Arrays.toString(arr));
+ } catch (Throwable t) {
+ throw new RuntimeException(t);
+ } finally {
+ System.currentTimeMillis();
+ }
+ } catch (Exception e) {
+ // Ignore.
+ }
+ }
+
+ @NeverInline
+ private static void simpleSynchronized3() {
+ // Does not optimize because allocation has different catch handlers.
+ int[] arr = new int[2];
+ synchronized (Main.class) {
+ arr[0] = 1;
+ arr[1] = 2;
+ }
+ System.out.println(Arrays.toString(arr));
+ }
+
+ @NeverInline
+ private static void simpleSynchronized4() {
+ // Does not optimize because allocation has different catch handlers.
+ int[] arr;
+ synchronized (Main.class) {
+ arr = new int[2];
+ }
+ arr[0] = 1;
+ arr[1] = 2;
+ System.out.println(Arrays.toString(arr));
+ }
+
+ @NeverInline
+ private static void catchHandlerWithoutSideeffects() {
+ // If we added logic to show that catch handlers exit without side-effects, we could optimize
+ // this case.z
+ int[] arr1;
+ try {
+ arr1 = new int[3];
+ } catch (Throwable t) {
+ throw new RuntimeException("1");
+ }
+ try {
+ arr1[0] = 0;
+ } catch (Throwable t) {
+ throw new RuntimeException("2");
+ }
+ arr1[1] = 1;
+ try {
arr1[2] = 2;
} catch (Throwable t) {
- throw new RuntimeException(t);
+ throw new RuntimeException("3");
}
System.out.println(Arrays.toString(arr1));
-
- try {
- // Test filled-new-array with a throwing instruction before the last array-put.
- int[] arr2 = new int[1];
- System.currentTimeMillis();
- arr2[0] = 0;
- System.out.println(Arrays.toString(arr2));
-
- // Test filled-array-data with a throwing instruction before the last array-put.
- short[] arr3 = new short[3];
- arr3[0] = 0;
- arr3[1] = 1;
- System.currentTimeMillis();
- arr3[2] = 2;
- System.out.println(Arrays.toString(arr3));
- } catch (Throwable t) {
- throw new RuntimeException(t);
- }
}
@NeverInline
@@ -485,11 +626,123 @@
@NeverInline
private static void phiFilledNewArray() {
// The presence of ? should not affect use of filled-new-array.
- Integer[] phiArray = {1, System.nanoTime() > 0 ? 2 : 3};
+ Integer[] phiArray = {1, System.nanoTime() > 0 ? 2 : 3, 3};
System.out.println(Arrays.toString(phiArray));
}
@NeverInline
+ private static void phiFilledNewArrayBlocks() {
+ int[] phiArray = new int[3];
+ if (System.currentTimeMillis() > 0) {
+ phiArray[0] = 2;
+ phiArray[1] = 2;
+ phiArray[2] = 2;
+ }
+ System.out.println(Arrays.toString(phiArray));
+ }
+
+ @NeverInline
+ private static void arrayWithDominatingPhiUsers() {
+ int[] phiArray = null;
+ try {
+ phiArray = new int[2];
+ phiArray[0] = 6;
+ phiArray[1] = 7;
+ } catch (Throwable t) {
+ System.out.println("Not reached");
+ }
+ System.out.println(Arrays.toString(phiArray));
+ }
+
+ @NeverInline
+ private static void arrayWithNonDominatingPhiUsers() {
+ int[] phiArray = null;
+ try {
+ phiArray = new int[1];
+ // If currentTimeMillis() throws, phiArray will have value of [0].
+ phiArray[0] = System.currentTimeMillis() > 0 ? 7 : 0;
+ } catch (Throwable t) {
+ System.out.println("Not reached");
+ }
+ System.out.println(Arrays.toString(phiArray));
+ }
+
+ @NeverInline
+ private static void phiWithNestedCatchHandler() {
+ int[] phiArray = null;
+ try {
+ phiArray = new int[2];
+ // If currentTimeMillis() throws, phiArray will have value of [0, 0].
+ try {
+ System.currentTimeMillis();
+ } catch (RuntimeException r) {
+ throw new RuntimeException(r);
+ }
+ phiArray[0] = 3;
+ phiArray[1] = 4;
+ } catch (Throwable t) {
+ System.out.println("Not reached");
+ }
+ System.out.println(Arrays.toString(phiArray));
+ }
+
+ @NeverInline
+ private static void phiWithExceptionalPhiUser() {
+ int[] arr = null;
+ try {
+ // Both of these should optimize, but care must be taken to ensure the phiUsers are properly
+ // dominated post-optimization.
+ if (System.currentTimeMillis() > 0) {
+ arr = new int[1];
+ arr[0] = 99;
+ } else {
+ arr = new int[] {1, 2};
+ }
+ } catch (RuntimeException e) {
+ // fall through
+ }
+ System.out.println(Arrays.toString(arr));
+ }
+
+ @NeverInline
+ private static void multiUseArray() {
+ int[] arr = new int[2];
+ arr[0] = 0;
+ arr[1] = System.nanoTime() > 0 ? 1 : 2;
+ System.out.println(Arrays.toString(arr));
+ System.out.println(Arrays.toString(arr));
+ // Usage in a different basic block.
+ if (System.nanoTime() > 0) {
+ System.nanoTime();
+ }
+ System.out.println(Arrays.toString(arr));
+ }
+
+ @NeverInline
+ private static void arrayWithHole() {
+ int[] arr = new int[2];
+ arr[1] = 1;
+ System.out.println(Arrays.toString(arr));
+ }
+
+ @NeverInline
+ private static void reassignmentDoesNotOptimize() {
+ // Reassignment in same block, and of last index.
+ Integer[] arr = new Integer[2];
+ arr[0] = 0;
+ arr[1] = 2;
+ arr[1] = 1;
+ System.out.println(Arrays.toString(arr));
+
+ // Reassignment across blocks, of non-last index.
+ arr = new Integer[2];
+ arr[0] = 3;
+ arr[1] = System.nanoTime() > 0 ? 1 : 2;
+ arr[0] = 0;
+ System.out.println(Arrays.toString(arr));
+ }
+
+ @NeverInline
private static void intsThatUseFilledNewArray() {
// Up to 5 ints uses filled-new-array rather than filled-array-data.
int[] intArr = {1, 2, 3, 4, 5};
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/StaticGetArrayWithNonUniqueValuesTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/StaticGetArrayWithNonUniqueValuesTest.java
index be54d34..68b80fc 100644
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/StaticGetArrayWithNonUniqueValuesTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/StaticGetArrayWithNonUniqueValuesTest.java
@@ -46,9 +46,9 @@
private static final String EXPECTED_OUTPUT = StringUtils.lines("100", "50");
- private void inspect(MethodSubject method, int staticGets, int puts, boolean insideCatchHandler) {
- boolean expectingFilledNewArray =
- canUseFilledNewArrayOfNonStringObjects(parameters) && !insideCatchHandler;
+ private void inspect(MethodSubject method, int staticGets, int puts, boolean isD8) {
+ // D8 cannot optimize due to risk of NoClassDefFoundError.
+ boolean expectingFilledNewArray = !isD8 && canUseFilledNewArrayOfNonStringObjects(parameters);
assertEquals(
expectingFilledNewArray ? 0 : puts,
method.streamInstructions().filter(InstructionSubject::isArrayPut).count());
@@ -66,18 +66,9 @@
}
private void inspectD8(CodeInspector inspector) {
- inspect(
- inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"),
- canUseFilledNewArrayOfNonStringObjects(parameters) ? 100 : 1,
- 100,
- false);
- inspect(
- inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"),
- canUseFilledNewArrayOfNonStringObjects(parameters)
- ? 50
- : (maxMaterializingConstants == 2 ? 42 : 10),
- 50,
- false);
+ // D8 cannot optimize due to risk of NoClassDefFoundError.
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), 100, 100, true);
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"), 50, 50, true);
}
@Test
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/StaticGetArrayWithUniqueValuesTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/StaticGetArrayWithUniqueValuesTest.java
index fce1e79..f64d0e8 100644
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/StaticGetArrayWithUniqueValuesTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/StaticGetArrayWithUniqueValuesTest.java
@@ -52,9 +52,9 @@
EXPECTING_APUTOBJECT
}
- private void inspect(MethodSubject method, int puts, boolean insideCatchHandler) {
- boolean expectingFilledNewArray =
- canUseFilledNewArrayOfNonStringObjects(parameters) && !insideCatchHandler;
+ private void inspect(boolean isR8, MethodSubject method, int puts) {
+ // D8 cannot optimize due to risk of NoClassDefFoundError.
+ boolean expectingFilledNewArray = isR8 && canUseFilledNewArrayOfNonStringObjects(parameters);
assertEquals(
expectingFilledNewArray ? 0 : puts,
method.streamInstructions().filter(InstructionSubject::isArrayPut).count());
@@ -84,10 +84,10 @@
}
}
- private void inspect(CodeInspector inspector) {
- inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), 5, false);
- inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"), 5, true);
- inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m3"), 100, false);
+ private void inspect(CodeInspector inspector, boolean isR8) {
+ inspect(isR8, inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), 5);
+ inspect(isR8, inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"), 5);
+ inspect(isR8, inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m3"), 100);
}
@Test
@@ -97,7 +97,7 @@
.addInnerClasses(getClass())
.setMinApi(parameters)
.run(parameters.getRuntime(), TestClass.class)
- .inspect(this::inspect)
+ .inspect(inspector -> inspect(inspector, false))
.assertSuccessWithOutput(EXPECTED_OUTPUT);
}
@@ -110,7 +110,7 @@
.enableInliningAnnotations()
.addDontObfuscate()
.run(parameters.getRuntime(), TestClass.class)
- .inspect(this::inspect)
+ .inspect(inspector -> inspect(inspector, true))
.assertSuccessWithOutput(EXPECTED_OUTPUT);
}
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/StringArrayWithNonUniqueValuesTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/StringArrayWithNonUniqueValuesTest.java
index c433e6d..cf12961 100644
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/StringArrayWithNonUniqueValuesTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/StringArrayWithNonUniqueValuesTest.java
@@ -46,10 +46,8 @@
private static final String EXPECTED_OUTPUT = StringUtils.lines("100", "104");
- private void inspect(
- MethodSubject method, int constStrings, int puts, boolean insideCatchHandler) {
- boolean expectingFilledNewArray =
- canUseFilledNewArrayOfStringObjects(parameters) && !insideCatchHandler;
+ private void inspect(MethodSubject method, int constStrings, int puts) {
+ boolean expectingFilledNewArray = canUseFilledNewArrayOfStringObjects(parameters);
assertEquals(
expectingFilledNewArray ? 0 : puts,
method.streamInstructions().filter(InstructionSubject::isArrayPut).count());
@@ -68,12 +66,11 @@
}
private void inspect(CodeInspector inspector) {
- inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), 1, 100, false);
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), 1, 100);
inspect(
inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"),
maxMaterializingConstants == 2 ? 32 : 26,
- 104,
- false);
+ 104);
}
@Test
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/StringArrayWithUniqueValuesTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/StringArrayWithUniqueValuesTest.java
index 837bea2..53f7f1f 100644
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/StringArrayWithUniqueValuesTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/StringArrayWithUniqueValuesTest.java
@@ -52,9 +52,8 @@
EXPECTING_APUTOBJECT
}
- private void inspect(MethodSubject method, int puts, boolean insideCatchHandler) {
- boolean expectingFilledNewArray =
- canUseFilledNewArrayOfStringObjects(parameters) && !insideCatchHandler;
+ private void inspect(MethodSubject method, int puts) {
+ boolean expectingFilledNewArray = canUseFilledNewArrayOfStringObjects(parameters);
assertEquals(
expectingFilledNewArray ? 0 : puts,
method.streamInstructions().filter(InstructionSubject::isArrayPut).count());
@@ -86,9 +85,9 @@
}
private void inspect(CodeInspector inspector) {
- inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), 5, false);
- inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"), 5, true);
- inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m3"), 100, false);
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), 5);
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"), 5);
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m3"), 100);
}
@Test
diff --git a/src/test/java/com/android/tools/r8/workaround/FilledNewArrayFromSubtypeWithMissingInterfaceWorkaroundTest.java b/src/test/java/com/android/tools/r8/workaround/FilledNewArrayFromSubtypeWithMissingInterfaceWorkaroundTest.java
index 3f7aba6..53794e3 100644
--- a/src/test/java/com/android/tools/r8/workaround/FilledNewArrayFromSubtypeWithMissingInterfaceWorkaroundTest.java
+++ b/src/test/java/com/android/tools/r8/workaround/FilledNewArrayFromSubtypeWithMissingInterfaceWorkaroundTest.java
@@ -6,14 +6,13 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoVerticalClassMerging;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
-import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.InstructionSubject;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
@@ -44,7 +43,7 @@
.release()
.setMinApi(parameters)
.compile()
- .inspect(inspector -> inspect(inspector, true))
+ .inspect(this::inspect)
.apply(
compileResult ->
compileResult.runDex2Oat(parameters.getRuntime()).assertNoVerificationErrors())
@@ -66,18 +65,17 @@
parameters.isDexRuntime(),
compileResult ->
compileResult
- .inspect(inspector -> inspect(inspector, false))
+ .inspect(this::inspect)
.runDex2Oat(parameters.getRuntime())
.assertNoVerificationErrors())
.run(parameters.getRuntime(), Main.class)
.assertFailureWithErrorThatThrows(NoClassDefFoundError.class);
}
- private void inspect(CodeInspector inspector, boolean isD8) {
+ private void inspect(CodeInspector inspector) {
MethodSubject mainMethodSubject = inspector.clazz(Main.class).mainMethod();
assertThat(mainMethodSubject, isPresent());
- assertEquals(
- isD8 && parameters.getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.N),
+ assertFalse(
mainMethodSubject.streamInstructions().anyMatch(InstructionSubject::isFilledNewArray));
}
