Merge commit 'e98da634de940e3d0098e0467d08af80c532139d' into dev-release
Change-Id: I5a07ae67971916c550577b6e84a60da721e8ba3f
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/library/StringMethodOptimizer.java b/src/main/java/com/android/tools/r8/ir/optimize/library/StringMethodOptimizer.java
index 15092b9..ab1c629 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/library/StringMethodOptimizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/library/StringMethodOptimizer.java
@@ -301,8 +301,7 @@
if (paramsValue.isAlwaysNull(appView)) {
elementValues = Collections.emptyList();
} else {
- ArrayValues arrayValues =
- ValueUtils.computeSingleUseArrayValues(paramsValue, formatInvoke, code);
+ ArrayValues arrayValues = ValueUtils.computeSingleUseArrayValues(paramsValue, formatInvoke);
if (arrayValues == null) {
return instructionIterator;
}
diff --git a/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java b/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java
index 200fbc6..1fcb8e6 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java
@@ -474,6 +474,7 @@
return new ConsequentRootSet(
neverInlineDueToSingleCaller,
neverClassInline,
+ noHorizontalClassMerging,
dependentMinimumKeepInfo,
dependentKeepClassCompatRule,
Lists.newArrayList(delayedRootSetActionItems),
@@ -1793,6 +1794,7 @@
final Set<DexMethod> neverInlineDueToSingleCaller;
final Set<DexType> neverClassInline;
+ final Set<DexType> noHorizontalClassMerging;
private final DependentMinimumKeepInfoCollection dependentMinimumKeepInfo;
final Map<DexType, Set<ProguardKeepRuleBase>> dependentKeepClassCompatRule;
final List<DelayedRootSetActionItem> delayedRootSetActionItems;
@@ -1801,12 +1803,14 @@
RootSetBase(
Set<DexMethod> neverInlineDueToSingleCaller,
Set<DexType> neverClassInline,
+ Set<DexType> noHorizontalClassMerging,
DependentMinimumKeepInfoCollection dependentMinimumKeepInfo,
Map<DexType, Set<ProguardKeepRuleBase>> dependentKeepClassCompatRule,
List<DelayedRootSetActionItem> delayedRootSetActionItems,
ProgramMethodMap<ProgramMethod> pendingMethodMoveInverse) {
this.neverInlineDueToSingleCaller = neverInlineDueToSingleCaller;
this.neverClassInline = neverClassInline;
+ this.noHorizontalClassMerging = noHorizontalClassMerging;
this.dependentMinimumKeepInfo = dependentMinimumKeepInfo;
this.dependentKeepClassCompatRule = dependentKeepClassCompatRule;
this.delayedRootSetActionItems = delayedRootSetActionItems;
@@ -1833,7 +1837,6 @@
public final PredicateSet<DexType> alwaysClassInline;
public final Set<DexType> noUnusedInterfaceRemoval;
public final Set<DexType> noVerticalClassMerging;
- public final Set<DexType> noHorizontalClassMerging;
public final Set<DexMember<?, ?>> neverPropagateValue;
public final Map<DexReference, ProguardMemberRule> mayHaveSideEffects;
public final Set<DexMember<?, ?>> identifierNameStrings;
@@ -1863,6 +1866,7 @@
super(
neverInlineDueToSingleCaller,
neverClassInline,
+ noHorizontalClassMerging,
dependentMinimumKeepInfo,
dependentKeepClassCompatRule,
delayedRootSetActionItems,
@@ -1876,7 +1880,6 @@
this.alwaysClassInline = alwaysClassInline;
this.noUnusedInterfaceRemoval = noUnusedInterfaceRemoval;
this.noVerticalClassMerging = noVerticalClassMerging;
- this.noHorizontalClassMerging = noHorizontalClassMerging;
this.neverPropagateValue = neverPropagateValue;
this.mayHaveSideEffects = mayHaveSideEffects;
this.identifierNameStrings = Collections.unmodifiableSet(identifierNameStrings);
@@ -1909,6 +1912,7 @@
void addConsequentRootSet(ConsequentRootSet consequentRootSet) {
neverInlineDueToSingleCaller.addAll(consequentRootSet.neverInlineDueToSingleCaller);
neverClassInline.addAll(consequentRootSet.neverClassInline);
+ noHorizontalClassMerging.addAll(consequentRootSet.noHorizontalClassMerging);
consequentRootSet.dependentKeepClassCompatRule.forEach(
(type, rules) ->
dependentKeepClassCompatRule
@@ -2235,6 +2239,7 @@
ConsequentRootSet(
Set<DexMethod> neverInlineDueToSingleCaller,
Set<DexType> neverClassInline,
+ Set<DexType> noHorizontalClassMerging,
DependentMinimumKeepInfoCollection dependentMinimumKeepInfo,
Map<DexType, Set<ProguardKeepRuleBase>> dependentKeepClassCompatRule,
List<DelayedRootSetActionItem> delayedRootSetActionItems,
@@ -2242,6 +2247,7 @@
super(
neverInlineDueToSingleCaller,
neverClassInline,
+ noHorizontalClassMerging,
dependentMinimumKeepInfo,
dependentKeepClassCompatRule,
delayedRootSetActionItems,
diff --git a/src/main/java/com/android/tools/r8/utils/DominatorChecker.java b/src/main/java/com/android/tools/r8/utils/DominatorChecker.java
new file mode 100644
index 0000000..46c6716
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/utils/DominatorChecker.java
@@ -0,0 +1,175 @@
+// Copyright (c) 2023, 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.utils;
+
+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;
+
+/**
+ * Given a subgraph defined by sourceBlock / destBlock, where sourceBlock dominates destBlock,
+ * allows querying whether other blocks within this subgraph dominate destBlock.
+ */
+public interface DominatorChecker {
+ boolean check(BasicBlock targetBlock);
+
+ DominatorChecker TRUE_CHECKER = targetBlock -> true;
+ DominatorChecker FALSE_CHECKER = targetBlock -> false;
+
+ class PrecomputedDominatorChecker implements DominatorChecker {
+ private final Set<BasicBlock> dominators;
+
+ public PrecomputedDominatorChecker(Set<BasicBlock> dominators) {
+ this.dominators = dominators;
+ }
+
+ @Override
+ public boolean check(BasicBlock targetBlock) {
+ return dominators.contains(targetBlock);
+ }
+ }
+
+ class TraversingDominatorChecker implements DominatorChecker {
+ private final BasicBlock sourceBlock;
+ private final BasicBlock destBlock;
+ private final Set<BasicBlock> knownDominators;
+ private final ArrayDeque<BasicBlock> workQueue = new ArrayDeque<>();
+ private final Set<BasicBlock> visited;
+ private BasicBlock prevTargetBlock;
+
+ private TraversingDominatorChecker(
+ BasicBlock sourceBlock, BasicBlock destBlock, Set<BasicBlock> knownDominators) {
+ this.sourceBlock = sourceBlock;
+ this.destBlock = destBlock;
+ this.knownDominators = knownDominators;
+ this.visited = Sets.newIdentityHashSet();
+ prevTargetBlock = destBlock;
+ }
+
+ @Override
+ public boolean check(BasicBlock targetBlock) {
+ assert prevTargetBlock != null : "DominatorChecker cannot be used after returning false.";
+ Set<BasicBlock> knownDominators = this.knownDominators;
+ if (knownDominators.contains(targetBlock)) {
+ return true;
+ }
+ // See if a block on the same linear path has already been checked.
+ BasicBlock firstSplittingBlock = targetBlock;
+ if (firstSplittingBlock.hasUniqueSuccessor()) {
+ do {
+ // knownDominators prevents firstSplittingBlock from being destBlock.
+ assert firstSplittingBlock != destBlock;
+ firstSplittingBlock = firstSplittingBlock.getUniqueSuccessor();
+ } while (firstSplittingBlock.hasUniqueSuccessor());
+
+ if (knownDominators.contains(firstSplittingBlock)) {
+ knownDominators.add(targetBlock);
+ return true;
+ }
+ }
+
+ 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);
+ prevTargetBlock = firstSplittingBlock;
+ } else {
+ ret = checkWithTraversal(sourceBlock, prevTargetBlock, targetBlock, visited, workQueue);
+ prevTargetBlock = targetBlock;
+ }
+ if (ret) {
+ knownDominators.add(targetBlock);
+ if (firstSplittingBlock != targetBlock) {
+ knownDominators.add(firstSplittingBlock);
+ }
+ } else {
+ prevTargetBlock = null;
+ }
+ return ret;
+ }
+
+ private static boolean checkWithTraversal(
+ BasicBlock sourceBlock,
+ BasicBlock destBlock,
+ BasicBlock targetBlock,
+ Set<BasicBlock> visited,
+ ArrayDeque<BasicBlock> workQueue) {
+ assert workQueue.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.
+ return false;
+ }
+ assert !curBlock.isEntry() : "sourceBlock did not dominate destBlock";
+ workQueue.addAll(curBlock.getPredecessors());
+ } while (!workQueue.isEmpty());
+
+ return true;
+ }
+ }
+
+ static DominatorChecker create(BasicBlock sourceBlock, BasicBlock destBlock) {
+ // 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));
+ }
+
+ // Shrink the subgraph by moving sourceBlock 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)) {
+ // Hit an infinite loop. Code would not verify in this case.
+ assert false;
+ return FALSE_CHECKER;
+ }
+ if (sourceBlock == destBlock) {
+ // 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()) {
+ 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.
+ return new PrecomputedDominatorChecker(headAndTailDominators);
+ }
+ // Hit an infinite loop. Code would not verify in this case.
+ assert false;
+ return FALSE_CHECKER;
+ }
+ }
+
+ if (destBlock.isEntry()) {
+ return FALSE_CHECKER;
+ }
+
+ return new TraversingDominatorChecker(sourceBlock, destBlock, headAndTailDominators);
+ }
+}
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 dcb72ad..509816c 100644
--- a/src/main/java/com/android/tools/r8/utils/ValueUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/ValueUtils.java
@@ -8,27 +8,18 @@
import com.android.tools.r8.ir.analysis.type.TypeElement;
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.InvokeVirtual;
import com.android.tools.r8.ir.code.NewArrayEmpty;
import com.android.tools.r8.ir.code.NewArrayFilled;
import com.android.tools.r8.ir.code.NewInstance;
import com.android.tools.r8.ir.code.Value;
-import com.google.common.collect.HashMultiset;
-import com.google.common.collect.Multiset;
-import com.google.common.collect.Sets;
-import java.util.ArrayDeque;
import java.util.Arrays;
-import java.util.Collections;
import java.util.List;
-import java.util.Set;
public class ValueUtils {
// We allocate an array of this size, so guard against it getting too big.
private static int MAX_ARRAY_SIZE = 100000;
- private static boolean DEBUG =
- System.getProperty("com.android.tools.r8.debug.computeSingleUseArrayValues") != null;
@SuppressWarnings("ReferenceEquality")
public static boolean isStringBuilder(Value value, DexItemFactory dexItemFactory) {
@@ -94,163 +85,6 @@
}
}
- private static BasicBlock findNextUnseen(
- int startIdx, List<BasicBlock> predecessors, Set<BasicBlock> seen) {
- int size = predecessors.size();
- for (int i = startIdx; i < size; ++i) {
- BasicBlock next = predecessors.get(i);
- if (!seen.contains(next)) {
- return next;
- }
- }
- return null;
- }
-
- private static void debugLog(IRCode code, String message) {
- System.err.println(message + " method=" + code.context().getReference());
- }
-
- /**
- * Returns all dominator blocks of destBlock between (and including) it and sourceBlock. Returns
- * null if sourceBlock is not a dominator of destBlock. Returns null if the algorithm is taking
- * too long.
- */
- private static Set<BasicBlock> computeSimpleCaseDominatorBlocks(
- BasicBlock sourceBlock, BasicBlock destBlock, IRCode code) {
- // Fast-path: blocks are the same.
- // As of Nov 2023 in Chrome for String.format() optimization, this is hit 77% of the time .
- if (destBlock == sourceBlock) {
- if (DEBUG) {
- debugLog(code, "computeSimpleCaseDominatorBlocks: SAME BLOCK");
- }
- return Collections.singleton(sourceBlock);
- }
-
- // Fast-path: Linear path from source -> dest.
- // As of Nov 2023 in Chrome for String.format() optimization, this is hit 14% of the time .
- BasicBlock curBlock = destBlock;
- List<BasicBlock> curPredecessors;
- Set<BasicBlock> ret = Sets.newIdentityHashSet();
- while (true) {
- curPredecessors = curBlock.getPredecessors();
- ret.add(curBlock);
- if (curBlock == sourceBlock) {
- if (DEBUG) {
- debugLog(code, "computeSimpleCaseDominatorBlocks: LINEAR PATH");
- }
- return ret;
- }
-
- BasicBlock nextBlock = findNextUnseen(0, curPredecessors, ret);
- if (nextBlock == null) {
- debugLog(code, "computeSimpleCaseDominatorBlocks: Not a dominator.");
- return null;
- }
- if (findNextUnseen(curPredecessors.indexOf(nextBlock) + 1, curPredecessors, ret) != null) {
- // Multiple predecessors.
- break;
- }
-
- curBlock = nextBlock;
- }
-
- // Algorithm: Traverse all predecessor paths between curBlock and sourceBlock, tracking the
- // number of times each block is visited.
- // Returns blocks with a "visit count" equal to the number of paths.
- // This algorithm has worst case exponential complexity (for a fully connected graph).
- // Rather than falling back to a DominatorTree for complex cases, this currently just returns
- // an empty set when the number of paths is not small. Thus, it should not be used when false
- // negatives are not acceptable.
- //
- // As of Nov 2023 in Chrome for String.format() optimization, the totalPathCounts were:
- // 2 * 12, 3 * 6, 4 * 2, 5 * 1, 12 * 1, 22 * 1, 24 * 1, 35 * 4,
- // MAX_PATH_COUNT * 2 (even with MAX_PATH_COUNT=3200)
- final int MAX_PATH_COUNT = 36;
-
- // TODO(agrieve): Lower MAX_PATH_COUNT and use DominatorTree for complicated cases. Or...
- // Algorithm vNext:
- // Track the fraction of paths that reach each node. Doing so would be ~linear for graphs
- // without cycles. E.g.:
- // DestNodeValue=1
- // NodeValue=SUM(block.NodeValue / block.numSuccessors for block in successors)
- // Dominators=(b for b in blocks if b.NodeValue == 1)
- // To choose which block to visit next, choose any where all predecessors have already been
- // visited. If no block has all predecessors visited, then an unvisited block is from a
- // cycle (they cannot be from outside of the sourceBlock->destBlock subgraph so long as
- // sourceBlock dominates destBlock).
- // To deal with cycles (clearly not linear time now):
- // * Find all blocks that participate in a cycle by doing a full traversal starting from
- // each candidate until one is found. Store the set of edges that do not reach sourceBlock
- // (except through the cycle).
- // * When computing the value of a block whose successors participate in a cycle:
- // NodeValue=SUM(block.NodeValue / effectiveNumSuccessors for block in successors)
- // where effectiveNumSuccessors=SUM(1 for s in successors if (b->s) not in cycleOnlyEdges)
-
- Multiset<BasicBlock> blockCounts = HashMultiset.create();
- int totalPathCount = 0;
-
- // Should never need to re-visit initial single-track nodes.
- Set<BasicBlock> seen = Sets.newIdentityHashSet();
- seen.addAll(ret);
- ArrayDeque<BasicBlock> pathStack = new ArrayDeque<>();
-
- pathStack.add(curBlock);
- pathStack.add(curPredecessors.get(0));
- while (true) {
- curBlock = pathStack.getLast();
- curPredecessors = curBlock.getPredecessors();
- if (curBlock == sourceBlock) {
- // Add every block for every connected path.
- blockCounts.addAll(pathStack);
- totalPathCount += 1;
- if (totalPathCount > MAX_PATH_COUNT) {
- if (DEBUG) {
- debugLog(code, "computeSimpleCaseDominatorBlocks: Reached MAX_PATH_COUNT");
- }
- return null;
- }
- } else if (!seen.contains(curBlock)) {
- if (curPredecessors.isEmpty()) {
- // Finding the entry block means the sourceBlock is not a dominator.
- if (DEBUG) {
- debugLog(code, "computeSimpleCaseDominatorBlocks: sourceBlock not a dominator");
- }
- return null;
- }
- // Going deeper.
- BasicBlock nextBlock = findNextUnseen(0, curPredecessors, seen);
- if (nextBlock != null) {
- seen.add(curBlock);
- pathStack.add(nextBlock);
- continue;
- }
- } else {
- seen.remove(curBlock);
- }
- // Popping.
- pathStack.removeLast();
- List<BasicBlock> prevPredecessors = pathStack.getLast().getPredecessors();
- int nextBlockIdx = prevPredecessors.indexOf(curBlock) + 1;
- BasicBlock nextBlock = findNextUnseen(nextBlockIdx, prevPredecessors, seen);
- if (nextBlock != null) {
- pathStack.add(nextBlock);
- } else if (pathStack.size() == 1) {
- break;
- }
- }
-
- for (var entry : blockCounts.entrySet()) {
- if (entry.getCount() == totalPathCount) {
- ret.add(entry.getElement());
- }
- }
- if (DEBUG) {
- debugLog(code, "computeSimpleCaseDominatorBlocks: PATH COUNT " + totalPathCount);
- }
-
- return ret;
- }
-
/**
* Attempts to determine all values for the given array. This will work only when:
*
@@ -260,7 +94,10 @@
* * When users are in different blocks, their order is hard to know.
* 2) The array size is a constant.
* 3) All array-put instructions have constant and unique indices.
- * * Indices must be unique because order is hard to know when multiple blocks are concerned.
+ * * 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>
*
@@ -268,8 +105,7 @@
* @param singleUser The only non-array-put user, or null to auto-detect.
* @return The computed array values, or null if they could not be determined.
*/
- public static ArrayValues computeSingleUseArrayValues(
- Value arrayValue, Instruction singleUser, IRCode code) {
+ 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()) {
@@ -309,15 +145,8 @@
}
}
- // Ensure that all paths from new-array-empty to |usage| contain all array-put instructions.
- Set<BasicBlock> dominatorBlocks =
- computeSimpleCaseDominatorBlocks(definition.getBlock(), singleUser.getBlock(), code);
- if (dominatorBlocks == null) {
- return null;
- }
- BasicBlock usageBlock = singleUser.getBlock();
-
ArrayPut[] arrayPutsByIndex = new ArrayPut[arraySize];
+ BasicBlock usageBlock = singleUser.getBlock();
for (Instruction user : arrayValue.uniqueUsers()) {
ArrayPut arrayPut = user.asArrayPut();
if (arrayPut == null || arrayPut.array() != arrayValue || arrayPut.value() == arrayValue) {
@@ -327,22 +156,26 @@
// Found a second non-array-put user.
return null;
}
- int index = arrayPut.indexIfConstAndInBounds(arraySize);
- if (index < 0) {
- return null;
- }
- if (arrayPut.getBlock() == usageBlock) {
- // Process these later.
+ // Process same-block instructions later.
+ if (user.getBlock() == usageBlock) {
continue;
- } else if (!dominatorBlocks.contains(arrayPut.getBlock())) {
- return null;
}
+ int index = arrayPut.indexIfConstAndInBounds(arraySize);
// We do not know what order blocks are in, so do not allow re-assignment.
- if (arrayPutsByIndex[index] != null) {
+ if (index < 0 || arrayPutsByIndex[index] != null) {
return null;
}
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);
+ for (Instruction user : arrayValue.uniqueUsers()) {
+ if (!dominatorChecker.check(user.getBlock())) {
+ return null;
+ }
+ }
+
boolean seenSingleUser = false;
for (Instruction inst : usageBlock.getInstructions()) {
if (inst == singleUser) {
@@ -357,7 +190,10 @@
// Found an array-put after the array was used. This is too uncommon of a thing to support.
return null;
}
- int index = arrayPut.index().getConstInstruction().asConstNumber().getIntValue();
+ int index = arrayPut.indexIfConstAndInBounds(arraySize);
+ if (index < 0) {
+ return null;
+ }
// We can allow reassignment at this point since we are visiting in order.
arrayPutsByIndex[index] = arrayPut;
}
diff --git a/src/resourceshrinker/java/com/android/build/shrinker/PossibleResourcesMarker.java b/src/resourceshrinker/java/com/android/build/shrinker/PossibleResourcesMarker.java
index 5089061..b710916 100644
--- a/src/resourceshrinker/java/com/android/build/shrinker/PossibleResourcesMarker.java
+++ b/src/resourceshrinker/java/com/android/build/shrinker/PossibleResourcesMarker.java
@@ -16,6 +16,7 @@
package com.android.build.shrinker;
+import static com.android.ide.common.resources.ResourcesUtil.resourceNameToFieldName;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.lang.Character.isDigit;
@@ -122,7 +123,7 @@
haveSlash |= c == '/';
formatting |= c == '%';
justName =
- justName && !(c == '.' || c == ':' || c == '%' || c == '/');
+ justName && !(c == ':' || c == '%' || c == '/');
}
Stream<Resource> reachable = Streams.concat(
@@ -170,7 +171,7 @@
// Check for a simple prefix match, e.g. as in
// getResources().getIdentifier("ic_video_codec_" + codecName, "drawable", ...)
return resourceStore.getResources().stream()
- .filter(resource -> resource.name.startsWith(string));
+ .filter(resource -> resource.name.startsWith(resourceNameToFieldName(string)));
}
private Stream<Resource> possibleFormatting(String string) {
@@ -191,7 +192,7 @@
// Try to pick out the resource name pieces; if we can find the
// resource type unambiguously; if not, just match on names
int slash = string.indexOf('/');
- String name = string.substring(slash + 1);
+ String name = resourceNameToFieldName(string.substring(slash + 1));
if (name.isEmpty() || !names.contains(name)) {
return Stream.empty();
}
diff --git a/src/test/java/com/android/tools/r8/androidresources/ResourceNameWithDotsRegressionTest.java b/src/test/java/com/android/tools/r8/androidresources/ResourceNameWithDotsRegressionTest.java
new file mode 100644
index 0000000..1e28ef0
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/androidresources/ResourceNameWithDotsRegressionTest.java
@@ -0,0 +1,77 @@
+// Copyright (c) 2023, 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.androidresources;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResource;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResourceBuilder;
+import com.android.tools.r8.utils.BooleanUtils;
+import java.nio.file.Path;
+import java.util.List;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class ResourceNameWithDotsRegressionTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameter(1)
+ public boolean debug;
+
+ @Parameter(2)
+ public boolean optimized;
+
+ @Parameters(name = "{0}, debug: {1}, optimized: {2}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withDefaultDexRuntime().withAllApiLevels().build(),
+ BooleanUtils.values(),
+ BooleanUtils.values());
+ }
+
+ public static AndroidTestResource getTestResources(TemporaryFolder temp) throws Exception {
+ return new AndroidTestResourceBuilder()
+ .withSimpleManifestAndAppNameString()
+ .addStringValue("foo.bar", "the foobar string")
+ .build(temp);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ AndroidTestResource testResources = getTestResources(temp);
+ Path resourcesClass = AndroidResourceTestingUtils.resourcesClassAsDex(temp);
+ byte[] withAnroidResourcesReference =
+ AndroidResourceTestingUtils.transformResourcesReferences(FooBar.class);
+ testForR8(parameters.getBackend())
+ .setMinApi(parameters)
+ .addProgramClassFileData(withAnroidResourcesReference)
+ .addRunClasspathFiles(resourcesClass)
+ .addAndroidResources(testResources)
+ .addKeepMainRule(FooBar.class)
+ .compile()
+ .inspectShrunkenResources(
+ resourceTableInspector -> {
+ resourceTableInspector.assertContainsResourceWithName("string", "foo.bar");
+ })
+ .run(parameters.getRuntime(), FooBar.class)
+ .assertSuccess();
+ }
+
+ public static class FooBar {
+
+ public static void main(String[] args) {
+ if (System.currentTimeMillis() == 0) {
+ Resources resources = new Resources();
+ resources.getIdentifier("foo.bar", "string", "mypackage");
+ }
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/androidresources/Resources.java b/src/test/java/com/android/tools/r8/androidresources/Resources.java
index ef01a99..e702f2a 100644
--- a/src/test/java/com/android/tools/r8/androidresources/Resources.java
+++ b/src/test/java/com/android/tools/r8/androidresources/Resources.java
@@ -13,4 +13,8 @@
public String getString(int id) {
return GET_STRING_VALUE;
}
+
+ public int getIdentifier(String name, String defType, String defPackage) {
+ return 42;
+ }
}
diff --git a/src/test/java/com/android/tools/r8/graph/invokespecial/InvokeSpecialToImmediateInterfaceTest.java b/src/test/java/com/android/tools/r8/graph/invokespecial/InvokeSpecialToImmediateInterfaceTest.java
new file mode 100644
index 0000000..15f1e9c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/graph/invokespecial/InvokeSpecialToImmediateInterfaceTest.java
@@ -0,0 +1,129 @@
+// Copyright (c) 2023, 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.graph.invokespecial;
+
+import static com.android.tools.r8.utils.DescriptorUtils.getBinaryNameFromJavaType;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.objectweb.asm.Opcodes.INVOKESPECIAL;
+import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL;
+
+import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.ToolHelper.DexVm.Version;
+import com.android.tools.r8.utils.Box;
+import com.android.tools.r8.utils.codeinspector.AssertUtils;
+import java.io.IOException;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class InvokeSpecialToImmediateInterfaceTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ @Test
+ public void testJvm() throws Exception {
+ parameters.assumeJvmTestParameters();
+ testForJvm(parameters)
+ .addProgramClasses(I.class, Main.class)
+ .addProgramClassFileData(getClassWithTransformedInvoked())
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("Hello, world!");
+ }
+
+ @Test
+ public void testD8Release() throws Exception {
+ parameters.assumeDexRuntime();
+ testForD8(parameters.getBackend())
+ .addProgramClasses(I.class, Main.class)
+ .addProgramClassFileData(getClassWithTransformedInvoked())
+ .release()
+ .setMinApi(parameters)
+ .run(parameters.getRuntime(), Main.class)
+ // TODO(b/313065227): Should succeed.
+ .applyIf(
+ parameters.getDexRuntimeVersion().isEqualToOneOf(Version.V5_1_1, Version.V6_0_1),
+ runResult -> runResult.assertFailureWithErrorThatMatches(containsString("SIGSEGV")),
+ runResult -> runResult.assertFailureWithErrorThatThrows(NoSuchMethodError.class));
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ Box<R8TestCompileResult> compileResult = new Box<>();
+
+ // TODO(b/313065227): Should succeed.
+ AssertUtils.assertFailsCompilationIf(
+ parameters.isCfRuntime(),
+ () ->
+ testForR8(parameters.getBackend())
+ .addProgramClasses(I.class, Main.class)
+ .addProgramClassFileData(getClassWithTransformedInvoked())
+ .addKeepMainRule(Main.class)
+ .setMinApi(parameters)
+ .compile()
+ .apply(compileResult::set));
+
+ if (!compileResult.isSet()) {
+ assertTrue(parameters.isCfRuntime());
+ return;
+ }
+
+ // TODO(b/313065227): Should succeed.
+ compileResult
+ .get()
+ .run(parameters.getRuntime(), Main.class)
+ .assertFailureWithErrorThatThrows(NullPointerException.class);
+ }
+
+ private byte[] getClassWithTransformedInvoked() throws IOException {
+ return transformer(A.class)
+ .transformMethodInsnInMethod(
+ "bar",
+ (opcode, owner, name, descriptor, isInterface, continuation) -> {
+ assertEquals(INVOKEVIRTUAL, opcode);
+ assertEquals(owner, getBinaryNameFromJavaType(A.class.getTypeName()));
+ continuation.visitMethodInsn(
+ INVOKESPECIAL,
+ getBinaryNameFromJavaType(A.class.getTypeName()),
+ name,
+ descriptor,
+ isInterface);
+ })
+ .transform();
+ }
+
+ public interface I {
+
+ default void foo() {
+ System.out.println("Hello, world!");
+ }
+ }
+
+ public static class A implements I {
+
+ public void bar() {
+ foo(); // Will be rewritten to invoke-special A.foo()
+ }
+ }
+
+ public static class Main {
+
+ public static void main(String[] args) {
+ new A().bar();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/keepanno/doctests/ConditionalMethodRulesAndHorizontalMergingTest.java b/src/test/java/com/android/tools/r8/keepanno/doctests/ConditionalMethodRulesAndHorizontalMergingTest.java
new file mode 100644
index 0000000..ee1748d
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/keepanno/doctests/ConditionalMethodRulesAndHorizontalMergingTest.java
@@ -0,0 +1,139 @@
+// Copyright (c) 2023, 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.keepanno.doctests;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.keepanno.doctests.ConditionalMethodRulesAndHorizontalMergingTest.Example1.BaseClass;
+import com.android.tools.r8.keepanno.doctests.ConditionalMethodRulesAndHorizontalMergingTest.Example1.MyHiddenMethodCaller;
+import com.android.tools.r8.keepanno.doctests.ConditionalMethodRulesAndHorizontalMergingTest.Example2.MyFieldValuePrinter;
+import com.android.tools.r8.keepanno.doctests.ConditionalMethodRulesAndHorizontalMergingTest.Example2.PrintableFieldInterface;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.StringUtils;
+import com.google.common.collect.ImmutableList;
+import java.lang.reflect.Field;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class ConditionalMethodRulesAndHorizontalMergingTest extends TestBase {
+
+ static final String EXPECTED =
+ StringUtils.lines("on Base", "on Sub", "intField = 42", "stringField = Hello!");
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withDefaultRuntimes().withApiLevel(AndroidApiLevel.B).build();
+ }
+
+ public ConditionalMethodRulesAndHorizontalMergingTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testReference() throws Exception {
+ testForRuntime(parameters)
+ .addProgramClasses(TestClass.class)
+ .addProgramClassesAndInnerClasses(getExampleClasses())
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addKeepMainRule(TestClass.class)
+ // The following rules are what was generated by the keepanno extraction for the doc tests.
+ .addKeepRules(
+ "-if class " + typeName(MyHiddenMethodCaller.class) + " {",
+ " void callHiddenMethod(" + typeName(BaseClass.class) + ");",
+ "}",
+ "-keepclassmembers class * extends " + typeName(BaseClass.class) + " {",
+ " *** hiddenMethod();",
+ "}",
+ "-if class " + typeName(MyHiddenMethodCaller.class) + " {",
+ " void callHiddenMethod(" + typeName(BaseClass.class) + ");",
+ "}",
+ "-keepclassmembers class " + typeName(BaseClass.class) + " {",
+ " *** hiddenMethod();",
+ "}",
+ "-if class " + typeName(MyFieldValuePrinter.class) + " {",
+ " void printFieldValues(" + typeName(PrintableFieldInterface.class) + ");",
+ "}",
+ "-keepclassmembers class * extends " + typeName(PrintableFieldInterface.class) + " {",
+ " *** *;",
+ "}"
+ // Removed the rule variant on the base interface as it does not have fields.
+ )
+ .addProgramClasses(TestClass.class)
+ .addProgramClassesAndInnerClasses(getExampleClasses())
+ .setMinApi(parameters)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ public List<Class<?>> getExampleClasses() {
+ return ImmutableList.of(Example1.class, Example2.class);
+ }
+
+ static class TestClass {
+ public static void main(String[] args) throws Exception {
+ Example1.run();
+ Example2.run();
+ }
+ }
+
+ static class Example1 {
+
+ static class BaseClass {
+ void hiddenMethod() {
+ System.out.println("on Base");
+ }
+ }
+
+ static class SubClass extends BaseClass {
+ void hiddenMethod() {
+ System.out.println("on Sub");
+ }
+ }
+
+ public static class MyHiddenMethodCaller {
+ public void callHiddenMethod(BaseClass base) throws Exception {
+ base.getClass().getDeclaredMethod("hiddenMethod").invoke(base);
+ }
+ }
+
+ static void run() throws Exception {
+ new MyHiddenMethodCaller().callHiddenMethod(new BaseClass());
+ new MyHiddenMethodCaller().callHiddenMethod(new SubClass());
+ }
+ }
+
+ static class Example2 {
+
+ interface PrintableFieldInterface {}
+
+ static class ClassWithFields implements PrintableFieldInterface {
+ final int intField = 42;
+ String stringField = "Hello!";
+ }
+
+ public static class MyFieldValuePrinter {
+ public void printFieldValues(PrintableFieldInterface objectWithFields) throws Exception {
+ for (Field field : objectWithFields.getClass().getDeclaredFields()) {
+ System.out.println(field.getName() + " = " + field.get(objectWithFields));
+ }
+ }
+ }
+
+ static void run() throws Exception {
+ new MyFieldValuePrinter().printFieldValues(new ClassWithFields());
+ }
+ }
+}
diff --git a/tools/compiledump.py b/tools/compiledump.py
index ce09db8..3abe651 100755
--- a/tools/compiledump.py
+++ b/tools/compiledump.py
@@ -95,6 +95,21 @@
'Disable Java assertions when running the compiler (default enabled)',
default=False,
action='store_true')
+ parser.add_argument(
+ '--enable-test-assertions',
+ '--enable_test_assertions',
+ help=
+ 'Enable additional test assertions when running the compiler (default disabled)',
+ default=False,
+ action='store_true')
+ parser.add_argument(
+ '--java-opts',
+ '--java-opts',
+ '-J',
+ metavar='<JVM argument(s)>',
+ default=[],
+ action='append',
+ help='Additional options to pass to JVM invocation')
parser.add_argument('--classfile',
help='Run with classfile output',
default=False,
@@ -559,6 +574,7 @@
cmd.append('-Xmx' + args.xmx)
if not args.disable_assertions:
cmd.append('-ea')
+ if args.enable_test_assertions:
cmd.append('-Dcom.android.tools.r8.enableTestAssertions=1')
if args.print_times:
cmd.append('-Dcom.android.tools.r8.printtimes=1')
@@ -567,6 +583,7 @@
if hasattr(args, 'properties'):
cmd.extend(args.properties)
cmd.extend(determine_properties(build_properties))
+ cmd.extend(args.java_opts)
cmd.extend(['-cp', '%s:%s' % (temp, jar)])
if compiler == 'd8':
prepare_d8_wrapper(jar, temp, jdkhome)
diff --git a/tools/run_on_app_dump.py b/tools/run_on_app_dump.py
index c06e878..964d1dc 100755
--- a/tools/run_on_app_dump.py
+++ b/tools/run_on_app_dump.py
@@ -743,6 +743,7 @@
'properties': app.compiler_properties,
'disable_desugared_lib': False,
'print_times': options.print_times,
+ 'java_opts': [],
})
app_jar = os.path.join(
diff --git a/tools/tag_versions.py b/tools/tag_versions.py
index 243625a..6f62ff7 100755
--- a/tools/tag_versions.py
+++ b/tools/tag_versions.py
@@ -125,7 +125,8 @@
]).decode('utf-8')
version = output.split(' ')[0]
run(args, ['git', 'tag', '-f', tag, '-m', tag, '%s^{}' % version])
- run(args, ['git', 'push', 'origin', tag])
+ run(args, ['git', 'push', '-o', 'push-justification=b/313360935',
+ 'origin', tag])
def tag_r8_branch(branch, args):
@@ -145,7 +146,8 @@
result = get_tag_info_on_origin(version)
if not result:
run(args, ['git', 'tag', '-a', version, '-m', version, hash])
- run(args, ['git', 'push', 'origin', version])
+ run(args, ['git', 'push', '-o', 'push-justification=b/313360935',
+ 'origin', version])
if args.dry_run:
print('Dry run complete. None of the above have been executed.')
