Reapply "Maintain LIR past repackaging"
This reverts commit 96a5fc3f9a34ddc33e4a56861239d813ce9c297b.
Change-Id: I186133ac9d3c7c5f693226b2bd3cf0891b7c17cc
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 45b7c53..24f0c28 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -55,6 +55,7 @@
import com.android.tools.r8.ir.optimize.SwitchMapCollector;
import com.android.tools.r8.ir.optimize.enums.EnumUnboxingCfMethods;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackSimple;
+import com.android.tools.r8.ir.optimize.info.OptimizationInfoRemover;
import com.android.tools.r8.ir.optimize.templates.CfUtilityMethodsForCodeOptimizations;
import com.android.tools.r8.jar.CfApplicationWriter;
import com.android.tools.r8.keepanno.annotations.KeepForApi;
@@ -533,6 +534,7 @@
new PrimaryR8IRConverter(appViewWithLiveness, timing)
.optimize(appViewWithLiveness, executorService);
+ assert LirConverter.verifyLirOnly(appView);
assert ArtProfileCompletenessChecker.verify(
appView, ALLOW_MISSING_ENUM_UNBOXING_UTILITY_METHODS);
@@ -716,17 +718,16 @@
SyntheticFinalization.finalizeWithClassHierarchy(appView, executorService, timing);
}
- // Rewrite LIR with lens to allow building IR from LIR in class mergers.
- LirConverter.rewriteLirWithLens(appView, timing, executorService);
-
- // TODO(b/225838009): Move further down.
- LirConverter.finalizeLirToOutputFormat(appView, timing, executorService);
-
// Read any -applymapping input to allow for repackaging to not relocate the classes.
timing.begin("read -applymapping file");
appView.loadApplyMappingSeedMapper();
timing.end();
+ // Remove optimization info before remaining optimizations, since these optimization currently
+ // do not rewrite the optimization info, which is OK since the optimization info should
+ // already have been leveraged.
+ OptimizationInfoRemover.run(appView, executorService);
+
// Perform repackaging.
if (appView.hasLiveness()) {
if (options.isRepackagingEnabled()) {
@@ -735,18 +736,21 @@
assert Repackaging.verifyIdentityRepackaging(appView.withLiveness(), executorService);
}
- // Clear the reference type lattice element cache. This is required since class merging may
- // need to build IR.
- appView.dexItemFactory().clearTypeElementsCache();
-
- GenericSignatureContextBuilder genericContextBuilderBeforeFinalMerging =
- GenericSignatureContextBuilder.create(appView);
+ // Rewrite LIR with lens to allow building IR from LIR in class mergers.
+ LirConverter.rewriteLirWithLens(appView, timing, executorService);
if (appView.hasLiveness()) {
VerticalClassMerger.createForFinalClassMerging(appView.withLiveness())
.runIfNecessary(executorService, timing);
}
+ // TODO(b/225838009): Move further down.
+ LirConverter.finalizeLirToOutputFormat(appView, timing, executorService);
+ assert appView.dexItemFactory().verifyNoCachedTypeElements();
+
+ GenericSignatureContextBuilder genericContextBuilderBeforeFinalMerging =
+ GenericSignatureContextBuilder.create(appView);
+
// Run horizontal class merging. This runs even if shrinking is disabled to ensure synthetics
// are always merged.
HorizontalClassMerger.createForFinalClassMerging(appView)
diff --git a/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java b/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
index 557bb44..af6a62f 100644
--- a/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
+++ b/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
@@ -48,6 +48,10 @@
// Mapping from register to local for currently open/visible locals.
private Int2ReferenceMap<DebugLocalInfo> pendingLocals = null;
+ // The method's preamble position. This is used in release mode to preserve the preamble position
+ // for methods without throwing instructions that have been moved (i.e., have a caller position).
+ private Position preamblePosition;
+
// Conservative pending-state of locals to avoid some equality checks on locals.
// pendingLocalChanges == true ==> localsEqual(emittedLocals, pendingLocals).
private boolean pendingLocalChanges = false;
@@ -81,6 +85,9 @@
// Initialize locals state on block entry.
if (isBlockEntry) {
updateBlockEntry(instruction);
+ if (preamblePosition == null) {
+ preamblePosition = instruction.getPosition();
+ }
}
assert pendingLocals != null;
@@ -116,8 +123,17 @@
public DexDebugInfo build() {
assert pendingLocals == null;
assert !pendingLocalChanges;
+ assert preamblePosition != null;
if (startLine == NO_LINE_INFO) {
- return null;
+ if (!preamblePosition.hasCallerPosition()) {
+ return null;
+ }
+ return new EventBasedDebugInfo(
+ preamblePosition.getLine(),
+ new DexString[method.getReference().getArity()],
+ new DexDebugEvent[] {
+ factory.createPositionFrame(preamblePosition), factory.zeroChangeDefaultEvent
+ });
}
DexString[] params = new DexString[method.getReference().getArity()];
if (arguments != null) {
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index 10d4010..3c3533a 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -1310,6 +1310,11 @@
optimizationInfo = info;
}
+ public void unsetOptimizationInfo() {
+ checkIfObsolete();
+ optimizationInfo = DefaultMethodOptimizationInfo.getInstance();
+ }
+
public void copyMetadata(AppView<?> appView, DexEncodedMethod from) {
checkIfObsolete();
if (from.hasClassFileVersion()) {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
index 9f48b43..c03f772 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
@@ -57,6 +57,7 @@
import com.android.tools.r8.ir.optimize.PeepholeOptimizer;
import com.android.tools.r8.ir.optimize.PhiOptimizations;
import com.android.tools.r8.ir.optimize.peepholes.BasicBlockMuncher;
+import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.Timing;
import com.android.tools.r8.utils.WorkList;
import com.google.common.collect.Sets;
@@ -90,6 +91,7 @@
private CfRegisterAllocator registerAllocator;
private Position currentPosition = Position.none();
+ private Position preamblePosition = null;
private final Int2ReferenceMap<DebugLocalInfo> emittedLocals = new Int2ReferenceOpenHashMap<>();
private Int2ReferenceMap<DebugLocalInfo> pendingLocals = null;
@@ -413,6 +415,7 @@
if (method.getDefinition().getCode().isCfCode()) {
diagnosticPosition = method.getDefinition().getCode().asCfCode().getDiagnosticPosition();
}
+ materializePreamblePosition();
return new CfCode(
method.getHolderType(),
stackHeightTracker.maxHeight,
@@ -424,6 +427,25 @@
bytecodeMetadataBuilder.build());
}
+ private void materializePreamblePosition() {
+ if (!currentPosition.isNone()
+ || preamblePosition == null
+ || !preamblePosition.hasCallerPosition()) {
+ return;
+ }
+ CfLabel existingLabel = ListUtils.first(instructions).asLabel();
+ int instructionIncrement = existingLabel != null ? 1 : 2;
+ List<CfInstruction> newInstructions =
+ new ArrayList<>(instructions.size() + instructionIncrement);
+ CfLabel label = existingLabel != null ? existingLabel : new CfLabel();
+ newInstructions.add(label);
+ newInstructions.add(new CfPosition(label, preamblePosition));
+ for (int i = existingLabel == null ? 0 : 1; i < instructions.size(); i++) {
+ newInstructions.add(instructions.get(i));
+ }
+ instructions = newInstructions;
+ }
+
private static boolean isNopInstruction(Instruction instruction, BasicBlock nextBlock) {
// From DexBuilder
return instruction.isArgument()
@@ -559,6 +581,9 @@
@SuppressWarnings("ReferenceEquality")
private void updatePositionAndLocals(Instruction instruction) {
Position position = instruction.getPosition();
+ if (preamblePosition == null) {
+ preamblePosition = position;
+ }
boolean didLocalsChange = localsChanged();
boolean didPositionChange =
position.isSome()
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 ecf769b..19bb6dc 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
@@ -6,6 +6,8 @@
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.Code;
+import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.bytecodemetadata.BytecodeMetadataProvider;
import com.android.tools.r8.graph.lens.GraphLens;
@@ -84,6 +86,7 @@
assert appView.testing().canUseLir(appView);
assert appView.testing().isSupportedLirPhase();
assert !appView.getSyntheticItems().hasPendingSyntheticClasses();
+ assert verifyLirOnly(appView);
GraphLens graphLens = appView.graphLens();
assert graphLens.isNonIdentityLens();
@@ -151,6 +154,7 @@
assert appView.testing().canUseLir(appView);
assert appView.testing().isSupportedLirPhase();
assert !appView.getSyntheticItems().hasPendingSyntheticClasses();
+ assert verifyLirOnly(appView);
appView.testing().exitLirSupportedPhase();
LensCodeRewriterUtils rewriterUtils = new LensCodeRewriterUtils(appView, true);
DeadCodeRemover deadCodeRemover = new DeadCodeRemover(appView);
@@ -265,4 +269,16 @@
IRFinalizer<?> finalizer = conversionOptions.getFinalizer(deadCodeRemover, appView);
method.setCode(finalizer.finalizeCode(irCode, noMetadata, onThreadTiming), appView);
}
+
+ public static boolean verifyLirOnly(AppView<? extends AppInfoWithClassHierarchy> appView) {
+ for (DexProgramClass clazz : appView.appInfo().classes()) {
+ for (DexEncodedMethod method : clazz.methods(DexEncodedMethod::hasCode)) {
+ assert method.getCode().isLirCode()
+ || method.getCode().isSharedCodeObject()
+ || appView.isCfByteCodePassThrough(method)
+ || appView.options().skipIR;
+ }
+ }
+ return true;
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/DefaultMethodOptimizationInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/DefaultMethodOptimizationInfo.java
index ea5f65d..36c1bce 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/DefaultMethodOptimizationInfo.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/DefaultMethodOptimizationInfo.java
@@ -122,7 +122,6 @@
@Override
public int getReturnedArgument() {
- assert returnsArgument();
return UNKNOWN_RETURNED_ARGUMENT;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MutableFieldOptimizationInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MutableFieldOptimizationInfo.java
index 9617c85..6c7a777 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/MutableFieldOptimizationInfo.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MutableFieldOptimizationInfo.java
@@ -84,6 +84,10 @@
abstractValue.rewrittenWithLens(appView, field.getType(), lens, codeLens), field);
}
+ public void unsetAbstractValue() {
+ abstractValue = AbstractValue.unknown();
+ }
+
@Override
public int getReadBits() {
return readBits;
@@ -111,6 +115,10 @@
this.dynamicType = dynamicType;
}
+ public void unsetDynamicType() {
+ setDynamicType(DynamicType.unknown());
+ }
+
@Override
public boolean isDead() {
return (flags & FLAGS_IS_DEAD) != 0;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MutableMethodOptimizationInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MutableMethodOptimizationInfo.java
index 33f1160..300c8c0 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/MutableMethodOptimizationInfo.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MutableMethodOptimizationInfo.java
@@ -33,7 +33,6 @@
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.OptionalBool;
import java.util.BitSet;
-import java.util.Collections;
import java.util.Set;
public class MutableMethodOptimizationInfo extends MethodOptimizationInfo
@@ -297,6 +296,10 @@
return this;
}
+ public void unsetArgumentInfos() {
+ argumentInfos = CallSiteOptimizationInfo.top();
+ }
+
@Override
public ClassInlinerMethodConstraint getClassInlinerMethodConstraint() {
return classInlinerConstraint;
@@ -630,11 +633,16 @@
}
void markInitializesClassesOnNormalExit(Set<DexType> initializedClassesOnNormalExit) {
- this.initializedClassesOnNormalExit = initializedClassesOnNormalExit;
+ if (initializedClassesOnNormalExit.isEmpty()) {
+ unsetInitializedClassesOnNormalExit();
+ } else {
+ this.initializedClassesOnNormalExit = initializedClassesOnNormalExit;
+ }
}
void unsetInitializedClassesOnNormalExit() {
- initializedClassesOnNormalExit = Collections.emptySet();
+ initializedClassesOnNormalExit =
+ DefaultMethodOptimizationInfo.getInstance().getInitializedClassesOnNormalExit();
}
void markReturnsArgument(int returnedArgumentIndex) {
@@ -767,6 +775,30 @@
return isFlagSet(RETURN_VALUE_HAS_BEEN_PROPAGATED_FLAG);
}
+ @SuppressWarnings("ReferenceEquality")
+ public boolean isEffectivelyDefault() {
+ DefaultMethodOptimizationInfo top = DefaultMethodOptimizationInfo.getInstance();
+ return argumentInfos == top.getArgumentInfos()
+ && initializedClassesOnNormalExit == top.getInitializedClassesOnNormalExit()
+ && returnedArgument == top.getReturnedArgument()
+ && abstractReturnValue == top.getAbstractReturnValue()
+ && classInlinerConstraint == top.getClassInlinerMethodConstraint()
+ && convertCheckNotNull == top.isConvertCheckNotNull()
+ && enumUnboxerMethodClassification == top.getEnumUnboxerMethodClassification()
+ && dynamicType == top.getDynamicType()
+ && inlining == InlinePreference.Default
+ && isReturnValueUsed == top.isReturnValueUsed()
+ && bridgeInfo == top.getBridgeInfo()
+ && instanceInitializerInfoCollection.isEmpty()
+ && nonNullParamOrThrow == top.getNonNullParamOrThrow()
+ && nonNullParamOnNormalExits == top.getNonNullParamOnNormalExits()
+ && simpleInliningConstraint == top.getSimpleInliningConstraint()
+ && maxRemovedAndroidLogLevel == top.getMaxRemovedAndroidLogLevel()
+ && parametersWithBitwiseOperations == top.getParametersWithBitwiseOperations()
+ && unusedArguments == top.getUnusedArguments()
+ && flags == DEFAULT_FLAGS;
+ }
+
@Override
public boolean isMutableOptimizationInfo() {
return true;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationInfoRemover.java b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationInfoRemover.java
new file mode 100644
index 0000000..a336a3d
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationInfoRemover.java
@@ -0,0 +1,66 @@
+// 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.optimize.info;
+
+import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexEncodedField;
+import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.utils.ThreadUtils;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+
+/**
+ * Clears any optimization info on fields and methods that need lens code rewriting. This avoids the
+ * need to lens code rewrite such optimization info in repackaging and other optimizations where the
+ * optimization info is mostly unused, since no more optimizations passes will be run.
+ */
+public class OptimizationInfoRemover {
+
+ public static void run(
+ AppView<? extends AppInfoWithClassHierarchy> appView, ExecutorService executorService)
+ throws ExecutionException {
+ ThreadUtils.processItems(
+ appView.appInfo().classes(),
+ OptimizationInfoRemover::processClass,
+ appView.options().getThreadingModule(),
+ executorService);
+ }
+
+ private static void processClass(DexProgramClass clazz) {
+ for (DexEncodedField field : clazz.fields()) {
+ processField(field);
+ }
+ for (DexEncodedMethod method : clazz.methods()) {
+ processMethod(method);
+ }
+ }
+
+ private static void processField(DexEncodedField field) {
+ MutableFieldOptimizationInfo optimizationInfo =
+ field.getOptimizationInfo().asMutableFieldOptimizationInfo();
+ if (optimizationInfo == null) {
+ return;
+ }
+ optimizationInfo.unsetAbstractValue();
+ optimizationInfo.unsetDynamicType();
+ }
+
+ private static void processMethod(DexEncodedMethod method) {
+ MutableMethodOptimizationInfo optimizationInfo =
+ method.getOptimizationInfo().asMutableMethodOptimizationInfo();
+ if (optimizationInfo == null) {
+ return;
+ }
+ optimizationInfo.unsetAbstractReturnValue();
+ optimizationInfo.unsetArgumentInfos();
+ optimizationInfo.unsetDynamicType();
+ optimizationInfo.unsetInitializedClassesOnNormalExit();
+ optimizationInfo.unsetInstanceInitializerInfoCollection();
+ if (optimizationInfo.isEffectivelyDefault()) {
+ method.unsetOptimizationInfo();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingMappingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingMappingTest.java
index f7c850a..c23da1d 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingMappingTest.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingMappingTest.java
@@ -18,22 +18,20 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
import org.junit.runners.Parameterized.Parameters;
@RunWith(Parameterized.class)
public class EnumUnboxingMappingTest extends EnumUnboxingTestBase {
- private final TestParameters parameters;
+ @Parameter(0)
+ public TestParameters parameters;
@Parameters(name = "{0}")
public static TestParametersCollection data() {
return getTestParameters().withDexRuntimes().withAllApiLevels().build();
}
- public EnumUnboxingMappingTest(TestParameters parameters) {
- this.parameters = parameters;
- }
-
@Test
public void testEnumUnboxing() throws Exception {
testForR8(parameters.getBackend())
@@ -64,14 +62,14 @@
assertEquals("int", debugInfoMethod.getFinalSignature().asMethodSignature().parameters[0]);
assertEquals("int", noDebugInfoMethod.getFinalSignature().asMethodSignature().parameters[0]);
- assertEquals(MyEnum.class.getName(), debugInfoMethod.getOriginalSignature().parameters[0]);
- // TODO(b/314076309): The original parameter should be MyEnum.class but is int.
- assertEquals("int", noDebugInfoMethod.getOriginalSignature().parameters[0]);
+ assertEquals(MyEnum.class.getTypeName(), debugInfoMethod.getOriginalSignature().parameters[0]);
+ assertEquals(
+ MyEnum.class.getTypeName(), noDebugInfoMethod.getOriginalSignature().parameters[0]);
ClassSubject indirection = codeInspector.clazz(Indirection.class);
MethodSubject abstractMethod = indirection.uniqueMethodWithOriginalName("intermediate");
assertTrue(abstractMethod.isAbstract());
- assertEquals(MyEnum.class.getName(), abstractMethod.getOriginalSignature().parameters[0]);
+ assertEquals(MyEnum.class.getTypeName(), abstractMethod.getOriginalSignature().parameters[0]);
}
@NeverClassInline
diff --git a/src/test/java/com/android/tools/r8/shaking/TreeShakingSpecificTest.java b/src/test/java/com/android/tools/r8/shaking/TreeShakingSpecificTest.java
index 2e03043..a2a1119 100644
--- a/src/test/java/com/android/tools/r8/shaking/TreeShakingSpecificTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/TreeShakingSpecificTest.java
@@ -95,7 +95,7 @@
"shaking1.Used -> a.a:",
"# {'id':'sourceFile','fileName':'Used.java'}",
" java.lang.String name -> a",
- " 1:14:void <init>(java.lang.String):0:13 -> <init>",
+ " 1:2:void <init>(java.lang.String):12:13 -> <init>",
" 1:1:java.lang.String method():17:17 -> a",
" 1:1:java.lang.String aMethodThatIsNotUsedButKept():21:21 "
+ "-> aMethodThatIsNotUsedButKept");