Reland "Adjust accessibility for desugared lambdas prior to IR processing in R8"
This reverts commit ff2c5273052f483102a1f28b0214edf6ba471eee.
Change-Id: I95d84a932e26b2678d5c643e99c43631896ce362
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index d7dc501..2ecc549 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -823,8 +823,6 @@
assert (dexCode.getDebugInfo() == null)
|| (arity == dexCode.getDebugInfo().parameters.length);
} else {
- assert appView.options().isDesugaredLibraryCompilation()
- || appView.options().enableCfInterfaceMethodDesugaring;
assert code.isCfCode();
CfCode cfCode = code.asCfCode();
cfCode.addFakeThisParameter(appView.dexItemFactory());
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 6ef43a6..477925a 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
@@ -274,9 +274,7 @@
assert appView.rootSet() != null;
AppView<AppInfoWithLiveness> appViewWithLiveness = appView.withLiveness();
this.classInliner =
- options.enableClassInlining && options.enableInlining
- ? new ClassInliner(lambdaRewriter)
- : null;
+ options.enableClassInlining && options.enableInlining ? new ClassInliner() : null;
this.classStaticizer =
options.enableClassStaticizer ? new ClassStaticizer(appViewWithLiveness, this) : null;
this.dynamicTypeOptimization =
@@ -402,9 +400,11 @@
private void synthesizeLambdaClasses(Builder<?> builder, ExecutorService executorService)
throws ExecutionException {
if (lambdaRewriter != null) {
- lambdaRewriter.adjustAccessibility(this);
- lambdaRewriter.synthesizeLambdaClasses(builder);
- lambdaRewriter.optimizeSynthesizedClasses(this, executorService);
+ if (appView.enableWholeProgramOptimizations()) {
+ lambdaRewriter.finalizeLambdaDesugaringForR8(builder);
+ } else {
+ lambdaRewriter.finalizeLambdaDesugaringForD8(builder, this, executorService);
+ }
}
}
@@ -625,7 +625,6 @@
}
public DexApplication optimize(ExecutorService executorService) throws ExecutionException {
-
if (options.isShrinking()) {
assert !removeLambdaDeserializationMethods();
} else {
@@ -638,6 +637,10 @@
collectLambdaMergingCandidates(application);
collectStaticizerCandidates(application);
+ if (lambdaRewriter != null) {
+ lambdaRewriter.installGraphLens();
+ }
+
// The process is in two phases in general.
// 1) Subject all DexEncodedMethods to optimization, except some optimizations that require
// reprocessing IR code of methods, e.g., outlining, double-inlining, class staticizer, etc.
@@ -815,7 +818,7 @@
if (lambdaRewriter != null) {
lambdaRewriter.synthesizeLambdaClassesForWave(
- wave, executorService, delayedOptimizationFeedback, lensCodeRewriter, this);
+ wave, this, executorService, delayedOptimizationFeedback, lensCodeRewriter);
}
}
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 18edf5f..0dcaf2a 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
@@ -27,7 +27,6 @@
import com.android.tools.r8.graph.ParameterAnnotationsList;
import com.android.tools.r8.ir.code.Invoke;
import com.android.tools.r8.ir.code.Invoke.Type;
-import com.android.tools.r8.ir.conversion.IRConverter;
import com.android.tools.r8.ir.synthetic.SynthesizedCode;
import com.android.tools.r8.origin.SynthesizedOrigin;
import com.android.tools.r8.utils.InternalOptions;
@@ -502,6 +501,9 @@
final DexMethod callTarget;
final Invoke.Type invokeType;
+ private boolean hasEnsuredAccessibility;
+ private DexEncodedMethod accessibilityBridge;
+
Target(DexMethod callTarget, Invoke.Type invokeType) {
assert callTarget != null;
assert invokeType != null;
@@ -510,7 +512,16 @@
}
// Ensure access of the referenced symbol(s).
- abstract void ensureAccessibility(IRConverter converter);
+ abstract DexEncodedMethod ensureAccessibility();
+
+ // Ensure access of the referenced symbol(s).
+ DexEncodedMethod ensureAccessibilityIfNeeded() {
+ if (!hasEnsuredAccessibility) {
+ accessibilityBridge = ensureAccessibility();
+ hasEnsuredAccessibility = true;
+ }
+ return accessibilityBridge;
+ }
DexClass definitionFor(DexType type) {
return rewriter.getAppInfo().app().definitionFor(type);
@@ -553,7 +564,9 @@
}
@Override
- void ensureAccessibility(IRConverter converter) {}
+ DexEncodedMethod ensureAccessibility() {
+ return null;
+ }
}
// Used for static private lambda$ methods. Only needs access relaxation.
@@ -564,7 +577,7 @@
}
@Override
- void ensureAccessibility(IRConverter converter) {
+ DexEncodedMethod ensureAccessibility() {
// We already found the static method to be called, just relax its accessibility.
assert descriptor.getAccessibility() != null;
descriptor.getAccessibility().unsetPrivate();
@@ -572,6 +585,7 @@
if (implMethodHolder.isInterface()) {
descriptor.getAccessibility().setPublic();
}
+ return null;
}
}
@@ -584,7 +598,7 @@
}
@Override
- void ensureAccessibility(IRConverter converter) {
+ DexEncodedMethod ensureAccessibility() {
// For all instantiation points for which the compiler creates lambda$
// methods, it creates these methods in the same class/interface.
DexMethod implMethod = descriptor.implHandle.asMethod();
@@ -611,16 +625,18 @@
encodedMethod.getCode(),
true);
newMethod.copyMetadata(encodedMethod);
- rewriter.methodMapping.put(encodedMethod.method, callTarget);
+ rewriter.originalMethodSignatures.put(callTarget, encodedMethod.method);
DexEncodedMethod.setDebugInfoWithFakeThisParameter(
newMethod.getCode(), callTarget.getArity(), rewriter.getAppView());
implMethodHolder.setDirectMethod(i, newMethod);
- return;
+
+ return newMethod;
}
}
assert false
: "Unexpected failure to find direct lambda target for: " + implMethod.qualifiedName();
+ return null;
}
}
@@ -632,7 +648,7 @@
}
@Override
- void ensureAccessibility(IRConverter converter) {
+ DexEncodedMethod ensureAccessibility() {
// For all instantiation points for which the compiler creates lambda$
// methods, it creates these methods in the same class/interface.
DexMethod implMethod = descriptor.implHandle.asMethod();
@@ -656,13 +672,15 @@
encodedMethod.getCode(),
true);
newMethod.copyMetadata(encodedMethod);
- rewriter.methodMapping.put(encodedMethod.method, callTarget);
+ rewriter.originalMethodSignatures.put(callTarget, encodedMethod.method);
// Move the method from the direct methods to the virtual methods set.
implMethodHolder.removeDirectMethod(i);
implMethodHolder.appendVirtualMethod(newMethod);
- return;
+
+ return newMethod;
}
}
+ return null;
}
}
@@ -675,7 +693,7 @@
}
@Override
- void ensureAccessibility(IRConverter converter) {
+ DexEncodedMethod ensureAccessibility() {
// Create a static accessor with proper accessibility.
DexProgramClass accessorClass = programDefinitionFor(callTarget.holder);
assert accessorClass != null;
@@ -702,7 +720,7 @@
accessorClass.appendDirectMethod(accessorEncodedMethod);
}
- converter.optimizeSynthesizedMethod(accessorEncodedMethod);
+ return accessorEncodedMethod;
}
}
}
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 92d04f0..f093a78 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
@@ -16,7 +16,6 @@
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
-import com.android.tools.r8.graph.DexProto;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.analysis.type.Nullability;
@@ -73,7 +72,8 @@
final DexString classConstructorName;
final DexString instanceFieldName;
- final BiMap<DexMethod, DexMethod> methodMapping = HashBiMap.create();
+ private final LambdaRewriterGraphLense graphLens;
+ final BiMap<DexMethod, DexMethod> originalMethodSignatures = HashBiMap.create();
// Maps call sites seen so far to inferred lambda descriptor. It is intended
// to help avoid re-matching call sites we already seen. Note that same call
@@ -93,10 +93,14 @@
public LambdaRewriter(AppView<?> appView) {
this.appView = appView;
+ this.graphLens = new LambdaRewriterGraphLense(appView);
this.constructorName = getFactory().createString(Constants.INSTANCE_INITIALIZER_NAME);
- DexProto initProto = getFactory().createProto(getFactory().voidType);
this.objectInitMethod =
- getFactory().createMethod(getFactory().objectType, initProto, constructorName);
+ getFactory()
+ .createMethod(
+ getFactory().objectType,
+ getFactory().createProto(getFactory().voidType),
+ constructorName);
this.classConstructorName = getFactory().createString(Constants.CLASS_INITIALIZER_NAME);
this.instanceFieldName = getFactory().createString(LAMBDA_INSTANCE_FIELD_NAME);
}
@@ -113,26 +117,40 @@
return getAppView().dexItemFactory();
}
+ public void installGraphLens() {
+ appView.setGraphLense(graphLens);
+ }
+
public void synthesizeLambdaClassesForWave(
Collection<DexEncodedMethod> wave,
+ IRConverter converter,
ExecutorService executorService,
OptimizationFeedbackDelayed feedback,
- LensCodeRewriter lensCodeRewriter,
- IRConverter converter)
+ LensCodeRewriter lensCodeRewriter)
throws ExecutionException {
- Set<DexProgramClass> synthesizedLambdaClasses = Sets.newIdentityHashSet();
+ Set<LambdaClass> synthesizedLambdaClasses = Sets.newIdentityHashSet();
+ Set<DexProgramClass> synthesizedLambdaProgramClasses = Sets.newIdentityHashSet();
for (DexEncodedMethod method : wave) {
- synthesizeLambdaClassesForMethod(method, synthesizedLambdaClasses::add, lensCodeRewriter);
+ synthesizeLambdaClassesForMethod(
+ method,
+ lambdaClass -> {
+ synthesizedLambdaClasses.add(lambdaClass);
+ synthesizedLambdaProgramClasses.add(lambdaClass.getOrCreateLambdaClass());
+ },
+ lensCodeRewriter);
}
if (synthesizedLambdaClasses.isEmpty()) {
return;
}
+ synthesizeAccessibilityBridgesForLambdaClasses(
+ synthesizedLambdaClasses, converter, executorService, feedback, lensCodeRewriter);
+
// Record that the static fields on each lambda class are only written inside the static
// initializer of the lambdas.
Map<DexEncodedField, Set<DexEncodedMethod>> writesWithContexts = new IdentityHashMap<>();
- for (DexProgramClass synthesizedLambdaClass : synthesizedLambdaClasses) {
+ for (DexProgramClass synthesizedLambdaClass : synthesizedLambdaProgramClasses) {
DexEncodedMethod clinit = synthesizedLambdaClass.getClassInitializer();
if (clinit != null) {
for (DexEncodedField field : synthesizedLambdaClass.staticFields()) {
@@ -145,14 +163,50 @@
appViewWithLiveness.setAppInfo(
appViewWithLiveness.appInfo().withStaticFieldWrites(writesWithContexts));
- converter.optimizeSynthesizedLambdaClasses(synthesizedLambdaClasses, executorService);
+ converter.optimizeSynthesizedLambdaClasses(synthesizedLambdaProgramClasses, executorService);
feedback.updateVisibleOptimizationInfo();
}
+ private void synthesizeAccessibilityBridgesForLambdaClasses(
+ Collection<LambdaClass> lambdaClasses, IRConverter converter, ExecutorService executorService)
+ throws ExecutionException {
+ synthesizeAccessibilityBridgesForLambdaClasses(
+ lambdaClasses, converter, executorService, null, null);
+ }
+
+ private void synthesizeAccessibilityBridgesForLambdaClasses(
+ Collection<LambdaClass> lambdaClasses,
+ IRConverter converter,
+ ExecutorService executorService,
+ OptimizationFeedbackDelayed feedback,
+ LensCodeRewriter lensCodeRewriter)
+ throws ExecutionException {
+ Set<DexEncodedMethod> nonDexAccessibilityBridges = Sets.newIdentityHashSet();
+ for (LambdaClass lambdaClass : lambdaClasses) {
+ // This call may cause originalMethodSignatures to be updated.
+ DexEncodedMethod accessibilityBridge = lambdaClass.target.ensureAccessibilityIfNeeded();
+ if (accessibilityBridge != null && !accessibilityBridge.getCode().isDexCode()) {
+ nonDexAccessibilityBridges.add(accessibilityBridge);
+ }
+ }
+ if (appView.enableWholeProgramOptimizations()) {
+ if (!originalMethodSignatures.isEmpty()) {
+ graphLens.addOriginalMethodSignatures(originalMethodSignatures);
+ originalMethodSignatures.clear();
+ }
+
+ // Ensure that all lambda classes referenced from accessibility bridges are synthesized
+ // prior to the IR processing of these accessibility bridges.
+ synthesizeLambdaClassesForWave(
+ nonDexAccessibilityBridges, converter, executorService, feedback, lensCodeRewriter);
+ }
+ if (!nonDexAccessibilityBridges.isEmpty()) {
+ converter.processMethodsConcurrently(nonDexAccessibilityBridges, executorService);
+ }
+ }
+
public void synthesizeLambdaClassesForMethod(
- DexEncodedMethod method,
- Consumer<DexProgramClass> consumer,
- LensCodeRewriter lensCodeRewriter) {
+ DexEncodedMethod method, Consumer<LambdaClass> consumer, LensCodeRewriter lensCodeRewriter) {
if (!method.hasCode() || method.isProcessed()) {
// Nothing to desugar.
return;
@@ -175,9 +229,7 @@
LambdaDescriptor descriptor =
inferLambdaDescriptor(lensCodeRewriter.rewriteCallSite(callSite, method));
if (descriptor != LambdaDescriptor.MATCH_FAILED) {
- consumer.accept(
- getOrCreateLambdaClass(descriptor, method.method.holder)
- .getOrCreateLambdaClass());
+ consumer.accept(getOrCreateLambdaClass(descriptor, method.method.holder));
}
}
});
@@ -249,41 +301,21 @@
return false;
}
- /** Adjust accessibility of referenced application symbols or creates necessary accessors. */
- public void adjustAccessibility(IRConverter converter) {
- // For each lambda class perform necessary adjustment of the
- // referenced symbols to make them accessible. This can result in
- // method access relaxation or creation of accessor method.
- for (LambdaClass lambdaClass : knownLambdaClasses.values()) {
- // This call may cause methodMapping to be updated.
- lambdaClass.target.ensureAccessibility(converter);
- }
- if (getAppView().enableWholeProgramOptimizations() && !methodMapping.isEmpty()) {
- getAppView()
- .setGraphLense(
- new LambdaRewriterGraphLense(methodMapping, getAppView().graphLense(), getFactory()));
- }
- }
-
- /**
- * Returns a synthetic class for desugared lambda or `null` if the `type` does not represent one.
- * Method can be called concurrently.
- */
- public DexProgramClass getLambdaClass(DexType type) {
- LambdaClass lambdaClass = getKnown(knownLambdaClasses, type);
- return lambdaClass == null ? null : lambdaClass.getOrCreateLambdaClass();
- }
-
/** Generates lambda classes and adds them to the builder. */
- public void synthesizeLambdaClasses(Builder<?> builder) {
+ public void finalizeLambdaDesugaringForD8(
+ Builder<?> builder, IRConverter converter, ExecutorService executorService)
+ throws ExecutionException {
+ synthesizeAccessibilityBridgesForLambdaClasses(
+ knownLambdaClasses.values(), converter, executorService);
for (LambdaClass lambdaClass : knownLambdaClasses.values()) {
DexProgramClass synthesizedClass = lambdaClass.getOrCreateLambdaClass();
getAppInfo().addSynthesizedClass(synthesizedClass);
builder.addSynthesizedClass(synthesizedClass, lambdaClass.addToMainDexList.get());
}
+ optimizeSynthesizedClasses(converter, executorService);
}
- public void optimizeSynthesizedClasses(IRConverter converter, ExecutorService executorService)
+ private void optimizeSynthesizedClasses(IRConverter converter, ExecutorService executorService)
throws ExecutionException {
converter.optimizeSynthesizedClasses(
knownLambdaClasses.values().stream()
@@ -292,6 +324,18 @@
executorService);
}
+ /** Generates lambda classes and adds them to the builder. */
+ public void finalizeLambdaDesugaringForR8(Builder<?> builder) {
+ // Verify that all mappings have been published to the LambdaRewriterGraphLense.
+ assert originalMethodSignatures.isEmpty();
+ graphLens.markShouldMapInvocationType();
+ for (LambdaClass lambdaClass : knownLambdaClasses.values()) {
+ DexProgramClass synthesizedClass = lambdaClass.getOrCreateLambdaClass();
+ getAppInfo().addSynthesizedClass(synthesizedClass);
+ builder.addSynthesizedClass(synthesizedClass, lambdaClass.addToMainDexList.get());
+ }
+ }
+
public Set<DexCallSite> getDesugaredCallSites() {
synchronized (knownCallSites) {
return knownCallSites.keySet();
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriterGraphLense.java b/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriterGraphLense.java
index d5ceeb1..9903607 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriterGraphLense.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriterGraphLense.java
@@ -3,36 +3,55 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.desugar;
-import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexMethod;
-import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.graph.GraphLense.NestedGraphLense;
-import com.android.tools.r8.ir.code.Invoke.Type;
-import com.google.common.collect.BiMap;
+import com.android.tools.r8.ir.code.Invoke;
+import com.google.common.collect.HashBiMap;
import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.ImmutableMap;
+import java.util.Map;
class LambdaRewriterGraphLense extends NestedGraphLense {
- LambdaRewriterGraphLense(
- BiMap<DexMethod, DexMethod> methodMapping, GraphLense previous, DexItemFactory factory) {
+ // We don't map the invocation type in the lens code rewriter for lambda accessibility bridges to
+ // ensure that we always compute the same unique id for invoke-custom instructions.
+ //
+ // TODO(b/129458850): It might be possible to avoid this by performing lambda desugaring prior to
+ // lens code rewriting in the IR converter.
+ private boolean shouldMapInvocationType = false;
+
+ LambdaRewriterGraphLense(AppView<?> appView) {
super(
ImmutableMap.of(),
- methodMapping,
+ ImmutableMap.of(),
ImmutableMap.of(),
ImmutableBiMap.of(),
- methodMapping.inverse(),
- previous,
- factory);
+ HashBiMap.create(),
+ appView.graphLense(),
+ appView.dexItemFactory());
}
@Override
- protected Type mapInvocationType(DexMethod newMethod, DexMethod originalMethod, Type type) {
- if (methodMap.get(originalMethod) == newMethod) {
- assert type == Type.VIRTUAL || type == Type.DIRECT;
- return Type.STATIC;
+ protected boolean isLegitimateToHaveEmptyMappings() {
+ return true;
+ }
+
+ void addOriginalMethodSignatures(Map<DexMethod, DexMethod> originalMethodSignatures) {
+ this.originalMethodSignatures.putAll(originalMethodSignatures);
+ }
+
+ void markShouldMapInvocationType() {
+ shouldMapInvocationType = true;
+ }
+
+ @Override
+ protected Invoke.Type mapInvocationType(
+ DexMethod newMethod, DexMethod originalMethod, Invoke.Type type) {
+ if (shouldMapInvocationType && methodMap.get(originalMethod) == newMethod) {
+ assert type == Invoke.Type.VIRTUAL || type == Invoke.Type.DIRECT;
+ return Invoke.Type.STATIC;
}
return super.mapInvocationType(newMethod, originalMethod, type);
}
-
}
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 36fd914..538cba6 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
@@ -13,7 +13,6 @@
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionOrPhi;
import com.android.tools.r8.ir.code.Value;
-import com.android.tools.r8.ir.desugar.LambdaRewriter;
import com.android.tools.r8.ir.optimize.CodeRewriter;
import com.android.tools.r8.ir.optimize.Inliner;
import com.android.tools.r8.ir.optimize.InliningOracle;
@@ -60,14 +59,9 @@
ELIGIBLE
}
- private final LambdaRewriter lambdaRewriter;
private final ConcurrentHashMap<DexClass, EligibilityStatus> knownClasses =
new ConcurrentHashMap<>();
- public ClassInliner(LambdaRewriter lambdaRewriter) {
- this.lambdaRewriter = lambdaRewriter;
- }
-
private void logEligibilityStatus(
DexEncodedMethod context, Instruction root, EligibilityStatus status) {
if (Log.ENABLED && Log.isLoggingEnabledFor(ClassInliner.class)) {
@@ -192,7 +186,6 @@
InlineCandidateProcessor processor =
new InlineCandidateProcessor(
appView,
- lambdaRewriter,
inliner,
clazz -> isClassEligible(appView, clazz),
isProcessedConcurrently,
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 d4d43f4..157e7b4 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
@@ -35,7 +35,6 @@
import com.android.tools.r8.ir.code.InvokeMethodWithReceiver;
import com.android.tools.r8.ir.code.StaticGet;
import com.android.tools.r8.ir.code.Value;
-import com.android.tools.r8.ir.desugar.LambdaRewriter;
import com.android.tools.r8.ir.optimize.Inliner;
import com.android.tools.r8.ir.optimize.Inliner.InlineAction;
import com.android.tools.r8.ir.optimize.Inliner.InliningInfo;
@@ -78,7 +77,6 @@
ImmutableSet.of(If.Type.EQ, If.Type.NE);
private final AppView<AppInfoWithLiveness> appView;
- private final LambdaRewriter lambdaRewriter;
private final Inliner inliner;
private final Function<DexClass, EligibilityStatus> isClassEligible;
private final Predicate<DexEncodedMethod> isProcessedConcurrently;
@@ -88,7 +86,6 @@
private Value eligibleInstance;
private DexType eligibleClass;
private DexProgramClass eligibleClassDefinition;
- private boolean isDesugaredLambda;
private final Map<InvokeMethodWithReceiver, InliningInfo> methodCallsOnInstance =
new IdentityHashMap<>();
@@ -106,14 +103,12 @@
InlineCandidateProcessor(
AppView<AppInfoWithLiveness> appView,
- LambdaRewriter lambdaRewriter,
Inliner inliner,
Function<DexClass, EligibilityStatus> isClassEligible,
Predicate<DexEncodedMethod> isProcessedConcurrently,
DexEncodedMethod method,
Instruction root) {
this.appView = appView;
- this.lambdaRewriter = lambdaRewriter;
this.inliner = inliner;
this.isClassEligible = isClassEligible;
this.method = method;
@@ -165,11 +160,6 @@
if (!eligibleClass.isClassType()) {
return EligibilityStatus.NON_CLASS_TYPE;
}
- if (lambdaRewriter != null) {
- // Check if the class is synthesized for a desugared lambda
- eligibleClassDefinition = lambdaRewriter.getLambdaClass(eligibleClass);
- isDesugaredLambda = eligibleClassDefinition != null;
- }
if (eligibleClassDefinition == null) {
eligibleClassDefinition = asProgramClassOrNull(appView.definitionFor(eligibleClass));
}
@@ -213,65 +203,6 @@
}
assert root.isStaticGet();
-
- // We know that desugared lambda classes satisfy eligibility requirements.
- if (isDesugaredLambda) {
- return EligibilityStatus.ELIGIBLE;
- }
-
- // Checking if we can safely inline class implemented following singleton-like
- // pattern, by which we assume a static final field holding on to the reference
- // initialized in class constructor.
- //
- // In general we are targeting cases when the class is defined as:
- //
- // class X {
- // static final X F;
- // static {
- // F = new X();
- // }
- // }
- //
- // and being used as follows:
- //
- // void foo() {
- // f = X.F;
- // f.bar();
- // }
- //
- // The main difference from the similar case of class inliner with 'new-instance'
- // instruction is that in this case the instance we inline is not just leaked, but
- // is actually published via X.F field. There are several risks we need to address
- // in this case:
- //
- // Risk: instance stored in field X.F has changed after it was initialized in
- // class initializer
- // Solution: we assume that final field X.F is not modified outside the class
- // initializer. In rare cases when it is (e.g. via reflections) it should
- // be marked with keep rules
- //
- // Risk: instance stored in field X.F is not initialized yet
- // Solution: not initialized instance can only be visible if X.<clinit>
- // triggers other class initialization which references X.F. This
- // situation should never happen if we:
- // -- don't allow any superclasses to have static initializer,
- // -- don't allow any subclasses,
- // -- guarantee the class has trivial class initializer
- // (see CodeRewriter::computeClassInitializerInfo), and
- // -- guarantee the instance is initialized with trivial instance
- // initializer (see CodeRewriter::computeInstanceInitializerInfo)
- //
- // Risk: instance stored in field X.F was mutated
- // Solution: we require that class X does not have any instance fields, and
- // if any of its superclasses has instance fields, accessing them will make
- // this instance not eligible for inlining. I.e. even though the instance is
- // publicized and its state has been mutated, it will not effect the logic
- // of class inlining
- //
-
- if (!eligibleClassDefinition.instanceFields().isEmpty()) {
- return EligibilityStatus.HAS_INSTANCE_FIELDS;
- }
return EligibilityStatus.ELIGIBLE;
}
@@ -305,10 +236,24 @@
indirectUsers.addAll(alias.uniqueUsers());
continue;
}
- // Field read/write.
- if (user.isInstanceGet()
- || (user.isInstancePut()
- && receivers.addIllegalReceiverAlias(user.asInstancePut().value()))) {
+
+ if (user.isInstanceGet()) {
+ if (root.isStaticGet()) {
+ // We don't have a replacement for this field read.
+ return user; // Not eligible.
+ }
+ DexEncodedField field =
+ appView.appInfo().resolveField(user.asFieldInstruction().getField());
+ if (field == null || field.isStatic()) {
+ return user; // Not eligible.
+ }
+ continue;
+ }
+
+ if (user.isInstancePut()) {
+ if (!receivers.addIllegalReceiverAlias(user.asInstancePut().value())) {
+ return user; // Not eligible.
+ }
DexEncodedField field =
appView.appInfo().resolveField(user.asFieldInstruction().getField());
if (field == null || field.isStatic()) {
@@ -707,12 +652,6 @@
return null;
}
- if (isDesugaredLambda) {
- // Lambda desugaring synthesizes eligible constructors.
- markSizeForInlining(invoke, singleTarget);
- return new InliningInfo(singleTarget, eligibleClass);
- }
-
// Check that the entire constructor chain can be inlined into the current context.
DexItemFactory dexItemFactory = appView.dexItemFactory();
DexMethod parent = instanceInitializerInfo.getParent();
@@ -879,6 +818,15 @@
return null;
}
+ if (root.isStaticGet()) {
+ // If we are class inlining a singleton instance from a static-get, then we don't the value of
+ // the fields.
+ ParameterUsage receiverUsage = optimizationInfo.getParameterUsages(0);
+ if (receiverUsage == null || receiverUsage.hasFieldRead) {
+ return null;
+ }
+ }
+
// If the method returns receiver and the return value is actually
// used in the code we need to make some additional checks.
if (!eligibilityAcceptanceCheck.test(eligibility)) {
@@ -1014,6 +962,14 @@
return false;
}
+ if (root.isStaticGet()) {
+ // If we are class inlining a singleton instance from a static-get, then we don't the value of
+ // the fields.
+ if (parameterUsage.hasFieldRead) {
+ return false;
+ }
+ }
+
if (parameterUsage.isReturned) {
if (invoke.outValue() != null && invoke.outValue().hasAnyUsers()) {
// Used as return value which is not ignored.
@@ -1081,12 +1037,6 @@
private boolean exemptFromInstructionLimit(DexEncodedMethod inlinee) {
DexType inlineeHolder = inlinee.method.holder;
- if (isDesugaredLambda && inlineeHolder == eligibleClass) {
- return true;
- }
- if (appView.appInfo().isPinned(inlineeHolder)) {
- return false;
- }
DexClass inlineeClass = appView.definitionFor(inlineeHolder);
assert inlineeClass != null;
@@ -1117,15 +1067,6 @@
if (isProcessedConcurrently.test(singleTarget)) {
return false;
}
- if (isDesugaredLambda && !singleTarget.accessFlags.isBridge()) {
- // OK if this is the call to the main method of a desugared lambda (for both direct and
- // indirect calls).
- //
- // Note: This is needed because lambda methods are generally processed in the same batch as
- // they are class inlined, which means that the call to isInliningCandidate() below will
- // return false.
- return true;
- }
if (!singleTarget.isInliningCandidate(
method, Reason.SIMPLE, appView.appInfo(), NopWhyAreYouNotInliningReporter.getInstance())) {
// If `singleTarget` is not an inlining candidate, we won't be able to inline it here.
diff --git a/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java b/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
index 8154dfc..92d02ee 100644
--- a/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
@@ -174,7 +174,7 @@
.withBuilderTransformation(ToolHelper::allowTestProguardOptions)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS_N_PLUS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 5, "lambdadesugaringnplus"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 7, "lambdadesugaringnplus"))
.run();
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetNameTestBase.java b/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetNameTestBase.java
index dde75c48..a452c16 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetNameTestBase.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetNameTestBase.java
@@ -29,7 +29,7 @@
@Parameterized.Parameters(name = "{0} minification: {1}")
public static Collection<Object[]> data() {
return buildParameters(
- getTestParameters().withAllRuntimes().withAllApiLevels().build(), BooleanUtils.values());
+ getTestParameters().withAllRuntimesAndApiLevels().build(), BooleanUtils.values());
}
public GetNameTestBase(TestParameters parameters, boolean enableMinification) {
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/analysis/ProtoApplicationStats.java b/src/test/java/com/android/tools/r8/utils/codeinspector/analysis/ProtoApplicationStats.java
index 21e821d..28075e6 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/analysis/ProtoApplicationStats.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/analysis/ProtoApplicationStats.java
@@ -32,6 +32,8 @@
"com.google.protobuf.GeneratedMessageLite";
private static final String GENERATED_MESSAGE_LITE_BUILDER_TYPE =
GENERATED_MESSAGE_LITE_TYPE + "$Builder";
+ private static final String GENERATED_MESSAGE_LITE_EXTENDABLE_BUILDER_TYPE =
+ GENERATED_MESSAGE_LITE_TYPE + "$ExtendableBuilder";
abstract static class Stats {
@@ -165,6 +167,7 @@
break;
case GENERATED_MESSAGE_LITE_BUILDER_TYPE:
+ case GENERATED_MESSAGE_LITE_EXTENDABLE_BUILDER_TYPE:
protoBuilderStats.builders.add(
dexItemFactory.createType(classSubject.getOriginalDescriptor()));
break;