Version 2.0.64 Cherry-pick: Reland "Adjust accessibility for desugared lambdas prior to IR processing in R8" CL: https://r8-review.googlesource.com/47320 Bug: 152865051 Change-Id: I5862890fe77fe1dbaac8a24cdbf5448869702f93
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java index b9b47c0..12c3ae2 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 = "2.0.63"; + public static final String LABEL = "2.0.64"; private Version() { }
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 ea851e7..8c067cc 100644 --- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java +++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -835,8 +835,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 54d65ab..77f0397 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
@@ -276,9 +276,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 = @@ -405,9 +403,11 @@ Builder<?> builder, OptimizationFeedback feedback, ExecutorService executorService) throws ExecutionException { if (lambdaRewriter != null) { - lambdaRewriter.adjustAccessibility(this, feedback); - lambdaRewriter.synthesizeLambdaClasses(builder); - lambdaRewriter.optimizeSynthesizedClasses(this, executorService); + if (appView.enableWholeProgramOptimizations()) { + lambdaRewriter.finalizeLambdaDesugaringForR8(builder); + } else { + lambdaRewriter.finalizeLambdaDesugaringForD8(builder, this, executorService); + } } } @@ -629,7 +629,6 @@ } public DexApplication optimize(ExecutorService executorService) throws ExecutionException { - if (options.isShrinking()) { assert !removeLambdaDeserializationMethods(); } else { @@ -642,6 +641,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. @@ -821,7 +824,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 4254d23..e9e45b4 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.optimize.info.OptimizationFeedback; import com.android.tools.r8.ir.synthetic.SynthesizedCode; import com.android.tools.r8.origin.SynthesizedOrigin; @@ -503,6 +502,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; @@ -511,7 +513,16 @@ } // Ensure access of the referenced symbol(s). - abstract void ensureAccessibility(IRConverter converter, OptimizationFeedback feedback); + abstract DexEncodedMethod ensureAccessibility(OptimizationFeedback feedback); + + // Ensure access of the referenced symbol(s). + DexEncodedMethod ensureAccessibilityIfNeeded(OptimizationFeedback feedback) { + if (!hasEnsuredAccessibility) { + accessibilityBridge = ensureAccessibility(feedback); + hasEnsuredAccessibility = true; + } + return accessibilityBridge; + } DexClass definitionFor(DexType type) { return rewriter.getAppInfo().app().definitionFor(type); @@ -554,7 +565,9 @@ } @Override - void ensureAccessibility(IRConverter converter, OptimizationFeedback feedback) {} + DexEncodedMethod ensureAccessibility(OptimizationFeedback feedback) { + return null; + } } // Used for static private lambda$ methods. Only needs access relaxation. @@ -565,7 +578,7 @@ } @Override - void ensureAccessibility(IRConverter converter, OptimizationFeedback feedback) { + DexEncodedMethod ensureAccessibility(OptimizationFeedback feedback) { // We already found the static method to be called, just relax its accessibility. assert descriptor.getAccessibility() != null; descriptor.getAccessibility().unsetPrivate(); @@ -573,6 +586,7 @@ if (implMethodHolder.isInterface()) { descriptor.getAccessibility().setPublic(); } + return null; } } @@ -585,7 +599,7 @@ } @Override - void ensureAccessibility(IRConverter converter, OptimizationFeedback feedback) { + DexEncodedMethod ensureAccessibility(OptimizationFeedback feedback) { // 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(); @@ -612,16 +626,18 @@ encodedMethod.getCode(), true); newMethod.copyMetadata(encodedMethod, feedback); - 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; } } @@ -633,7 +649,7 @@ } @Override - void ensureAccessibility(IRConverter converter, OptimizationFeedback feedback) { + DexEncodedMethod ensureAccessibility(OptimizationFeedback feedback) { // 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(); @@ -657,13 +673,15 @@ encodedMethod.getCode(), true); newMethod.copyMetadata(encodedMethod, feedback); - 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; } } @@ -676,7 +694,7 @@ } @Override - void ensureAccessibility(IRConverter converter, OptimizationFeedback feedback) { + DexEncodedMethod ensureAccessibility(OptimizationFeedback feedback) { // Create a static accessor with proper accessibility. DexProgramClass accessorClass = programDefinitionFor(callTarget.holder); assert accessorClass != null; @@ -703,7 +721,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 17b227e..97ebc58 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; @@ -33,7 +32,6 @@ import com.android.tools.r8.ir.code.Value; import com.android.tools.r8.ir.conversion.IRConverter; import com.android.tools.r8.ir.conversion.LensCodeRewriter; -import com.android.tools.r8.ir.optimize.info.OptimizationFeedback; import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackDelayed; import com.android.tools.r8.shaking.AppInfoWithLiveness; import com.android.tools.r8.utils.DescriptorUtils; @@ -74,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 @@ -94,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); } @@ -114,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()) { @@ -146,14 +163,51 @@ 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(feedback); + 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; @@ -176,9 +230,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)); } } }); @@ -252,41 +304,21 @@ return anyRemoved; } - /** Adjust accessibility of referenced application symbols or creates necessary accessors. */ - public void adjustAccessibility(IRConverter converter, OptimizationFeedback feedback) { - // 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, feedback); - } - 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() @@ -295,6 +327,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 3e5280d..2e44132 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)); } @@ -214,65 +204,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; } @@ -726,12 +657,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(); @@ -899,8 +824,8 @@ } if (root.isStaticGet()) { - // If we are class inlining a singleton instance from a static-get, then we don't know the - // value of the fields. + // 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; @@ -1046,6 +971,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. @@ -1117,12 +1050,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; @@ -1153,15 +1080,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 9c0766c..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().build(), BooleanUtils.values()); + getTestParameters().withAllRuntimesAndApiLevels().build(), BooleanUtils.values()); } public GetNameTestBase(TestParameters parameters, boolean enableMinification) {
diff --git a/src/test/java/com/android/tools/r8/regress/b152865051/CallSiteOptimizationWithLambdaMethodReference.java b/src/test/java/com/android/tools/r8/regress/b152865051/CallSiteOptimizationWithLambdaMethodReference.java new file mode 100644 index 0000000..b2b70ad --- /dev/null +++ b/src/test/java/com/android/tools/r8/regress/b152865051/CallSiteOptimizationWithLambdaMethodReference.java
@@ -0,0 +1,76 @@ +// Copyright (c) 2020, 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.regress.b152865051; + +import com.android.tools.r8.CompilationFailedException; +import com.android.tools.r8.TestBase; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.TestParametersCollection; +import java.io.IOException; +import java.util.concurrent.ExecutionException; +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 CallSiteOptimizationWithLambdaMethodReference extends TestBase { + + private final TestParameters parameters; + + @Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withAllRuntimesAndApiLevels().build(); + } + + public CallSiteOptimizationWithLambdaMethodReference(TestParameters parameters) { + this.parameters = parameters; + } + + @Test + public void testR8() throws IOException, CompilationFailedException, ExecutionException { + testForR8(parameters.getBackend()) + .addInnerClasses(CallSiteOptimizationWithLambdaMethodReference.class) + .setMinApi(parameters.getApiLevel()) + .addKeepMainRule(Main.class) + .addOptionsModification( + internalOptions -> { + internalOptions.enableClassInlining = false; + }) + .run(parameters.getRuntime(), Main.class) + .disassemble() + .assertSuccessWithOutputLines("NULL", "NOT NULL"); + } + + @FunctionalInterface + public interface I { + void action(Object a); + } + + public static class Holder { + + private I i; + + Holder(I i) { + this.i = i; + } + + void callAction() { + i.action(new Object()); + } + } + + public static class Main { + + public static void main(String[] args) { + ((I) Main::action).action(null); + new Holder(Main::action).callAction(); + } + + private static void action(Object a) { + System.out.println(a == null ? "NULL" : "NOT NULL"); + } + } +}
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;