Reland "Extend the range of assume instructions, part 6." Just same as go/r8g/43581 (minus a couple separations). Necessary fix in inliner is made separately. Test: tools/run_on_app.py --app gmail --compiler r8 --version 170604.16 Test: tools/run_on_app.py --app youtube --compiler r8 --version 14.19 Test: tools/run_on_as_app.py --app tivi --shrinker r8-full Bug: 120920488, 142607578 Change-Id: I45487f221d7fc5f4f745e6bc734ee620eef31dc1
diff --git a/src/main/java/com/android/tools/r8/ir/code/Assume.java b/src/main/java/com/android/tools/r8/ir/code/Assume.java index 8a9a863..37de422 100644 --- a/src/main/java/com/android/tools/r8/ir/code/Assume.java +++ b/src/main/java/com/android/tools/r8/ir/code/Assume.java
@@ -293,11 +293,13 @@ @Override public String toString() { - // During branch simplification, the origin `if` could be simplified. - // It means the assumption became "truth." - assert origin.hasBlock() || isAssumeNonNull(); + // `origin` could become obsolete: + // 1) during branch simplification, the origin `if` could be simplified, which means the + // assumption became "truth." + // 2) invoke-interface could be devirtualized, while its dynamic type and/or non-null receiver + // are still valid. String originString = - origin.hasBlock() ? " (origin: `" + origin.toString() + "`)" : " (origin simplified)"; + origin.hasBlock() ? " (origin: `" + origin.toString() + "`)" : " (obsolete origin)"; if (isAssumeNone() || isAssumeNonNull()) { return super.toString() + originString; }
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 3eeddc8..e3712f9 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
@@ -1397,24 +1397,6 @@ classStaticizer.examineMethodCode(method, code); } - if (nonNullTracker != null) { - // TODO(b/139246447): Once we extend this optimization to, e.g., constants of primitive args, - // this may not be the right place to collect call site optimization info. - // Collecting call-site optimization info depends on the existence of non-null IRs. - // Arguments can be changed during the debug mode. - if (!isDebugMode && appView.callSiteOptimizationInfoPropagator() != null) { - appView.callSiteOptimizationInfoPropagator().collectCallSiteOptimizationInfo(code); - } - // Computation of non-null parameters on normal exits rely on the existence of non-null IRs. - methodOptimizationInfoCollector.computeNonNullParamOnNormalExits(feedback, code); - } - if (aliasIntroducer != null || nonNullTracker != null || dynamicTypeOptimization != null) { - codeRewriter.removeAssumeInstructions(code); - assert code.isConsistentSSA(); - } - // Assert that we do not have unremoved non-sense code in the output, e.g., v <- non-null NULL. - assert code.verifyNoNullabilityBottomTypes(); - assert code.verifyTypes(appView); if (appView.enableWholeProgramOptimizations()) { @@ -1426,31 +1408,30 @@ fieldBitAccessAnalysis.recordFieldAccesses(code, feedback); } + // Arguments can be changed during the debug mode. + if (!isDebugMode && appView.callSiteOptimizationInfoPropagator() != null) { + appView.callSiteOptimizationInfoPropagator().collectCallSiteOptimizationInfo(code); + } + // Compute optimization info summary for the current method unless it is pinned // (in that case we should not be making any assumptions about the behavior of the method). if (!appView.appInfo().withLiveness().isPinned(method.method)) { - methodOptimizationInfoCollector.identifyClassInlinerEligibility(method, code, feedback); - methodOptimizationInfoCollector.identifyParameterUsages(method, code, feedback); - methodOptimizationInfoCollector.identifyReturnsArgument(method, code, feedback); - methodOptimizationInfoCollector.identifyTrivialInitializer(method, code, feedback); - - if (options.enableInlining && inliner != null) { - methodOptimizationInfoCollector - .identifyInvokeSemanticsForInlining(method, code, appView, feedback); - } - methodOptimizationInfoCollector - .computeDynamicReturnType(dynamicTypeOptimization, feedback, method, code); + .collectMethodOptimizationInfo(method, code, feedback, dynamicTypeOptimization); FieldValueAnalysis.run(appView, code, feedback, method); - methodOptimizationInfoCollector - .computeInitializedClassesOnNormalExit(feedback, method, code); - methodOptimizationInfoCollector.computeMayHaveSideEffects(feedback, method, code); - methodOptimizationInfoCollector - .computeReturnValueOnlyDependsOnArguments(feedback, method, code); - methodOptimizationInfoCollector.computeNonNullParamOrThrow(feedback, method, code); } } + if (aliasIntroducer != null || nonNullTracker != null || dynamicTypeOptimization != null) { + codeRewriter.removeAssumeInstructions(code); + assert code.isConsistentSSA(); + } + + // Assert that we do not have unremoved non-sense code in the output, e.g., v <- non-null NULL. + assert code.verifyNoNullabilityBottomTypes(); + + assert code.verifyTypes(appView); + previous = printMethod(code, "IR after computation of optimization info summary (SSA)", previous);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/AssumeDynamicTypeRemover.java b/src/main/java/com/android/tools/r8/ir/optimize/AssumeDynamicTypeRemover.java index 257d002..73214ba 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/AssumeDynamicTypeRemover.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/AssumeDynamicTypeRemover.java
@@ -53,7 +53,7 @@ } public void markUsersForRemoval(Value value) { - for (Instruction user : value.uniqueUsers()) { + for (Instruction user : value.aliasedUsers()) { if (user.isAssumeDynamicType()) { assert value.numberOfAllUsers() == 1 : "Expected value flowing into Assume<DynamicTypeAssumption> instruction to have a "
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 4e43761..c99d9d7 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
@@ -1248,10 +1248,10 @@ assumeDynamicTypeRemover.finish(); if (!blocksToBeRemoved.isEmpty()) { code.removeBlocks(blocksToBeRemoved); - code.removeAllTrivialPhis(); + code.removeAllTrivialPhis(affectedValues); assert code.getUnreachableBlocks().isEmpty(); } else if (mayHaveRemovedTrivialPhi || assumeDynamicTypeRemover.mayHaveIntroducedTrivialPhi()) { - code.removeAllTrivialPhis(); + code.removeAllTrivialPhis(affectedValues); } if (!affectedValues.isEmpty()) { new TypeAnalysis(appView).narrowing(affectedValues); @@ -1408,7 +1408,10 @@ // Removing check-cast may result in a trivial phi: // v3 <- phi(v1, v1) if (needToRemoveTrivialPhis) { - code.removeAllTrivialPhis(); + code.removeAllTrivialPhis(affectedValues); + if (!affectedValues.isEmpty()) { + typeAnalysis.narrowing(affectedValues); + } } assert code.isConsistentSSA(); }
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 62d5bd8..cea6258 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
@@ -3,6 +3,9 @@ // BSD-style license that can be found in the LICENSE file. package com.android.tools.r8.ir.optimize; +import static com.android.tools.r8.ir.analysis.type.Nullability.definitelyNotNull; +import static com.android.tools.r8.ir.analysis.type.TypeLatticeElement.stringClassType; + import com.android.tools.r8.graph.AppView; import com.android.tools.r8.graph.DebugLocalInfo; import com.android.tools.r8.graph.DexDefinition; @@ -193,7 +196,8 @@ appView.dexItemFactory().stringType, typeLattice.asClassTypeLatticeElement().getClassType()) .isTrue(); - Value returnedValue = code.createValue(typeLattice, debugLocalInfo); + Value returnedValue = + code.createValue(stringClassType(appView, definitelyNotNull()), debugLocalInfo); ConstString instruction = new ConstString( returnedValue, constant, ThrowingInfo.defaultForConstString(appView.options()));
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java b/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java index d85587e..5088d99 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
@@ -422,12 +422,12 @@ } } } + code.removeBlocks(blocksToBeRemoved); + code.removeAllTrivialPhis(valuesToNarrow); + code.removeUnreachableBlocks(); if (!valuesToNarrow.isEmpty()) { new TypeAnalysis(appView).narrowing(valuesToNarrow); } - code.removeBlocks(blocksToBeRemoved); - code.removeAllTrivialPhis(); - code.removeUnreachableBlocks(); assert code.isConsistentSSA(); }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java index 78369e3..e23fa01 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
@@ -46,6 +46,7 @@ import com.android.tools.r8.kotlin.Kotlin; import com.android.tools.r8.shaking.AppInfoWithLiveness; import com.android.tools.r8.utils.InternalOptions; +import com.android.tools.r8.utils.ListUtils; import com.android.tools.r8.utils.MethodSignatureEquivalence; import com.google.common.base.Equivalence.Wrapper; import com.google.common.collect.Sets; @@ -70,7 +71,6 @@ this.dexItemFactory = appView.dexItemFactory(); } - // TODO(b/141656615): Use this and then make all utils in this collector `private`. public void collectMethodOptimizationInfo( DexEncodedMethod method, IRCode code, @@ -91,7 +91,7 @@ computeNonNullParamOnNormalExits(feedback, code); } - public void identifyClassInlinerEligibility( + private void identifyClassInlinerEligibility( DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) { // Method eligibility is calculated in similar way for regular method // and for the constructor. To be eligible method should only be using its @@ -126,7 +126,11 @@ boolean receiverUsedAsReturnValue = false; boolean seenSuperInitCall = false; - for (Instruction insn : receiver.uniqueUsers()) { + for (Instruction insn : receiver.aliasedUsers()) { + if (insn.isAssume()) { + continue; + } + if (insn.isMonitor()) { continue; } @@ -140,11 +144,11 @@ if (insn.isInstancePut()) { InstancePut instancePutInstruction = insn.asInstancePut(); // Only allow field writes to the receiver. - if (instancePutInstruction.object() != receiver) { + if (instancePutInstruction.object().getAliasedValue() != receiver) { return; } // Do not allow the receiver to escape via a field write. - if (instancePutInstruction.value() == receiver) { + if (instancePutInstruction.value().getAliasedValue() == receiver) { return; } } @@ -162,7 +166,8 @@ DexMethod invokedMethod = invokedDirect.getInvokedMethod(); if (dexItemFactory.isConstructor(invokedMethod) && invokedMethod.holder == clazz.superType - && invokedDirect.inValues().lastIndexOf(receiver) == 0 + && ListUtils.lastIndexMatching( + invokedDirect.inValues(), v -> v.getAliasedValue() == receiver) == 0 && !seenSuperInitCall && instanceInitializer) { seenSuperInitCall = true; @@ -184,7 +189,7 @@ method, new ClassInlinerEligibility(receiverUsedAsReturnValue)); } - public void identifyParameterUsages( + private void identifyParameterUsages( DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) { List<ParameterUsage> usages = new ArrayList<>(); List<Value> values = code.collectArguments(); @@ -203,7 +208,7 @@ private ParameterUsage collectParameterUsages(int i, Value value) { ParameterUsageBuilder builder = new ParameterUsageBuilder(value, i); - for (Instruction user : value.uniqueUsers()) { + for (Instruction user : value.aliasedUsers()) { if (!builder.note(user)) { return null; } @@ -211,7 +216,7 @@ return builder.build(); } - public void identifyReturnsArgument( + private void identifyReturnsArgument( DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) { List<BasicBlock> normalExits = code.computeNormalExitBlocks(); if (normalExits.isEmpty()) { @@ -233,18 +238,19 @@ isNeverNull &= value.getTypeLattice().isReference() && value.isNeverNull(); } if (returnValue != null) { - if (returnValue.isArgument()) { + Value aliasedValue = returnValue.getAliasedValue(); + if (aliasedValue.isArgument()) { // Find the argument number. - int index = code.collectArguments().indexOf(returnValue); - assert index != -1; + int index = aliasedValue.computeArgumentPosition(code); + assert index >= 0; feedback.methodReturnsArgument(method, index); } - if (returnValue.isConstant()) { - if (returnValue.definition.isConstNumber()) { - long value = returnValue.definition.asConstNumber().getRawValue(); + if (aliasedValue.isConstant()) { + if (aliasedValue.definition.isConstNumber()) { + long value = aliasedValue.definition.asConstNumber().getRawValue(); feedback.methodReturnsConstantNumber(method, value); - } else if (returnValue.definition.isConstString()) { - ConstString constStringInstruction = returnValue.definition.asConstString(); + } else if (aliasedValue.definition.isConstString()) { + ConstString constStringInstruction = aliasedValue.definition.asConstString(); if (!constStringInstruction.instructionInstanceCanThrow()) { feedback.methodReturnsConstantString(method, constStringInstruction.getValue()); } @@ -256,7 +262,7 @@ } } - public void identifyTrivialInitializer( + private void identifyTrivialInitializer( DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) { if (!method.isInstanceInitializer() && !method.isClassInitializer()) { return; @@ -303,6 +309,10 @@ continue; } + if (insn.isAssume()) { + continue; + } + if (insn.isNewInstance()) { NewInstance newInstance = insn.asNewInstance(); if (createdSingletonInstance != null @@ -374,6 +384,10 @@ continue; } + if (insn.isAssume()) { + continue; + } + if (insn.isArgument()) { continue; } @@ -440,7 +454,7 @@ return TrivialInstanceInitializer.INSTANCE; } - public void identifyInvokeSemanticsForInlining( + private void identifyInvokeSemanticsForInlining( DexEncodedMethod method, IRCode code, AppView<?> appView, OptimizationFeedback feedback) { if (method.isStatic()) { // Identifies if the method preserves class initialization after inlining. @@ -692,7 +706,7 @@ return true; } - public void computeDynamicReturnType( + private void computeDynamicReturnType( DynamicTypeOptimization dynamicTypeOptimization, OptimizationFeedback feedback, DexEncodedMethod method, @@ -721,7 +735,7 @@ } } - public void computeInitializedClassesOnNormalExit( + private void computeInitializedClassesOnNormalExit( OptimizationFeedback feedback, DexEncodedMethod method, IRCode code) { if (options.enableInitializedClassesAnalysis && appView.appInfo().hasLiveness()) { AppView<AppInfoWithLiveness> appViewWithLiveness = appView.withLiveness(); @@ -734,7 +748,7 @@ } } - public void computeMayHaveSideEffects( + private void computeMayHaveSideEffects( OptimizationFeedback feedback, DexEncodedMethod method, IRCode code) { // If the method is native, we don't know what could happen. assert !method.accessFlags.isNative(); @@ -802,7 +816,7 @@ return false; } - public void computeReturnValueOnlyDependsOnArguments( + private void computeReturnValueOnlyDependsOnArguments( OptimizationFeedback feedback, DexEncodedMethod method, IRCode code) { if (!options.enableDeterminismAnalysis) { return; @@ -815,7 +829,7 @@ } // Track usage of parameters and compute their nullability and possibility of NPE. - public void computeNonNullParamOrThrow( + private void computeNonNullParamOrThrow( OptimizationFeedback feedback, DexEncodedMethod method, IRCode code) { if (method.getOptimizationInfo().getNonNullParamOrThrow() != null) { return; @@ -843,7 +857,7 @@ } } - public void computeNonNullParamOnNormalExits(OptimizationFeedback feedback, IRCode code) { + private void computeNonNullParamOnNormalExits(OptimizationFeedback feedback, IRCode code) { Set<BasicBlock> normalExits = Sets.newIdentityHashSet(); normalExits.addAll(code.computeNormalExitBlocks()); DominatorTree dominatorTree = new DominatorTree(code, MAY_HAVE_UNREACHABLE_BLOCKS);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/ParameterUsagesInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/ParameterUsagesInfo.java index 0495bba..7e61600 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/info/ParameterUsagesInfo.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/info/ParameterUsagesInfo.java
@@ -14,6 +14,7 @@ import com.android.tools.r8.ir.code.InvokeMethodWithReceiver; import com.android.tools.r8.ir.code.Return; import com.android.tools.r8.ir.code.Value; +import com.android.tools.r8.utils.ListUtils; import com.android.tools.r8.utils.Pair; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -110,6 +111,10 @@ // Returns false if the instruction is not supported. public boolean note(Instruction instruction) { + if (instruction.isAssume()) { + // Keep examining other users, but the param usage builder should consider aliased users. + return true; + } if (instruction.isIf()) { return note(instruction.asIf()); } @@ -141,7 +146,8 @@ private boolean note(If ifInstruction) { if (ifInstruction.asIf().isZeroTest()) { - assert ifInstruction.inValues().size() == 1 && ifInstruction.inValues().get(0) == arg; + assert ifInstruction.inValues().size() == 1 + && ifInstruction.inValues().get(0).getAliasedValue() == arg; ifZeroTestTypes.add(ifInstruction.asIf().getType()); return true; } @@ -150,7 +156,7 @@ private boolean note(InstanceGet instanceGetInstruction) { assert arg != instanceGetInstruction.outValue(); - if (instanceGetInstruction.object() == arg) { + if (instanceGetInstruction.object().getAliasedValue() == arg) { hasFieldRead = true; return true; } @@ -159,12 +165,12 @@ private boolean note(InstancePut instancePutInstruction) { assert arg != instancePutInstruction.outValue(); - if (instancePutInstruction.object() == arg) { + if (instancePutInstruction.object().getAliasedValue() == arg) { hasFieldAssignment = true; - isAssignedToField |= instancePutInstruction.value() == arg; + isAssignedToField |= instancePutInstruction.value().getAliasedValue() == arg; return true; } - if (instancePutInstruction.value() == arg) { + if (instancePutInstruction.value().getAliasedValue() == arg) { isAssignedToField = true; return true; } @@ -172,7 +178,8 @@ } private boolean note(InvokeMethodWithReceiver invokeInstruction) { - if (invokeInstruction.inValues().lastIndexOf(arg) == 0) { + if (ListUtils.lastIndexMatching( + invokeInstruction.inValues(), v -> v.getAliasedValue() == arg) == 0) { callsOnReceiver.add( new Pair<>( invokeInstruction.asInvokeMethodWithReceiver().getType(), @@ -183,7 +190,8 @@ } private boolean note(Return returnInstruction) { - assert returnInstruction.inValues().size() == 1 && returnInstruction.inValues().get(0) == arg; + assert returnInstruction.inValues().size() == 1 + && returnInstruction.inValues().get(0).getAliasedValue() == arg; isReturned = true; return true; }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/UpdatableMethodOptimizationInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/UpdatableMethodOptimizationInfo.java index 06e7332..09a8d13 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/info/UpdatableMethodOptimizationInfo.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/info/UpdatableMethodOptimizationInfo.java
@@ -371,7 +371,12 @@ // We may get more precise type information if the method is reprocessed (e.g., due to // optimization info collected from all call sites), and hence the `returnsObjectOfType` is // allowed to become more precise. - assert returnsObjectOfType == UNKNOWN_TYPE || type.lessThanOrEqual(returnsObjectOfType, appView) + // TODO(b/142559221): non-materializable assume instructions? + // Nullability could be less precise, though. For example, suppose a value is known to be + // non-null after a safe invocation, hence recorded with the non-null variant. If that call is + // inlined and the method is reprocessed, such non-null assumption cannot be made again. + assert returnsObjectOfType == UNKNOWN_TYPE + || type.lessThanOrEqualUpToNullability(returnsObjectOfType, appView) : "return type changed from " + returnsObjectOfType + " to " + type; returnsObjectOfType = type; }