Convert into DexItemBasedConstString in CF->LIR conversion
Bug: b/344939647
Change-Id: Ia001bfcfa55d333392156f339566646d78fee216
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 7cabfb6..851df61 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -465,7 +465,7 @@
new CfOpenClosedInterfacesAnalysis(appViewWithLiveness).run(executorService);
// TODO(b/225838009): Move higher up.
- LirConverter.enterLirSupportedPhase(appView, executorService);
+ LirConverter.enterLirSupportedPhase(appViewWithLiveness, executorService);
assert verifyNoJarApplicationReaders(appView.appInfo().classes());
assert appView.checkForTesting(() -> allReferencesAssignedApiLevel(appViewWithLiveness));
diff --git a/src/main/java/com/android/tools/r8/ir/code/FieldPut.java b/src/main/java/com/android/tools/r8/ir/code/FieldPut.java
index 488e60a..91e6d2d 100644
--- a/src/main/java/com/android/tools/r8/ir/code/FieldPut.java
+++ b/src/main/java/com/android/tools/r8/ir/code/FieldPut.java
@@ -12,6 +12,8 @@
public interface FieldPut {
+ BasicBlock getBlock();
+
DexField getField();
Position getPosition();
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index ad479bd..96bfdc4 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -589,13 +589,6 @@
CheckNotNullConverter.runIfNecessary(appView, code);
previous = printMethod(code, "IR after disable assertions (SSA)", previous);
- if (identifierNameStringMarker != null) {
- timing.begin("Decouple identifier-name strings");
- identifierNameStringMarker.decoupleIdentifierNameStringsInMethod(code);
- timing.end();
- previous = printMethod(code, "IR after identifier-name strings (SSA)", previous);
- }
-
if (memberValuePropagation != null) {
timing.begin("Propagate member values");
memberValuePropagation.run(code);
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 dacdd2f..c067459 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
@@ -27,8 +27,10 @@
import com.android.tools.r8.lightir.IR2LirConverter;
import com.android.tools.r8.lightir.LirCode;
import com.android.tools.r8.lightir.LirStrategy;
+import com.android.tools.r8.naming.IdentifierNameStringMarker;
import com.android.tools.r8.naming.RecordInvokeDynamicInvokeCustomRewriter;
import com.android.tools.r8.optimize.MemberRebindingIdentityLens;
+import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.ObjectUtils;
import com.android.tools.r8.utils.ThreadUtils;
import com.android.tools.r8.utils.Timing;
@@ -39,14 +41,16 @@
public class LirConverter {
public static void enterLirSupportedPhase(
- AppView<? extends AppInfoWithClassHierarchy> appView, ExecutorService executorService)
+ AppView<AppInfoWithLiveness> appView, ExecutorService executorService)
throws ExecutionException {
assert appView.testing().canUseLir(appView);
assert appView.testing().isPreLirPhase();
appView.testing().enterLirSupportedPhase();
CodeRewriterPassCollection codeRewriterPassCollection =
new CodeRewriterPassCollection(
- new ConstResourceNumberRewriter(appView), new StringSwitchConverter(appView));
+ new ConstResourceNumberRewriter(appView),
+ new IdentifierNameStringMarker(appView),
+ new StringSwitchConverter(appView));
// Convert code objects to LIR.
ThreadUtils.processItems(
appView.appInfo().classes(),
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 87fc847..c56ddbf 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
@@ -29,8 +29,8 @@
}
@SuppressWarnings("unchecked")
- protected AppView<? extends T> appView() {
- return (AppView<? extends T>) appView;
+ protected AppView<T> appView() {
+ return (AppView<T>) appView;
}
public final CodeRewriterResult run(
diff --git a/src/main/java/com/android/tools/r8/naming/IdentifierNameStringMarker.java b/src/main/java/com/android/tools/r8/naming/IdentifierNameStringMarker.java
index 14a837a..cdec5d9 100644
--- a/src/main/java/com/android/tools/r8/naming/IdentifierNameStringMarker.java
+++ b/src/main/java/com/android/tools/r8/naming/IdentifierNameStringMarker.java
@@ -9,6 +9,7 @@
import static com.android.tools.r8.naming.IdentifierNameStringUtils.isClassNameComparison;
import static com.android.tools.r8.naming.IdentifierNameStringUtils.isReflectionMethod;
+import com.android.tools.r8.contexts.CompilationContext.MethodProcessingContext;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexField;
@@ -23,7 +24,7 @@
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.ConstString;
import com.android.tools.r8.ir.code.DexItemBasedConstString;
-import com.android.tools.r8.ir.code.FieldInstruction;
+import com.android.tools.r8.ir.code.FieldPut;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.InstancePut;
import com.android.tools.r8.ir.code.Instruction;
@@ -32,16 +33,19 @@
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.StaticPut;
import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.ir.conversion.MethodProcessor;
+import com.android.tools.r8.ir.conversion.passes.CodeRewriterPass;
+import com.android.tools.r8.ir.conversion.passes.result.CodeRewriterResult;
import com.android.tools.r8.naming.dexitembasedstring.ClassNameComputationInfo;
import com.android.tools.r8.naming.identifiernamestring.IdentifierNameStringLookupResult;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.position.TextPosition;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.utils.ArrayUtils;
import com.android.tools.r8.utils.StringDiagnostic;
import com.android.tools.r8.utils.ThreadUtils;
import com.google.common.collect.Streams;
import it.unimi.dsi.fastutil.objects.Object2BooleanMap;
-import java.util.Arrays;
import java.util.List;
import java.util.ListIterator;
import java.util.Objects;
@@ -50,16 +54,33 @@
import java.util.concurrent.ExecutorService;
import java.util.stream.Collectors;
-public class IdentifierNameStringMarker {
+public class IdentifierNameStringMarker extends CodeRewriterPass<AppInfoWithLiveness> {
- private final AppView<AppInfoWithLiveness> appView;
private final Object2BooleanMap<DexMember<?, ?>> identifierNameStrings;
public IdentifierNameStringMarker(AppView<AppInfoWithLiveness> appView) {
- this.appView = appView;
+ super(appView);
this.identifierNameStrings = appView.appInfo().identifierNameStrings;
}
+ @Override
+ protected CodeRewriterResult rewriteCode(
+ IRCode code,
+ MethodProcessor methodProcessor,
+ MethodProcessingContext methodProcessingContext) {
+ return decoupleIdentifierNameStringsInBlocks(code, null);
+ }
+
+ @Override
+ protected boolean shouldRewriteCode(IRCode code, MethodProcessor methodProcessor) {
+ return code.metadata().mayHaveConstString();
+ }
+
+ @Override
+ protected String getRewriterId() {
+ return "IdentifierNameStringMarker";
+ }
+
public void decoupleIdentifierNameStringsInFields(
ExecutorService executorService) throws ExecutionException {
ThreadUtils.processItems(
@@ -83,21 +104,18 @@
return;
}
DexString original = staticValue.getValue();
- DexReference itemBasedString = inferMemberOrTypeFromNameString(appView, original);
+ DexReference itemBasedString = inferMemberOrTypeFromNameString(appView(), original);
if (itemBasedString != null) {
encodedField.setStaticValue(
new DexItemBasedValueString(itemBasedString, ClassNameComputationInfo.none()));
}
}
- public void decoupleIdentifierNameStringsInMethod(IRCode code) {
- decoupleIdentifierNameStringsInBlocks(code, null);
- assert code.isConsistentSSA(appView);
- }
-
- public void decoupleIdentifierNameStringsInBlocks(IRCode code, Set<BasicBlock> blocks) {
+ public CodeRewriterResult decoupleIdentifierNameStringsInBlocks(
+ IRCode code, Set<BasicBlock> blocks) {
+ CodeRewriterResult result = CodeRewriterResult.NO_CHANGE;
if (!code.metadata().mayHaveConstString()) {
- return;
+ return result;
}
ListIterator<BasicBlock> blockIterator = code.listIterator();
while (blockIterator.hasNext()) {
@@ -119,56 +137,69 @@
// ...
// v_n' <- DexItemBasedString("Lx/y/z;") // decoupled
// this.fld <- v_n' // fieldPut
- if (instruction.isStaticPut() || instruction.isInstancePut()) {
- iterator =
- decoupleIdentifierNameStringForFieldPutInstruction(
- code, blockIterator, iterator, instruction.asFieldInstruction());
+ if (instruction.isFieldPut()) {
+ FieldPut fieldPut = instruction.asFieldPut();
+ DexReference itemBasedString = getItemBasedStringForFieldPut(code, fieldPut);
+ if (itemBasedString != null) {
+ iterator =
+ decoupleIdentifierNameStringForFieldPutInstruction(
+ code, blockIterator, iterator, fieldPut, itemBasedString);
+ result = CodeRewriterResult.HAS_CHANGED;
+ }
} else if (instruction.isInvokeMethod()) {
+ InvokeMethod invoke = instruction.asInvokeMethod();
iterator =
decoupleIdentifierNameStringForInvokeInstruction(
- code, blockIterator, iterator, instruction.asInvokeMethod());
+ code, blockIterator, iterator, invoke);
+ result = CodeRewriterResult.HAS_CHANGED;
}
}
}
+ return result;
+ }
+
+ private DexReference getItemBasedStringForFieldPut(IRCode code, FieldPut fieldPut) {
+ DexField field = fieldPut.getField();
+ if (!identifierNameStrings.containsKey(field)) {
+ return null;
+ }
+ Value in = fieldPut.value();
+ if (in.isDexItemBasedConstString()) {
+ return null;
+ }
+ if (!in.isConstString()) {
+ warnUndeterminedIdentifierIfNecessary(
+ field, code.context(), fieldPut.asFieldInstruction(), null);
+ return null;
+ }
+ DexString original = in.getConstInstruction().asConstString().getValue();
+ DexReference itemBasedString = inferMemberOrTypeFromNameString(appView(), original);
+ if (itemBasedString == null) {
+ warnUndeterminedIdentifierIfNecessary(
+ field, code.context(), fieldPut.asFieldInstruction(), original);
+ return null;
+ }
+ return itemBasedString;
}
private InstructionListIterator decoupleIdentifierNameStringForFieldPutInstruction(
IRCode code,
ListIterator<BasicBlock> blocks,
InstructionListIterator iterator,
- FieldInstruction instruction) {
- assert instruction.isInstancePut() || instruction.isStaticPut();
- FieldInstruction fieldPut = instruction.asFieldInstruction();
- DexField field = fieldPut.getField();
- if (!identifierNameStrings.containsKey(field)) {
- return iterator;
- }
- Value in = instruction.value();
- if (in.isDexItemBasedConstString()) {
- return iterator;
- }
- if (!in.isConstString()) {
- warnUndeterminedIdentifierIfNecessary(field, code.context(), instruction, null);
- return iterator;
- }
- DexString original = in.getConstInstruction().asConstString().getValue();
- DexReference itemBasedString = inferMemberOrTypeFromNameString(appView, original);
- if (itemBasedString == null) {
- warnUndeterminedIdentifierIfNecessary(field, code.context(), instruction, original);
- return iterator;
- }
+ FieldPut fieldPut,
+ DexReference itemBasedString) {
// Move the cursor back to $fieldPut
assert iterator.peekPrevious() == fieldPut;
iterator.previous();
// Prepare $decoupled just before $fieldPut
- Value newIn = code.createValue(in.getType(), in.getLocalInfo());
+ Value newIn = code.createValue(fieldPut.value().getType(), fieldPut.value().getLocalInfo());
DexItemBasedConstString decoupled =
new DexItemBasedConstString(newIn, itemBasedString, ClassNameComputationInfo.none());
decoupled.setPosition(fieldPut.getPosition());
// If the current block has catch handler, split into two blocks.
// Because const-string we're about to add is also a throwing instr, we need to split
// before adding it.
- BasicBlock block = instruction.getBlock();
+ BasicBlock block = fieldPut.getBlock();
BasicBlock blockWithFieldInstruction =
block.hasCatchHandlers() ? iterator.split(code, blocks) : block;
if (blockWithFieldInstruction != block) {
@@ -186,12 +217,13 @@
assert iterator.peekNext() == fieldPut;
iterator.next();
}
- if (instruction.isStaticPut()) {
- iterator.replaceCurrentInstruction(new StaticPut(newIn, field));
+ if (fieldPut.isStaticPut()) {
+ iterator.replaceCurrentInstruction(new StaticPut(newIn, fieldPut.getField()));
} else {
- assert instruction.isInstancePut();
- InstancePut instancePut = instruction.asInstancePut();
- iterator.replaceCurrentInstruction(new InstancePut(field, instancePut.object(), newIn));
+ assert fieldPut.isInstancePut();
+ InstancePut instancePut = fieldPut.asInstancePut();
+ iterator.replaceCurrentInstruction(
+ new InstancePut(fieldPut.getField(), instancePut.object(), newIn));
}
return iterator;
}
@@ -282,7 +314,7 @@
continue;
}
DexString original = in.getConstInstruction().asConstString().getValue();
- DexReference itemBasedString = inferMemberOrTypeFromNameString(appView, original);
+ DexReference itemBasedString = inferMemberOrTypeFromNameString(appView(), original);
if (itemBasedString == null) {
warnUndeterminedIdentifierIfNecessary(invokedMethod, code.context(), invoke, original);
continue;
@@ -320,16 +352,17 @@
}
}
}
- if (!Arrays.stream(changes).allMatch(Objects::isNull)) {
- List<Value> newIns =
- Streams.mapWithIndex(
- ins.stream(),
- (in, index) -> changes[(int) index] != null ? changes[(int) index] : in)
- .collect(Collectors.toList());
- iterator.replaceCurrentInstruction(
- Invoke.create(
- invoke.getType(), invokedMethod, invokedMethod.proto, invoke.outValue(), newIns));
+ if (ArrayUtils.none(changes, Objects::nonNull)) {
+ return iterator;
}
+ List<Value> newIns =
+ Streams.mapWithIndex(
+ ins.stream(),
+ (in, index) -> changes[(int) index] != null ? changes[(int) index] : in)
+ .collect(Collectors.toList());
+ iterator.replaceCurrentInstruction(
+ Invoke.create(
+ invoke.getType(), invokedMethod, invokedMethod.proto, invoke.outValue(), newIns));
return iterator;
}
@@ -372,8 +405,10 @@
}
Origin origin = method.getOrigin();
String kind = member.isDexField() ? "field" : "method";
- String originalMessage = original == null ? "what identifier string flows to "
- : "what '" + original.toString() + "' refers to, which flows to ";
+ String originalMessage =
+ original == null
+ ? "what identifier string flows to "
+ : "what '" + original + "' refers to, which flows to ";
String message =
"Cannot determine " + originalMessage + member.toSourceString()
+ " that is specified in -identifiernamestring rules."
diff --git a/src/test/java/com/android/tools/r8/naming/IdentifierNameStringVerticallyMergedClassTest.java b/src/test/java/com/android/tools/r8/naming/IdentifierNameStringVerticallyMergedClassTest.java
index 5e0cc75..022d375 100644
--- a/src/test/java/com/android/tools/r8/naming/IdentifierNameStringVerticallyMergedClassTest.java
+++ b/src/test/java/com/android/tools/r8/naming/IdentifierNameStringVerticallyMergedClassTest.java
@@ -39,8 +39,7 @@
.setMinApi(parameters)
.compile()
.run(parameters.getRuntime(), Main.class)
- // TODO(b/344939647): Should be "A".
- .assertSuccessWithOutputLines("null");
+ .assertSuccessWithOutputLines("A");
}
static class Main {
