Disallow single caller inlining of already inlined methods
This fixes an issue in the second optimization pass when there is a call chain m1 -> m2 -> m3, where m2 is the single caller of m3.
If m2 is first inlined into m1 (e.g., due to meeting the simple inlining constraint), then m1 is now also a caller of m3. When processing m2 it is therefore invalid to single caller inline m3.
Note that this issue does not exist in the first optimization pass. Notably, if m2 is processed before m1, then m3 is correctly single caller inlined. If m1 is processed before m2, then m2 is not eligible for inlining into m1, since m2 is still unprocessed.
Fixes: b/343768871
Change-Id: I575b264dc25767560df4871a3d8387b3ce44f869
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/callgraph/CallSiteInformation.java b/src/main/java/com/android/tools/r8/ir/conversion/callgraph/CallSiteInformation.java
index 25c381e..84685ae 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/callgraph/CallSiteInformation.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/callgraph/CallSiteInformation.java
@@ -13,11 +13,12 @@
import com.android.tools.r8.synthesis.SyntheticItems;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.classhierarchy.MethodOverridesCollector;
+import com.android.tools.r8.utils.collections.BidirectionalManyToOneHashMap;
+import com.android.tools.r8.utils.collections.MutableBidirectionalManyToOneMap;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
import com.google.common.collect.Sets;
-import java.util.IdentityHashMap;
-import java.util.Map;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
public abstract class CallSiteInformation {
@@ -39,6 +40,8 @@
public abstract boolean isMultiCallerInlineCandidate(ProgramMethod method);
+ public abstract void notifyMethodInlined(ProgramMethod caller, ProgramMethod callee);
+
public abstract void unsetCallSiteInformation(ProgramMethod method);
public static CallSiteInformation empty() {
@@ -65,6 +68,9 @@
}
@Override
+ public void notifyMethodInlined(ProgramMethod caller, ProgramMethod callee) {}
+
+ @Override
public void unsetCallSiteInformation(ProgramMethod method) {
// Intentionally empty.
}
@@ -72,15 +78,21 @@
static class CallGraphBasedCallSiteInformation extends CallSiteInformation {
+ private final MethodProcessorWithWave methodProcessor;
+
// Single callers track their calling context to ensure that the predicate is stable after
// inlining of the caller.
- private final Map<DexMethod, DexMethod> singleCallerMethods = new IdentityHashMap<>();
+ private final MutableBidirectionalManyToOneMap<DexMethod, DexMethod> singleCallerMethods =
+ BidirectionalManyToOneHashMap.newIdentityHashMap();
private final Set<DexMethod> multiCallerInlineCandidates = Sets.newIdentityHashSet();
+ private final Set<DexMethod> noSingleCallerInliningInto = ConcurrentHashMap.newKeySet();
CallGraphBasedCallSiteInformation(
AppView<AppInfoWithLiveness> appView,
CallGraph graph,
MethodProcessorWithWave methodProcessor) {
+ this.methodProcessor = methodProcessor;
+
InternalOptions options = appView.options();
ProgramMethodSet pinned =
MethodOverridesCollector.findAllMethodsAndOverridesThatMatches(
@@ -135,8 +147,8 @@
assert callersWithDeterministicOrder.size() == 1;
caller = callersWithDeterministicOrder.iterator().next().getMethod().getReference();
}
- DexMethod existing = singleCallerMethods.put(reference, caller);
- assert existing == null;
+ assert !singleCallerMethods.containsKey(reference);
+ singleCallerMethods.put(reference, caller);
} else if (numberOfCallSites > 1 && methodProcessor.isPrimaryMethodProcessor()) {
multiCallerInlineCandidates.add(reference);
}
@@ -150,9 +162,9 @@
* library method this always returns false.
*/
@Override
- @SuppressWarnings("ReferenceEquality")
public boolean hasSingleCallSite(ProgramMethod method, ProgramMethod context) {
- return singleCallerMethods.get(method.getReference()) == context.getReference();
+ return !noSingleCallerInliningInto.contains(context.getReference())
+ && context.getReference().isIdenticalTo(singleCallerMethods.get(method.getReference()));
}
/**
@@ -178,6 +190,19 @@
}
@Override
+ public void notifyMethodInlined(ProgramMethod caller, ProgramMethod callee) {
+ if (methodProcessor.isPrimaryMethodProcessor()) {
+ return;
+ }
+ assert methodProcessor.isPostMethodProcessor();
+ // If the callee is the single caller of another method, then disallow that single caller
+ // inlining, since we have now duplicated the call site as a result of this inlining.
+ if (singleCallerMethods.containsValue(callee.getReference())) {
+ noSingleCallerInliningInto.add(callee.getReference());
+ }
+ }
+
+ @Override
public void unsetCallSiteInformation(ProgramMethod method) {
singleCallerMethods.remove(method.getReference());
multiCallerInlineCandidates.remove(method.getReference());
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index a03e4cc..3d78982 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -1122,6 +1122,8 @@
}
}
+ methodProcessor.getCallSiteInformation().notifyMethodInlined(context, singleTarget);
+
classInitializationAnalysis.notifyCodeHasChanged();
postProcessInlineeBlocks(
code, blockIterator, block, affectedValues, blocksToRemove, timing);
diff --git a/src/main/java/com/android/tools/r8/optimize/singlecaller/SingleCallerInliner.java b/src/main/java/com/android/tools/r8/optimize/singlecaller/SingleCallerInliner.java
index 614cf22..5645247 100644
--- a/src/main/java/com/android/tools/r8/optimize/singlecaller/SingleCallerInliner.java
+++ b/src/main/java/com/android/tools/r8/optimize/singlecaller/SingleCallerInliner.java
@@ -181,6 +181,9 @@
}
@Override
+ public void notifyMethodInlined(ProgramMethod caller, ProgramMethod callee) {}
+
+ @Override
public void unsetCallSiteInformation(ProgramMethod method) {
throw new Unreachable();
}
