Revert "Redundant field elimination: large enum analysis"
This reverts commit aa170d82c478d49dab8e0f0efe8986ad698c2e53.
Reason for revert: Failure on internal bot.
Log: tools/internal_test.py --print_logs a098ba26f0f22926039e57793d0172c87162eab6
Repro: ./tools/run_on_app.py --app gmail --version 180826.15 --compiler r8 --compiler-build full
Works on 741fbcc9e
Change-Id: I68bffc299be8e4ae4986911033b7232fbf344236
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/StaticFieldValueAnalysis.java b/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/StaticFieldValueAnalysis.java
index 281215f..03455fb 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/StaticFieldValueAnalysis.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/StaticFieldValueAnalysis.java
@@ -316,11 +316,9 @@
return computeObjectState(definition.outValue());
}
if (definition.isStaticGet()) {
- // Enums with many instance rely on staticGets to set the $VALUES data instead of directly
- // keeping the values in registers, due to the max capacity of the redundant field load
- // elimination. The capacity has already been increased, so that this case is extremely
- // uncommon (very large enums).
- // TODO(b/169050248): We could consider analysing these to answer the object state here.
+ // TODO(b/166532388) : Enums with many instance rely on staticGets to set the $VALUES data
+ // instead of directly keeping the values in registers. We could consider analysing these
+ // and answer the analysed object state here.
return ObjectState.empty();
}
return ObjectState.empty();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java b/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java
index 10b8b25..cabff87 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java
@@ -54,12 +54,11 @@
public class RedundantFieldLoadElimination {
private static final int MAX_CAPACITY = 10000;
- private static final int MIN_CAPACITY_PER_BLOCK = 50;
+ private static final int MAX_CAPACITY_PER_BLOCK = 50;
private final AppView<?> appView;
private final ProgramMethod method;
private final IRCode code;
- private final int maxCapacityPerBlock;
// Values that may require type propagation.
private final Set<Value> affectedValues = Sets.newIdentityHashSet();
@@ -75,7 +74,6 @@
this.appView = appView;
this.method = code.context();
this.code = code;
- this.maxCapacityPerBlock = Math.max(MIN_CAPACITY_PER_BLOCK, MAX_CAPACITY / code.blocks.size());
}
public static boolean shouldRun(AppView<?> appView, IRCode code) {
@@ -187,7 +185,7 @@
// Already visited.
continue;
}
- activeState = activeStates.computeActiveStateOnBlockEntry(head, maxCapacityPerBlock);
+ activeState = activeStates.computeActiveStateOnBlockEntry(head);
activeStates.removeDeadBlockExitStates(head, pendingNormalSuccessors);
BasicBlock block = head;
BasicBlock end = null;
@@ -446,20 +444,19 @@
private int capacity = MAX_CAPACITY;
- BlockState computeActiveStateOnBlockEntry(BasicBlock block, int maxCapacityPerBlock) {
+ BlockState computeActiveStateOnBlockEntry(BasicBlock block) {
if (block.isEntry()) {
- return new BlockState(maxCapacityPerBlock);
+ return new BlockState();
}
List<BasicBlock> predecessors = block.getPredecessors();
Iterator<BasicBlock> predecessorIterator = predecessors.iterator();
- BlockState state =
- new BlockState(maxCapacityPerBlock, activeStateAtExit.get(predecessorIterator.next()));
+ BlockState state = new BlockState(activeStateAtExit.get(predecessorIterator.next()));
while (predecessorIterator.hasNext()) {
BasicBlock predecessor = predecessorIterator.next();
BlockState predecessorExitState = activeStateAtExit.get(predecessor);
if (predecessorExitState == null) {
// Not processed yet.
- return new BlockState(maxCapacityPerBlock);
+ return new BlockState();
}
state.intersect(predecessorExitState);
}
@@ -482,7 +479,7 @@
private void ensureCapacity(BlockState state) {
int stateSize = state.size();
- assert stateSize <= state.maxCapacity;
+ assert stateSize <= MAX_CAPACITY_PER_BLOCK;
int numberOfItemsToRemove = stateSize - capacity;
if (numberOfItemsToRemove <= 0) {
return;
@@ -571,14 +568,9 @@
private LinkedHashMap<DexField, FieldValue> nonFinalStaticFieldValues;
- private final int maxCapacity;
+ public BlockState() {}
- public BlockState(int maxCapacity) {
- this.maxCapacity = maxCapacity;
- }
-
- public BlockState(int maxCapacity, BlockState state) {
- this(maxCapacity);
+ public BlockState(BlockState state) {
if (state != null) {
if (state.finalInstanceFieldValues != null && !state.finalInstanceFieldValues.isEmpty()) {
finalInstanceFieldValues = new LinkedHashMap<>();
@@ -614,8 +606,8 @@
public void ensureCapacityForNewElement() {
int size = size();
- assert size <= maxCapacity;
- if (size == maxCapacity) {
+ assert size <= MAX_CAPACITY_PER_BLOCK;
+ if (size == MAX_CAPACITY_PER_BLOCK) {
reduceSize(1);
}
}
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/LargeEnumUnboxingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/LargeEnumUnboxingTest.java
deleted file mode 100644
index 66c861a..0000000
--- a/src/test/java/com/android/tools/r8/enumunboxing/LargeEnumUnboxingTest.java
+++ /dev/null
@@ -1,246 +0,0 @@
-// 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.enumunboxing;
-
-import com.android.tools.r8.NeverClassInline;
-import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.utils.StringUtils;
-import java.util.List;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameters;
-
-@RunWith(Parameterized.class)
-public class LargeEnumUnboxingTest extends EnumUnboxingTestBase {
-
- private static final String EXPECTED_RESULT =
- StringUtils.lines(
- "B1falsetruefalse1",
- "B2falsetruefalse2",
- "B3falsetruefalse3",
- "B4falsetruefalse4",
- "B5falsetruefalse5",
- "B6falsetruefalse6",
- "B7falsetruefalse7",
- "B8falsetruefalse8",
- "B9falsetruefalse9",
- "B10falsetruefalse10",
- "B11falsetruefalse11",
- "B12falsetruefalse12",
- "B13falsetruefalse13",
- "E1falsefalsetrue14",
- "E2falsefalsetrue15",
- "E3falsefalsetrue16",
- "E4falsefalsetrue17",
- "E5falsefalsetrue18",
- "E6falsefalsetrue19",
- "E7falsefalsetrue20",
- "E8falsefalsetrue21",
- "E9falsefalsetrue22",
- "E10falsefalsetrue23",
- "E11falsefalsetrue24",
- "E12falsefalsetrue25",
- "E13falsefalsetrue26",
- "E14falsefalsetrue27",
- "E15falsefalsetrue28",
- "G1falsefalsefalse29",
- "G2falsefalsefalse30",
- "G3falsefalsefalse31",
- "G4falsefalsefalse32",
- "G5falsefalsefalse33",
- "G6falsefalsefalse34",
- "G7falsefalsefalse35",
- "G8falsefalsefalse36",
- "G9falsefalsefalse37",
- "I1truefalsefalse38",
- "I2truefalsefalse39",
- "I3truefalsefalse40",
- "I4truefalsefalse41",
- "I5truefalsefalse42",
- "I6truefalsefalse43",
- "I7truefalsefalse44",
- "I8truefalsefalse45",
- "I9truefalsefalse46",
- "I10truefalsefalse47",
- "I11truefalsefalse48",
- "I12truefalsefalse49",
- "I13truefalsefalse50",
- "I14truefalsefalse51",
- "I15truefalsefalse52",
- "I16truefalsefalse53",
- "J1falsefalsefalse54",
- "J2falsefalsefalse55");
-
- private final TestParameters parameters;
- private final boolean enumValueOptimization;
- private final EnumKeepRules enumKeepRules;
-
- @Parameters(name = "{0} valueOpt: {1} keep: {2}")
- public static List<Object[]> data() {
- return enumUnboxingTestParameters();
- }
-
- public LargeEnumUnboxingTest(
- TestParameters parameters, boolean enumValueOptimization, EnumKeepRules enumKeepRules) {
- this.parameters = parameters;
- this.enumValueOptimization = enumValueOptimization;
- this.enumKeepRules = enumKeepRules;
- }
-
- @Test
- public void testEnumUnboxing() throws Exception {
- Class<?> mainClass = Main.class;
- testForR8(parameters.getBackend())
- .addProgramClasses(mainClass, LargeEnum.class)
- .addKeepMainRule(mainClass)
- .addKeepRules(enumKeepRules.getKeepRules())
- .enableNeverClassInliningAnnotations()
- .addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
- .allowDiagnosticInfoMessages()
- .setMinApi(parameters.getApiLevel())
- .compile()
- .inspectDiagnosticMessages(
- m -> assertEnumIsUnboxed(LargeEnum.class, mainClass.getSimpleName(), m))
- .run(parameters.getRuntime(), mainClass)
- .assertSuccessWithOutput(EXPECTED_RESULT);
- }
-
- @NeverClassInline
- enum LargeEnum {
- B1("1"),
- B2("2"),
- B3("3"),
- B4("4"),
- B5("5"),
- B6("6"),
- B7("7"),
- B8("8"),
- B9("9"),
- B10("10"),
- B11("11"),
- B12("12"),
- B13("13"),
-
- E1("14"),
- E2("15"),
- E3("16"),
- E4("17"),
- E5("18"),
- E6("19"),
- E7("20"),
- E8("21"),
- E9("22"),
- E10("23"),
- E11("24"),
- E12("25"),
- E13("26"),
- E14("27"),
- E15("28"),
-
- G1("29"),
- G2("30"),
- G3("31"),
- G4("32"),
- G5("33"),
- G6("34"),
- G7("35"),
- G8("36"),
- G9("37"),
-
- I1("38"),
- I2("39"),
- I3("40"),
- I4("41"),
- I5("42"),
- I6("43"),
- I7("44"),
- I8("45"),
- I9("46"),
- I10("47"),
- I11("48"),
- I12("49"),
- I13("50"),
- I14("51"),
- I15("52"),
- I16("53"),
-
- J1("54"),
- J2("55");
-
- private final String num;
-
- LargeEnum(String num) {
- this.num = num;
- }
-
- public String getNum() {
- return num;
- }
-
- public boolean isI() {
- return this == I1
- || this == I2
- || this == I3
- || this == I4
- || this == I5
- || this == I6
- || this == I7
- || this == I8
- || this == I9
- || this == I10
- || this == I11
- || this == I12
- || this == I13
- || this == I14
- || this == I15
- || this == I16;
- }
-
- public boolean isB() {
- return this == B1
- || this == B2
- || this == B3
- || this == B4
- || this == B5
- || this == B6
- || this == B7
- || this == B8
- || this == B9
- || this == B10
- || this == B11
- || this == B12
- || this == B13;
- }
-
- public boolean isE() {
- return this == E1
- || this == E2
- || this == E3
- || this == E4
- || this == E5
- || this == E6
- || this == E7
- || this == E8
- || this == E9
- || this == E10
- || this == E11
- || this == E12
- || this == E13
- || this == E14
- || this == E15;
- }
- }
-
- static class Main {
-
- public static void main(String[] args) {
- for (LargeEnum value : LargeEnum.values()) {
- System.out.println(
- value.toString() + value.isI() + value.isB() + value.isE() + value.getNum());
- }
- }
- }
-}
