Merge "Add latest gmscore build to x20 dependencies."
diff --git a/src/main/java/com/android/tools/r8/graph/GraphLense.java b/src/main/java/com/android/tools/r8/graph/GraphLense.java
index e6c2d00..b9173dd 100644
--- a/src/main/java/com/android/tools/r8/graph/GraphLense.java
+++ b/src/main/java/com/android/tools/r8/graph/GraphLense.java
@@ -6,6 +6,19 @@
import java.util.IdentityHashMap;
import java.util.Map;
+/**
+ * A GraphLense implements a virtual view on top of the graph, used to delay global rewrites until
+ * later IR processing stages.
+ * <p>
+ * Valid remappings are limited to the following operations:
+ * <ul>
+ * <li>Mapping a classes type to one of the super/subtypes.</li>
+ * <li>Renaming private methods/fields.</li>
+ * <li>Moving methods/fields to a super/subclass.</li>
+ * <li>Replacing method/field references by the same method/field on a super/subtype</li>
+ * </ul>
+ * Note that the latter two have to take visibility into account.
+ */
public abstract class GraphLense {
public static class Builder {
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 8bb31d2..2bef174 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
@@ -694,7 +694,7 @@
StringBuilder builder, List<BasicBlock> list, Function<BasicBlock, String> postfix) {
if (list.size() > 0) {
for (BasicBlock block : list) {
- builder.append(block.getNumber());
+ builder.append(block.number >= 0 ? block.number : "<unknown>");
builder.append(postfix.apply(block));
builder.append(" ");
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
index 7e47ed2..b0ab940 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
@@ -277,7 +277,7 @@
// Check that we don't ever have instructions that can throw and have targets.
assert !dex.canThrow();
for (int relativeOffset : targets) {
- builder.ensureSuccessorBlock(offset + relativeOffset);
+ builder.ensureNormalSuccessorBlock(offset, offset + relativeOffset);
}
return true;
}
@@ -297,7 +297,7 @@
builder.ensureBlockWithoutEnqueuing(tryRangeStartAddress);
// Edge to exceptional successors.
for (Integer handlerOffset : getUniqueTryHandlerOffsets(tryRange)) {
- builder.ensureSuccessorBlock(handlerOffset);
+ builder.ensureExceptionalSuccessorBlock(offset, handlerOffset);
}
// If the following instruction is a move-result include it in this (the invokes) block.
if (index + 1 < code.instructions.length && isMoveResult(code.instructions[index + 1])) {
@@ -307,7 +307,7 @@
}
// Edge to normal successor if any (fallthrough).
if (!(dex instanceof Throw)) {
- builder.ensureSuccessorBlock(dex.getOffset() + dex.getSize());
+ builder.ensureNormalSuccessorBlock(offset, dex.getOffset() + dex.getSize());
}
return true;
}
@@ -319,9 +319,9 @@
switchPayloadResolver.addPayloadUser(dex);
for (int target : switchPayloadResolver.absoluteTargets(dex)) {
- builder.ensureSuccessorBlock(target);
+ builder.ensureNormalSuccessorBlock(offset, target);
}
- builder.ensureSuccessorBlock(offset + dex.getSize());
+ builder.ensureNormalSuccessorBlock(offset, offset + dex.getSize());
return true;
}
// TODO(zerny): Remove this from block computation.
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
index bd8a469..b3d5266 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
@@ -83,7 +83,14 @@
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.StringUtils.BraceType;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceAVLTreeMap;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceSortedMap;
+import it.unimi.dsi.fastutil.ints.IntArraySet;
+import it.unimi.dsi.fastutil.ints.IntIterator;
+import it.unimi.dsi.fastutil.ints.IntSet;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
@@ -101,6 +108,8 @@
*/
public class IRBuilder {
+ private static final int INITIAL_BLOCK_OFFSET = -1;
+
// SSA construction uses a worklist of basic blocks reachable from the entry and their
// instruction offsets.
private static class WorklistItem {
@@ -158,8 +167,64 @@
}
}
+ private static class BlockInfo {
+ BasicBlock block = new BasicBlock();
+ IntSet normalPredecessors = new IntArraySet();
+ IntSet normalSuccessors = new IntArraySet();
+ IntSet exceptionalPredecessors = new IntArraySet();
+ IntSet exceptionalSuccessors = new IntArraySet();
+
+ void addNormalPredecessor(int offset) {
+ normalPredecessors.add(offset);
+ }
+
+ void addNormalSuccessor(int offset) {
+ normalSuccessors.add(offset);
+ }
+
+ void replaceNormalPredecessor(int existing, int replacement) {
+ normalPredecessors.remove(existing);
+ normalPredecessors.add(replacement);
+ }
+
+ void addExceptionalPredecessor(int offset) {
+ exceptionalPredecessors.add(offset);
+ }
+
+ void addExceptionalSuccessor(int offset) {
+ exceptionalSuccessors.add(offset);
+ }
+
+ int predecessorCount() {
+ return normalPredecessors.size() + exceptionalPredecessors.size();
+ }
+
+ BlockInfo split(
+ int blockStartOffset, int fallthroughOffset, Int2ReferenceMap<BlockInfo> targets) {
+ BlockInfo fallthroughInfo = new BlockInfo();
+ fallthroughInfo.normalPredecessors = new IntArraySet(Collections.singleton(blockStartOffset));
+ fallthroughInfo.block.incrementUnfilledPredecessorCount();
+ // Move all normal successors to the fallthrough block.
+ IntIterator normalSuccessorIterator = normalSuccessors.iterator();
+ while (normalSuccessorIterator.hasNext()) {
+ BlockInfo normalSuccessor = targets.get(normalSuccessorIterator.nextInt());
+ normalSuccessor.replaceNormalPredecessor(blockStartOffset, fallthroughOffset);
+ }
+ fallthroughInfo.normalSuccessors = normalSuccessors;
+ normalSuccessors = new IntArraySet(Collections.singleton(fallthroughOffset));
+ // Copy all exceptional successors to the fallthrough block.
+ IntIterator exceptionalSuccessorIterator = fallthroughInfo.exceptionalSuccessors.iterator();
+ while (exceptionalSuccessorIterator.hasNext()) {
+ BlockInfo exceptionalSuccessor = targets.get(exceptionalSuccessorIterator.nextInt());
+ exceptionalSuccessor.addExceptionalPredecessor(fallthroughOffset);
+ }
+ fallthroughInfo.exceptionalSuccessors = new IntArraySet(this.exceptionalSuccessors);
+ return fallthroughInfo;
+ }
+ }
+
// Mapping from instruction offsets to basic-block targets.
- private final Map<Integer, BasicBlock> targets = new HashMap<>();
+ private final Int2ReferenceSortedMap<BlockInfo> targets = new Int2ReferenceAVLTreeMap<>();
// Worklist of reachable blocks.
private final Queue<Integer> traceBlocksWorklist = new LinkedList<>();
@@ -239,13 +304,8 @@
assert source != null;
source.setUp();
- // Create entry block.
- setCurrentBlock(new BasicBlock());
-
- // If the method needs a prelude, the entry block must never be the target of other blocks.
- if (!source.needsPrelude()) {
- targets.put(0, currentBlock);
- }
+ // Create entry block (at a non-targetable address).
+ targets.put(INITIAL_BLOCK_OFFSET, new BlockInfo());
// Process reachable code paths starting from instruction 0.
processedInstructions = new boolean[source.instructionCount()];
@@ -267,8 +327,8 @@
// If the next instruction starts a block, fall through to it.
if (index + 1 < source.instructionCount()) {
int nextOffset = source.instructionOffset(index + 1);
- if (getTarget(nextOffset) != null) {
- ensureSuccessorBlock(nextOffset);
+ if (targets.get(nextOffset) != null) {
+ ensureNormalSuccessorBlock(startOfBlockOffset, nextOffset);
break;
}
}
@@ -276,6 +336,7 @@
}
processedInstructions = null;
+ setCurrentBlock(targets.get(INITIAL_BLOCK_OFFSET).block);
source.buildPrelude(this);
// Process normal blocks reachable from the entry block using a worklist of reachable
@@ -340,11 +401,36 @@
private boolean verifyFilledPredecessors() {
for (BasicBlock block : blocks) {
- assert block.verifyFilledPredecessors();
+ assert verifyFilledPredecessors(block);
}
return true;
}
+ private boolean verifyFilledPredecessors(BasicBlock block) {
+ assert block.verifyFilledPredecessors();
+ // TODO(zerny): Consider moving the validation of the initial control-flow graph to after its
+ // construction and prior to building the IR.
+ for (BlockInfo info : targets.values()) {
+ if (info != null && info.block == block) {
+ assert info.predecessorCount() == block.getPredecessors().size();
+ assert info.normalSuccessors.size() == block.getNormalSucessors().size();
+ if (block.hasCatchHandlers()) {
+ assert info.exceptionalSuccessors.size()
+ == block.getCatchHandlers().getUniqueTargets().size();
+ } else {
+ assert !block.canThrow()
+ || info.exceptionalSuccessors.isEmpty()
+ || (info.exceptionalSuccessors.size() == 1
+ && info.exceptionalSuccessors.iterator().nextInt() < 0);
+ }
+ return true;
+ }
+ }
+ // There are places where we add in new blocks that we do not represent in the initial CFG.
+ // TODO(zerny): Should we maintain the initial CFG after instruction building?
+ return true;
+ }
+
private int processWorklist(int blockNumber) {
for (WorklistItem item = ssaWorklist.poll(); item != null; item = ssaWorklist.poll()) {
if (item.block.isFilled()) {
@@ -359,11 +445,11 @@
source.closedCurrentBlock();
break;
}
- BasicBlock block = getTarget(source.instructionOffset(i));
- if (block != null && block != currentBlock) {
- closeCurrentBlockWithFallThrough(block);
+ BlockInfo info = targets.get(source.instructionOffset(i));
+ if (info != null && info.block != currentBlock) {
+ closeCurrentBlockWithFallThrough(info.block);
source.closedCurrentBlockWithFallthrough(i);
- addToWorklist(block, i);
+ addToWorklist(info.block, i);
break;
}
source.buildInstruction(this, i);
@@ -1151,7 +1237,8 @@
// If this was a packed switch with only fallthrough cases we can make it a goto.
// Oddly, this does happen.
if (numberOfFallthroughs == numberOfTargets) {
- targets.get(fallthroughOffset).decrementUnfilledPredecessorCount(numberOfFallthroughs);
+ BlockInfo info = targets.get(fallthroughOffset);
+ info.block.decrementUnfilledPredecessorCount(numberOfFallthroughs);
addGoto(fallthroughOffset);
return;
}
@@ -1161,7 +1248,8 @@
int bytesSaved = packedSwitchPayloadSize - sparseSwitchPayloadSize;
// Perform the rewrite if we can reduce the payload size by more than 20%.
if (bytesSaved > (packedSwitchPayloadSize / 5)) {
- targets.get(fallthroughOffset).decrementUnfilledPredecessorCount(numberOfFallthroughs);
+ BlockInfo info = targets.get(fallthroughOffset);
+ info.block.decrementUnfilledPredecessorCount(numberOfFallthroughs);
int nextCaseIndex = 0;
int currentKey = keys[0];
keys = new int[numberOfSparseTargets];
@@ -1458,10 +1546,16 @@
BasicBlock block = new BasicBlock();
blocks.add(block);
block.incrementUnfilledPredecessorCount();
- for (Integer offset : source.getCurrentCatchHandlers().getUniqueTargets()) {
- BasicBlock target = getTarget(offset);
- assert !target.isSealed();
- target.incrementUnfilledPredecessorCount();
+ int freshOffset = INITIAL_BLOCK_OFFSET - 1;
+ while (targets.containsKey(freshOffset)) {
+ freshOffset--;
+ }
+ targets.put(freshOffset, null);
+ for (int offset : source.getCurrentCatchHandlers().getUniqueTargets()) {
+ BlockInfo target = targets.get(offset);
+ assert !target.block.isSealed();
+ target.block.incrementUnfilledPredecessorCount();
+ target.addExceptionalPredecessor(freshOffset);
}
addInstruction(new Goto());
currentBlock.link(block);
@@ -1499,21 +1593,32 @@
// Package (ie, SourceCode accessed) helpers.
// Ensure there is a block starting at offset.
- BasicBlock ensureBlockWithoutEnqueuing(int offset) {
- BasicBlock block = targets.get(offset);
- if (block == null) {
- block = new BasicBlock();
- targets.put(offset, block);
+ BlockInfo ensureBlockWithoutEnqueuing(int offset) {
+ assert offset != INITIAL_BLOCK_OFFSET;
+ BlockInfo info = targets.get(offset);
+ if (info == null) {
// If this is a processed instruction, the block split and it has a fall-through predecessor.
if (offset >= 0 && isOffsetProcessed(offset)) {
- block.incrementUnfilledPredecessorCount();
+ int blockStartOffset = getBlockStartOffset(offset);
+ BlockInfo existing = targets.get(blockStartOffset);
+ info = existing.split(blockStartOffset, offset, targets);
+ } else {
+ info = new BlockInfo();
}
+ targets.put(offset, info);
}
- return block;
+ return info;
+ }
+
+ private int getBlockStartOffset(int offset) {
+ if (targets.containsKey(offset)) {
+ return offset;
+ }
+ return targets.headMap(offset).lastIntKey();
}
// Ensure there is a block starting at offset and add it to the work-list if it needs processing.
- private BasicBlock ensureBlock(int offset) {
+ private BlockInfo ensureBlock(int offset) {
// We don't enqueue negative targets (these are special blocks, eg, an argument prelude).
if (offset >= 0 && !isOffsetProcessed(offset)) {
traceBlocksWorklist.add(offset);
@@ -1550,16 +1655,32 @@
}
// Ensure there is a block at offset and add a predecessor to it.
- BasicBlock ensureSuccessorBlock(int offset) {
- BasicBlock block = ensureBlock(offset);
- block.incrementUnfilledPredecessorCount();
- return block;
+ private void ensureSuccessorBlock(int sourceOffset, int targetOffset, boolean normal) {
+ BlockInfo targetInfo = ensureBlock(targetOffset);
+ int sourceStartOffset = getBlockStartOffset(sourceOffset);
+ BlockInfo sourceInfo = targets.get(sourceStartOffset);
+ if (normal) {
+ sourceInfo.addNormalSuccessor(targetOffset);
+ targetInfo.addNormalPredecessor(sourceStartOffset);
+ } else {
+ sourceInfo.addExceptionalSuccessor(targetOffset);
+ targetInfo.addExceptionalPredecessor(sourceStartOffset);
+ }
+ targetInfo.block.incrementUnfilledPredecessorCount();
+ }
+
+ void ensureNormalSuccessorBlock(int sourceOffset, int targetOffset) {
+ ensureSuccessorBlock(sourceOffset, targetOffset, true);
+ }
+
+ void ensureExceptionalSuccessorBlock(int sourceOffset, int targetOffset) {
+ ensureSuccessorBlock(sourceOffset, targetOffset, false);
}
// Private block helpers.
private BasicBlock getTarget(int offset) {
- return targets.get(offset);
+ return targets.get(offset).block;
}
private void closeCurrentBlock() {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
index 5647878..1d8f18d 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
@@ -66,7 +66,6 @@
import org.objectweb.asm.util.Textifier;
import org.objectweb.asm.util.TraceMethodVisitor;
-
public class JarSourceCode implements SourceCode {
// Try-catch block wrapper containing resolved offsets.
@@ -144,7 +143,7 @@
// thus that the monitor-entry prelude (part of the argument block which must not have a try-catch
// successor) is not joined with the first instruction block (which likely will have a try-catch
// successor).
- private static final int EXCEPTIONAL_SYNC_EXIT_OFFSET = -1;
+ private static final int EXCEPTIONAL_SYNC_EXIT_OFFSET = -2;
private static final TryCatchBlock EXCEPTIONAL_SYNC_EXIT =
new TryCatchBlock(EXCEPTIONAL_SYNC_EXIT_OFFSET, 0, Integer.MAX_VALUE, null);
@@ -534,7 +533,7 @@
if (targets != NO_TARGETS) {
assert !canThrow(insn);
for (int target : targets) {
- builder.ensureSuccessorBlock(target);
+ builder.ensureNormalSuccessorBlock(index, target);
}
return true;
}
@@ -549,12 +548,12 @@
int handler = tryCatchBlock.getHandler();
if (!seenHandlerOffsets.contains(handler)) {
seenHandlerOffsets.add(handler);
- builder.ensureSuccessorBlock(handler);
+ builder.ensureExceptionalSuccessorBlock(index, handler);
}
}
// Edge to normal successor if any (fallthrough).
if (!isThrow(insn)) {
- builder.ensureSuccessorBlock(getOffset(insn.getNext()));
+ builder.ensureNormalSuccessorBlock(index, getOffset(insn.getNext()));
}
return true;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
index b122d23..df5cb27 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
@@ -236,8 +236,7 @@
private boolean isFieldRead(DexEncodedField field, boolean isStatic) {
// Without live set information we cannot tell and assume true.
if (liveSet == null
- || isStatic && liveSet.staticFieldsRead.contains(field.field)
- || !isStatic && liveSet.instanceFieldsRead.contains(field.field)
+ || liveSet.fieldsRead.contains(field.field)
|| liveSet.pinnedItems.contains(field)) {
return true;
}
diff --git a/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java b/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
index 88d371b..17da59b 100644
--- a/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
+++ b/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
@@ -4,7 +4,6 @@
package com.android.tools.r8.optimize;
import com.android.tools.r8.errors.Unreachable;
-import com.android.tools.r8.graph.Descriptor;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
@@ -98,11 +97,6 @@
return searchClass.type;
}
- private boolean isNotFromLibrary(Descriptor item) {
- DexClass clazz = appInfo.definitionFor(item.getHolder());
- return clazz != null && !clazz.isLibraryClass();
- }
-
private DexEncodedMethod virtualLookup(DexMethod method) {
return appInfo.lookupVirtualDefinition(method.getHolder(), method);
}
@@ -190,13 +184,23 @@
for (DexField field : fields) {
field = lense.lookupField(field, null);
DexEncodedField target = lookup.apply(field.getHolder(), field);
- // Rebind to the lowest library class or program class.
- if (target != null && target.field != field) {
+ // Rebind to the lowest library class or program class. Do not rebind accesses to fields that
+ // are not public, as this might lead to access violation errors.
+ if (target != null && target.field != field && isVisibleFromOtherClasses(target)) {
builder.map(field, validTargetFor(target.field, field, lookupTargetOnClass));
}
}
}
+ private boolean isVisibleFromOtherClasses(DexEncodedField field) {
+ // If the field is not public, the visibility on the class can not be a further constraint.
+ if (!field.accessFlags.isPublic()) {
+ return true;
+ }
+ // If the field is public, then a non-public holder class will further constrain visibility.
+ return appInfo.definitionFor(field.field.getHolder()).accessFlags.isPublic();
+ }
+
private static void privateMethodsCheck(DexProgramClass clazz, DexEncodedMethod method) {
throw new Unreachable("Direct invokes should not require forwarding.");
}
@@ -211,9 +215,9 @@
computeMethodRebinding(appInfo.staticInvokes, appInfo::lookupStaticTarget,
DexClass::findDirectTarget, DexProgramClass::addStaticMethod);
- computeFieldRebinding(Sets.union(appInfo.staticFieldsRead, appInfo.staticFieldsWritten),
+ computeFieldRebinding(Sets.union(appInfo.staticFieldReads, appInfo.staticFieldWrites),
appInfo::lookupStaticTarget, DexClass::findStaticTarget);
- computeFieldRebinding(Sets.union(appInfo.instanceFieldsRead, appInfo.instanceFieldsWritten),
+ computeFieldRebinding(Sets.union(appInfo.instanceFieldReads, appInfo.instanceFieldWrites),
appInfo::lookupInstanceTarget, DexClass::findInstanceTarget);
return builder.build(lense, appInfo.dexItemFactory);
}
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index 2dab755..abdda65 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -37,6 +37,7 @@
import com.google.common.collect.Sets;
import com.google.common.collect.Sets.SetView;
import java.util.ArrayDeque;
+import java.util.Collection;
import java.util.Collections;
import java.util.Deque;
import java.util.HashMap;
@@ -47,6 +48,7 @@
import java.util.Queue;
import java.util.Set;
import java.util.function.BiFunction;
+import java.util.function.Function;
import java.util.stream.Collectors;
/**
@@ -856,9 +858,8 @@
}
private Set<DexField> collectFields(Map<DexType, Set<DexField>> map) {
- Set<DexField> set = Sets.newIdentityHashSet();
- map.values().forEach(set::addAll);
- return set;
+ return map.values().stream().flatMap(Collection::stream)
+ .collect(Collectors.toCollection(Sets::newIdentityHashSet));
}
Set<DexField> collectInstanceFieldsRead() {
@@ -877,6 +878,32 @@
return Collections.unmodifiableSet(collectFields(staticFieldsWritten));
}
+ private Set<DexField> collectReachedFields(Map<DexType, Set<DexField>> map,
+ Function<DexField, DexField> lookup) {
+ return map.values().stream().flatMap(set -> set.stream().map(lookup))
+ .collect(Collectors.toCollection(Sets::newIdentityHashSet));
+ }
+
+ private DexField tryLookupInstanceField(DexField field) {
+ DexEncodedField target = appInfo.lookupInstanceTarget(field.clazz, field);
+ return target == null ? field : target.field;
+ }
+
+ private DexField tryLookupStaticField(DexField field) {
+ DexEncodedField target = appInfo.lookupStaticTarget(field.clazz, field);
+ return target == null ? field : target.field;
+ }
+
+ Set<DexField> collectFieldsRead() {
+ return Sets.union(collectReachedFields(instanceFieldsRead, this::tryLookupInstanceField),
+ collectReachedFields(staticFieldsRead, this::tryLookupStaticField));
+ }
+
+ Set<DexField> collectFieldsWritten() {
+ return Sets.union(collectReachedFields(instanceFieldsWritten, this::tryLookupInstanceField),
+ collectReachedFields(staticFieldsWritten, this::tryLookupStaticField));
+ }
+
private static class Action {
final Kind kind;
@@ -981,31 +1008,29 @@
*/
final Set<DexField> liveFields;
/**
- * Set of all fields which are read. Combines {@link #instanceFieldsRead} and
- * {@link #staticFieldsRead}.
+ * Set of all fields which may be touched by a get operation. This is actual field definitions.
*/
public final Set<DexField> fieldsRead;
/**
- * Set of all fields which are written. Combines {@link #instanceFieldsWritten} and
- * {@link #staticFieldsWritten}.
+ * Set of all fields which may be touched by a put operation. This is actual field definitions.
*/
public final Set<DexField> fieldsWritten;
/**
- * Set of all instance fields which are read.
+ * Set of all field ids used in instance field reads.
*/
- public final Set<DexField> instanceFieldsRead;
+ public final Set<DexField> instanceFieldReads;
/**
- * Set of all instance fields which are written.
+ * Set of all field ids used in instance field writes.
*/
- public final Set<DexField> instanceFieldsWritten;
+ public final Set<DexField> instanceFieldWrites;
/**
- * Set of all static fields which are read.
+ * Set of all field ids used in static static field reads.
*/
- public final Set<DexField> staticFieldsRead;
+ public final Set<DexField> staticFieldReads;
/**
- * Set of all static fields which are written.
+ * Set of all field ids used in static field writes.
*/
- public final Set<DexField> staticFieldsWritten;
+ public final Set<DexField> staticFieldWrites;
/**
* Set of all methods referenced in virtual invokes;
*/
@@ -1042,12 +1067,12 @@
this.targetedMethods = toDescriptorSet(enqueuer.targetedMethods.getItems());
this.liveMethods = toDescriptorSet(enqueuer.liveMethods.getItems());
this.liveFields = toDescriptorSet(enqueuer.liveFields.getItems());
- this.instanceFieldsRead = enqueuer.collectInstanceFieldsRead();
- this.instanceFieldsWritten = enqueuer.collectInstanceFieldsWritten();
- this.staticFieldsRead = enqueuer.collectStaticFieldsRead();
- this.staticFieldsWritten = enqueuer.collectStaticFieldsWritten();
- this.fieldsRead = Sets.union(staticFieldsRead, instanceFieldsRead);
- this.fieldsWritten = Sets.union(staticFieldsWritten, instanceFieldsWritten);
+ this.instanceFieldReads = enqueuer.collectInstanceFieldsRead();
+ this.instanceFieldWrites = enqueuer.collectInstanceFieldsWritten();
+ this.staticFieldReads = enqueuer.collectStaticFieldsRead();
+ this.staticFieldWrites = enqueuer.collectStaticFieldsWritten();
+ this.fieldsRead = enqueuer.collectFieldsRead();
+ this.fieldsWritten = enqueuer.collectFieldsWritten();
this.pinnedItems = Collections.unmodifiableSet(enqueuer.pinnedItems);
this.virtualInvokes = joinInvokedMethods(enqueuer.virtualInvokes);
this.superInvokes = joinInvokedMethods(enqueuer.superInvokes);
@@ -1055,8 +1080,8 @@
this.staticInvokes = joinInvokedMethods(enqueuer.staticInvokes);
this.noSideEffects = enqueuer.rootSet.noSideEffects;
this.assumedValues = enqueuer.rootSet.assumedValues;
- assert Sets.intersection(instanceFieldsRead, staticFieldsRead).size() == 0;
- assert Sets.intersection(instanceFieldsWritten, staticFieldsWritten).size() == 0;
+ assert Sets.intersection(instanceFieldReads, staticFieldReads).size() == 0;
+ assert Sets.intersection(instanceFieldWrites, staticFieldWrites).size() == 0;
}
private AppInfoWithLiveness(AppInfoWithLiveness previous, DexApplication application) {
@@ -1066,11 +1091,12 @@
this.targetedMethods = previous.targetedMethods;
this.liveMethods = previous.liveMethods;
this.liveFields = previous.liveFields;
- this.instanceFieldsRead = previous.instanceFieldsRead;
- this.instanceFieldsWritten = previous.instanceFieldsWritten;
- this.staticFieldsRead = previous.staticFieldsRead;
- this.staticFieldsWritten = previous.staticFieldsWritten;
+ this.instanceFieldReads = previous.instanceFieldReads;
+ this.instanceFieldWrites = previous.instanceFieldWrites;
+ this.staticFieldReads = previous.staticFieldReads;
+ this.staticFieldWrites = previous.staticFieldWrites;
this.fieldsRead = previous.fieldsRead;
+ // TODO(herhut): We remove fields that are only written, so maybe update this.
this.fieldsWritten = previous.fieldsWritten;
this.pinnedItems = previous.pinnedItems;
this.noSideEffects = previous.noSideEffects;
@@ -1079,8 +1105,8 @@
this.superInvokes = previous.superInvokes;
this.directInvokes = previous.directInvokes;
this.staticInvokes = previous.staticInvokes;
- assert Sets.intersection(instanceFieldsRead, staticFieldsRead).size() == 0;
- assert Sets.intersection(instanceFieldsWritten, staticFieldsWritten).size() == 0;
+ assert Sets.intersection(instanceFieldReads, staticFieldReads).size() == 0;
+ assert Sets.intersection(instanceFieldWrites, staticFieldWrites).size() == 0;
}
private AppInfoWithLiveness(AppInfoWithLiveness previous, GraphLense lense) {
@@ -1090,12 +1116,12 @@
this.targetedMethods = rewriteItems(previous.targetedMethods, lense::lookupMethod);
this.liveMethods = rewriteItems(previous.liveMethods, lense::lookupMethod);
this.liveFields = rewriteItems(previous.liveFields, lense::lookupField);
- this.instanceFieldsRead = rewriteItems(previous.instanceFieldsRead, lense::lookupField);
- this.instanceFieldsWritten = rewriteItems(previous.instanceFieldsWritten, lense::lookupField);
- this.staticFieldsRead = rewriteItems(previous.staticFieldsRead, lense::lookupField);
- this.staticFieldsWritten = rewriteItems(previous.staticFieldsWritten, lense::lookupField);
- this.fieldsRead = Sets.union(staticFieldsRead, instanceFieldsRead);
- this.fieldsWritten = Sets.union(staticFieldsWritten, instanceFieldsWritten);
+ this.instanceFieldReads = rewriteItems(previous.instanceFieldReads, lense::lookupField);
+ this.instanceFieldWrites = rewriteItems(previous.instanceFieldWrites, lense::lookupField);
+ this.staticFieldReads = rewriteItems(previous.staticFieldReads, lense::lookupField);
+ this.staticFieldWrites = rewriteItems(previous.staticFieldWrites, lense::lookupField);
+ this.fieldsRead = rewriteItems(previous.fieldsRead, lense::lookupField);
+ this.fieldsWritten = rewriteItems(previous.fieldsWritten, lense::lookupField);
// TODO(herhut): Migrate these to Descriptors, as well.
this.pinnedItems = previous.pinnedItems;
this.noSideEffects = previous.noSideEffects;
@@ -1104,8 +1130,8 @@
this.superInvokes = rewriteItems(previous.superInvokes, lense::lookupMethod);
this.directInvokes = rewriteItems(previous.directInvokes, lense::lookupMethod);
this.staticInvokes = rewriteItems(previous.staticInvokes, lense::lookupMethod);
- assert Sets.intersection(instanceFieldsRead, staticFieldsRead).size() == 0;
- assert Sets.intersection(instanceFieldsWritten, staticFieldsWritten).size() == 0;
+ assert Sets.intersection(instanceFieldReads, staticFieldReads).size() == 0;
+ assert Sets.intersection(instanceFieldWrites, staticFieldWrites).size() == 0;
}
private Set<DexMethod> joinInvokedMethods(Map<DexType, Set<DexMethod>> invokes) {
diff --git a/src/main/java/com/android/tools/r8/shaking/SimpleClassMerger.java b/src/main/java/com/android/tools/r8/shaking/SimpleClassMerger.java
index a2864e4..d03c74b 100644
--- a/src/main/java/com/android/tools/r8/shaking/SimpleClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/SimpleClassMerger.java
@@ -27,9 +27,7 @@
import com.android.tools.r8.utils.Timing;
import com.google.common.base.Equivalence;
import com.google.common.base.Equivalence.Wrapper;
-import com.google.common.collect.Iterables;
import com.google.common.collect.Iterators;
-import com.google.common.collect.Sets;
import it.unimi.dsi.fastutil.ints.Int2IntMap;
import it.unimi.dsi.fastutil.ints.Int2IntOpenHashMap;
import it.unimi.dsi.fastutil.objects.Reference2IntMap;
@@ -44,6 +42,7 @@
import java.util.Map;
import java.util.Set;
import java.util.function.BiFunction;
+import java.util.stream.Collectors;
/**
* Merges Supertypes with a single implementation into their single subtype.
@@ -61,7 +60,7 @@
private final GraphLense.Builder renamedMembersLense = GraphLense.builder();
private final Map<DexType, DexType> mergedClasses = new IdentityHashMap<>();
private final Timing timing;
- private Set<DexMethod> invokes;
+ private Collection<DexMethod> invokes;
private int numberOfMerges = 0;
public SimpleClassMerger(DexApplication application, AppInfoWithLiveness appInfo,
@@ -81,25 +80,51 @@
&& clazz.type.getSingleSubtype() != null;
}
- private Set<DexMethod> getInvokes() {
- if (invokes == null) {
+ private void addProgramMethods(Set<Wrapper<DexMethod>> set, DexMethod method,
+ Equivalence<DexMethod> equivalence) {
+ DexClass definition = appInfo.definitionFor(method.holder);
+ if (definition != null && definition.isProgramClass()) {
+ set.add(equivalence.wrap(method));
+ }
+ }
- // TODO(herhut): Ignore invokes into the library, as those can only reference library types.
- invokes = Sets.newIdentityHashSet();
- invokes.addAll(appInfo.directInvokes);
- invokes.addAll(appInfo.staticInvokes);
- invokes.addAll(appInfo.superInvokes);
- invokes.addAll(appInfo.virtualInvokes);
- invokes.addAll(appInfo.targetedMethods);
- invokes.addAll(appInfo.liveMethods);
- for (DexEncodedMethod method : Iterables
- .filter(appInfo.pinnedItems, DexEncodedMethod.class)) {
- invokes.add(method.method);
- }
+ private Collection<DexMethod> getInvokes() {
+ if (invokes == null) {
+ // Collect all reachable methods that are not within a library class. Those defined on
+ // library classes are known not to have program classes in their signature.
+ // Also filter methods that only use types from library classes in their signatures. We
+ // know that those won't conflict.
+ Set<Wrapper<DexMethod>> filteredInvokes = new HashSet<>();
+ Equivalence<DexMethod> equivalence = MethodSignatureEquivalence.get();
+ appInfo.targetedMethods.forEach(m -> addProgramMethods(filteredInvokes, m, equivalence));
+ invokes = filteredInvokes.stream().map(Wrapper::get).filter(this::removeNonProgram)
+ .collect(Collectors.toList());
}
return invokes;
}
+ private boolean isProgramClass(DexType type) {
+ if (type.isArrayType()) {
+ type = type.toBaseType(appInfo.dexItemFactory);
+ }
+ if (type.isClassType()) {
+ DexClass clazz = appInfo.definitionFor(type);
+ if (clazz != null && clazz.isProgramClass()) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private boolean removeNonProgram(DexMethod dexMethod) {
+ for (DexType type : dexMethod.proto.parameters.values) {
+ if (isProgramClass(type)) {
+ return true;
+ }
+ }
+ return isProgramClass(dexMethod.proto.returnType);
+ }
+
public GraphLense run() {
timing.begin("merge");
GraphLense mergingGraphLense = mergeClasses(graphLense);
@@ -426,7 +451,8 @@
if (previous != null) {
if (!previous.accessFlags.isBridge()) {
if (!method.accessFlags.isBridge()) {
- throw new CompilationError("Class merging produced invalid result.");
+ throw new CompilationError(
+ "Class merging produced invalid result on: " + previous.toSourceString());
} else {
filtered.put(previous.method, previous);
}
diff --git a/src/test/examples/memberrebinding/Test.java b/src/test/examples/memberrebinding/Memberrebinding.java
similarity index 97%
rename from src/test/examples/memberrebinding/Test.java
rename to src/test/examples/memberrebinding/Memberrebinding.java
index 62a2440..74c99fd 100644
--- a/src/test/examples/memberrebinding/Test.java
+++ b/src/test/examples/memberrebinding/Memberrebinding.java
@@ -6,7 +6,7 @@
import memberrebinding.subpackage.PublicClass;
import memberrebindinglib.AnIndependentInterface;
-public class Test {
+public class Memberrebinding {
public static void main(String[] args) {
ClassAtBottomOfChain bottomInstance = new ClassAtBottomOfChain();
diff --git a/src/test/examples/memberrebinding2/Test.java b/src/test/examples/memberrebinding2/Memberrebinding.java
similarity index 96%
rename from src/test/examples/memberrebinding2/Test.java
rename to src/test/examples/memberrebinding2/Memberrebinding.java
index d4b005e..13cde8c 100644
--- a/src/test/examples/memberrebinding2/Test.java
+++ b/src/test/examples/memberrebinding2/Memberrebinding.java
@@ -5,7 +5,7 @@
import memberrebinding2.subpackage.PublicClass;
-public class Test {
+public class Memberrebinding {
public static void main(String[] args) {
ClassAtBottomOfChain bottomInstance = new ClassAtBottomOfChain();
diff --git a/src/test/examples/memberrebinding2/keep-rules.txt b/src/test/examples/memberrebinding2/keep-rules.txt
new file mode 100644
index 0000000..ae40c43
--- /dev/null
+++ b/src/test/examples/memberrebinding2/keep-rules.txt
@@ -0,0 +1,12 @@
+# Copyright (c) 2016, 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.
+
+# Keep the application entry point. Get rid of everything that is not
+# reachable from there.
+-keep public class memberrebinding2.Memberrebinding {
+ public static void main(...);
+}
+
+# Remove once b/62048823 is fixed.
+-allowaccessmodification
\ No newline at end of file
diff --git a/src/test/examples/memberrebinding3/Test.java b/src/test/examples/memberrebinding3/Memberrebinding.java
similarity index 78%
rename from src/test/examples/memberrebinding3/Test.java
rename to src/test/examples/memberrebinding3/Memberrebinding.java
index d37f263..83d8bbb 100644
--- a/src/test/examples/memberrebinding3/Test.java
+++ b/src/test/examples/memberrebinding3/Memberrebinding.java
@@ -3,16 +3,19 @@
// BSD-style license that can be found in the LICENSE file.
package memberrebinding3;
-public class Test extends ClassAtBottomOfChain {
+public class Memberrebinding extends ClassAtBottomOfChain {
+ @Override
void bottomMethod() {
}
+ @Override
void middleMethod() {
}
+ @Override
void topMethod() {
}
@@ -24,6 +27,6 @@
}
public static void main(String[] args) {
- new Test().test();
+ new Memberrebinding().test();
}
}
diff --git a/src/test/examplesAndroidN/memberrebinding4/Test.java b/src/test/examplesAndroidN/memberrebinding4/Memberrebinding.java
similarity index 93%
rename from src/test/examplesAndroidN/memberrebinding4/Test.java
rename to src/test/examplesAndroidN/memberrebinding4/Memberrebinding.java
index 6fc2ad5..a699537 100644
--- a/src/test/examplesAndroidN/memberrebinding4/Test.java
+++ b/src/test/examplesAndroidN/memberrebinding4/Memberrebinding.java
@@ -5,7 +5,7 @@
import memberrebinding4.subpackage.PublicInterface;
-public class Test {
+public class Memberrebinding {
static class Inner implements PublicInterface {
diff --git a/src/test/java/com/android/tools/r8/R8RunExamplesTest.java b/src/test/java/com/android/tools/r8/R8RunExamplesTest.java
index a264f48..9be6930 100644
--- a/src/test/java/com/android/tools/r8/R8RunExamplesTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunExamplesTest.java
@@ -103,8 +103,8 @@
"regress_37875803.Regress",
"regress_37955340.Regress",
"regress_62300145.Regress",
- "memberrebinding2.Test",
- "memberrebinding3.Test",
+ "memberrebinding2.Memberrebinding",
+ "memberrebinding3.Memberrebinding",
"minification.Minification",
"enclosingmethod.Main",
"interfaceinlining.Main",
@@ -123,7 +123,7 @@
private static String[] makeTest(
DexTool tool, CompilerUnderTest compiler, CompilationMode mode, String clazz) {
String pkg = clazz.substring(0, clazz.lastIndexOf('.'));
- return new String[] {pkg, tool.name(), compiler.name(), mode.name(), clazz};
+ return new String[]{pkg, tool.name(), compiler.name(), mode.name(), clazz};
}
@Rule
diff --git a/src/test/java/com/android/tools/r8/dex/ExtraFileTest.java b/src/test/java/com/android/tools/r8/dex/ExtraFileTest.java
index 26067c7..376577c 100644
--- a/src/test/java/com/android/tools/r8/dex/ExtraFileTest.java
+++ b/src/test/java/com/android/tools/r8/dex/ExtraFileTest.java
@@ -25,7 +25,7 @@
private static final String EXAMPLE_DIR = ToolHelper.EXAMPLES_BUILD_DIR;
private static final String EXAMPLE_DEX = "memberrebinding/classes.dex";
private static final String EXAMPLE_LIB = "memberrebindinglib/classes.dex";
- private static final String EXAMPLE_CLASS = "memberrebinding.Test";
+ private static final String EXAMPLE_CLASS = "memberrebinding.Memberrebinding";
private static final String EXAMPLE_PACKAGE_MAP = "memberrebinding/package.map";
private static final String EXAMPLE_PROGUARD_MAP = "memberrebinding/proguard.map";
diff --git a/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingTest.java b/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingTest.java
index 55bd373..c617c25 100644
--- a/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingTest.java
+++ b/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingTest.java
@@ -105,7 +105,8 @@
}
private static void inspectOriginalMain(DexInspector inspector) {
- MethodSubject main = inspector.clazz("memberrebinding.Test").method(DexInspector.MAIN);
+ MethodSubject main = inspector.clazz("memberrebinding.Memberrebinding")
+ .method(DexInspector.MAIN);
Iterator<InvokeInstructionSubject> iterator =
main.iterateInstructions(MemberRebindingTest::coolInvokes);
assertTrue(iterator.next().holder().is("memberrebinding.ClassAtBottomOfChain"));
@@ -132,7 +133,8 @@
}
private static void inspectMain(DexInspector inspector) {
- MethodSubject main = inspector.clazz("memberrebinding.Test").method(DexInspector.MAIN);
+ MethodSubject main = inspector.clazz("memberrebinding.Memberrebinding")
+ .method(DexInspector.MAIN);
Iterator<InvokeInstructionSubject> iterator =
main.iterateInstructions(MemberRebindingTest::coolInvokes);
assertTrue(iterator.next().holder().is("memberrebinding.ClassAtBottomOfChain"));
@@ -162,7 +164,8 @@
}
private static void inspectOriginalMain2(DexInspector inspector) {
- MethodSubject main = inspector.clazz("memberrebinding2.Test").method(DexInspector.MAIN);
+ MethodSubject main = inspector.clazz("memberrebinding2.Memberrebinding")
+ .method(DexInspector.MAIN);
Iterator<FieldAccessInstructionSubject> iterator =
main.iterateInstructions(InstructionSubject::isFieldAccess);
// Run through instance put, static put, instance get and instance get.
@@ -177,7 +180,8 @@
}
private static void inspectMain2(DexInspector inspector) {
- MethodSubject main = inspector.clazz("memberrebinding2.Test").method(DexInspector.MAIN);
+ MethodSubject main = inspector.clazz("memberrebinding2.Memberrebinding")
+ .method(DexInspector.MAIN);
Iterator<FieldAccessInstructionSubject> iterator =
main.iterateInstructions(InstructionSubject::isFieldAccess);
// Run through instance put, static put, instance get and instance get.
@@ -185,7 +189,7 @@
assertTrue(iterator.next().holder().is("memberrebinding2.ClassAtBottomOfChain"));
assertTrue(iterator.next().holder().is("memberrebinding2.ClassInMiddleOfChain"));
assertTrue(iterator.next().holder().is("memberrebinding2.SuperClassOfAll"));
- assertTrue(iterator.next().holder().is("memberrebinding2.subpackage.PackagePrivateClass"));
+ assertTrue(iterator.next().holder().is("memberrebinding2.subpackage.PublicClass"));
}
assertTrue(iterator.next().holder().is("java.lang.System"));
assertFalse(iterator.hasNext());
@@ -195,7 +199,7 @@
new MethodSignature("test", "void", new String[]{});
private static void inspectOriginal3(DexInspector inspector) {
- MethodSubject main = inspector.clazz("memberrebinding3.Test").method(TEST);
+ MethodSubject main = inspector.clazz("memberrebinding3.Memberrebinding").method(TEST);
Iterator<InvokeInstructionSubject> iterator =
main.iterateInstructions(InstructionSubject::isInvoke);
assertTrue(iterator.next().holder().is("memberrebinding3.ClassAtBottomOfChain"));
@@ -205,7 +209,7 @@
}
private static void inspect3(DexInspector inspector) {
- MethodSubject main = inspector.clazz("memberrebinding3.Test").method(TEST);
+ MethodSubject main = inspector.clazz("memberrebinding3.Memberrebinding").method(TEST);
Iterator<InvokeInstructionSubject> iterator =
main.iterateInstructions(InstructionSubject::isInvoke);
assertTrue(iterator.next().holder().is("memberrebinding3.ClassAtBottomOfChain"));
@@ -215,19 +219,19 @@
}
private static void inspectOriginal4(DexInspector inspector) {
- MethodSubject main = inspector.clazz("memberrebinding4.Test").method(TEST);
+ MethodSubject main = inspector.clazz("memberrebinding4.Memberrebinding").method(TEST);
Iterator<InvokeInstructionSubject> iterator =
main.iterateInstructions(InstructionSubject::isInvoke);
- assertTrue(iterator.next().holder().is("memberrebinding4.Test$Inner"));
+ assertTrue(iterator.next().holder().is("memberrebinding4.Memberrebinding$Inner"));
assertTrue(iterator.next().holder().is("memberrebinding4.subpackage.PublicInterface"));
assertFalse(iterator.hasNext());
}
private static void inspect4(DexInspector inspector) {
- MethodSubject main = inspector.clazz("memberrebinding4.Test").method(TEST);
+ MethodSubject main = inspector.clazz("memberrebinding4.Memberrebinding").method(TEST);
Iterator<InvokeInstructionSubject> iterator =
main.iterateInstructions(InstructionSubject::isInvoke);
- assertTrue(iterator.next().holder().is("memberrebinding4.Test$Inner"));
+ assertTrue(iterator.next().holder().is("memberrebinding4.Memberrebinding$Inner"));
assertTrue(iterator.next().holder().is("memberrebinding4.subpackage.PublicInterface"));
assertFalse(iterator.hasNext());
}
diff --git a/src/test/java/com/android/tools/r8/shaking/TreeShakingTest.java b/src/test/java/com/android/tools/r8/shaking/TreeShakingTest.java
index 4530e2e..6a8cfb8 100644
--- a/src/test/java/com/android/tools/r8/shaking/TreeShakingTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/TreeShakingTest.java
@@ -542,7 +542,9 @@
"assumevalues3",
"assumevalues4",
"assumevalues5",
- "annotationremoval");
+ "annotationremoval",
+ "memberrebinding2",
+ "memberrebinding3");
// Keys can be the name of the test or the name of the test followed by a colon and the name
// of the keep file.
