Version 2.1.51
Cherry-pick: Fix the fix for enum single field values.
CL: https://r8-review.googlesource.com/c/r8/+/52528
Cherry-pick: Determine enum single field values using holder.
CL: https://r8-review.googlesource.com/c/r8/+/52525
Bug: 160831625
Cherry-pick: Remove trivial phis in class inliner.
CL: https://r8-review.googlesource.com/c/r8/+/52507
Bug: 160394262
Change-Id: If3d67b8c3ebcfbc958e54bd705bdb3ec33842cef
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index e9f227a..5f079ea 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "2.1.50";
+ public static final String LABEL = "2.1.51";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/value/SingleFieldValue.java b/src/main/java/com/android/tools/r8/ir/analysis/value/SingleFieldValue.java
index 14dee6f..7bdcc84 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/value/SingleFieldValue.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/value/SingleFieldValue.java
@@ -109,12 +109,14 @@
@Override
public SingleValue rewrittenWithLens(AppView<AppInfoWithLiveness> appView, GraphLense lens) {
AbstractValueFactory factory = appView.abstractValueFactory();
- EnumValueInfoMap unboxedEnumInfo = appView.unboxedEnums().getEnumValueInfoMap(field.type);
- if (unboxedEnumInfo != null) {
- // Return the ordinal of the unboxed enum.
- assert unboxedEnumInfo.hasEnumValueInfo(field);
- return factory.createSingleNumberValue(
- unboxedEnumInfo.getEnumValueInfo(field).convertToInt());
+ if (field.holder == field.type) {
+ EnumValueInfoMap unboxedEnumInfo = appView.unboxedEnums().getEnumValueInfoMap(field.type);
+ if (unboxedEnumInfo != null) {
+ // Return the ordinal of the unboxed enum.
+ assert unboxedEnumInfo.hasEnumValueInfo(field);
+ return factory.createSingleNumberValue(
+ unboxedEnumInfo.getEnumValueInfo(field).convertToInt());
+ }
}
return factory.createSingleFieldValue(
lens.lookupField(field), getState().rewrittenWithLens(appView, lens));
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerCostAnalysis.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerCostAnalysis.java
index f80e1d7..3feac87 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerCostAnalysis.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerCostAnalysis.java
@@ -12,11 +12,8 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.ProgramMethod;
-import com.android.tools.r8.ir.analysis.type.TypeElement;
-import com.android.tools.r8.ir.code.AssumeAndCheckCastAliasedValueConfiguration;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
-import com.android.tools.r8.ir.code.InstructionIterator;
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.optimize.inliner.InliningIRProvider;
@@ -26,7 +23,6 @@
import java.util.Map;
import java.util.Set;
-// TODO(b/160634549): Rename or refactor this to reflect its non-cost related analysis.
/** Analysis that estimates the cost of class inlining an object allocation. */
class ClassInlinerCostAnalysis {
@@ -77,28 +73,6 @@
continue;
}
IRCode inliningIR = inliningIRProvider.getAndCacheInliningIR(invoke, inlinee);
-
- // If the instance is part of a phi, then inlining will invalidate the inliner assumptions.
- // TODO(b/160634549): This is not a budget miss but a hard requirement.
- InstructionIterator iterator = inliningIR.entryBlock().iterator();
- while (iterator.hasNext()) {
- Instruction next = iterator.next();
- if (!next.isArgument()) {
- break;
- }
- Value argumentValue = next.outValue();
- TypeElement argumentType = argumentValue.getType();
- if (argumentType.isClassType()
- && argumentType.asClassType().getClassType() == eligibleClass.type) {
- assert argumentValue.uniqueUsers().stream()
- .noneMatch(
- AssumeAndCheckCastAliasedValueConfiguration.getInstance()::isIntroducingAnAlias);
- if (argumentValue.hasPhiUsers()) {
- return true;
- }
- }
- }
-
int increment =
inlinee.getDefinition().getCode().estimatedSizeForInlining()
- estimateNumberOfNonMaterializingInstructions(invoke, inliningIR);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
index da8c4cc..74d07d2 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
@@ -8,6 +8,7 @@
import static com.android.tools.r8.graph.DexProgramClass.asProgramClassOrNull;
import static com.google.common.base.Predicates.alwaysFalse;
+import com.android.tools.r8.errors.InternalCompilerError;
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexClass;
@@ -40,6 +41,7 @@
import com.android.tools.r8.ir.code.InvokeDirect;
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.InvokeMethodWithReceiver;
+import com.android.tools.r8.ir.code.Phi;
import com.android.tools.r8.ir.code.StaticGet;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.conversion.MethodProcessor;
@@ -57,10 +59,10 @@
import com.android.tools.r8.ir.optimize.inliner.NopWhyAreYouNotInliningReporter;
import com.android.tools.r8.kotlin.KotlinClassLevelInfo;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
-import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.Pair;
import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.Timing;
+import com.android.tools.r8.utils.WorkList;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
@@ -568,11 +570,35 @@
while (!currentUsers.isEmpty()) {
Set<Instruction> indirectOutValueUsers = Sets.newIdentityHashSet();
for (Instruction instruction : currentUsers) {
- if (instruction.isAssume() || instruction.isCheckCast()) {
- Value src = ListUtils.first(instruction.inValues());
+ if (aliasesThroughAssumeAndCheckCasts.isIntroducingAnAlias(instruction)) {
+ Value src = aliasesThroughAssumeAndCheckCasts.getAliasForOutValue(instruction);
Value dest = instruction.outValue();
- indirectOutValueUsers.addAll(dest.uniqueUsers());
+ if (dest.hasPhiUsers()) {
+ // It is possible that a trivial phi is constructed upon IR building for the eligible
+ // value. It must actually be trivial so verify that it is indeed trivial and replace
+ // all of the phis involved with the value.
+ WorkList<Phi> worklist = WorkList.newIdentityWorkList(dest.uniquePhiUsers());
+ while (worklist.hasNext()) {
+ Phi phi = worklist.next();
+ for (Value operand : phi.getOperands()) {
+ operand = operand.getAliasedValue(aliasesThroughAssumeAndCheckCasts);
+ if (operand.isPhi()) {
+ worklist.addIfNotSeen(operand.asPhi());
+ } else if (src != operand) {
+ throw new InternalCompilerError(
+ "Unexpected non-trivial phi in method eligible for class inlining");
+ }
+ }
+ }
+ // The only value flowing into any of the phis is src, so replace all phis by src.
+ for (Phi phi : worklist.getSeenSet()) {
+ indirectOutValueUsers.addAll(phi.uniqueUsers());
+ phi.replaceUsers(src);
+ phi.removeDeadPhi();
+ }
+ }
assert !dest.hasPhiUsers();
+ indirectOutValueUsers.addAll(dest.uniqueUsers());
dest.replaceUsers(src);
removeInstruction(instruction);
}
diff --git a/src/main/java/com/android/tools/r8/utils/WorkList.java b/src/main/java/com/android/tools/r8/utils/WorkList.java
index 3e35802..c6e554a 100644
--- a/src/main/java/com/android/tools/r8/utils/WorkList.java
+++ b/src/main/java/com/android/tools/r8/utils/WorkList.java
@@ -6,6 +6,7 @@
import com.google.common.collect.Sets;
import java.util.ArrayDeque;
+import java.util.Collections;
import java.util.Deque;
import java.util.HashSet;
import java.util.Set;
@@ -70,6 +71,10 @@
return workingList.removeFirst();
}
+ public Set<T> getSeenSet() {
+ return Collections.unmodifiableSet(seen);
+ }
+
public enum EqualityTest {
HASH,
IDENTITY
diff --git a/src/test/java/com/android/tools/r8/regress/Regress160394262Test.java b/src/test/java/com/android/tools/r8/regress/Regress160394262Test.java
index 0c3206c..383a7ef 100644
--- a/src/test/java/com/android/tools/r8/regress/Regress160394262Test.java
+++ b/src/test/java/com/android/tools/r8/regress/Regress160394262Test.java
@@ -57,15 +57,13 @@
}
private void checkJoinerIsClassInlined(CodeInspector inspector) {
- // TODO(b/160640028): When compiling to DEX a trivial phi remains in the inline code preventing
- // class inlining of Joiner and the anonymous Joiner subclass.
+ assertThat(inspector.clazz(Joiner.class.getTypeName() + "$1"), not(isPresent()));
+ // TODO(b/160640028): When compiling to DEX the outer Joiner class is not inlined.
if (parameters.isDexRuntime()) {
assertThat(inspector.clazz(Joiner.class), isPresent());
- assertThat(inspector.clazz(Joiner.class.getTypeName() + "$1"), isPresent());
- return;
+ } else {
+ assertThat(inspector.clazz(Joiner.class), not(isPresent()));
}
- assertThat(inspector.clazz(Joiner.class), not(isPresent()));
- assertThat(inspector.clazz(Joiner.class.getTypeName() + "$1"), not(isPresent()));
}
static class TestClass {
diff --git a/src/test/java/com/android/tools/r8/regress/Regress160831625Test.java b/src/test/java/com/android/tools/r8/regress/Regress160831625Test.java
new file mode 100644
index 0000000..15c0c75
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/regress/Regress160831625Test.java
@@ -0,0 +1,72 @@
+// Copyright (c) 2020, 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.regress;
+
+import com.android.tools.r8.NeverPropagateValue;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.StringUtils;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class Regress160831625Test extends TestBase {
+
+ static final String EXPECTED = StringUtils.lines("A", "0");
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimes().withAllApiLevels().build();
+ }
+
+ public Regress160831625Test(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testReference() throws Exception {
+ testForRuntime(parameters)
+ .addInnerClasses(getClass())
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .setMinApi(parameters.getApiLevel())
+ .addInnerClasses(getClass())
+ .enableMemberValuePropagationAnnotations()
+ .addKeepMainRule(TestClass.class)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ enum MyEnum {
+ // Ensure that the enum field value is not inlined in the alias in MyClass.B
+ @NeverPropagateValue
+ A
+ }
+
+ static class MyClass {
+
+ private static final MyEnum B = MyEnum.A;
+
+ public static MyEnum getB() {
+ return B;
+ }
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ System.out.println(MyClass.getB().name());
+ System.out.println(MyClass.getB().ordinal());
+ }
+ }
+}
