Store parameter names info in DexEncodedMethod
Bug: 132549918
Change-Id: I20d016defac237286865f515aa314d1c311ca739
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 0b8569c..fed2064 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -62,11 +62,14 @@
import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceArrayMap;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.BitSet;
import java.util.Collections;
import java.util.List;
+import java.util.Map;
import java.util.Set;
import java.util.function.Consumer;
import java.util.function.IntPredicate;
@@ -114,6 +117,8 @@
public static final DexEncodedMethod[] EMPTY_ARRAY = {};
public static final DexEncodedMethod SENTINEL =
new DexEncodedMethod(null, null, null, ParameterAnnotationsList.empty(), null);
+ public static final Int2ReferenceMap<DebugLocalInfo> NO_PARAMETER_INFO =
+ new Int2ReferenceArrayMap<>(0);
public final DexMethod method;
public final MethodAccessFlags accessFlags;
@@ -131,6 +136,8 @@
private OptionalBool isLibraryMethodOverride = OptionalBool.unknown();
+ private Int2ReferenceMap<DebugLocalInfo> parameterInfo = NO_PARAMETER_INFO;
+
// This flag indicates the current instance is no longer up-to-date as another instance was
// created based on this. Any further (public) operations on this instance will raise an error
// to catch potential bugs due to the inconsistency (e.g., http://b/111893131)
@@ -429,6 +436,19 @@
setCode(builder.build());
}
+ public void setParameterInfo(Int2ReferenceMap<DebugLocalInfo> parameterInfo) {
+ assert this.parameterInfo == NO_PARAMETER_INFO;
+ this.parameterInfo = parameterInfo;
+ }
+
+ public boolean hasParameterInfo() {
+ return parameterInfo != NO_PARAMETER_INFO;
+ }
+
+ public Map<Integer, DebugLocalInfo> getParameterInfo() {
+ return parameterInfo;
+ }
+
@Override
public String toString() {
checkIfObsolete();
diff --git a/src/main/java/com/android/tools/r8/graph/JarCode.java b/src/main/java/com/android/tools/r8/graph/JarCode.java
index 8e6314c..fa4e3d1 100644
--- a/src/main/java/com/android/tools/r8/graph/JarCode.java
+++ b/src/main/java/com/android/tools/r8/graph/JarCode.java
@@ -24,11 +24,11 @@
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.OffOrAuto;
import it.unimi.dsi.fastutil.ints.Int2ReferenceArrayMap;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.BitSet;
import java.util.Iterator;
-import java.util.Map;
import java.util.function.BiFunction;
import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
@@ -154,14 +154,14 @@
triggerDelayedParsingIfNeccessary();
if (!keepLocals(encodedMethod, appView.options())) {
// If the locals are not kept, we might still need a bit of locals information to satisfy
- // -keepparameternames for R8.
- Map<Integer, DebugLocalInfo> parameterInfo = collectParameterInfo(encodedMethod, appView);
- // We strip locals here because we will not be able to recover from invalid info.
- if (canStripLocals(encodedMethod, appView)) {
- node.localVariables.clear();
+ // -keepparameternames for R8. As locals are stripped after collection the parameter names
+ // this information can only be retrieved the first time IR is build for a method, so stick
+ // to the information if already present.
+ if (!encodedMethod.hasParameterInfo()) {
+ encodedMethod.setParameterInfo(collectParameterInfo(encodedMethod, appView));
}
- return internalBuild(
- context, encodedMethod, appView, generator, callerPosition, parameterInfo);
+ node.localVariables.clear();
+ return internalBuild(context, encodedMethod, appView, generator, callerPosition);
} else {
return internalBuildWithLocals(context, encodedMethod, appView, generator, callerPosition);
}
@@ -174,13 +174,11 @@
ValueNumberGenerator generator,
Position callerPosition) {
try {
- return internalBuild(
- context, encodedMethod, appView, generator, callerPosition, IRCode.NO_PARAMETER_INFO);
+ return internalBuild(context, encodedMethod, appView, generator, callerPosition);
} catch (InvalidDebugInfoException e) {
appView.options().warningInvalidDebugInfo(encodedMethod, origin, e);
node.localVariables.clear();
- return internalBuild(
- context, encodedMethod, appView, generator, callerPosition, IRCode.NO_PARAMETER_INFO);
+ return internalBuild(context, encodedMethod, appView, generator, callerPosition);
}
}
@@ -194,17 +192,17 @@
return false;
}
- private Map<Integer, DebugLocalInfo> collectParameterInfo(
+ private Int2ReferenceMap<DebugLocalInfo> collectParameterInfo(
DexEncodedMethod encodedMethod, AppView<?> appView) {
if (!appView.options().hasProguardConfiguration()
|| !appView.options().getProguardConfiguration().isKeepParameterNames()) {
- return IRCode.NO_PARAMETER_INFO;
+ return DexEncodedMethod.NO_PARAMETER_INFO;
}
// The enqueuer might build IR to trace reflective behaviour. At that point liveness is not
// known, so be conservative with collection parameter name information.
if (appView.appInfo().hasLiveness()
&& !appView.appInfo().withLiveness().isPinned(encodedMethod.method)) {
- return IRCode.NO_PARAMETER_INFO;
+ return DexEncodedMethod.NO_PARAMETER_INFO;
}
// Collect the local slots used for parameters.
BitSet localSlotsForParameters = new BitSet(0);
@@ -220,7 +218,7 @@
// assuming that that does actually describe the parameter (name, type and possibly
// signature).
DexItemFactory factory = appView.options().itemFactory;
- Map<Integer, DebugLocalInfo> parameterInfo =
+ Int2ReferenceMap<DebugLocalInfo> parameterInfo =
new Int2ReferenceArrayMap<>(localSlotsForParameters.cardinality());
for (Object o : node.localVariables) {
LocalVariableNode node = (LocalVariableNode) o;
@@ -238,31 +236,13 @@
return parameterInfo;
}
- private boolean canStripLocals(DexEncodedMethod encodedMethod, AppView<?> appView) {
- // If not keeping parameter names the locals can always be stripped.
- if (!appView.options().hasProguardConfiguration()
- || !appView.options().getProguardConfiguration().isKeepParameterNames()) {
- return true;
- }
- // The enqueuer might build IR to trace reflective behaviour. At that point liveness is not
- // known, so locals cannot be stripped as IR will built again in the IR converter.
- if (appView.appInfo().hasLiveness()
- && !appView.appInfo().withLiveness().isPinned(encodedMethod.method)) {
- return true;
- }
- return false;
- }
-
private IRCode internalBuild(
DexEncodedMethod context,
DexEncodedMethod encodedMethod,
AppView<?> appView,
ValueNumberGenerator generator,
- Position callerPosition,
- Map<Integer, DebugLocalInfo> parameterInfo) {
- assert node.localVariables.isEmpty()
- || keepLocals(encodedMethod, appView.options())
- || !canStripLocals(encodedMethod, appView);
+ Position callerPosition) {
+ assert node.localVariables.isEmpty() || keepLocals(encodedMethod, appView.options());
JarSourceCode source =
new JarSourceCode(
method.holder,
@@ -271,7 +251,7 @@
appView.graphLense().getOriginalMethodSignature(encodedMethod.method),
callerPosition);
IRBuilder builder = new IRBuilder(encodedMethod, appView, source, origin, generator);
- return builder.build(context, parameterInfo);
+ return builder.build(context);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCode.java b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
index a48f44a..6b84e42 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCode.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
@@ -21,7 +21,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
-import it.unimi.dsi.fastutil.ints.Int2ReferenceArrayMap;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Arrays;
@@ -90,8 +89,6 @@
// When numbering instructions we number instructions only with even numbers. This allows us to
// use odd instruction numbers for the insertion of moves during spilling.
public static final int INSTRUCTION_NUMBER_DELTA = 2;
- public static final Map<Integer, DebugLocalInfo> NO_PARAMETER_INFO =
- new Int2ReferenceArrayMap<>(0);
public final DexEncodedMethod method;
@@ -116,8 +113,6 @@
public final Origin origin;
- public final Map<Integer, DebugLocalInfo> parameterInfo;
-
public IRCode(
InternalOptions options,
DexEncodedMethod method,
@@ -126,8 +121,7 @@
boolean hasDebugPositions,
boolean hasMonitorInstruction,
boolean hasConstString,
- Origin origin,
- Map<Integer, DebugLocalInfo> parameterInfo) {
+ Origin origin) {
assert options != null;
this.options = options;
this.method = method;
@@ -137,10 +131,8 @@
this.hasMonitorInstruction = hasMonitorInstruction;
this.hasConstString = hasConstString;
this.origin = origin;
- this.parameterInfo = parameterInfo;
// TODO(zerny): Remove or update this property now that all instructions have positions.
allThrowingInstructionsHavePositions = computeAllThrowingInstructionsHavePositions();
- assert parameterInfo != null;
}
public void copyMetadataFromInlinee(IRCode inlinee) {
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 ffe8806..657032a 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
@@ -311,7 +311,7 @@
assert !block.exit().isReturn() || stackHeightTracker.isEmpty();
if (firstBlock) {
- addParameterNamesIfRequired(block, code.parameterInfo);
+ addParameterNamesIfRequired(block);
firstBlock = false;
}
@@ -631,15 +631,19 @@
}
}
- private void addParameterNamesIfRequired(
- BasicBlock block, Map<Integer, DebugLocalInfo> parameterInfo) {
+ private void addParameterNamesIfRequired(BasicBlock block) {
// Don't add this information if the code already have full debug information.
if (appView.options().debug) {
return;
}
- if (code.parameterInfo != IRCode.NO_PARAMETER_INFO) {
- for (Map.Entry<Integer, DebugLocalInfo> entries : parameterInfo.entrySet()) {
+ if (appView.appInfo().hasLiveness()
+ && !appView.appInfo().withLiveness().isPinned(method.method)) {
+ return;
+ }
+
+ if (method.hasParameterInfo()) {
+ for (Map.Entry<Integer, DebugLocalInfo> entries : method.getParameterInfo().entrySet()) {
LocalVariableInfo localVariableInfo =
new LocalVariableInfo(entries.getKey(), entries.getValue(), getLabel(block));
CfLabel endLabel = ensureLabel();
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 eae998d..e85b1cc 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
@@ -495,19 +495,6 @@
* @return The list of basic blocks. First block is the main entry.
*/
public IRCode build(DexEncodedMethod context) {
- return build(context, IRCode.NO_PARAMETER_INFO);
- }
-
- /**
- * Build the high-level IR in SSA form.
- *
- * @param context Under what context this IRCode is built. Either the current method or caller.
- * @param parameterInfo Parameter information to include in the output. Pass <code>
- * IRCode.NO_PARAMETER_INFO</code> if this is not relevant. This information is only used if
- * the generated code does not contain any debug information.
- * @return The list of basic blocks. First block is the main entry.
- */
- public IRCode build(DexEncodedMethod context, Map<Integer, DebugLocalInfo> parameterInfo) {
assert source != null;
source.setUp();
@@ -608,8 +595,7 @@
hasDebugPositions,
hasMonitorInstruction,
hasConstString,
- origin,
- parameterInfo);
+ origin);
// Verify critical edges are split so we have a place to insert phi moves if necessary.
assert ir.verifySplitCriticalEdges();
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/ConstantRemovalTest.java b/src/test/java/com/android/tools/r8/ir/optimize/ConstantRemovalTest.java
index 3284be0..d32606d 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/ConstantRemovalTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/ConstantRemovalTest.java
@@ -141,8 +141,7 @@
false,
false,
false,
- Origin.unknown(),
- IRCode.NO_PARAMETER_INFO);
+ Origin.unknown());
PeepholeOptimizer.optimize(code, new MockLinearScanRegisterAllocator(appView, code));
// Check that all four constant number instructions remain.
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/TrivialGotoEliminationTest.java b/src/test/java/com/android/tools/r8/ir/optimize/TrivialGotoEliminationTest.java
index 40ac512..c3e6823 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/TrivialGotoEliminationTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/TrivialGotoEliminationTest.java
@@ -81,8 +81,7 @@
false,
false,
false,
- Origin.unknown(),
- IRCode.NO_PARAMETER_INFO);
+ Origin.unknown());
CodeRewriter.collapseTrivialGotos(null, code);
assertTrue(code.entryBlock().isTrivialGoto());
assertTrue(blocks.contains(block0));
@@ -169,8 +168,7 @@
false,
false,
false,
- Origin.unknown(),
- IRCode.NO_PARAMETER_INFO);
+ Origin.unknown());
CodeRewriter.collapseTrivialGotos(null, code);
assertTrue(block0.getInstructions().get(1).isIf());
assertEquals(block1, block0.getInstructions().get(1).asIf().fallthroughBlock());
