Allow sharing within a shared context in intermediate mode.
This also relands "Assert that equivalences groups are ordered."
Bug: 172410912
Bug: 172194101
Change-Id: I1875b93b7000e0b0aca4793f2edf90e37cc84fbf
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticDefinition.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticDefinition.java
index 1d037cc..30d232e 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticDefinition.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticDefinition.java
@@ -26,7 +26,7 @@
abstract DexProgramClass getHolder();
- abstract HashCode computeHash();
+ abstract HashCode computeHash(boolean intermediate);
- abstract boolean isEquivalentTo(SyntheticDefinition other);
+ abstract boolean isEquivalentTo(SyntheticDefinition other, boolean intermediate);
}
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java
index bdb76ed..00af9d5 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java
@@ -16,7 +16,6 @@
import com.android.tools.r8.shaking.MainDexClasses;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions;
-import com.android.tools.r8.utils.ListUtils;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -98,13 +97,10 @@
lookupSyntheticMethodDefinitions(application);
Collection<List<SyntheticMethodDefinition>> potentialEquivalences =
- // Don't share synthetics in intermediate mode builds.
- options.intermediate
- ? ListUtils.map(methodDefinitions, Collections::singletonList)
- : computePotentialEquivalences(methodDefinitions);
+ computePotentialEquivalences(methodDefinitions, options.intermediate);
Map<DexType, EquivalenceGroup<SyntheticMethodDefinition>> equivalences =
- computeActualEquivalences(potentialEquivalences, options.itemFactory);
+ computeActualEquivalences(potentialEquivalences, options.intermediate, options.itemFactory);
Builder lensBuilder = NestedGraphLens.builder();
List<DexProgramClass> newProgramClasses = new ArrayList<>();
@@ -332,11 +328,11 @@
private static <T extends SyntheticDefinition & Comparable<T>>
Map<DexType, EquivalenceGroup<T>> computeActualEquivalences(
- Collection<List<T>> potentialEquivalences, DexItemFactory factory) {
+ Collection<List<T>> potentialEquivalences, boolean intermediate, DexItemFactory factory) {
Map<DexType, List<EquivalenceGroup<T>>> groupsPerContext = new IdentityHashMap<>();
potentialEquivalences.forEach(
members -> {
- List<List<T>> groups = groupEquivalent(members);
+ List<List<T>> groups = groupEquivalent(members, intermediate);
for (List<T> group : groups) {
T representative = findDeterministicRepresentative(group);
// The representative is required to be the first element of the group.
@@ -356,6 +352,9 @@
groups.sort(EquivalenceGroup::compareTo);
for (int i = 0; i < groups.size(); i++) {
EquivalenceGroup<T> group = groups.get(i);
+ // Two equivalence groups in same context type must be distinct otherwise the assignment
+ // of the synthetic name will be non-deterministic between the two.
+ assert i == 0 || checkGroupsAreDistict(groups.get(i - 1), group);
DexType representativeType = createExternalType(context, i, factory);
equivalences.put(representativeType, group);
}
@@ -364,13 +363,13 @@
}
private static <T extends SyntheticDefinition & Comparable<T>> List<List<T>> groupEquivalent(
- List<T> potentialEquivalence) {
+ List<T> potentialEquivalence, boolean intermediate) {
List<List<T>> groups = new ArrayList<>();
// Each other member is in a shared group if it is actually equivalent to the first member.
for (T synthetic : potentialEquivalence) {
boolean requireNewGroup = true;
for (List<T> group : groups) {
- if (synthetic.isEquivalentTo(group.get(0))) {
+ if (synthetic.isEquivalentTo(group.get(0), intermediate)) {
requireNewGroup = false;
group.add(synthetic);
break;
@@ -385,6 +384,12 @@
return groups;
}
+ private static <T extends SyntheticDefinition & Comparable<T>> boolean checkGroupsAreDistict(
+ EquivalenceGroup<T> g1, EquivalenceGroup<T> g2) {
+ assert g1.compareTo(g2) != 0;
+ return true;
+ }
+
private static <T extends SyntheticDefinition & Comparable<T>> T findDeterministicRepresentative(
List<T> members) {
// Pick a deterministic member as representative.
@@ -408,10 +413,10 @@
}
private static <T extends SyntheticDefinition> Collection<List<T>> computePotentialEquivalences(
- List<T> definitions) {
+ List<T> definitions, boolean intermediate) {
Map<HashCode, List<T>> equivalences = new HashMap<>(definitions.size());
for (T definition : definitions) {
- HashCode hash = definition.computeHash();
+ HashCode hash = definition.computeHash(intermediate);
equivalences.computeIfAbsent(hash, k -> new ArrayList<>()).add(definition);
}
return equivalences.values();
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodDefinition.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodDefinition.java
index ec7794d..8cddcfc 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodDefinition.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodDefinition.java
@@ -41,17 +41,29 @@
}
@Override
- HashCode computeHash() {
+ HashCode computeHash(boolean intermediate) {
Hasher hasher = Hashing.sha256().newHasher();
+ if (intermediate) {
+ // If in intermediate mode, include the context type as sharing is restricted to within a
+ // single context.
+ hasher.putInt(getContext().getSynthesizingContextType().hashCode());
+ }
method.getDefinition().hashSyntheticContent(hasher);
return hasher.hash();
}
@Override
- boolean isEquivalentTo(SyntheticDefinition other) {
+ boolean isEquivalentTo(SyntheticDefinition other, boolean intermediate) {
if (!(other instanceof SyntheticMethodDefinition)) {
return false;
}
+ if (intermediate
+ && getContext().getSynthesizingContextType()
+ != other.getContext().getSynthesizingContextType()) {
+ // If in intermediate mode, only synthetics within the same context should be considered
+ // equal.
+ return false;
+ }
SyntheticMethodDefinition o = (SyntheticMethodDefinition) other;
return method.getDefinition().isSyntheticContentEqual(o.method.getDefinition());
}
diff --git a/src/test/java/com/android/tools/r8/desugar/backports/BackportMainDexTest.java b/src/test/java/com/android/tools/r8/desugar/backports/BackportMainDexTest.java
index 4a2e645..45f8957 100644
--- a/src/test/java/com/android/tools/r8/desugar/backports/BackportMainDexTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/backports/BackportMainDexTest.java
@@ -147,8 +147,9 @@
.writeToZip();
GenerateMainDexListRunResult mainDexListFromDex =
traceMainDex(Collections.emptyList(), Collections.singleton(out));
- // Compiling in intermediate will not share the synthetics so there is one per call site.
- assertEquals(MAIN_DEX_LIST_CLASSES.size() + 6, mainDexListFromDex.getMainDexList().size());
+ // Compiling in intermediate will share the synthetics within the context types so there is one
+ // synthetic class per backport in User2: Character.compare and Integer.compare.
+ assertEquals(MAIN_DEX_LIST_CLASSES.size() + 2, mainDexListFromDex.getMainDexList().size());
}
@Test
