Lens code rewriting for Argument instructions
Change-Id: Idd84efbe168e13c959d068e38dbda034bfacd0a3
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
index c3fd49f..5f318cd 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
@@ -120,9 +120,7 @@
import com.android.tools.r8.ir.code.ValueType;
import com.android.tools.r8.ir.code.ValueTypeConstraint;
import com.android.tools.r8.ir.code.Xor;
-import com.android.tools.r8.ir.optimize.info.CallSiteOptimizationInfo;
import com.android.tools.r8.naming.dexitembasedstring.NameComputationInfo;
-import com.android.tools.r8.optimize.argumentpropagation.ArgumentPropagatorIROptimizer;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.Pair;
@@ -748,26 +746,6 @@
new TypeAnalysis(appView).narrowing(ir);
}
- // Update the IR code if collected call site optimization info has something useful.
- // While aggregation of parameter information at call sites would be more precise than static
- // types, those could be still less precise at one single call site, where specific arguments
- // will be passed during (double) inlining. Instead of adding assumptions and removing invalid
- // ones, it's better not to insert assumptions for inlinee in the beginning.
- CallSiteOptimizationInfo callSiteOptimizationInfo =
- getMethod().getOptimizationInfo().getArgumentInfos();
- if (callSiteOptimizationInfo.isConcreteCallSiteOptimizationInfo() && method == context) {
- // TODO(b/190154391): Consider pruning all argument information from the optimization info
- // after the second optimization pass. That way we save memory and can assert here that
- // !appView.hasLiveness() (which currently may happen due to the reflective behavior
- // handling in the final round of tree shaking).
- if (appView.hasLiveness()) {
- ArgumentPropagatorIROptimizer.optimize(
- appView.withLiveness(),
- ir,
- callSiteOptimizationInfo.asConcreteCallSiteOptimizationInfo());
- }
- }
-
if (appView.options().isStringSwitchConversionEnabled()) {
StringSwitchConverter.convertToStringSwitchInstructions(ir, 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 c30c8b6..c4dba5e 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
@@ -81,6 +81,7 @@
import com.android.tools.r8.ir.optimize.classinliner.ClassInliner;
import com.android.tools.r8.ir.optimize.enums.EnumUnboxer;
import com.android.tools.r8.ir.optimize.enums.EnumValueOptimizer;
+import com.android.tools.r8.ir.optimize.info.CallSiteOptimizationInfo;
import com.android.tools.r8.ir.optimize.info.MethodOptimizationInfoCollector;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedback;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackDelayed;
@@ -96,6 +97,7 @@
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.naming.IdentifierNameStringMarker;
import com.android.tools.r8.optimize.argumentpropagation.ArgumentPropagator;
+import com.android.tools.r8.optimize.argumentpropagation.ArgumentPropagatorIROptimizer;
import com.android.tools.r8.position.MethodPosition;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.shaking.KeepMethodInfo;
@@ -1213,6 +1215,20 @@
previous = printMethod(code, "IR after disable assertions (SSA)", previous);
+ // Update the IR code if collected call site optimization info has something useful.
+ // While aggregation of parameter information at call sites would be more precise than static
+ // types, those could be still less precise at one single call site, where specific arguments
+ // will be passed during (double) inlining. Instead of adding assumptions and removing invalid
+ // ones, it's better not to insert assumptions for inlinee in the beginning.
+ CallSiteOptimizationInfo callSiteOptimizationInfo =
+ context.getOptimizationInfo().getArgumentInfos();
+ if (callSiteOptimizationInfo.isConcreteCallSiteOptimizationInfo() && appView.hasLiveness()) {
+ ArgumentPropagatorIROptimizer.optimize(
+ appView.withLiveness(),
+ code,
+ callSiteOptimizationInfo.asConcreteCallSiteOptimizationInfo());
+ }
+
if (assumeInserter != null) {
assumeInserter.insertAssumeInstructions(code, timing);
}
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 3dc74b2..31f99e5 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
@@ -6,6 +6,7 @@
import static com.android.tools.r8.graph.UseRegistry.MethodHandleUse.NOT_ARGUMENT_TO_LAMBDA_METAFACTORY;
import static com.android.tools.r8.ir.code.Invoke.Type.STATIC;
import static com.android.tools.r8.ir.code.Invoke.Type.VIRTUAL;
+import static com.android.tools.r8.ir.code.Opcodes.ARGUMENT;
import static com.android.tools.r8.ir.code.Opcodes.ASSUME;
import static com.android.tools.r8.ir.code.Opcodes.CHECK_CAST;
import static com.android.tools.r8.ir.code.Opcodes.CONST_CLASS;
@@ -60,7 +61,7 @@
import com.android.tools.r8.ir.analysis.type.Nullability;
import com.android.tools.r8.ir.analysis.type.TypeElement;
import com.android.tools.r8.ir.analysis.value.SingleNumberValue;
-import com.android.tools.r8.ir.code.Assume;
+import com.android.tools.r8.ir.code.Argument;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.BasicBlockIterator;
import com.android.tools.r8.ir.code.CatchHandlers;
@@ -685,49 +686,30 @@
}
break;
- case ASSUME:
+ case ARGUMENT:
{
- // TODO(b/174543992): It's not clear we should rewrite the assumes here. The code
- // present fixes the problem for enum unboxing, but not for lambda merging.
- // The LensCodeRewriter is run before most assume instructions are inserted, however,
- // the call site optimization may propagate assumptions at IR building time, and such
- // assumes are already present.
- // R8 clears the assumes if the type is rewritten to a primitive type.
- Assume assume = current.asAssume();
- if (assume.hasOutValue()) {
- TypeElement type = assume.getOutType();
- TypeElement substituted = type.rewrittenWithLens(appView, graphLens);
- if (substituted != type) {
- assert type.isArrayType() || type.isClassType();
- affectedPhis.addAll(assume.outValue().uniquePhiUsers());
- if (substituted.isPrimitiveType()) {
- assert type.isClassType();
- assert appView.unboxedEnums().isUnboxedEnum(type.asClassType().getClassType());
- // Any assumption of a class type being converted to a primitive type is
- // invalid. Dynamic type is irrelevant and non null is incorrect.
- assume.outValue().replaceUsers(assume.src());
- iterator.removeOrReplaceByDebugLocalRead();
- } else if (substituted.isPrimitiveArrayType()) {
- assert type.isArrayType();
- // Non-null assumptions on a class array type being converted to a primitive
- // array type remains, but dynamic type becomes irrelevant.
- assume.unsetDynamicTypeAssumption();
- if (assume.hasNonNullAssumption()) {
- assume.outValue().setType(substituted);
- } else {
- assume.outValue().replaceUsers(assume.src());
- iterator.removeOrReplaceByDebugLocalRead();
- }
- } else {
- assert !substituted.isPrimitiveType();
- assert !substituted.isPrimitiveArrayType();
- current.outValue().setType(substituted);
- }
- }
+ Argument argument = current.asArgument();
+ TypeElement currentArgumentType = argument.getOutType();
+ TypeElement newArgumentType =
+ method
+ .getArgumentType(argument.getIndex())
+ .toTypeElement(appView, currentArgumentType.nullability());
+ if (!newArgumentType.equals(currentArgumentType)) {
+ affectedPhis.addAll(argument.outValue().uniquePhiUsers());
+ iterator.replaceCurrentInstruction(
+ Argument.builder()
+ .setIndex(argument.getIndex())
+ .setFreshOutValue(code, newArgumentType)
+ .setPosition(argument)
+ .build());
}
}
break;
+ case ASSUME:
+ assert false;
+ break;
+
default:
if (current.hasOutValue()) {
// For all other instructions, substitute any changed type.
diff --git a/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingRemoveInterfaceBridgeWithSubTypeTest.java b/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingRemoveInterfaceBridgeWithSubTypeTest.java
index f1fc86d..25674a2 100644
--- a/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingRemoveInterfaceBridgeWithSubTypeTest.java
+++ b/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingRemoveInterfaceBridgeWithSubTypeTest.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoHorizontalClassMerging;
+import com.android.tools.r8.NoParameterTypeStrengthening;
import com.android.tools.r8.NoVerticalClassMerging;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
@@ -51,6 +52,7 @@
.addKeepClassAndMembersRules(I.class)
.enableInliningAnnotations()
.enableNoHorizontalClassMergingAnnotations()
+ .enableNoParameterTypeStrengtheningAnnotations()
.enableNoVerticalClassMergingAnnotations()
.addHorizontallyMergedClassesInspector(
HorizontallyMergedClassesInspector::assertNoClassesMerged)
@@ -94,6 +96,7 @@
}
@NeverInline
+ @NoParameterTypeStrengthening
private static void callJ(A a) {
a.foo();
}
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageWithOverridesOfPackagePrivateMethodsTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageWithOverridesOfPackagePrivateMethodsTest.java
index 951a2b6..4eff919 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageWithOverridesOfPackagePrivateMethodsTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageWithOverridesOfPackagePrivateMethodsTest.java
@@ -8,6 +8,7 @@
import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NoParameterTypeStrengthening;
import com.android.tools.r8.NoVerticalClassMerging;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
@@ -31,6 +32,7 @@
.apply(this::configureRepackaging)
.enableInliningAnnotations()
.enableNeverClassInliningAnnotations()
+ .enableNoParameterTypeStrengtheningAnnotations()
.enableNoVerticalClassMergingAnnotations()
.setMinApi(parameters.getApiLevel())
.compile()
@@ -52,11 +54,13 @@
}
@NeverInline
+ @NoParameterTypeStrengthening
static void greet(HelloGreeterBase greeter) {
greeter.greet();
}
@NeverInline
+ @NoParameterTypeStrengthening
static void greet(WorldGreeterBase greeter) {
greeter.greet();
}
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageWithOverridesOfProtectedMethodsTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageWithOverridesOfProtectedMethodsTest.java
index 7747cbd..56f6a41 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageWithOverridesOfProtectedMethodsTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageWithOverridesOfProtectedMethodsTest.java
@@ -8,6 +8,7 @@
import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NoParameterTypeStrengthening;
import com.android.tools.r8.NoVerticalClassMerging;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
@@ -31,6 +32,7 @@
.apply(this::configureRepackaging)
.enableInliningAnnotations()
.enableNeverClassInliningAnnotations()
+ .enableNoParameterTypeStrengtheningAnnotations()
.enableNoVerticalClassMergingAnnotations()
.setMinApi(parameters.getApiLevel())
.compile()
@@ -52,11 +54,13 @@
}
@NeverInline
+ @NoParameterTypeStrengthening
static void greet(HelloGreeterBase greeter) {
greeter.greet();
}
@NeverInline
+ @NoParameterTypeStrengthening
static void greet(WorldGreeterBase greeter) {
greeter.greet();
}
