Simplify StringSwitch conversion
Fixes: b/325395722
Change-Id: Ie6ae28aa1bcf781394268d049a8f600fbf2c2111
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
index 994ef53..b95529b 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
@@ -742,10 +742,6 @@
new TypeAnalysis(appView, ir).narrowing();
}
- if (conversionOptions.isStringSwitchConversionEnabled()) {
- StringSwitchConverter.convertToStringSwitchInstructions(ir, appView.dexItemFactory());
- }
-
ir.removeRedundantBlocks();
assert ir.isConsistentSSABeforeTypesAreCorrect(appView);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LirConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/LirConverter.java
index 846d0c7..25df747 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/LirConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LirConverter.java
@@ -19,6 +19,7 @@
import com.android.tools.r8.ir.conversion.passes.DexItemBasedConstStringRemover;
import com.android.tools.r8.ir.conversion.passes.FilledNewArrayRewriter;
import com.android.tools.r8.ir.conversion.passes.InitClassRemover;
+import com.android.tools.r8.ir.conversion.passes.StringSwitchConverter;
import com.android.tools.r8.ir.conversion.passes.StringSwitchRemover;
import com.android.tools.r8.ir.optimize.ConstantCanonicalizer;
import com.android.tools.r8.ir.optimize.DeadCodeRemover;
@@ -42,8 +43,9 @@
assert appView.testing().canUseLir(appView);
assert appView.testing().isPreLirPhase();
appView.testing().enterLirSupportedPhase();
- ConstResourceNumberRewriter constResourceNumberRewriter =
- new ConstResourceNumberRewriter(appView);
+ CodeRewriterPassCollection codeRewriterPassCollection =
+ new CodeRewriterPassCollection(
+ new ConstResourceNumberRewriter(appView), new StringSwitchConverter(appView));
// Convert code objects to LIR.
ThreadUtils.processItems(
appView.appInfo().classes(),
@@ -53,7 +55,7 @@
method -> {
assert !method.getDefinition().getCode().hasExplicitCodeLens();
IRCode code = method.buildIR(appView, MethodConversionOptions.forLirPhase(appView));
- constResourceNumberRewriter.run(code, Timing.empty());
+ codeRewriterPassCollection.run(code, null, null, Timing.empty());
LirCode<Integer> lirCode =
IR2LirConverter.translate(
code,
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/MethodConversionOptions.java b/src/main/java/com/android/tools/r8/ir/conversion/MethodConversionOptions.java
index 24f7613..fd3c580 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/MethodConversionOptions.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/MethodConversionOptions.java
@@ -7,7 +7,6 @@
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.ir.optimize.DeadCodeRemover;
-import com.android.tools.r8.utils.InternalOptions;
public abstract class MethodConversionOptions {
@@ -16,7 +15,7 @@
return forD8(appView);
}
assert appView.testing().isPreLirPhase();
- return new MutableMethodConversionOptions(Target.CF, appView.options());
+ return new MutableMethodConversionOptions(Target.CF);
}
public static MutableMethodConversionOptions forPostLirPhase(AppView<?> appView) {
@@ -25,8 +24,7 @@
}
assert appView.testing().isPostLirPhase();
Target target = appView.options().isGeneratingClassFiles() ? Target.CF : Target.DEX;
- return new MutableMethodConversionOptions(target, appView.options())
- .disableStringSwitchConversion();
+ return new MutableMethodConversionOptions(target);
}
public static MutableMethodConversionOptions forLirPhase(AppView<?> appView) {
@@ -34,12 +32,12 @@
return forD8(appView);
}
assert appView.testing().isSupportedLirPhase();
- return new MutableMethodConversionOptions(determineTarget(appView), appView.options());
+ return new MutableMethodConversionOptions(determineTarget(appView));
}
public static MutableMethodConversionOptions forD8(AppView<?> appView) {
assert !appView.enableWholeProgramOptimizations();
- return new MutableMethodConversionOptions(determineTarget(appView), appView.options());
+ return new MutableMethodConversionOptions(determineTarget(appView));
}
public static MutableMethodConversionOptions nonConverting() {
@@ -82,24 +80,16 @@
public abstract boolean isPeepholeOptimizationsEnabled();
- public abstract boolean isStringSwitchConversionEnabled();
-
public abstract boolean shouldFinalizeAfterLensCodeRewriter();
public static class MutableMethodConversionOptions extends MethodConversionOptions {
- private Target target;
+ private final Target target;
private boolean enablePeepholeOptimizations = true;
- private boolean enableStringSwitchConversion;
private boolean finalizeAfterLensCodeRewriter;
- private MutableMethodConversionOptions(Target target, boolean enableStringSwitchConversion) {
+ private MutableMethodConversionOptions(Target target) {
this.target = target;
- this.enableStringSwitchConversion = enableStringSwitchConversion;
- }
-
- private MutableMethodConversionOptions(Target target, InternalOptions options) {
- this(target, options.isStringSwitchConversionEnabled());
}
public void disablePeepholeOptimizations(MethodProcessor methodProcessor) {
@@ -107,11 +97,6 @@
enablePeepholeOptimizations = false;
}
- public MutableMethodConversionOptions disableStringSwitchConversion() {
- enableStringSwitchConversion = false;
- return this;
- }
-
public MutableMethodConversionOptions setFinalizeAfterLensCodeRewriter() {
finalizeAfterLensCodeRewriter = true;
return this;
@@ -138,11 +123,6 @@
}
@Override
- public boolean isStringSwitchConversionEnabled() {
- return enableStringSwitchConversion;
- }
-
- @Override
public boolean shouldFinalizeAfterLensCodeRewriter() {
return finalizeAfterLensCodeRewriter;
}
@@ -151,7 +131,7 @@
public static class ThrowingMethodConversionOptions extends MutableMethodConversionOptions {
private ThrowingMethodConversionOptions() {
- super(null, true);
+ super(null);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/CodeRewriterPass.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/CodeRewriterPass.java
index 98a92e8..87fc847 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/passes/CodeRewriterPass.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/CodeRewriterPass.java
@@ -59,7 +59,7 @@
return noChange();
}
- private boolean verifyConsistentCode(IRCode code, boolean ssa, String preposition) {
+ protected boolean verifyConsistentCode(IRCode code, boolean ssa, String preposition) {
boolean result;
String message = "Invalid code " + preposition + " " + getRewriterId();
try {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/ConstResourceNumberRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/ConstResourceNumberRewriter.java
index e6806fc..7783c5c 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/passes/ConstResourceNumberRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/ConstResourceNumberRewriter.java
@@ -62,4 +62,10 @@
}
return CodeRewriterResult.hasChanged(hasChanged);
}
+
+ @Override
+ protected boolean verifyConsistentCode(IRCode code, boolean ssa, String preposition) {
+ // Skip verification since this runs prior to the removal of invalid code.
+ return true;
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/StringSwitchConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/StringSwitchConverter.java
similarity index 95%
rename from src/main/java/com/android/tools/r8/ir/conversion/StringSwitchConverter.java
rename to src/main/java/com/android/tools/r8/ir/conversion/passes/StringSwitchConverter.java
index 5d7faef..a49b93b 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/StringSwitchConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/StringSwitchConverter.java
@@ -1,12 +1,15 @@
-// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file
+// Copyright (c) 2024, 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.ir.conversion;
+package com.android.tools.r8.ir.conversion.passes;
import static com.android.tools.r8.ir.code.ConstNumber.asConstNumberOrNull;
+import com.android.tools.r8.contexts.CompilationContext.MethodProcessingContext;
import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.ir.code.BasicBlock;
@@ -24,6 +27,8 @@
import com.android.tools.r8.ir.code.Phi;
import com.android.tools.r8.ir.code.StringSwitch;
import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.ir.conversion.MethodProcessor;
+import com.android.tools.r8.ir.conversion.passes.result.CodeRewriterResult;
import com.google.common.collect.Sets;
import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
import it.unimi.dsi.fastutil.ints.Int2ReferenceOpenHashMap;
@@ -105,10 +110,28 @@
* }
* </pre>
*/
-public class StringSwitchConverter {
+public class StringSwitchConverter extends CodeRewriterPass<AppInfo> {
- public static boolean convertToStringSwitchInstructions(
- IRCode code, DexItemFactory dexItemFactory) {
+ public StringSwitchConverter(AppView<?> appView) {
+ super(appView);
+ }
+
+ @Override
+ protected String getRewriterId() {
+ return "StringSwitchConverter";
+ }
+
+ @Override
+ protected boolean shouldRewriteCode(IRCode code, MethodProcessor methodProcessor) {
+ return options.shouldCompileMethodInReleaseMode(appView, code.context())
+ && options.enableStringSwitchConversion;
+ }
+
+ @Override
+ protected CodeRewriterResult rewriteCode(
+ IRCode code,
+ MethodProcessor methodProcessor,
+ MethodProcessingContext methodProcessingContext) {
List<BasicBlock> rewritingCandidates = getRewritingCandidates(code, dexItemFactory);
if (rewritingCandidates != null) {
boolean changed = false;
@@ -121,9 +144,15 @@
code.removeAllDeadAndTrivialPhis();
code.removeUnreachableBlocks();
}
- return changed;
+ return CodeRewriterResult.hasChanged(changed);
}
- return false;
+ return CodeRewriterResult.NO_CHANGE;
+ }
+
+ @Override
+ protected boolean verifyConsistentCode(IRCode code, boolean ssa, String preposition) {
+ // Skip verification since this runs prior to the removal of invalid code.
+ return true;
}
private static List<BasicBlock> getRewritingCandidates(
diff --git a/src/main/java/com/android/tools/r8/lightir/Lir2IRConverter.java b/src/main/java/com/android/tools/r8/lightir/Lir2IRConverter.java
index e8deff4..42c94e5 100644
--- a/src/main/java/com/android/tools/r8/lightir/Lir2IRConverter.java
+++ b/src/main/java/com/android/tools/r8/lightir/Lir2IRConverter.java
@@ -100,7 +100,6 @@
import com.android.tools.r8.ir.code.Xor;
import com.android.tools.r8.ir.conversion.ExtraParameter;
import com.android.tools.r8.ir.conversion.MethodConversionOptions.MutableMethodConversionOptions;
-import com.android.tools.r8.ir.conversion.StringSwitchConverter;
import com.android.tools.r8.lightir.LirBuilder.IntSwitchPayload;
import com.android.tools.r8.lightir.LirBuilder.StringSwitchPayload;
import com.android.tools.r8.lightir.LirCode.PositionEntry;
@@ -146,13 +145,6 @@
IRCode irCode = parser.getIRCode(method, conversionOptions);
// Some instructions have bottom types (e.g., phis). Compute their actual types by widening.
new TypeAnalysis(appView, irCode).widening();
-
- if (conversionOptions.isStringSwitchConversionEnabled()) {
- if (StringSwitchConverter.convertToStringSwitchInstructions(
- irCode, appView.dexItemFactory())) {
- irCode.removeRedundantBlocks();
- }
- }
return irCode;
}
diff --git a/src/main/java/com/android/tools/r8/lightir/LirLensCodeRewriter.java b/src/main/java/com/android/tools/r8/lightir/LirLensCodeRewriter.java
index b2331cc..de48365 100644
--- a/src/main/java/com/android/tools/r8/lightir/LirLensCodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/lightir/LirLensCodeRewriter.java
@@ -366,11 +366,7 @@
@SuppressWarnings("unchecked")
private LirCode<EV> removeUnreachableBlocks(LirCode<EV> rewritten) {
- IRCode code =
- rewritten.buildIR(
- context,
- appView,
- MethodConversionOptions.forLirPhase(appView).disableStringSwitchConversion());
+ IRCode code = rewritten.buildIR(context, appView, MethodConversionOptions.forLirPhase(appView));
AffectedValues affectedValues = code.removeUnreachableBlocks();
affectedValues.narrowingWithAssumeRemoval(appView, code);
new DeadCodeRemover(appView).run(code, Timing.empty());
@@ -386,7 +382,6 @@
context.buildIR(
appView,
MethodConversionOptions.forLirPhase(appView)
- .disableStringSwitchConversion()
.setFinalizeAfterLensCodeRewriter());
// MethodProcessor argument is only used by unboxing lenses.
MethodProcessor methodProcessor = null;
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 126960b..10731fb 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
@@ -116,10 +116,7 @@
.setApiLevelForCode(
appView.apiLevelCompute().computeInitialMinApiLevel(appView.options()));
}
- IRCode code =
- method.buildIR(
- appView,
- MethodConversionOptions.forLirPhase(appView).disableStringSwitchConversion());
+ IRCode code = method.buildIR(appView, MethodConversionOptions.forLirPhase(appView));
inliner.performInlining(
method, code, getSimpleFeedback(), methodProcessor, Timing.empty());
CodeRewriter.removeAssumeInstructions(appView, code);
diff --git a/src/main/java/com/android/tools/r8/profile/startup/instrumentation/StartupInstrumentation.java b/src/main/java/com/android/tools/r8/profile/startup/instrumentation/StartupInstrumentation.java
index ba3abda..192178b 100644
--- a/src/main/java/com/android/tools/r8/profile/startup/instrumentation/StartupInstrumentation.java
+++ b/src/main/java/com/android/tools/r8/profile/startup/instrumentation/StartupInstrumentation.java
@@ -176,8 +176,7 @@
private void instrumentMethod(ProgramMethod method, boolean skipMethodLogging) {
// Disable StringSwitch conversion to avoid having to run the StringSwitchRemover before
// finalizing the code.
- MutableMethodConversionOptions conversionOptions =
- MethodConversionOptions.forD8(appView).disableStringSwitchConversion();
+ MutableMethodConversionOptions conversionOptions = MethodConversionOptions.forD8(appView);
IRCode code = method.buildIR(appView, conversionOptions);
InstructionListIterator instructionIterator = code.entryBlock().listIterator(code);
instructionIterator.positionBeforeNextInstructionThatMatches(not(Instruction::isArgument));
diff --git a/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracingImpl.java b/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracingImpl.java
index 759ebd7..1eacba7 100644
--- a/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracingImpl.java
+++ b/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracingImpl.java
@@ -266,7 +266,6 @@
mode.isInitialTreeShaking()
? MethodConversionOptions.forPreLirPhase(appView)
: MethodConversionOptions.forLirPhase(appView);
- conversionOptions.disableStringSwitchConversion();
IRCode ir = method.buildIR(appView, conversionOptions);
diff --git a/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java b/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java
index 867f9c9..181c0c5 100644
--- a/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java
+++ b/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java
@@ -151,11 +151,6 @@
}
@Override
- public boolean isStringSwitchConversionEnabled() {
- return false;
- }
-
- @Override
public boolean shouldFinalizeAfterLensCodeRewriter() {
return false;
}
