Version 1.4.50
Cherry-pick: Cache synthesized classes at AppInfo level.
CL: https://r8-review.googlesource.com/c/r8/+/34320
Cherry-pick: Refactor InstructionIterator to use ListIterator
CL: https://r8-review.googlesource.com/c/r8/+/33111
Cherry-pick: Add tests and TODOs for rebinding of fields
CL: https://r8-review.googlesource.com/c/r8/+/33840
Cherry-pick: Revert "Revert "Disallow const instructions after throwing instructions""
CL: https://r8-review.googlesource.com/c/r8/+/33227
Cherry-pick: Don't allow fill-array-data in consistency check.
CL: https://r8-review.googlesource.com/c/r8/+/33109
Cherry-pick: Refactor rewriteWithConstant to make code clearer
CL: https://r8-review.googlesource.com/c/r8/+/33225
Cherry-pick: Ignore <clinit> of field holder in member value propagation
CL: https://r8-review.googlesource.com/c/r8/+/33844
Bug: 123857022, 113374256, 124155517, 124593221, 74379749
Change-Id: Ia74e53a026ff1c70fc9c859675dd87275c50d2c8
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index b31bcbd..a6b94b0 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.4.49";
+ public static final String LABEL = "1.4.50";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfo.java b/src/main/java/com/android/tools/r8/graph/AppInfo.java
index 028f5a2..db83f6a 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfo.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfo.java
@@ -3,22 +3,17 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.graph;
-import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMap.Builder;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Sets;
-import java.util.ArrayDeque;
import java.util.ArrayList;
-import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
-import java.util.Queue;
-import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Consumer;
@@ -28,6 +23,10 @@
public final DexItemFactory dexItemFactory;
private final ConcurrentHashMap<DexType, Map<Descriptor<?,?>, KeyedDexItem<?>>> definitions =
new ConcurrentHashMap<>();
+ // For some optimizations, e.g. optimizing synthetic classes, we may need to resolve the current
+ // class being optimized.
+ private ConcurrentHashMap<DexType, DexProgramClass> synthesizedClasses =
+ new ConcurrentHashMap<>();
public AppInfo(DexApplication application) {
this.app = application;
@@ -48,6 +47,16 @@
this(application);
}
+ public void addSynthesizedClass(DexProgramClass clazz) {
+ assert clazz.type.isD8R8SynthesizedClassType();
+ DexProgramClass previous = synthesizedClasses.put(clazz.type, clazz);
+ assert previous == null || previous == clazz;
+ }
+
+ public Collection<DexProgramClass> getSynthesizedClassesForSanityCheck() {
+ return Collections.unmodifiableCollection(synthesizedClasses.values());
+ }
+
private Map<Descriptor<?,?>, KeyedDexItem<?>> computeDefinitions(DexType type) {
Builder<Descriptor<?,?>, KeyedDexItem<?>> builder = ImmutableMap.builder();
DexClass clazz = app.definitionFor(type);
@@ -78,6 +87,11 @@
}
public DexClass definitionFor(DexType type) {
+ DexProgramClass cached = synthesizedClasses.get(type);
+ if (cached != null) {
+ assert app.definitionFor(type) == null;
+ return cached;
+ }
return app.definitionFor(type);
}
@@ -524,51 +538,6 @@
return result;
}
- public boolean canTriggerStaticInitializer(DexType type, boolean ignoreTypeItself) {
- DexClass clazz = definitionFor(type);
- assert clazz != null;
- return canTriggerStaticInitializer(clazz, ignoreTypeItself);
- }
-
- public boolean canTriggerStaticInitializer(DexClass clazz, boolean ignoreTypeItself) {
- Set<DexType> knownInterfaces = Sets.newIdentityHashSet();
-
- // Process superclass chain.
- DexClass current = clazz;
- while (current != null && current.type != dexItemFactory.objectType) {
- if (canTriggerStaticInitializer(current) && (!ignoreTypeItself || current != clazz)) {
- return true;
- }
- knownInterfaces.addAll(Arrays.asList(current.interfaces.values));
- current = current.superType != null ? definitionFor(current.superType) : null;
- }
-
- // Process interfaces.
- Queue<DexType> queue = new ArrayDeque<>(knownInterfaces);
- while (!queue.isEmpty()) {
- DexType iface = queue.remove();
- DexClass definition = definitionFor(iface);
- if (canTriggerStaticInitializer(definition)) {
- return true;
- }
- if (!definition.isInterface()) {
- throw new Unreachable(iface.toSourceString() + " is expected to be an interface");
- }
-
- for (DexType superIface : definition.interfaces.values) {
- if (knownInterfaces.add(superIface)) {
- queue.add(superIface);
- }
- }
- }
- return false;
- }
-
- public static boolean canTriggerStaticInitializer(DexClass clazz) {
- // Assume it *may* trigger if we didn't find the definition.
- return clazz == null || clazz.hasClassInitializer();
- }
-
public interface ResolutionResult {
DexEncodedMethod asResultOfResolve();
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index 828d98d..5e65aa3 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -567,7 +567,8 @@
}
public boolean classInitializationMayHaveSideEffects(AppInfo appInfo, Predicate<DexType> ignore) {
- if (ignore.test(type)) {
+ if (ignore.test(type)
+ || appInfo.dexItemFactory.libraryTypesWithoutStaticInitialization.contains(type)) {
return false;
}
if (hasNonTrivialClassInitializer()) {
@@ -576,6 +577,15 @@
if (defaultValuesForStaticFieldsMayTriggerAllocation()) {
return true;
}
+ return initializationOfParentTypesMayHaveSideEffects(appInfo, ignore);
+ }
+
+ public boolean initializationOfParentTypesMayHaveSideEffects(AppInfo appInfo) {
+ return initializationOfParentTypesMayHaveSideEffects(appInfo, Predicates.alwaysFalse());
+ }
+
+ public boolean initializationOfParentTypesMayHaveSideEffects(
+ AppInfo appInfo, Predicate<DexType> ignore) {
for (DexType iface : interfaces.values) {
if (iface.classInitializationMayHaveSideEffects(appInfo, ignore)) {
return true;
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedField.java b/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
index 42cc0ce..96f34bc 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
@@ -7,6 +7,7 @@
import com.android.tools.r8.dex.MixedSectionCollection;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
public class DexEncodedField extends KeyedDexItem<DexField> {
@@ -109,22 +110,23 @@
}
// Returns a const instructions if this field is a compile time final const.
- public Instruction valueAsConstInstruction(AppInfo appInfo, Value dest) {
+ public Instruction valueAsConstInstruction(AppInfoWithLiveness appInfo, Value dest) {
// The only way to figure out whether the DexValue contains the final value
// is ensure the value is not the default or check <clinit> is not present.
- if (accessFlags.isStatic() && accessFlags.isPublic() && accessFlags.isFinal()) {
- DexClass clazz = appInfo.definitionFor(field.getHolder());
+ boolean isEffectivelyFinal =
+ (accessFlags.isFinal() || !appInfo.isFieldWritten(field))
+ && !appInfo.isPinned(field);
+ if (!isEffectivelyFinal) {
+ return null;
+ }
+ if (accessFlags.isStatic()) {
+ DexClass clazz = appInfo.definitionFor(field.clazz);
assert clazz != null : "Class for the field must be present";
return getStaticValue().asConstInstruction(clazz.hasClassInitializer(), dest);
}
return null;
}
- public DexEncodedField toRenamedField(DexString name, DexItemFactory dexItemFactory) {
- return new DexEncodedField(dexItemFactory.createField(field.clazz, field.type, name),
- accessFlags, annotations, staticValue);
- }
-
public DexEncodedField toTypeSubstitutedField(DexField field) {
if (this.field == field) {
return this;
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index 8bdf115..8cd38d0 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -298,6 +298,7 @@
public final DexType metafactoryType = createType("Ljava/lang/invoke/LambdaMetafactory;");
public final DexType callSiteType = createType("Ljava/lang/invoke/CallSite;");
public final DexType lookupType = createType("Ljava/lang/invoke/MethodHandles$Lookup;");
+ public final DexType iteratorType = createType("Ljava/util/Iterator;");
public final DexType serializableType = createType("Ljava/io/Serializable;");
public final DexType externalizableType = createType("Ljava/io/Externalizable;");
public final DexType comparableType = createType("Ljava/lang/Comparable;");
@@ -348,6 +349,9 @@
createString("makeConcat")
);
+ public final Set<DexType> libraryTypesWithoutStaticInitialization =
+ ImmutableSet.of(iteratorType, serializableType);
+
private boolean skipNameValidationForTesting = false;
public void setSkipNameValidationForTesting(boolean skipNameValidationForTesting) {
diff --git a/src/main/java/com/android/tools/r8/graph/DexType.java b/src/main/java/com/android/tools/r8/graph/DexType.java
index 9182811..1a880f6 100644
--- a/src/main/java/com/android/tools/r8/graph/DexType.java
+++ b/src/main/java/com/android/tools/r8/graph/DexType.java
@@ -10,10 +10,13 @@
import com.android.tools.r8.dex.IndexedItemCollection;
import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.ir.desugar.Java8MethodRewriter;
+import com.android.tools.r8.ir.desugar.TwrCloseResourceRewriter;
import com.android.tools.r8.naming.NamingLens;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions.OutlineOptions;
import com.google.common.collect.ImmutableList;
+import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
@@ -121,11 +124,25 @@
return implementedInterfaces(appInfo).contains(appInfo.dexItemFactory.serializableType);
}
+ public boolean classInitializationMayHaveSideEffects(AppInfo appInfo) {
+ return classInitializationMayHaveSideEffects(appInfo, Predicates.alwaysFalse());
+ }
+
public boolean classInitializationMayHaveSideEffects(AppInfo appInfo, Predicate<DexType> ignore) {
DexClass clazz = appInfo.definitionFor(this);
return clazz == null || clazz.classInitializationMayHaveSideEffects(appInfo, ignore);
}
+ public boolean initializationOfParentTypesMayHaveSideEffects(AppInfo appInfo) {
+ return initializationOfParentTypesMayHaveSideEffects(appInfo, Predicates.alwaysFalse());
+ }
+
+ public boolean initializationOfParentTypesMayHaveSideEffects(
+ AppInfo appInfo, Predicate<DexType> ignore) {
+ DexClass clazz = appInfo.definitionFor(this);
+ return clazz == null || clazz.initializationOfParentTypesMayHaveSideEffects(appInfo, ignore);
+ }
+
public boolean isUnknown() {
return hierarchyLevel == UNKNOWN_LEVEL;
}
@@ -453,7 +470,9 @@
|| name.contains(DISPATCH_CLASS_NAME_SUFFIX)
|| name.contains(LAMBDA_CLASS_NAME_PREFIX)
|| name.contains(LAMBDA_GROUP_CLASS_NAME_PREFIX)
- || name.contains(OutlineOptions.CLASS_NAME);
+ || name.contains(OutlineOptions.CLASS_NAME)
+ || name.contains(TwrCloseResourceRewriter.UTILITY_CLASS_NAME)
+ || name.contains(Java8MethodRewriter.UTILITY_CLASS_NAME_PREFIX);
}
public int elementSizeForPrimitiveArrayType() {
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 bac74f6..402b56d 100644
--- a/src/main/java/com/android/tools/r8/graph/GraphLense.java
+++ b/src/main/java/com/android/tools/r8/graph/GraphLense.java
@@ -388,9 +388,7 @@
public abstract DexMethod getRenamedMethodSignature(DexMethod originalMethod);
public DexEncodedMethod mapDexEncodedMethod(
- DexEncodedMethod originalEncodedMethod,
- AppInfo appInfo,
- Map<DexType, DexProgramClass> synthesizedClasses) {
+ DexEncodedMethod originalEncodedMethod, AppInfo appInfo) {
DexMethod newMethod = getRenamedMethodSignature(originalEncodedMethod.method);
// Note that:
// * Even if `newMethod` is the same as `originalEncodedMethod.method`, we still need to look it
@@ -398,14 +396,7 @@
// * We can't directly use AppInfo#definitionFor(DexMethod) since definitions may not be
// updated either yet.
DexClass newHolder = appInfo.definitionFor(newMethod.holder);
-
- // TODO(b/120130831): Need to ensure that all synthesized classes are part of the application.
- if (newHolder == null) {
- newHolder = synthesizedClasses.get(newMethod.holder);
- }
-
assert newHolder != null;
-
DexEncodedMethod newEncodedMethod = newHolder.lookupMethod(newMethod);
assert newEncodedMethod != null;
return newEncodedMethod;
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCode.java b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
index f961b80..649d1af 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCode.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
@@ -662,13 +662,9 @@
}
// After the throwing instruction only debug instructions and the final jump
// instruction is allowed.
- // TODO(ager): For now allow const instructions due to the way consts are pushed
- // towards their use
if (seenThrowing) {
assert instruction.isDebugInstruction()
|| instruction.isJumpInstruction()
- || instruction.isConstInstruction()
- || instruction.isNewArrayFilledData()
|| instruction.isStore()
|| instruction.isPop();
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionsIterator.java b/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionsIterator.java
index b98662c..cdb40bf 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionsIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionsIterator.java
@@ -19,10 +19,7 @@
@Override
public boolean hasNext() {
- if (instructionIterator.hasNext()) {
- return true;
- }
- return blockIterator.hasNext();
+ return instructionIterator.hasNext() || blockIterator.hasNext();
}
@Override
@@ -39,6 +36,35 @@
}
@Override
+ public boolean hasPrevious() {
+ return instructionIterator.hasPrevious() || blockIterator.hasPrevious();
+ }
+
+ @Override
+ public Instruction previous() {
+ if (instructionIterator.hasPrevious()) {
+ return instructionIterator.previous();
+ }
+ if (!blockIterator.hasPrevious()) {
+ throw new NoSuchElementException();
+ }
+ BasicBlock block = blockIterator.previous();
+ instructionIterator = block.listIterator(block.getInstructions().size());
+ assert instructionIterator.hasPrevious();
+ return instructionIterator.previous();
+ }
+
+ @Override
+ public int nextIndex() {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public int previousIndex() {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
public void add(Instruction instruction) {
instructionIterator.add(instruction);
}
@@ -49,6 +75,11 @@
}
@Override
+ public void set(Instruction instruction) {
+ instructionIterator.set(instruction);
+ }
+
+ @Override
public void replaceCurrentInstruction(Instruction newInstruction) {
instructionIterator.replaceCurrentInstruction(newInstruction);
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstructionIterator.java b/src/main/java/com/android/tools/r8/ir/code/InstructionIterator.java
index 2f2b3f1..72aa3ee 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstructionIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstructionIterator.java
@@ -4,7 +4,10 @@
package com.android.tools.r8.ir.code;
-public interface InstructionIterator extends NextUntilIterator<Instruction> {
+import java.util.ListIterator;
+
+public interface InstructionIterator
+ extends ListIterator<Instruction>, NextUntilIterator<Instruction> {
/**
* Replace the current instruction (aka the {@link Instruction} returned by the previous call to
* {@link #next} with the passed in <code>newInstruction</code>.
@@ -22,15 +25,6 @@
*/
void replaceCurrentInstruction(Instruction newInstruction);
- /**
- * Adds an instruction. The instruction will be added just before the current
- * cursor position.
- *
- * The instruction will be assigned to the block it is added to.
- *
- * @param instruction The instruction to add.
- */
- void add(Instruction instruction);
/**
* Safe removal function that will insert a DebugLocalRead to take over the debug values if any
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
index a5e5d78..c232d3f 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
@@ -11,9 +11,7 @@
import java.util.ListIterator;
public interface InstructionListIterator
- extends ListIterator<Instruction>,
- NextUntilIterator<Instruction>,
- PreviousUntilIterator<Instruction> {
+ extends InstructionIterator, PreviousUntilIterator<Instruction> {
/**
* Peek the previous instruction.
@@ -50,27 +48,6 @@
}
/**
- * Safe removal function that will insert a DebugLocalRead to take over the debug values if any
- * are associated with the current instruction.
- */
- void removeOrReplaceByDebugLocalRead();
-
- /**
- * Replace the current instruction (aka the {@link Instruction} returned by the previous call to
- * {@link #next} with the passed in <code>newInstruction</code>.
- *
- * The current instruction will be completely detached from the instruction stream with uses
- * of its in-values removed.
- *
- * If the current instruction produces an out-value the new instruction must also produce
- * an out-value, and all uses of the current instructions out-value will be replaced by the
- * new instructions out-value.
- *
- * @param newInstruction the instruction to insert instead of the current.
- */
- void replaceCurrentInstruction(Instruction newInstruction);
-
- /**
* Split the block into two blocks at the point of the {@link ListIterator} cursor. The existing
* block will have all the instructions before the cursor, and the new block all the
* instructions after the cursor.
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index 0343e8a..edd33c8 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -89,12 +89,10 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
-import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
-import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
@@ -149,10 +147,6 @@
private final OptimizationFeedback simpleOptimizationFeedback = new OptimizationFeedbackSimple();
private DexString highestSortingString;
- // For some optimizations, e.g. optimizing synthetic classes, we may need to resolve
- // the current class being optimized.
- private ConcurrentHashMap<DexType, DexProgramClass> cachedClasses = new ConcurrentHashMap<>();
-
// The argument `appView` is only available when full program optimizations are allowed
// (i.e., when running R8).
private IRConverter(
@@ -352,30 +346,24 @@
InterfaceMethodRewriter.Flavor includeAllResources,
ExecutorService executorService)
throws ExecutionException {
- desugarInterfaceMethods(builder, includeAllResources, executorService, null);
- }
-
- private void desugarInterfaceMethods(
- Builder<?> builder,
- InterfaceMethodRewriter.Flavor includeAllResources,
- ExecutorService executorService,
- Map<DexType, DexProgramClass> synthesizedClasses)
- throws ExecutionException {
if (interfaceMethodRewriter != null) {
interfaceMethodRewriter.desugarInterfaceMethods(
- builder, includeAllResources, executorService, synthesizedClasses);
+ builder, includeAllResources, executorService);
}
}
- private void synthesizeTwrCloseResourceUtilityClass(Builder<?> builder) {
+ private void synthesizeTwrCloseResourceUtilityClass(
+ Builder<?> builder, ExecutorService executorService)
+ throws ExecutionException {
if (twrCloseResourceRewriter != null) {
- twrCloseResourceRewriter.synthesizeUtilityClass(builder, options);
+ twrCloseResourceRewriter.synthesizeUtilityClass(builder, executorService, options);
}
}
- private void synthesizeJava8UtilityClass(Builder<?> builder) {
+ private void synthesizeJava8UtilityClass(
+ Builder<?> builder, ExecutorService executorService) throws ExecutionException {
if (java8MethodRewriter != null) {
- java8MethodRewriter.synthesizeUtilityClass(builder, options);
+ java8MethodRewriter.synthesizeUtilityClass(builder, executorService, options);
}
}
@@ -398,8 +386,8 @@
synthesizeLambdaClasses(builder, executor);
desugarInterfaceMethods(builder, ExcludeDexResources, executor);
- synthesizeTwrCloseResourceUtilityClass(builder);
- synthesizeJava8UtilityClass(builder);
+ synthesizeTwrCloseResourceUtilityClass(builder, executor);
+ synthesizeJava8UtilityClass(builder, executor);
processCovariantReturnTypeAnnotations(builder);
handleSynthesizedClassMapping(builder);
@@ -590,21 +578,20 @@
synthesizeLambdaClasses(builder, executorService);
printPhase("Interface method desugaring");
- Map<DexType, DexProgramClass> synthesizedClasses = new IdentityHashMap<>();
- desugarInterfaceMethods(builder, IncludeAllResources, executorService, synthesizedClasses);
+ desugarInterfaceMethods(builder, IncludeAllResources, executorService);
printPhase("Twr close resource utility class synthesis");
- synthesizeTwrCloseResourceUtilityClass(builder);
- synthesizeJava8UtilityClass(builder);
+ synthesizeTwrCloseResourceUtilityClass(builder, executorService);
+ synthesizeJava8UtilityClass(builder, executorService);
handleSynthesizedClassMapping(builder);
printPhase("Lambda merging finalization");
- finalizeLambdaMerging(application, feedback, builder, executorService, synthesizedClasses);
+ finalizeLambdaMerging(application, feedback, builder, executorService);
if (outliner != null) {
printPhase("Outlining");
timing.begin("IR conversion phase 2");
- if (outliner.selectMethodsForOutlining(synthesizedClasses)) {
+ if (outliner.selectMethodsForOutlining()) {
forEachSelectedOutliningMethod(
executorService,
(code, method) -> {
@@ -612,7 +599,8 @@
outliner.identifyOutlineSites(code, method);
});
DexProgramClass outlineClass = outliner.buildOutlinerClass(computeOutlineClassType());
- optimizeSynthesizedClass(outlineClass);
+ appInfo.addSynthesizedClass(outlineClass);
+ optimizeSynthesizedClass(outlineClass, executorService);
forEachSelectedOutliningMethod(
executorService,
(code, method) -> {
@@ -636,6 +624,12 @@
uninstantiatedTypeOptimization.logResults();
}
+ // Check if what we've added to the application builder as synthesized classes are same as
+ // what we've added and used through AppInfo.
+ assert appInfo.getSynthesizedClassesForSanityCheck()
+ .containsAll(builder.getSynthesizedClasses())
+ && builder.getSynthesizedClasses()
+ .containsAll(appInfo.getSynthesizedClassesForSanityCheck());
return builder.build();
}
@@ -687,14 +681,13 @@
private void finalizeLambdaMerging(
DexApplication application,
- OptimizationFeedback directFeedback,
+ OptimizationFeedback feedback,
Builder<?> builder,
- ExecutorService executorService,
- Map<DexType, DexProgramClass> synthesizedClasses)
+ ExecutorService executorService)
throws ExecutionException {
if (lambdaMerger != null) {
lambdaMerger.applyLambdaClassMapping(
- application, this, directFeedback, builder, executorService, synthesizedClasses);
+ application, this, feedback, builder, executorService);
}
}
@@ -744,49 +737,24 @@
return result;
}
- public DexClass definitionFor(DexType type) {
- DexProgramClass cached = cachedClasses.get(type);
- return cached != null ? cached : appInfo.definitionFor(type);
- }
-
- public void optimizeSynthesizedClass(DexProgramClass clazz) {
- try {
- enterCachedClass(clazz);
- // Process the generated class, but don't apply any outlining.
- clazz.forEachMethod(this::optimizeSynthesizedMethod);
- } finally {
- leaveCachedClass(clazz);
- }
+ public void optimizeSynthesizedClass(
+ DexProgramClass clazz, ExecutorService executorService)
+ throws ExecutionException {
+ Set<DexEncodedMethod> methods = Sets.newIdentityHashSet();
+ clazz.forEachMethod(methods::add);
+ // Process the generated class, but don't apply any outlining.
+ optimizeSynthesizedMethodsConcurrently(methods, executorService);
}
public void optimizeSynthesizedClasses(
Collection<DexProgramClass> classes, ExecutorService executorService)
throws ExecutionException {
Set<DexEncodedMethod> methods = Sets.newIdentityHashSet();
- try {
- for (DexProgramClass clazz : classes) {
- enterCachedClass(clazz);
- clazz.forEachMethod(methods::add);
- }
- // Process the generated class, but don't apply any outlining.
- optimizeSynthesizedMethods(methods, executorService);
- } finally {
- for (DexProgramClass clazz : classes) {
- leaveCachedClass(clazz);
- }
+ for (DexProgramClass clazz : classes) {
+ clazz.forEachMethod(methods::add);
}
- }
-
- public void optimizeMethodOnSynthesizedClass(DexProgramClass clazz, DexEncodedMethod method) {
- if (!method.isProcessed()) {
- try {
- enterCachedClass(clazz);
- // Process the generated method, but don't apply any outlining.
- optimizeSynthesizedMethod(method);
- } finally {
- leaveCachedClass(clazz);
- }
- }
+ // Process the generated class, but don't apply any outlining.
+ optimizeSynthesizedMethodsConcurrently(methods, executorService);
}
public void optimizeSynthesizedMethod(DexEncodedMethod method) {
@@ -801,7 +769,7 @@
}
}
- public void optimizeSynthesizedMethods(
+ public void optimizeSynthesizedMethodsConcurrently(
Collection<DexEncodedMethod> methods, ExecutorService executorService)
throws ExecutionException {
List<Future<?>> futures = new ArrayList<>();
@@ -821,16 +789,6 @@
ThreadUtils.awaitFutures(futures);
}
- private void enterCachedClass(DexProgramClass clazz) {
- DexProgramClass previous = cachedClasses.put(clazz.type, clazz);
- assert previous == null;
- }
-
- private void leaveCachedClass(DexProgramClass clazz) {
- DexProgramClass existing = cachedClasses.remove(clazz.type);
- assert existing == clazz;
- }
-
private String logCode(InternalOptions options, DexEncodedMethod method) {
return options.useSmaliSyntax ? method.toSmaliString(null) : method.codeToString();
}
@@ -1191,7 +1149,8 @@
// original method signature (this could have changed as a result of, for example, class
// merging). Then, we find the type that now corresponds to the the original holder.
DexMethod originalSignature = graphLense().getOriginalMethodSignature(method.method);
- DexClass originalHolder = definitionFor(graphLense().lookupType(originalSignature.holder));
+ DexClass originalHolder = appInfo.definitionFor(
+ graphLense().lookupType(originalSignature.holder));
if (originalHolder.hasKotlinInfo()) {
KotlinInfo kotlinInfo = originalHolder.getKotlinInfo();
if (kotlinInfo.hasNonNullParameterHints()) {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
index 7c22bbf..fd968d6 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
@@ -210,7 +210,15 @@
iterator.replaceCurrentInstruction(newInvoke);
if (constantReturnMaterializingInstruction != null) {
- iterator.add(constantReturnMaterializingInstruction);
+ if (block.hasCatchHandlers()) {
+ // Split the block to ensure no instructions after throwing instructions.
+ iterator
+ .split(code, blocks)
+ .listIterator()
+ .add(constantReturnMaterializingInstruction);
+ } else {
+ iterator.add(constantReturnMaterializingInstruction);
+ }
}
DexType actualReturnType = actualTarget.proto.returnType;
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java b/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java
index 968b200..a27b096 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java
@@ -60,7 +60,7 @@
DexType superType = clazz.superType;
// If superClass definition is missing, just skip this part and let real processing of its
// subclasses report the error if it is required.
- DexClass superClass = superType == null ? null : rewriter.findDefinitionFor(superType);
+ DexClass superClass = superType == null ? null : rewriter.appInfo.definitionFor(superType);
if (superClass != null && superType != rewriter.factory.objectType) {
if (superClass.isInterface()) {
throw new CompilationError("Interface `" + superClass.toSourceString()
@@ -96,10 +96,10 @@
private DexEncodedMethod addForwardingMethod(DexEncodedMethod defaultMethod, DexClass clazz) {
DexMethod method = defaultMethod.method;
+ DexClass target = rewriter.appInfo.definitionFor(method.holder);
// NOTE: Never add a forwarding method to methods of classes unknown or coming from android.jar
// even if this results in invalid code, these classes are never desugared.
- assert rewriter.findDefinitionFor(method.holder) != null
- && !rewriter.findDefinitionFor(method.holder).isLibraryClass();
+ assert target != null && !target.isLibraryClass();
// New method will have the same name, proto, and also all the flags of the
// default method, including bridge flag.
DexMethod newMethod = rewriter.factory.createMethod(clazz.type, method.proto, method.name);
@@ -167,7 +167,7 @@
if (current.superType == null) {
break;
} else {
- DexClass superClass = rewriter.findDefinitionFor(current.superType);
+ DexClass superClass = rewriter.appInfo.definitionFor(current.superType);
if (superClass != null) {
current = superClass;
} else {
@@ -205,7 +205,7 @@
DexType superType = current.superType;
DexClass superClass = null;
if (superType != null) {
- superClass = rewriter.findDefinitionFor(superType);
+ superClass = rewriter.appInfo.definitionFor(superType);
// It's available or we would have failed while analyzing the hierarchy for interfaces.
assert superClass != null;
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
index e79017d..abd38a5 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
@@ -6,6 +6,7 @@
import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.Unimplemented;
+import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexApplication.Builder;
@@ -80,6 +81,7 @@
public static final String PRIVATE_METHOD_PREFIX = "$private$";
private final AppView<? extends AppInfoWithLiveness> appView;
+ final AppInfo appInfo;
private final IRConverter converter;
private final InternalOptions options;
final DexItemFactory factory;
@@ -121,6 +123,7 @@
assert converter != null;
this.appView = appView;
this.converter = converter;
+ this.appInfo = converter.appInfo;
this.options = options;
this.factory = options.itemFactory;
}
@@ -156,7 +159,7 @@
if (instruction.isInvokeStatic()) {
InvokeStatic invokeStatic = instruction.asInvokeStatic();
DexMethod method = invokeStatic.getInvokedMethod();
- DexClass clazz = findDefinitionFor(method.holder);
+ DexClass clazz = appInfo.definitionFor(method.holder);
if (Java8MethodRewriter.hasJava8MethodRewritePrefix(method.holder)) {
// We did not create this code yet, but it will not require rewriting.
continue;
@@ -187,7 +190,7 @@
invokeStatic.outValue(), invokeStatic.arguments()));
requiredDispatchClasses
.computeIfAbsent(clazz.asLibraryClass(), k -> Sets.newConcurrentHashSet())
- .add(findDefinitionFor(encodedMethod.method.holder).asProgramClass());
+ .add(appInfo.definitionFor(encodedMethod.method.holder).asProgramClass());
}
} else {
instructions.replaceCurrentInstruction(
@@ -201,7 +204,7 @@
if (instruction.isInvokeSuper()) {
InvokeSuper invokeSuper = instruction.asInvokeSuper();
DexMethod method = invokeSuper.getInvokedMethod();
- DexClass clazz = findDefinitionFor(method.holder);
+ DexClass clazz = appInfo.definitionFor(method.holder);
if (clazz == null) {
// NOTE: leave unchanged those calls to undefined targets. This may lead to runtime
// exception but we can not report it as error since it can also be the intended
@@ -218,7 +221,7 @@
// WARNING: This may result in incorrect code on older platforms!
// Retarget call to an appropriate method of companion class.
DexMethod amendedMethod = amendDefaultMethod(
- findDefinitionFor(encodedMethod.method.holder), method);
+ appInfo.definitionFor(encodedMethod.method.holder), method);
instructions.replaceCurrentInstruction(
new InvokeStatic(defaultAsMethodOfCompanionClass(amendedMethod),
invokeSuper.outValue(), invokeSuper.arguments()));
@@ -233,7 +236,7 @@
continue;
}
- DexClass clazz = findDefinitionFor(method.holder);
+ DexClass clazz = appInfo.definitionFor(method.holder);
if (clazz == null) {
// Report missing class since we don't know if it is an interface.
warnMissingType(encodedMethod.method, method.holder);
@@ -279,7 +282,7 @@
private void reportStaticInterfaceMethodHandle(DexMethod referencedFrom, DexMethodHandle handle) {
if (handle.type.isInvokeStatic()) {
- DexClass holderClass = findDefinitionFor(handle.asMethod().holder);
+ DexClass holderClass = appInfo.definitionFor(handle.asMethod().holder);
// NOTE: If the class definition is missing we can't check. Let it be handled as any other
// missing call target.
if (holderClass == null) {
@@ -292,15 +295,6 @@
}
}
- /**
- * Returns the class definition for the specified type.
- *
- * @return may return null if no definition for the given type is available.
- */
- final DexClass findDefinitionFor(DexType type) {
- return converter.definitionFor(type);
- }
-
// Gets the companion class for the interface `type`.
final DexType getCompanionClassType(DexType type) {
assert type.isClassType();
@@ -334,7 +328,7 @@
}
private boolean isInMainDexList(DexType iface) {
- return converter.appInfo.isInMainDexList(iface);
+ return appInfo.isInMainDexList(iface);
}
// Represent a static interface method as a method of companion class.
@@ -393,8 +387,7 @@
public void desugarInterfaceMethods(
Builder<?> builder,
Flavor flavour,
- ExecutorService executorService,
- Map<DexType, DexProgramClass> synthesizedClasses)
+ ExecutorService executorService)
throws ExecutionException {
// Process all classes first. Add missing forwarding methods to
// replace desugared default interface methods.
@@ -409,13 +402,10 @@
// are just moved from interfaces and don't need to be re-processed.
DexProgramClass synthesizedClass = entry.getValue();
builder.addSynthesizedClass(synthesizedClass, isInMainDexList(entry.getKey()));
-
- if (synthesizedClasses != null) {
- synthesizedClasses.put(synthesizedClass.type, synthesizedClass);
- }
+ appInfo.addSynthesizedClass(synthesizedClass);
}
- converter.optimizeSynthesizedMethods(synthesizedMethods, executorService);
+ converter.optimizeSynthesizedMethodsConcurrently(synthesizedMethods, executorService);
// Cached data is not needed any more.
clear();
@@ -528,7 +518,7 @@
if (isCompanionClassType(holder)) {
holder = getInterfaceClassType(holder);
}
- DexClass clazz = converter.appInfo.definitionFor(holder);
+ DexClass clazz = appInfo.definitionFor(holder);
return clazz == null ? Origin.unknown() : clazz.getOrigin();
}
@@ -550,7 +540,7 @@
DexClass implementing,
DexType iface) {
DefaultMethodsHelper helper = new DefaultMethodsHelper();
- DexClass definedInterface = findDefinitionFor(iface);
+ DexClass definedInterface = appInfo.definitionFor(iface);
if (definedInterface == null) {
warnMissingInterface(classToDesugar, implementing, iface);
return helper.wrapInCollection();
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java
index 9d5e5fc..58ec9ed 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java
@@ -322,7 +322,7 @@
if (!seenBefore.add(superType)) {
continue;
}
- DexClass clazz = rewriter.findDefinitionFor(superType);
+ DexClass clazz = rewriter.appInfo.definitionFor(superType);
if (clazz != null) {
if (clazz.lookupVirtualMethod(method.method) != null) {
return false;
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/Java8MethodRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/Java8MethodRewriter.java
index 9e2e9ab..e930bcf 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/Java8MethodRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/Java8MethodRewriter.java
@@ -37,9 +37,12 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
import java.util.function.BiFunction;
public final class Java8MethodRewriter {
+ public static final String UTILITY_CLASS_NAME_PREFIX = "$r8$java8methods$utility";
private static final String UTILITY_CLASS_DESCRIPTOR_PREFIX = "L$r8$java8methods$utility";
private final Set<DexType> holders = Sets.newConcurrentHashSet();
private final IRConverter converter;
@@ -88,7 +91,9 @@
return clazz.descriptor.toString().startsWith(UTILITY_CLASS_DESCRIPTOR_PREFIX);
}
- public void synthesizeUtilityClass(Builder<?> builder, InternalOptions options) {
+ public void synthesizeUtilityClass(
+ Builder<?> builder, ExecutorService executorService, InternalOptions options)
+ throws ExecutionException {
if (holders.isEmpty()) {
return;
}
@@ -134,7 +139,8 @@
code.setUpContext(utilityClass);
boolean addToMainDexList = referencingClasses.stream()
.anyMatch(clazz -> converter.appInfo.isInMainDexList(clazz.type));
- converter.optimizeSynthesizedClass(utilityClass);
+ converter.appInfo.addSynthesizedClass(utilityClass);
+ converter.optimizeSynthesizedClass(utilityClass, executorService);
builder.addSynthesizedClass(utilityClass, addToMainDexList);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java b/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java
index 865a62c..cff5392 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java
@@ -153,8 +153,9 @@
synthesizeVirtualMethods(mainMethod),
rewriter.factory.getSkipNameValidationForTesting());
// Optimize main method.
- rewriter.converter.optimizeMethodOnSynthesizedClass(
- clazz, clazz.lookupVirtualMethod(mainMethod));
+ rewriter.converter.appInfo.addSynthesizedClass(clazz);
+ rewriter.converter.optimizeSynthesizedMethod(clazz.lookupVirtualMethod(mainMethod));
+
// The method addSynthesizedFrom() may be called concurrently. To avoid a Concurrent-
// ModificationException we must use synchronization.
synchronized (synthesizedFrom) {
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java
index ade6feb..d0faabc 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java
@@ -180,14 +180,15 @@
/** Generates lambda classes and adds them to the builder. */
public void synthesizeLambdaClasses(Builder<?> builder, ExecutorService executorService)
throws ExecutionException {
+ for (LambdaClass lambdaClass : knownLambdaClasses.values()) {
+ DexProgramClass synthesizedClass = lambdaClass.getLambdaClass();
+ appInfo.addSynthesizedClass(synthesizedClass);
+ builder.addSynthesizedClass(synthesizedClass, lambdaClass.addToMainDexList.get());
+ }
converter.optimizeSynthesizedClasses(
knownLambdaClasses.values().stream()
.map(LambdaClass::getLambdaClass).collect(ImmutableSet.toImmutableSet()),
executorService);
- for (LambdaClass lambdaClass : knownLambdaClasses.values()) {
- DexProgramClass synthesizedClass = lambdaClass.getLambdaClass();
- builder.addSynthesizedClass(synthesizedClass, lambdaClass.addToMainDexList.get());
- }
}
public Set<DexCallSite> getDesugaredCallSites() {
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/TwrCloseResourceRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/TwrCloseResourceRewriter.java
index e1e534e..c06e158 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/TwrCloseResourceRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/TwrCloseResourceRewriter.java
@@ -31,6 +31,8 @@
import java.lang.reflect.Method;
import java.util.Collections;
import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
// Try with resources outlining processor. Handles $closeResource methods
// synthesized by java 9 compiler.
@@ -45,6 +47,7 @@
// tree shaking to remove them since now they should not be referenced.
//
public final class TwrCloseResourceRewriter {
+ public static final String UTILITY_CLASS_NAME = "$r8$twr$utility";
public static final String UTILITY_CLASS_DESCRIPTOR = "L$r8$twr$utility;";
private final IRConverter converter;
@@ -107,7 +110,9 @@
&& original.proto == converter.appInfo.dexItemFactory.twrCloseResourceMethodProto;
}
- public void synthesizeUtilityClass(Builder<?> builder, InternalOptions options) {
+ public void synthesizeUtilityClass(
+ Builder<?> builder, ExecutorService executorService, InternalOptions options)
+ throws ExecutionException {
if (referencingClasses.isEmpty()) {
return;
}
@@ -144,7 +149,8 @@
// Process created class and method.
boolean addToMainDexList = referencingClasses.stream()
.anyMatch(clazz -> converter.appInfo.isInMainDexList(clazz.type));
- converter.optimizeSynthesizedClass(utilityClass);
+ converter.appInfo.addSynthesizedClass(utilityClass);
+ converter.optimizeSynthesizedClass(utilityClass, executorService);
builder.addSynthesizedClass(utilityClass, addToMainDexList);
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index a07410c..e723a55 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -1843,7 +1843,7 @@
return;
}
- DexClass clazz = definitionFor(method.method.getHolder());
+ DexClass clazz = appInfo.definitionFor(method.method.getHolder());
if (clazz == null) {
return;
}
@@ -2044,10 +2044,6 @@
}
}
- DexClass definitionFor(DexType type) {
- return converter.definitionFor(type);
- }
-
public void removeTrivialCheckCastAndInstanceOfInstructions(
IRCode code, boolean enableWholeProgramOptimizations) {
if (!enableWholeProgramOptimizations) {
@@ -2153,7 +2149,7 @@
if (baseType.isPrimitiveType()) {
return false;
}
- DexClass clazz = definitionFor(baseType);
+ DexClass clazz = appInfo.definitionFor(baseType);
if (clazz == null) {
// Conservatively say yes.
return true;
@@ -3502,7 +3498,7 @@
if (type == dexItemFactory.throwableType) {
return true;
}
- DexClass dexClass = definitionFor(type);
+ DexClass dexClass = appInfo.definitionFor(type);
if (dexClass == null) {
throw new CompilationError("Class or interface " + type.toSourceString() +
" required for desugaring of try-with-resources is not found.");
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 434bb04..7ebc21c 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
@@ -17,18 +17,20 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.analysis.type.TypeAnalysis;
import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
+import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.ConstNumber;
+import com.android.tools.r8.ir.code.FieldInstruction;
import com.android.tools.r8.ir.code.IRCode;
-import com.android.tools.r8.ir.code.InstancePut;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionIterator;
+import com.android.tools.r8.ir.code.InstructionListIterator;
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.StaticGet;
-import com.android.tools.r8.ir.code.StaticPut;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import com.android.tools.r8.shaking.ProguardMemberRule;
import com.google.common.collect.Sets;
+import java.util.ListIterator;
import java.util.Set;
import java.util.function.Predicate;
@@ -114,174 +116,207 @@
}
}
- private void replaceInstructionFromProguardRule(RuleType ruleType, InstructionIterator iterator,
- Instruction current, Instruction replacement) {
- if (ruleType == RuleType.ASSUME_NO_SIDE_EFFECTS) {
+ private boolean tryConstantReplacementFromProguard(
+ IRCode code,
+ Set<Value> affectedValues,
+ ListIterator<BasicBlock> blocks,
+ InstructionListIterator iterator,
+ Instruction current,
+ ProguardMemberRuleLookup lookup) {
+ Instruction replacement = constantReplacementFromProguardRule(lookup.rule, code, current);
+ if (replacement == null) {
+ // Check to see if a value range can be assumed.
+ setValueRangeFromProguardRule(lookup.rule, current.outValue());
+ return false;
+ }
+ affectedValues.add(replacement.outValue());
+ if (lookup.type == RuleType.ASSUME_NO_SIDE_EFFECTS) {
iterator.replaceCurrentInstruction(replacement);
} else {
+ assert lookup.type == RuleType.ASSUME_VALUES;
if (current.outValue() != null) {
assert replacement.outValue() != null;
current.outValue().replaceUsers(replacement.outValue());
}
replacement.setPosition(current.getPosition());
- iterator.add(replacement);
+ if (current.getBlock().hasCatchHandlers()) {
+ iterator.split(code, blocks).listIterator().add(replacement);
+ } else {
+ iterator.add(replacement);
+ }
+ }
+ return true;
+ }
+
+ private void rewriteInvokeMethodWithConstantValues(
+ IRCode code,
+ DexType callingContext,
+ Set<Value> affectedValues,
+ ListIterator<BasicBlock> blocks,
+ InstructionListIterator iterator,
+ InvokeMethod current) {
+ DexMethod invokedMethod = current.getInvokedMethod();
+ DexType invokedHolder = invokedMethod.getHolder();
+ if (!invokedHolder.isClassType()) {
+ return;
+ }
+ // TODO(70550443): Maybe check all methods here.
+ DexEncodedMethod definition = appInfo.lookup(current.getType(), invokedMethod, callingContext);
+ ProguardMemberRuleLookup lookup = lookupMemberRule(definition);
+ boolean invokeReplaced = false;
+ if (lookup != null) {
+ boolean outValueNullOrNotUsed = current.outValue() == null || !current.outValue().isUsed();
+ if (lookup.type == RuleType.ASSUME_NO_SIDE_EFFECTS && outValueNullOrNotUsed) {
+ // Remove invoke if marked as having no side effects and the return value is not used.
+ iterator.removeOrReplaceByDebugLocalRead();
+ invokeReplaced = true;
+ } else if (!outValueNullOrNotUsed) {
+ // Check to see if a constant value can be assumed.
+ invokeReplaced =
+ tryConstantReplacementFromProguard(
+ code, affectedValues, blocks, iterator, current, lookup);
+ }
+ }
+ if (invokeReplaced || current.outValue() == null) {
+ return;
+ }
+ // No Proguard rule could replace the instruction check for knowledge about the return value.
+ DexEncodedMethod target = current.lookupSingleTarget(appInfo, callingContext);
+ if (target == null) {
+ return;
+ }
+ if (target.getOptimizationInfo().neverReturnsNull() && current.outValue().canBeNull()) {
+ Value knownToBeNonNullValue = current.outValue();
+ knownToBeNonNullValue.markNeverNull();
+ TypeLatticeElement typeLattice = knownToBeNonNullValue.getTypeLattice();
+ assert typeLattice.isNullable() && typeLattice.isReference();
+ knownToBeNonNullValue.narrowing(appInfo, typeLattice.asNonNullable());
+ affectedValues.addAll(knownToBeNonNullValue.affectedValues());
+ }
+ if (target.getOptimizationInfo().returnsConstant()) {
+ long constant = target.getOptimizationInfo().getReturnedConstant();
+ ConstNumber replacement =
+ createConstNumberReplacement(
+ code, constant, current.outValue().getTypeLattice(), current.getLocalInfo());
+ affectedValues.add(replacement.outValue());
+ current.outValue().replaceUsers(replacement.outValue());
+ current.setOutValue(null);
+ replacement.setPosition(current.getPosition());
+ current.moveDebugValues(replacement);
+ if (current.getBlock().hasCatchHandlers()) {
+ iterator.split(code, blocks).listIterator().add(replacement);
+ } else {
+ iterator.add(replacement);
+ }
+ }
+ }
+
+ private void rewriteStaticGetWithConstantValues(
+ IRCode code,
+ Predicate<DexEncodedMethod> isProcessedConcurrently,
+ Set<Value> affectedValues,
+ ListIterator<BasicBlock> blocks,
+ InstructionListIterator iterator,
+ StaticGet current) {
+ DexField field = current.getField();
+
+ // TODO(b/123857022): Should be able to use definitionFor().
+ DexEncodedField target = appInfo.lookupStaticTarget(field.getHolder(), field);
+ if (target == null) {
+ return;
+ }
+ // Check if a this value is known const.
+ Instruction replacement = target.valueAsConstInstruction(appInfo, current.dest());
+ if (replacement != null) {
+ affectedValues.add(replacement.outValue());
+ iterator.replaceCurrentInstruction(replacement);
+ return;
+ }
+ ProguardMemberRuleLookup lookup = lookupMemberRule(target);
+ if (lookup != null
+ && lookup.type == RuleType.ASSUME_VALUES
+ && tryConstantReplacementFromProguard(
+ code, affectedValues, blocks, iterator, current, lookup)) {
+ return;
+ }
+ if (current.dest() != null) {
+ // In case the class holder of this static field satisfying following criteria:
+ // -- cannot trigger other static initializer except for its own
+ // -- is final
+ // -- has a class initializer which is classified as trivial
+ // (see CodeRewriter::computeClassInitializerInfo) and
+ // initializes the field being accessed
+ //
+ // ... and the field itself is not pinned by keep rules (in which case it might
+ // be updated outside the class constructor, e.g. via reflections), it is safe
+ // to assume that the static-get instruction reads the value it initialized value
+ // in class initializer and is never null.
+ DexClass holderDefinition = appInfo.definitionFor(field.getHolder());
+ if (holderDefinition != null
+ && holderDefinition.accessFlags.isFinal()
+ && !field.getHolder().initializationOfParentTypesMayHaveSideEffects(appInfo)) {
+ Value outValue = current.dest();
+ DexEncodedMethod classInitializer = holderDefinition.getClassInitializer();
+ if (classInitializer != null && !isProcessedConcurrently.test(classInitializer)) {
+ TrivialInitializer info =
+ classInitializer.getOptimizationInfo().getTrivialInitializerInfo();
+ if (info != null
+ && ((TrivialClassInitializer) info).field == field
+ && !appInfo.isPinned(field)
+ && outValue.canBeNull()) {
+ outValue.markNeverNull();
+ TypeLatticeElement typeLattice = outValue.getTypeLattice();
+ assert typeLattice.isNullable() && typeLattice.isReference();
+ outValue.narrowing(appInfo, typeLattice.asNonNullable());
+ affectedValues.addAll(outValue.affectedValues());
+ }
+ }
+ }
+ }
+ }
+
+ private void rewritePutWithConstantValues(
+ InstructionIterator iterator, FieldInstruction current) {
+ DexField field = current.getField();
+ // TODO(b/123857022): Should be possible to use definitionFor().
+ DexEncodedField target =
+ current.isInstancePut()
+ ? appInfo.lookupInstanceTarget(field.getHolder(), field)
+ : appInfo.lookupStaticTarget(field.getHolder(), field);
+ // TODO(b/123857022): Should be possible to use `!isFieldRead(field)`.
+ if (target != null && !appInfo.isFieldRead(target.field)) {
+ // Remove writes to dead (i.e. never read) fields.
+ iterator.removeOrReplaceByDebugLocalRead();
}
}
/**
* Replace invoke targets and field accesses with constant values where possible.
- * <p>
- * Also assigns value ranges to values where possible.
+ *
+ * <p>Also assigns value ranges to values where possible.
*/
public void rewriteWithConstantValues(
IRCode code, DexType callingContext, Predicate<DexEncodedMethod> isProcessedConcurrently) {
Set<Value> affectedValues = Sets.newIdentityHashSet();
- InstructionIterator iterator = code.instructionIterator();
- while (iterator.hasNext()) {
- Instruction current = iterator.next();
- if (current.isInvokeMethod()) {
- InvokeMethod invoke = current.asInvokeMethod();
- DexMethod invokedMethod = invoke.getInvokedMethod();
- DexType invokedHolder = invokedMethod.getHolder();
- if (!invokedHolder.isClassType()) {
- continue;
- }
- // TODO(70550443): Maybe check all methods here.
- DexEncodedMethod definition = appInfo
- .lookup(invoke.getType(), invokedMethod, callingContext);
-
- boolean invokeReplaced = false;
- ProguardMemberRuleLookup lookup = lookupMemberRule(definition);
- if (lookup != null) {
- if (lookup.type == RuleType.ASSUME_NO_SIDE_EFFECTS
- && (invoke.outValue() == null || !invoke.outValue().isUsed())) {
- // Remove invoke if marked as having no side effects and the return value is not used.
- iterator.remove();
- invokeReplaced = true;
- } else if (invoke.outValue() != null && invoke.outValue().isUsed()) {
- // Check to see if a constant value can be assumed.
- Instruction replacement =
- constantReplacementFromProguardRule(lookup.rule, code, invoke);
- if (replacement != null) {
- affectedValues.add(replacement.outValue());
- replaceInstructionFromProguardRule(lookup.type, iterator, current, replacement);
- invokeReplaced = true;
- } else {
- // Check to see if a value range can be assumed.
- setValueRangeFromProguardRule(lookup.rule, current.outValue());
- }
- }
- }
-
- // If no Proguard rule could replace the instruction check for knowledge about the
- // return value.
- if (!invokeReplaced && invoke.outValue() != null) {
- DexEncodedMethod target = invoke.lookupSingleTarget(appInfo, callingContext);
- if (target != null) {
- if (target.getOptimizationInfo().neverReturnsNull() && invoke.outValue().canBeNull()) {
- Value knownToBeNonNullValue = invoke.outValue();
- knownToBeNonNullValue.markNeverNull();
- TypeLatticeElement typeLattice = knownToBeNonNullValue.getTypeLattice();
- assert typeLattice.isNullable() && typeLattice.isReference();
- knownToBeNonNullValue.narrowing(appInfo, typeLattice.asNonNullable());
- affectedValues.addAll(knownToBeNonNullValue.affectedValues());
- }
- if (target.getOptimizationInfo().returnsConstant()) {
- long constant = target.getOptimizationInfo().getReturnedConstant();
- ConstNumber replacement = createConstNumberReplacement(
- code, constant, invoke.outValue().getTypeLattice(), invoke.getLocalInfo());
- affectedValues.add(replacement.outValue());
- invoke.outValue().replaceUsers(replacement.outValue());
- invoke.setOutValue(null);
- replacement.setPosition(invoke.getPosition());
- invoke.moveDebugValues(replacement);
- iterator.add(replacement);
- }
- }
- }
- } else if (current.isInstancePut()) {
- InstancePut instancePut = current.asInstancePut();
- DexField field = instancePut.getField();
- DexEncodedField target = appInfo.lookupInstanceTarget(field.getHolder(), field);
- if (target != null) {
- // Remove writes to dead (i.e. never read) fields.
- if (!isFieldRead(target, false) && instancePut.object().isNeverNull()) {
- iterator.remove();
- }
- }
- } else if (current.isStaticGet()) {
- StaticGet staticGet = current.asStaticGet();
- DexField field = staticGet.getField();
- DexEncodedField target = appInfo.lookupStaticTarget(field.getHolder(), field);
- ProguardMemberRuleLookup lookup = null;
- if (target != null) {
- // Check if a this value is known const.
- Instruction replacement = target.valueAsConstInstruction(appInfo, staticGet.dest());
- if (replacement == null) {
- lookup = lookupMemberRule(target);
- if (lookup != null) {
- replacement = constantReplacementFromProguardRule(lookup.rule, code, staticGet);
- }
- }
- if (replacement == null) {
- // If no const replacement was found, at least store the range information.
- if (lookup != null) {
- setValueRangeFromProguardRule(lookup.rule, staticGet.dest());
- }
- }
- if (replacement != null) {
- affectedValues.add(replacement.outValue());
- // Ignore assumenosideeffects for fields.
- if (lookup != null && lookup.type == RuleType.ASSUME_VALUES) {
- replaceInstructionFromProguardRule(lookup.type, iterator, current, replacement);
- } else {
- iterator.replaceCurrentInstruction(replacement);
- }
- } else if (staticGet.dest() != null) {
- // In case the class holder of this static field satisfying following criteria:
- // -- cannot trigger other static initializer except for its own
- // -- is final
- // -- has a class initializer which is classified as trivial
- // (see CodeRewriter::computeClassInitializerInfo) and
- // initializes the field being accessed
- //
- // ... and the field itself is not pinned by keep rules (in which case it might
- // be updated outside the class constructor, e.g. via reflections), it is safe
- // to assume that the static-get instruction reads the value it initialized value
- // in class initializer and is never null.
- //
- DexClass holderDefinition = appInfo.definitionFor(field.getHolder());
- if (holderDefinition != null
- && holderDefinition.accessFlags.isFinal()
- && !appInfo.canTriggerStaticInitializer(field.getHolder(), true)) {
- Value outValue = staticGet.dest();
- DexEncodedMethod classInitializer = holderDefinition.getClassInitializer();
- if (classInitializer != null && !isProcessedConcurrently.test(classInitializer)) {
- TrivialInitializer info =
- classInitializer.getOptimizationInfo().getTrivialInitializerInfo();
- if (info != null
- && ((TrivialClassInitializer) info).field == field
- && !appInfo.isPinned(field)
- && outValue.canBeNull()) {
- outValue.markNeverNull();
- TypeLatticeElement typeLattice = outValue.getTypeLattice();
- assert typeLattice.isNullable() && typeLattice.isReference();
- outValue.narrowing(appInfo, typeLattice.asNonNullable());
- affectedValues.addAll(outValue.affectedValues());
- }
- }
- }
- }
- }
- } else if (current.isStaticPut()) {
- StaticPut staticPut = current.asStaticPut();
- DexField field = staticPut.getField();
- DexEncodedField target = appInfo.lookupStaticTarget(field.getHolder(), field);
- if (target != null) {
- // Remove writes to dead (i.e. never read) fields.
- if (!isFieldRead(target, true)) {
- iterator.removeOrReplaceByDebugLocalRead();
- }
+ ListIterator<BasicBlock> blocks = code.blocks.listIterator();
+ while (blocks.hasNext()) {
+ BasicBlock block = blocks.next();
+ InstructionListIterator iterator = block.listIterator();
+ while (iterator.hasNext()) {
+ Instruction current = iterator.next();
+ if (current.isInvokeMethod()) {
+ rewriteInvokeMethodWithConstantValues(
+ code, callingContext, affectedValues, blocks, iterator, current.asInvokeMethod());
+ } else if (current.isInstancePut() || current.isStaticPut()) {
+ rewritePutWithConstantValues(iterator, current.asFieldInstruction());
+ } else if (current.isStaticGet()) {
+ rewriteStaticGetWithConstantValues(
+ code,
+ isProcessedConcurrently,
+ affectedValues,
+ blocks,
+ iterator,
+ current.asStaticGet());
}
}
}
@@ -290,14 +325,4 @@
}
assert code.isConsistentSSA();
}
-
- private boolean isFieldRead(DexEncodedField field, boolean isStatic) {
- if (appInfo.fieldsRead.contains(field.field)
- || appInfo.isPinned(field.field)) {
- return true;
- }
- // For library classes we don't know whether a field is read.
- DexClass holder = appInfo.definitionFor(field.field.clazz);
- return holder == null || holder.isLibraryClass();
- }
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java
index 87b2537..452f4ed 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java
@@ -1228,7 +1228,7 @@
}
}
- public boolean selectMethodsForOutlining(Map<DexType, DexProgramClass> synthesizedClasses) {
+ public boolean selectMethodsForOutlining() {
assert methodsSelectedForOutlining.size() == 0;
assert outlineSites.size() == 0;
for (List<DexEncodedMethod> outlineMethods : candidateMethodLists) {
@@ -1237,7 +1237,7 @@
methodsSelectedForOutlining.add(
converter
.graphLense()
- .mapDexEncodedMethod(outlineMethod, appInfo, synthesizedClasses));
+ .mapDexEncodedMethod(outlineMethod, appInfo));
}
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
index 2972ffd..1f4c1c3 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
@@ -234,6 +234,6 @@
}
// Check for static initializers in this class or any of interfaces it implements.
- return !appInfo.canTriggerStaticInitializer(clazz, true);
+ return !clazz.initializationOfParentTypesMayHaveSideEffects(appInfo);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
index cdf37c4..d6ca0e0 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
@@ -116,12 +116,14 @@
if (!eligibleClass.isClassType()) {
return false;
}
- eligibleClassDefinition = appInfo.definitionFor(eligibleClass);
- if (eligibleClassDefinition == null && lambdaRewriter != null) {
+ if (lambdaRewriter != null) {
// Check if the class is synthesized for a desugared lambda
eligibleClassDefinition = lambdaRewriter.getLambdaClass(eligibleClass);
isDesugaredLambda = eligibleClassDefinition != null;
}
+ if (eligibleClassDefinition == null) {
+ eligibleClassDefinition = appInfo.definitionFor(eligibleClass);
+ }
return eligibleClassDefinition != null;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/lambda/LambdaMerger.java b/src/main/java/com/android/tools/r8/ir/optimize/lambda/LambdaMerger.java
index b988a89..ab6f635 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/lambda/LambdaMerger.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/lambda/LambdaMerger.java
@@ -26,6 +26,7 @@
import com.android.tools.r8.ir.conversion.CallSiteInformation;
import com.android.tools.r8.ir.conversion.IRConverter;
import com.android.tools.r8.ir.conversion.OptimizationFeedback;
+import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget;
import com.android.tools.r8.ir.optimize.Outliner;
import com.android.tools.r8.ir.optimize.lambda.CodeProcessor.Strategy;
import com.android.tools.r8.ir.optimize.lambda.LambdaGroup.LambdaStructureError;
@@ -36,6 +37,7 @@
import com.android.tools.r8.utils.StringDiagnostic;
import com.android.tools.r8.utils.ThreadUtils;
import com.android.tools.r8.utils.ThrowingConsumer;
+import com.google.common.base.Predicates;
import com.google.common.collect.Sets;
import java.util.ArrayList;
import java.util.IdentityHashMap;
@@ -194,8 +196,7 @@
IRConverter converter,
OptimizationFeedback feedback,
Builder<?> builder,
- ExecutorService executorService,
- Map<DexType, DexProgramClass> synthesizedClasses)
+ ExecutorService executorService)
throws ExecutionException {
if (lambdas.isEmpty()) {
return;
@@ -218,18 +219,29 @@
this.strategyFactory = ApplyStrategy::new;
// Add synthesized lambda group classes to the builder.
- converter.optimizeSynthesizedClasses(lambdaGroupsClasses.values(), executorService);
for (Entry<LambdaGroup, DexProgramClass> entry : lambdaGroupsClasses.entrySet()) {
DexProgramClass synthesizedClass = entry.getValue();
- synthesizedClasses.put(synthesizedClass.type, synthesizedClass);
+ converter.appInfo.addSynthesizedClass(synthesizedClass);
builder.addSynthesizedClass(
synthesizedClass, entry.getKey().shouldAddToMainDex(converter.appInfo));
+ // Eventually, we need to process synthesized methods in the lambda group.
+ // Otherwise, abstract SynthesizedCode will be flown to Enqueuer.
+ // But that process should not see the holder. Otherwise, lambda calls in the main dispatch
+ // method became recursive calls via the lense rewriter. They should remain, then inliner
+ // will inline methods from mergee lambdas to the main dispatch method.
+ // Then, there is a dilemma: other sub optimizations trigger subtype lookup that will throw
+ // NPE if it cannot find the holder for this synthesized lambda group.
+ // One hack here is to mark those methods `processed` so that the lense rewriter is skipped.
+ synthesizedClass.forEachMethod(encodedMethod -> {
+ encodedMethod.markProcessed(ConstraintWithTarget.NEVER);
+ });
}
+ converter.optimizeSynthesizedClasses(lambdaGroupsClasses.values(), executorService);
// Rewrite lambda class references into lambda group class
// references inside methods from the processing queue.
- rewriteLambdaReferences(converter, synthesizedClasses, feedback);
+ rewriteLambdaReferences(converter, feedback);
this.strategyFactory = null;
}
@@ -304,10 +316,7 @@
}
}
- private void rewriteLambdaReferences(
- IRConverter converter,
- Map<DexType, DexProgramClass> synthesizedClasses,
- OptimizationFeedback feedback) {
+ private void rewriteLambdaReferences(IRConverter converter, OptimizationFeedback feedback) {
List<DexEncodedMethod> methods =
methodsToReprocess
.stream()
@@ -315,9 +324,9 @@
.collect(Collectors.toList());
for (DexEncodedMethod method : methods) {
DexEncodedMethod mappedMethod =
- converter.graphLense().mapDexEncodedMethod(method, converter.appInfo, synthesizedClasses);
+ converter.graphLense().mapDexEncodedMethod(method, converter.appInfo);
converter.processMethod(mappedMethod, feedback,
- x -> false, CallSiteInformation.empty(), Outliner::noProcessing);
+ Predicates.alwaysFalse(), CallSiteInformation.empty(), Outliner::noProcessing);
assert mappedMethod.isProcessed();
}
}
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 d08f3e6..e4d2cec 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1980,11 +1980,11 @@
/**
* Set of all fields which may be touched by a get operation. This is actual field definitions.
*/
- public final SortedSet<DexField> fieldsRead;
+ private final SortedSet<DexField> fieldsRead;
/**
* Set of all fields which may be touched by a put operation. This is actual field definitions.
*/
- public final SortedSet<DexField> fieldsWritten;
+ private final SortedSet<DexField> fieldsWritten;
/**
* Set of all field ids used in instance field reads, along with access context.
*/
@@ -2354,7 +2354,8 @@
public boolean isInstantiatedDirectly(DexType type) {
assert type.isClassType();
- return instantiatedTypes.contains(type)
+ return type.isD8R8SynthesizedClassType()
+ || instantiatedTypes.contains(type)
|| instantiatedLambdas.contains(type)
|| instantiatedAnnotationTypes.contains(type);
}
@@ -2381,6 +2382,31 @@
return isInstantiatedDirectly(type) || isInstantiatedIndirectly(type);
}
+ public boolean isFieldRead(DexField field) {
+ return fieldsRead.contains(field)
+ // TODO(b/121354886): Pinned fields should be in `fieldsRead`.
+ || isPinned(field)
+ // Fields in the class that is synthesized by D8/R8 would be used soon.
+ || field.getHolder().isD8R8SynthesizedClassType()
+ // For library classes we don't know whether a field is read.
+ || isLibraryField(field);
+ }
+
+ public boolean isFieldWritten(DexField field) {
+ return fieldsWritten.contains(field)
+ // TODO(b/121354886): Pinned fields should be in `fieldsWritten`.
+ || isPinned(field)
+ // Fields in the class that is synthesized by D8/R8 would be used soon.
+ || field.clazz.isD8R8SynthesizedClassType()
+ // For library classes we don't know whether a field is rewritten.
+ || isLibraryField(field);
+ }
+
+ private boolean isLibraryField(DexField field) {
+ DexClass holder = definitionFor(field.clazz);
+ return holder == null || holder.isLibraryClass();
+ }
+
private Object2BooleanMap<DexReference> joinIdentifierNameStrings(
Set<DexReference> explicit, Set<DexReference> implicit) {
Object2BooleanMap<DexReference> result = new Object2BooleanArrayMap<>();
diff --git a/src/main/java/com/android/tools/r8/shaking/TreePruner.java b/src/main/java/com/android/tools/r8/shaking/TreePruner.java
index acc1e78..8017878 100644
--- a/src/main/java/com/android/tools/r8/shaking/TreePruner.java
+++ b/src/main/java/com/android/tools/r8/shaking/TreePruner.java
@@ -228,8 +228,8 @@
Predicate<DexField> isReachableOrReferencedField =
field ->
appInfo.liveFields.contains(field)
- || appInfo.fieldsRead.contains(field)
- || appInfo.fieldsWritten.contains(field);
+ || appInfo.isFieldRead(field)
+ || appInfo.isFieldWritten(field);
int firstUnreachable =
firstUnreachableIndex(Arrays.asList(fields), isReachableOrReferencedField);
// Return the original array if all fields are used.
diff --git a/src/test/java/com/android/tools/r8/memberrebinding/IllegalFieldRebindingTest.java b/src/test/java/com/android/tools/r8/memberrebinding/IllegalFieldRebindingTest.java
new file mode 100644
index 0000000..cc35cbd
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/memberrebinding/IllegalFieldRebindingTest.java
@@ -0,0 +1,125 @@
+// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.memberrebinding;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.memberrebinding.testclasses.IllegalFieldRebindingTestClasses;
+import com.android.tools.r8.memberrebinding.testclasses.IllegalFieldRebindingTestClasses.B;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class IllegalFieldRebindingTest extends TestBase {
+
+ private final Backend backend;
+
+ @Parameters(name = "Backend: {0}")
+ public static Backend[] data() {
+ return Backend.values();
+ }
+
+ public IllegalFieldRebindingTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Test
+ public void test() throws Exception {
+ String expectedOutput = StringUtils.lines("42");
+
+ if (backend == Backend.CF) {
+ testForJvm().addTestClasspath().run(TestClass.class).assertSuccessWithOutput(expectedOutput);
+ }
+
+ CodeInspector inspector =
+ testForR8(Backend.DEX)
+ .addInnerClasses(IllegalFieldRebindingTest.class)
+ .addInnerClasses(IllegalFieldRebindingTestClasses.class)
+ .addKeepMainRule(TestClass.class)
+ .enableMergeAnnotations()
+ .run(TestClass.class)
+ .assertSuccessWithOutput(expectedOutput)
+ .inspector();
+
+ ClassSubject classSubject = inspector.clazz(TestClass.class);
+ assertThat(classSubject, isPresent());
+
+ MethodSubject methodSubject = classSubject.mainMethod();
+ assertThat(methodSubject, isPresent());
+
+ // Verify that the static-put instruction has not been removed.
+ assertEquals(
+ 1, methodSubject.streamInstructions().filter(InstructionSubject::isStaticPut).count());
+ }
+
+ @Test
+ public void otherTest() throws Exception {
+ String expectedOutput = StringUtils.lines("0");
+
+ if (backend == Backend.CF) {
+ testForJvm()
+ .addTestClasspath()
+ .run(OtherTestClass.class)
+ .assertSuccessWithOutput(expectedOutput);
+ }
+
+ CodeInspector inspector =
+ testForR8(Backend.DEX)
+ .addInnerClasses(IllegalFieldRebindingTest.class)
+ .addInnerClasses(IllegalFieldRebindingTestClasses.class)
+ .addKeepMainRule(OtherTestClass.class)
+ .enableMergeAnnotations()
+ .run(OtherTestClass.class)
+ .assertSuccessWithOutput(expectedOutput)
+ .inspector();
+
+ // The static-get instruction in OtherTestClass.main() should have been replaced by the
+ // constant 0, which makes the remaining classes unreachable.
+ assertEquals(1, inspector.allClasses().size());
+
+ ClassSubject classSubject = inspector.clazz(OtherTestClass.class);
+ assertThat(classSubject, isPresent());
+
+ MethodSubject methodSubject = classSubject.mainMethod();
+ assertThat(methodSubject, isPresent());
+
+ // Verify that a constant 0 is being loaded in main().
+ assertEquals(
+ 1,
+ methodSubject
+ .streamInstructions()
+ .filter(instruction -> instruction.isConstNumber(0))
+ .count());
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ // Cannot be member rebound because A is inaccessible to this package. However, the
+ // instruction cannot be removed as the field `A.f` is actually read.
+ B.f = 42;
+ B.print();
+ }
+ }
+
+ static class OtherTestClass {
+
+ public static void main(String[] args) {
+ // Cannot be member rebound because A is inaccessible to this package.
+ // However, the instruction should still be replaced by the constant 0.
+ System.out.println(B.f);
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/memberrebinding/testclasses/IllegalFieldRebindingTestClasses.java b/src/test/java/com/android/tools/r8/memberrebinding/testclasses/IllegalFieldRebindingTestClasses.java
new file mode 100644
index 0000000..64b4d1f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/memberrebinding/testclasses/IllegalFieldRebindingTestClasses.java
@@ -0,0 +1,22 @@
+// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.memberrebinding.testclasses;
+
+import com.android.tools.r8.NeverMerge;
+
+public class IllegalFieldRebindingTestClasses {
+
+ @NeverMerge
+ static class A {
+ public static int f;
+ }
+
+ public static class B extends A {
+
+ public static void print() {
+ System.out.println(A.f);
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/ImplicitlyKeptDefaultConstructorTest.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/ImplicitlyKeptDefaultConstructorTest.java
index d8631ee..6731209 100644
--- a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/ImplicitlyKeptDefaultConstructorTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/ImplicitlyKeptDefaultConstructorTest.java
@@ -287,8 +287,7 @@
mainClass,
ImmutableList.of(mainClass, StaticFieldNotInitialized.class),
keepMainProguardConfiguration(mainClass),
- // TODO(74379749): Proguard does not keep the class with the un-initialized static field.
- this::checkAllClassesPresentOnlyMainWithDefaultConstructor,
+ this::checkOnlyMainPresent,
this::checkOnlyMainPresent);
}
