No longer force IR for code that only requires instruction desugaring
Fixed missing rewritings and stack height adjustment which where
hidden by running the code through IR.
Bug: 147485959
Change-Id: I26c288b3ebab1a0a7b497bca15b3604df0ea222b
diff --git a/src/main/java/com/android/tools/r8/PrintUses.java b/src/main/java/com/android/tools/r8/PrintUses.java
index 926577f..dc284ed 100644
--- a/src/main/java/com/android/tools/r8/PrintUses.java
+++ b/src/main/java/com/android/tools/r8/PrintUses.java
@@ -233,7 +233,8 @@
if (typeMethods != null) {
DexClass holder = appInfo.definitionForHolder(method);
DexEncodedMethod definition = method.lookupOnClass(holder);
- assert definition != null : "Could not find method " + method.toString();
+ assert definition != null
+ : "Could not find method " + method.toString() + " in " + context.getTypeName();
if (!allowObfuscation) {
noObfuscationTypes.add(method.holder);
}
diff --git a/src/main/java/com/android/tools/r8/cf/code/CfFrame.java b/src/main/java/com/android/tools/r8/cf/code/CfFrame.java
index 98bc101..e55bf9a 100644
--- a/src/main/java/com/android/tools/r8/cf/code/CfFrame.java
+++ b/src/main/java/com/android/tools/r8/cf/code/CfFrame.java
@@ -30,6 +30,7 @@
import java.util.ArrayDeque;
import java.util.Deque;
import java.util.Objects;
+import java.util.SortedMap;
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;
@@ -342,10 +343,26 @@
this.stack = stack;
}
+ // This is used from tests. As fastutils are repackaged and minified the method above is
+ // not available from tests which use fastutils in their original namespace.
+ public CfFrame(SortedMap<Integer, FrameType> locals, Deque<FrameType> stack) {
+ this(
+ locals instanceof Int2ReferenceAVLTreeMap
+ ? (Int2ReferenceAVLTreeMap<FrameType>) locals
+ : new Int2ReferenceAVLTreeMap<>(locals),
+ stack);
+ }
+
public Int2ReferenceSortedMap<FrameType> getLocals() {
return locals;
}
+ // This is used from tests. As fastutils are repackaged and minified the method above is
+ // not available from tests which use fastutils in their original namespace.
+ public SortedMap<Integer, FrameType> getLocalsAsSortedMap() {
+ return locals;
+ }
+
public Deque<FrameType> getStack() {
return stack;
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index acd8b41..8abedbe 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -2299,7 +2299,7 @@
private DexType createStaticallyKnownType(Class<?> clazz) {
return createStaticallyKnownType(
- createString(DescriptorUtils.javaTypeToDescriptor(clazz.getName())));
+ createString(DescriptorUtils.javaTypeToDescriptor(clazz.getTypeName())));
}
private DexType createStaticallyKnownType(DexString descriptor) {
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 a241a05..2cbebcd 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
@@ -88,6 +88,7 @@
import com.android.tools.r8.ir.optimize.string.StringOptimizer;
import com.android.tools.r8.ir.regalloc.LinearScanRegisterAllocator;
import com.android.tools.r8.ir.regalloc.RegisterAllocator;
+import com.android.tools.r8.ir.synthetic.SynthesizedCode;
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.naming.IdentifierNameStringMarker;
import com.android.tools.r8.position.MethodPosition;
@@ -540,9 +541,6 @@
return;
}
checkPrefixMerging(method);
- if (!needsIRConversion(definition.getCode(), method)) {
- return;
- }
if (options.isGeneratingClassFiles()
|| !(options.passthroughDexCode && definition.getCode().isDexCode())) {
// We do not process in call graph order, so anything could be a leaf.
@@ -560,7 +558,10 @@
}
}
- private boolean needsIRConversion(Code code, ProgramMethod method) {
+ private boolean needsIRConversion(ProgramMethod method) {
+ if (appView.enableWholeProgramOptimizations()) {
+ return true;
+ }
if (options.testing.forceIRForCfToCfDesugar) {
return true;
}
@@ -570,23 +571,23 @@
if (!options.cfToCfDesugar) {
return true;
}
- if (instructionDesugaring.needsDesugaring(method)) {
- return true;
- }
if (desugaredLibraryAPIConverter != null
&& desugaredLibraryAPIConverter.shouldRegisterCallback(method)) {
return true;
}
-
- NeedsIRDesugarUseRegistry useRegistry =
- new NeedsIRDesugarUseRegistry(
- appView,
- desugaredLibraryRetargeter,
- interfaceMethodRewriter,
- desugaredLibraryAPIConverter);
- method.registerCodeReferences(useRegistry);
-
- return useRegistry.needsDesugaring();
+ if (method.getDefinition().getCode() instanceof SynthesizedCode) {
+ // SynthesizedCode needs IR to generate the code.
+ return true;
+ } else {
+ NeedsIRDesugarUseRegistry useRegistry =
+ new NeedsIRDesugarUseRegistry(
+ appView,
+ desugaredLibraryRetargeter,
+ interfaceMethodRewriter,
+ desugaredLibraryAPIConverter);
+ method.registerCodeReferences(useRegistry);
+ return useRegistry.needsDesugaring();
+ }
}
private void checkPrefixMerging(ProgramMethod method) {
@@ -1119,10 +1120,12 @@
if (options.testing.hookInIrConversion != null) {
options.testing.hookInIrConversion.run();
}
- if (options.skipIR) {
+
+ if (!needsIRConversion(method) || options.skipIR) {
feedback.markProcessed(method.getDefinition(), ConstraintWithTarget.NEVER);
return Timing.empty();
}
+
IRCode code = method.buildIR(appView);
if (code == null) {
feedback.markProcessed(method.getDefinition(), ConstraintWithTarget.NEVER);
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/RecordCfMethods.java b/src/main/java/com/android/tools/r8/ir/desugar/RecordCfMethods.java
index 85a7982..47bc98b 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/RecordCfMethods.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/RecordCfMethods.java
@@ -44,8 +44,6 @@
public final class RecordCfMethods {
public static void registerSynthesizedCodeReferences(DexItemFactory factory) {
- factory.createSynthesizedType(
- "Lcom/android/tools/r8/desugar/records/RecordMethods$RecordStub;");
factory.createSynthesizedType("Ljava/lang/Record;");
factory.createSynthesizedType("Ljava/util/Arrays;");
factory.createSynthesizedType("[Ljava/lang/Object;");
@@ -121,9 +119,7 @@
new Int2ReferenceAVLTreeMap<>(
new int[] {0, 1},
new FrameType[] {
- FrameType.initialized(
- options.itemFactory.createType(
- "Lcom/android/tools/r8/desugar/records/RecordMethods$RecordStub;")),
+ FrameType.initialized(options.itemFactory.createType("Ljava/lang/Record;")),
FrameType.initialized(options.itemFactory.objectType)
}),
new ArrayDeque<>(Arrays.asList())),
@@ -133,9 +129,7 @@
new Int2ReferenceAVLTreeMap<>(
new int[] {0, 1},
new FrameType[] {
- FrameType.initialized(
- options.itemFactory.createType(
- "Lcom/android/tools/r8/desugar/records/RecordMethods$RecordStub;")),
+ FrameType.initialized(options.itemFactory.createType("Ljava/lang/Record;")),
FrameType.initialized(options.itemFactory.objectType)
}),
new ArrayDeque<>(
@@ -240,9 +234,7 @@
new Int2ReferenceAVLTreeMap<>(
new int[] {0, 1, 2},
new FrameType[] {
- FrameType.initialized(
- options.itemFactory.createType(
- "Lcom/android/tools/r8/desugar/records/RecordMethods$RecordStub;")),
+ FrameType.initialized(options.itemFactory.createType("Ljava/lang/Record;")),
FrameType.initialized(options.itemFactory.stringType),
FrameType.initialized(options.itemFactory.stringType)
}),
@@ -263,9 +255,7 @@
new Int2ReferenceAVLTreeMap<>(
new int[] {0, 1, 2},
new FrameType[] {
- FrameType.initialized(
- options.itemFactory.createType(
- "Lcom/android/tools/r8/desugar/records/RecordMethods$RecordStub;")),
+ FrameType.initialized(options.itemFactory.createType("Ljava/lang/Record;")),
FrameType.initialized(options.itemFactory.stringType),
FrameType.initialized(options.itemFactory.stringType)
}),
@@ -325,9 +315,7 @@
new Int2ReferenceAVLTreeMap<>(
new int[] {0, 1, 2, 3, 4, 5, 6},
new FrameType[] {
- FrameType.initialized(
- options.itemFactory.createType(
- "Lcom/android/tools/r8/desugar/records/RecordMethods$RecordStub;")),
+ FrameType.initialized(options.itemFactory.createType("Ljava/lang/Record;")),
FrameType.initialized(options.itemFactory.stringType),
FrameType.initialized(options.itemFactory.stringType),
FrameType.initialized(options.itemFactory.createType("[Ljava/lang/String;")),
@@ -398,9 +386,7 @@
new Int2ReferenceAVLTreeMap<>(
new int[] {0, 1, 2, 3, 4, 5, 6},
new FrameType[] {
- FrameType.initialized(
- options.itemFactory.createType(
- "Lcom/android/tools/r8/desugar/records/RecordMethods$RecordStub;")),
+ FrameType.initialized(options.itemFactory.createType("Ljava/lang/Record;")),
FrameType.initialized(options.itemFactory.stringType),
FrameType.initialized(options.itemFactory.stringType),
FrameType.initialized(options.itemFactory.createType("[Ljava/lang/String;")),
@@ -416,9 +402,7 @@
new Int2ReferenceAVLTreeMap<>(
new int[] {0, 1, 2, 3, 4, 5},
new FrameType[] {
- FrameType.initialized(
- options.itemFactory.createType(
- "Lcom/android/tools/r8/desugar/records/RecordMethods$RecordStub;")),
+ FrameType.initialized(options.itemFactory.createType("Ljava/lang/Record;")),
FrameType.initialized(options.itemFactory.stringType),
FrameType.initialized(options.itemFactory.stringType),
FrameType.initialized(options.itemFactory.createType("[Ljava/lang/String;")),
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/RecordRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/RecordRewriter.java
index b9a8880..1b5e6aa 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/RecordRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/RecordRewriter.java
@@ -130,7 +130,11 @@
assert !instruction.isInitClass();
if (instruction.isInvokeDynamic() && needsDesugaring(instruction.asInvokeDynamic(), context)) {
return desugarInvokeDynamicOnRecord(
- instruction.asInvokeDynamic(), context, eventConsumer, methodProcessingContext);
+ instruction.asInvokeDynamic(),
+ localStackAllocator,
+ context,
+ eventConsumer,
+ methodProcessingContext);
}
if (instruction.isInvoke()) {
CfInvoke cfInvoke = instruction.asInvoke();
@@ -146,6 +150,7 @@
public List<CfInstruction> desugarInvokeDynamicOnRecord(
CfInvokeDynamic invokeDynamic,
+ LocalStackAllocator localStackAllocator,
ProgramMethod context,
CfInstructionDesugaringEventConsumer eventConsumer,
MethodProcessingContext methodProcessingContext) {
@@ -166,7 +171,13 @@
ClassNameComputationInfo.ClassNameMapping.SIMPLE_NAME.map(
recordValueType.getValue().toDescriptorString(), context.getHolder(), factory);
return desugarInvokeRecordToString(
- recordClass, fieldNames, fields, simpleName, eventConsumer, methodProcessingContext);
+ recordClass,
+ fieldNames,
+ fields,
+ simpleName,
+ localStackAllocator,
+ eventConsumer,
+ methodProcessingContext);
}
if (callSite.methodName == factory.hashCodeMethodName) {
return desugarInvokeRecordHashCode(
@@ -273,12 +284,14 @@
DexString fieldNames,
DexField[] fields,
DexString simpleName,
+ LocalStackAllocator localStackAllocator,
CfInstructionDesugaringEventConsumer eventConsumer,
MethodProcessingContext methodProcessingContext) {
ensureGetFieldsAsObjects(recordClass, fields, eventConsumer);
ArrayList<CfInstruction> instructions = new ArrayList<>();
instructions.add(new CfConstString(simpleName));
instructions.add(new CfConstString(fieldNames));
+ localStackAllocator.allocateLocalStack(2);
ProgramMethod programMethod =
synthesizeRecordHelper(
recordToStringHelperProto,
diff --git a/src/test/java/com/android/tools/r8/desugar/records/GenerateRecordMethods.java b/src/test/java/com/android/tools/r8/desugar/records/GenerateRecordMethods.java
index 53d6201..e3078a2 100644
--- a/src/test/java/com/android/tools/r8/desugar/records/GenerateRecordMethods.java
+++ b/src/test/java/com/android/tools/r8/desugar/records/GenerateRecordMethods.java
@@ -8,22 +8,30 @@
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.TestRuntime.CfVm;
+import com.android.tools.r8.cf.code.CfFrame;
+import com.android.tools.r8.cf.code.CfFrame.FrameType;
import com.android.tools.r8.cf.code.CfInstruction;
import com.android.tools.r8.cf.code.CfInvoke;
import com.android.tools.r8.cf.code.CfTypeInstruction;
import com.android.tools.r8.cfmethodgeneration.MethodGenerationBase;
+import com.android.tools.r8.desugar.records.RecordMethods.RecordStub;
import com.android.tools.r8.graph.CfCode;
-import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.desugar.RecordRewriter;
+import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.FileUtils;
import com.google.common.collect.ImmutableList;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceAVLTreeMap;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceSortedMap;
import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Comparator;
+import java.util.Deque;
import java.util.List;
+import java.util.SortedMap;
import java.util.stream.Collectors;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -34,6 +42,8 @@
public class GenerateRecordMethods extends MethodGenerationBase {
private final DexType GENERATED_TYPE =
factory.createType("Lcom/android/tools/r8/ir/desugar/RecordCfMethods;");
+ private final DexType RECORD_STUB_TYPE =
+ factory.createType(DescriptorUtils.javaTypeToDescriptor(RecordStub.class.getTypeName()));
private final List<Class<?>> METHOD_TEMPLATE_CLASSES = ImmutableList.of(RecordMethods.class);
protected final TestParameters parameters;
@@ -64,32 +74,30 @@
@Override
protected CfCode getCode(String holderName, String methodName, CfCode code) {
- DexType recordStubType =
- factory.createType("Lcom/android/tools/r8/desugar/records/RecordMethods$RecordStub;");
code.setInstructions(
code.getInstructions().stream()
- .map(instruction -> rewriteRecordStub(factory, instruction, recordStubType))
+ .map(instruction -> rewriteRecordStub(instruction))
.collect(Collectors.toList()));
return code;
}
- private CfInstruction rewriteRecordStub(
- DexItemFactory factory, CfInstruction instruction, DexType recordStubType) {
+ private CfInstruction rewriteRecordStub(CfInstruction instruction) {
if (instruction.isTypeInstruction()) {
CfTypeInstruction typeInstruction = instruction.asTypeInstruction();
- return typeInstruction.withType(
- rewriteType(factory, recordStubType, typeInstruction.getType()));
+ return typeInstruction.withType(rewriteType(typeInstruction.getType()));
}
if (instruction.isInvoke()) {
CfInvoke cfInvoke = instruction.asInvoke();
DexMethod method = cfInvoke.getMethod();
DexMethod newMethod =
- factory.createMethod(
- rewriteType(factory, recordStubType, method.holder),
- method.proto,
- rewriteName(method.name));
+ factory.createMethod(rewriteType(method.holder), method.proto, rewriteName(method.name));
return new CfInvoke(cfInvoke.getOpcode(), newMethod, cfInvoke.isInterface());
}
+ if (instruction.isFrame()) {
+ CfFrame cfFrame = instruction.asFrame();
+ return new CfFrame(
+ rewriteLocals(cfFrame.getLocalsAsSortedMap()), rewriteStack(cfFrame.getStack()));
+ }
return instruction;
}
@@ -99,8 +107,40 @@
: name.toString();
}
- private DexType rewriteType(DexItemFactory factory, DexType recordStubType, DexType type) {
- return type == recordStubType ? factory.recordType : type;
+ private DexType rewriteType(DexType type) {
+ DexType baseType = type.isArrayType() ? type.toBaseType(factory) : type;
+ if (baseType != RECORD_STUB_TYPE) {
+ return type;
+ }
+ return type.isArrayType()
+ ? type.replaceBaseType(factory.recordType, factory)
+ : factory.recordType;
+ }
+
+ private FrameType rewriteFrameType(FrameType frameType) {
+ if (frameType.isInitialized() && frameType.getInitializedType().isReferenceType()) {
+ DexType newType = rewriteType(frameType.getInitializedType());
+ if (newType == frameType.getInitializedType()) {
+ return frameType;
+ }
+ return FrameType.initialized(newType);
+ } else {
+ assert !frameType.isUninitializedNew();
+ assert !frameType.isUninitializedThis();
+ return frameType;
+ }
+ }
+
+ private SortedMap<Integer, FrameType> rewriteLocals(SortedMap<Integer, FrameType> locals) {
+ Int2ReferenceSortedMap<FrameType> newLocals = new Int2ReferenceAVLTreeMap<>();
+ locals.forEach((index, local) -> newLocals.put((int) index, rewriteFrameType(local)));
+ return newLocals;
+ }
+
+ private Deque<FrameType> rewriteStack(Deque<FrameType> stack) {
+ ArrayDeque<FrameType> newStack = new ArrayDeque<>();
+ stack.forEach(frameType -> newStack.add(rewriteFrameType(frameType)));
+ return newStack;
}
@Test
