Introduce a predicate for libraryMethodsWithoutSideEffects
Bug: 133745205
Change-Id: I937839ecf6c3ecac23f89b274f57ca5556d9ff1f
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index d5ebf00..9202120 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.graph;
import static com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement.computeLeastUpperBoundOfInterfaces;
+import static com.google.common.base.Predicates.alwaysTrue;
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.dex.Marker;
@@ -22,16 +23,19 @@
import com.android.tools.r8.ir.analysis.type.Nullability;
import com.android.tools.r8.ir.analysis.type.ReferenceTypeLatticeElement;
import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
+import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.kotlin.Kotlin;
import com.android.tools.r8.naming.NamingLens;
import com.android.tools.r8.utils.ArrayUtils;
import com.android.tools.r8.utils.LRUCacheTable;
+import com.android.tools.r8.utils.Pair;
import com.google.common.base.Strings;
import com.google.common.collect.BiMap;
import com.google.common.collect.HashBiMap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Streams;
import it.unimi.dsi.fastutil.ints.Int2ObjectArrayMap;
import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
@@ -47,7 +51,9 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Consumer;
import java.util.function.Function;
+import java.util.function.Predicate;
import java.util.stream.Collectors;
+import java.util.stream.Stream;
public class DexItemFactory {
@@ -448,14 +454,19 @@
// We assume library methods listed here are `public`, i.e., free from visibility side effects.
// If not, that library method should not be added here because it literally has side effects.
- public Set<DexMethod> libraryMethodsWithoutSideEffects =
- ImmutableSet.<DexMethod>builder()
- .add(objectMethods.constructor)
- .addAll(classMethods.getNames)
- .addAll(stringBufferMethods.constructorMethods)
- .addAll(stringBuilderMethods.constructorMethods)
- .addAll(boxedValueOfMethods())
- .build();
+ public Map<DexMethod, Predicate<InvokeMethod>> libraryMethodsWithoutSideEffects =
+ Streams.<Pair<DexMethod, Predicate<InvokeMethod>>>concat(
+ Stream.of(new Pair<>(objectMethods.constructor, alwaysTrue())),
+ mapToPredicate(classMethods.getNames, alwaysTrue()),
+ mapToPredicate(stringBufferMethods.constructorMethods, alwaysTrue()),
+ mapToPredicate(stringBuilderMethods.constructorMethods, alwaysTrue()),
+ mapToPredicate(boxedValueOfMethods(), alwaysTrue()))
+ .collect(Collectors.toMap(Pair::getFirst, Pair::getSecond));
+
+ private static Stream<Pair<DexMethod, Predicate<InvokeMethod>>> mapToPredicate(
+ Set<DexMethod> methods, Predicate<InvokeMethod> predicate) {
+ return methods.stream().map(method -> new Pair<>(method, predicate));
+ }
// TODO(b/119596718): More idempotent methods? Any singleton accessors? E.g.,
// java.util.Calendar#getInstance(...) // 4 variants
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java b/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java
index 9be25b6..a7d3edb 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java
@@ -163,7 +163,9 @@
}
// Check if it is a call to one of library methods that are known to be side-effect free.
- if (appView.dexItemFactory().libraryMethodsWithoutSideEffects.contains(getInvokedMethod())) {
+ Predicate<InvokeMethod> noSideEffectsPredicate =
+ appView.dexItemFactory().libraryMethodsWithoutSideEffects.get(getInvokedMethod());
+ if (noSideEffectsPredicate != null && noSideEffectsPredicate.test(this)) {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java b/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
index 56f852b..44b4790 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
@@ -25,6 +25,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.function.Predicate;
import org.objectweb.asm.Opcodes;
public class InvokeStatic extends InvokeMethod {
@@ -150,7 +151,9 @@
}
// Check if it is a call to one of library methods that are known to be side-effect free.
- if (appView.dexItemFactory().libraryMethodsWithoutSideEffects.contains(getInvokedMethod())) {
+ Predicate<InvokeMethod> noSideEffectsPredicate =
+ appView.dexItemFactory().libraryMethodsWithoutSideEffects.get(getInvokedMethod());
+ if (noSideEffectsPredicate != null && noSideEffectsPredicate.test(this)) {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeVirtual.java b/src/main/java/com/android/tools/r8/ir/code/InvokeVirtual.java
index a2ce1bc..2c1b5a6 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeVirtual.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeVirtual.java
@@ -23,6 +23,7 @@
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import java.util.Collection;
import java.util.List;
+import java.util.function.Predicate;
import org.objectweb.asm.Opcodes;
public class InvokeVirtual extends InvokeMethodWithReceiver {
@@ -138,7 +139,9 @@
}
// Check if it is a call to one of library methods that are known to be side-effect free.
- if (appView.dexItemFactory().libraryMethodsWithoutSideEffects.contains(getInvokedMethod())) {
+ Predicate<InvokeMethod> noSideEffectsPredicate =
+ appView.dexItemFactory().libraryMethodsWithoutSideEffects.get(getInvokedMethod());
+ if (noSideEffectsPredicate != null && noSideEffectsPredicate.test(this)) {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizer.java b/src/main/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizer.java
index 9b04a16..6e958f1 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizer.java
@@ -29,6 +29,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
+import java.util.function.Predicate;
/**
* Canonicalize idempotent function calls.
@@ -122,8 +123,7 @@
}
DexMethod invokedMethod = invoke.getInvokedMethod();
// Interested in known-to-be idempotent methods.
- if (!factory.libraryMethodsWithReturnValueDependingOnlyOnArguments.contains(invokedMethod)
- || !factory.libraryMethodsWithoutSideEffects.contains(invokedMethod)) {
+ if (!isIdempotentLibraryMethodInvoke(invoke)) {
if (!appView.enableWholeProgramOptimizations()) {
// Give up in D8
continue;
@@ -246,6 +246,16 @@
assert code.isConsistentSSA();
}
+ private boolean isIdempotentLibraryMethodInvoke(InvokeMethod invoke) {
+ DexMethod invokedMethod = invoke.getInvokedMethod();
+ Predicate<InvokeMethod> noSideEffectPredicate =
+ factory.libraryMethodsWithoutSideEffects.get(invokedMethod);
+ if (noSideEffectPredicate == null || !noSideEffectPredicate.test(invoke)) {
+ return false;
+ }
+ return factory.libraryMethodsWithReturnValueDependingOnlyOnArguments.contains(invokedMethod);
+ }
+
private static void insertCanonicalizedInvokeWithInValues(
IRCode code, Invoke canonicalizedInvoke) {
BasicBlock entryBlock = code.entryBlock();
diff --git a/src/test/java/com/android/tools/r8/smali/OutlineTest.java b/src/test/java/com/android/tools/r8/smali/OutlineTest.java
index 47e0cd0..8987418 100644
--- a/src/test/java/com/android/tools/r8/smali/OutlineTest.java
+++ b/src/test/java/com/android/tools/r8/smali/OutlineTest.java
@@ -42,6 +42,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
+import java.util.IdentityHashMap;
import java.util.List;
import java.util.function.Consumer;
import org.junit.Assert;
@@ -1331,7 +1332,7 @@
dexItemFactory.stringBuilderType);
// ... and not assuming that StringBuilder.<init>() cannot have side effects.
dexItemFactory.libraryMethodsWithoutSideEffects =
- new HashSet<>(dexItemFactory.libraryMethodsWithoutSideEffects);
+ new IdentityHashMap<>(dexItemFactory.libraryMethodsWithoutSideEffects);
dexItemFactory.libraryMethodsWithoutSideEffects.remove(
dexItemFactory.stringBuilderMethods.defaultConstructor);
});
