Refactor feature split optimization logic to single class
Change-Id: I8a77721e53c4472c26a51ff8ac62a5fff8b362a4
diff --git a/src/main/java/com/android/tools/r8/features/ClassToFeatureSplitMap.java b/src/main/java/com/android/tools/r8/features/ClassToFeatureSplitMap.java
index 68af78a..fd710bb 100644
--- a/src/main/java/com/android/tools/r8/features/ClassToFeatureSplitMap.java
+++ b/src/main/java/com/android/tools/r8/features/ClassToFeatureSplitMap.java
@@ -18,7 +18,6 @@
import com.android.tools.r8.graph.ProgramDefinition;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.PrunedItems;
-import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.synthesis.SyntheticItems;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.Reporter;
@@ -327,11 +326,13 @@
// Static helpers to avoid verbose predicates.
- private static ClassToFeatureSplitMap getMap(AppView<AppInfoWithLiveness> appView) {
+ private static ClassToFeatureSplitMap getMap(
+ AppView<? extends AppInfoWithClassHierarchy> appView) {
return appView.appInfo().getClassToFeatureSplitMap();
}
- public static boolean isInFeature(DexProgramClass clazz, AppView<AppInfoWithLiveness> appView) {
+ public static boolean isInFeature(
+ DexProgramClass clazz, AppView<? extends AppInfoWithClassHierarchy> appView) {
return getMap(appView)
.isInFeature(
clazz,
diff --git a/src/main/java/com/android/tools/r8/features/FeatureSplitBoundaryOptimizationUtils.java b/src/main/java/com/android/tools/r8/features/FeatureSplitBoundaryOptimizationUtils.java
new file mode 100644
index 0000000..451ff92
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/features/FeatureSplitBoundaryOptimizationUtils.java
@@ -0,0 +1,75 @@
+// Copyright (c) 2022, 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.features;
+
+import com.android.tools.r8.FeatureSplit;
+import com.android.tools.r8.experimental.startup.StartupOrder;
+import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexEncodedMember;
+import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.ProgramDefinition;
+import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget;
+import com.android.tools.r8.synthesis.SyntheticItems;
+import com.android.tools.r8.utils.InternalOptions;
+
+public class FeatureSplitBoundaryOptimizationUtils {
+
+ public static ConstraintWithTarget getInliningConstraintForResolvedMember(
+ ProgramMethod method,
+ DexEncodedMember<?, ?> resolvedMember,
+ AppView<? extends AppInfoWithClassHierarchy> appView) {
+ ClassToFeatureSplitMap classToFeatureSplitMap = appView.appInfo().getClassToFeatureSplitMap();
+ // We never inline into the base from a feature (calls should never happen) and we never inline
+ // between features, so this check should be sufficient.
+ if (classToFeatureSplitMap.isInBaseOrSameFeatureAs(
+ resolvedMember.getHolderType(), method, appView)) {
+ return ConstraintWithTarget.ALWAYS;
+ }
+ return ConstraintWithTarget.NEVER;
+ }
+
+ public static FeatureSplit getMergeKeyForHorizontalClassMerging(
+ DexProgramClass clazz, AppView<? extends AppInfoWithClassHierarchy> appView) {
+ ClassToFeatureSplitMap classToFeatureSplitMap = appView.appInfo().getClassToFeatureSplitMap();
+ return classToFeatureSplitMap.getFeatureSplit(clazz, appView);
+ }
+
+ public static boolean isSafeForAccess(
+ DexProgramClass accessedClass,
+ ProgramDefinition accessor,
+ ClassToFeatureSplitMap classToFeatureSplitMap,
+ InternalOptions options,
+ StartupOrder startupOrder,
+ SyntheticItems syntheticItems) {
+ return classToFeatureSplitMap.isInBaseOrSameFeatureAs(
+ accessedClass, accessor, options, startupOrder, syntheticItems);
+ }
+
+ public static boolean isSafeForInlining(
+ ProgramMethod caller,
+ ProgramMethod callee,
+ AppView<? extends AppInfoWithClassHierarchy> appView) {
+ ClassToFeatureSplitMap classToFeatureSplitMap = appView.appInfo().getClassToFeatureSplitMap();
+ if (classToFeatureSplitMap.isInSameFeatureOrBothInSameBase(callee, caller, appView)) {
+ return true;
+ }
+ // Still allow inlining if we inline from the base into a feature.
+ if (classToFeatureSplitMap.isInBase(callee.getHolder(), appView)) {
+ return true;
+ }
+ return false;
+ }
+
+ public static boolean isSafeForVerticalClassMerging(
+ DexProgramClass sourceClass,
+ DexProgramClass targetClass,
+ AppView<? extends AppInfoWithClassHierarchy> appView) {
+ ClassToFeatureSplitMap classToFeatureSplitMap = appView.appInfo().getClassToFeatureSplitMap();
+ return classToFeatureSplitMap.isInSameFeatureOrBothInSameBase(
+ sourceClass, targetClass, appView);
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/graph/AccessControl.java b/src/main/java/com/android/tools/r8/graph/AccessControl.java
index 43bbfa2..9211db6 100644
--- a/src/main/java/com/android/tools/r8/graph/AccessControl.java
+++ b/src/main/java/com/android/tools/r8/graph/AccessControl.java
@@ -5,6 +5,7 @@
import com.android.tools.r8.experimental.startup.StartupOrder;
import com.android.tools.r8.features.ClassToFeatureSplitMap;
+import com.android.tools.r8.features.FeatureSplitBoundaryOptimizationUtils;
import com.android.tools.r8.synthesis.SyntheticItems;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.OptionalBool;
@@ -42,9 +43,10 @@
}
if (clazz.isProgramClass()
&& context.isProgramDefinition()
- && !classToFeatureSplitMap.isInBaseOrSameFeatureAs(
+ && !FeatureSplitBoundaryOptimizationUtils.isSafeForAccess(
clazz.asProgramClass(),
context.asProgramDefinition(),
+ classToFeatureSplitMap,
options,
startupOrder,
syntheticItems)) {
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/SameFeatureSplit.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/SameFeatureSplit.java
index 73dfa2a..5ad8747 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/SameFeatureSplit.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/SameFeatureSplit.java
@@ -5,6 +5,7 @@
package com.android.tools.r8.horizontalclassmerging.policies;
import com.android.tools.r8.FeatureSplit;
+import com.android.tools.r8.features.FeatureSplitBoundaryOptimizationUtils;
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexProgramClass;
@@ -19,7 +20,8 @@
@Override
public FeatureSplit getMergeKey(DexProgramClass clazz) {
- return appView.appInfo().getClassToFeatureSplitMap().getFeatureSplit(clazz, appView);
+ return FeatureSplitBoundaryOptimizationUtils.getMergeKeyForHorizontalClassMerging(
+ clazz, appView);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
index d3d0ece..ac4c0ef 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -11,7 +11,7 @@
import com.android.tools.r8.dex.code.DexMoveResultObject;
import com.android.tools.r8.dex.code.DexMoveResultWide;
import com.android.tools.r8.errors.Unreachable;
-import com.android.tools.r8.features.ClassToFeatureSplitMap;
+import com.android.tools.r8.features.FeatureSplitBoundaryOptimizationUtils;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.Code;
import com.android.tools.r8.graph.DexEncodedField;
@@ -163,13 +163,9 @@
return false;
}
- ClassToFeatureSplitMap classToFeatureSplitMap = appView.appInfo().getClassToFeatureSplitMap();
- if (!classToFeatureSplitMap.isInSameFeatureOrBothInSameBase(singleTarget, method, appView)) {
- // Still allow inlining if we inline from the base into a feature.
- if (!classToFeatureSplitMap.isInBase(singleTarget.getHolder(), appView)) {
- whyAreYouNotInliningReporter.reportInliningAcrossFeatureSplit();
- return false;
- }
+ if (!FeatureSplitBoundaryOptimizationUtils.isSafeForInlining(method, singleTarget, appView)) {
+ whyAreYouNotInliningReporter.reportInliningAcrossFeatureSplit();
+ return false;
}
Set<Reason> validInliningReasons = appView.testing().validInliningReasons;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
index 96b79b1..6dc28aa 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
@@ -5,6 +5,7 @@
package com.android.tools.r8.ir.optimize;
import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.features.FeatureSplitBoundaryOptimizationUtils;
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexClass;
@@ -382,14 +383,13 @@
// This will fail at runtime.
return ConstraintWithTarget.NEVER;
}
- if (!appView
- .appInfo()
- .getClassToFeatureSplitMap()
- .isInBaseOrSameFeatureAs(
- resolvedMember.getHolderType(), context.asProgramMethod(), appView)) {
- // We never inline into the base from a feature (calls should never happen) and we
- // never inline between features, so this check should be sufficient.
- return ConstraintWithTarget.NEVER;
+ ConstraintWithTarget featureSplitInliningConstraint =
+ FeatureSplitBoundaryOptimizationUtils.getInliningConstraintForResolvedMember(
+ context, resolvedMember, appView);
+ assert featureSplitInliningConstraint == ConstraintWithTarget.ALWAYS
+ || featureSplitInliningConstraint == ConstraintWithTarget.NEVER;
+ if (featureSplitInliningConstraint == ConstraintWithTarget.NEVER) {
+ return featureSplitInliningConstraint;
}
DexType resolvedHolder = graphLens.lookupType(resolvedMember.getHolderType());
assert initialResolutionHolder != null;
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index 37641f0..8760fb8 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -15,6 +15,7 @@
import com.android.tools.r8.androidapi.ComputedApiLevel;
import com.android.tools.r8.cf.CfVersion;
import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.features.FeatureSplitBoundaryOptimizationUtils;
import com.android.tools.r8.graph.AccessControl;
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
import com.android.tools.r8.graph.AppView;
@@ -387,9 +388,8 @@
.map(DexEncodedMember::getReference)
.noneMatch(appInfo::isPinned);
- if (!appInfo
- .getClassToFeatureSplitMap()
- .isInSameFeatureOrBothInSameBase(sourceClass, targetClass, appView)) {
+ if (!FeatureSplitBoundaryOptimizationUtils.isSafeForVerticalClassMerging(
+ sourceClass, targetClass, appView)) {
return false;
}
if (appView.appServices().allServiceTypes().contains(sourceClass.type)
