Revert "Add support for outlining StringBuilders" This reverts commit bfb481e19d9f1c1ee919f01c42188aa04533b208. Reason for revert: Bot failures Change-Id: Ibca7bccf79eea5c52d664fa1856ab569fa7cf91f
diff --git a/src/main/java/com/android/tools/r8/ir/code/Instruction.java b/src/main/java/com/android/tools/r8/ir/code/Instruction.java index 2287bbc..003e17f 100644 --- a/src/main/java/com/android/tools/r8/ir/code/Instruction.java +++ b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
@@ -96,19 +96,10 @@ return prev; } - public boolean hasNext() { - return next != null; - } - public Instruction getNext() { return next; } - @SuppressWarnings("TypeParameterUnusedInFormals") - public <T extends Instruction> T nextUntilExclusive(Predicate<Instruction> predicate) { - return hasNext() ? getNext().nextUntilInclusive(predicate) : null; - } - @SuppressWarnings({"TypeParameterUnusedInFormals", "unchecked"}) public <T extends Instruction> T nextUntilInclusive(Predicate<Instruction> predicate) { Instruction current = this;
diff --git a/src/main/java/com/android/tools/r8/ir/code/ThrowBlockOutlineMarker.java b/src/main/java/com/android/tools/r8/ir/code/ThrowBlockOutlineMarker.java index 8f61c05..a7d82e0 100644 --- a/src/main/java/com/android/tools/r8/ir/code/ThrowBlockOutlineMarker.java +++ b/src/main/java/com/android/tools/r8/ir/code/ThrowBlockOutlineMarker.java
@@ -37,7 +37,7 @@ ListUtils.mapOrElse( inValues, (i, argument) -> { - if (outline.isArgumentConstant(i)) { + if (outline.getParentOrSelf().isArgumentConstant(i)) { argument.removeUser(this); return null; }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutline.java b/src/main/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutline.java index 353aba6..ae16771 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutline.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutline.java
@@ -295,14 +295,6 @@ return materializedOutlineMethod != null; } - public boolean isStringBuilderToStringOutline() { - return !isThrowOutline(); - } - - public boolean isThrowOutline() { - return proto.getReturnType().isVoidType(); - } - public void materialize(AppView<?> appView, MethodProcessingContext methodProcessingContext) { assert verifyNotMerged(); SyntheticItems syntheticItems = appView.getSyntheticItems();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutlineMarkerRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutlineMarkerRewriter.java index 04e8567..e9de503 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutlineMarkerRewriter.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutlineMarkerRewriter.java
@@ -5,7 +5,6 @@ import com.android.tools.r8.graph.AppView; import com.android.tools.r8.graph.Code; -import com.android.tools.r8.graph.DexItemFactory; import com.android.tools.r8.graph.DexType; import com.android.tools.r8.graph.ProgramMethod; import com.android.tools.r8.graph.bytecodemetadata.BytecodeMetadataProvider; @@ -19,6 +18,7 @@ import com.android.tools.r8.ir.code.Phi; import com.android.tools.r8.ir.code.Position; import com.android.tools.r8.ir.code.Return; +import com.android.tools.r8.ir.code.Throw; import com.android.tools.r8.ir.code.ThrowBlockOutlineMarker; import com.android.tools.r8.ir.code.UnusedArgument; import com.android.tools.r8.ir.code.Value; @@ -41,12 +41,10 @@ private final AppView<?> appView; private final DeadCodeRemover deadCodeRemover; - private final DexItemFactory factory; ThrowBlockOutlineMarkerRewriter(AppView<?> appView) { this.appView = appView; this.deadCodeRemover = new DeadCodeRemover(appView); - this.factory = appView.dexItemFactory(); } public void processOutlineMethod( @@ -111,92 +109,80 @@ private void processOutlineMarkers(IRCode code) { for (BasicBlock block : code.getBlocks()) { - ThrowBlockOutlineMarker outlineMarker = - block.entry().nextUntilInclusive(Instruction::isThrowBlockOutlineMarker); - while (outlineMarker != null) { - ThrowBlockOutline outline = outlineMarker.getOutline().getParentOrSelf(); - Instruction outlineEnd = getOutlineEnd(block, outline, outlineMarker); - outlineMarker.detachConstantOutlineArguments(outline); - if (outline.isMaterialized()) { - // Insert a call to the materialized outline method and load the return value. - BasicBlockInstructionListIterator instructionIterator = block.listIterator(outlineEnd); - InvokeStatic.Builder invokeBuilder = - InvokeStatic.builder() - .setArguments(outlineMarker.inValues()) - .setIsInterface(false) - .setMethod(outline.getMaterializedOutlineMethod()) - .setPosition(outlineEnd); - if (outline.isStringBuilderToStringOutline()) { - invokeBuilder.setFreshOutValue( - code, factory.stringType.toTypeElement(appView), outlineEnd.getLocalInfo()); - } - InvokeStatic invoke = invokeBuilder.build(); - if (outline.isStringBuilderToStringOutline()) { - outlineEnd.replace(invoke); - outlineEnd = invoke; - } else { - assert outline.isThrowOutline(); + Throw throwInstruction = block.exit().asThrow(); + if (throwInstruction != null) { + ThrowBlockOutlineMarker outlineMarker = + block.entry().nextUntilInclusive(Instruction::isThrowBlockOutlineMarker); + if (outlineMarker != null) { + ThrowBlockOutline outline = outlineMarker.getOutline(); + outlineMarker.detachConstantOutlineArguments(outline); + if (outline.isMaterialized()) { + // Insert a call to the materialized outline method and load the return value. + BasicBlockInstructionListIterator instructionIterator = + block.listIterator(block.exit()); + InvokeStatic invoke = + InvokeStatic.builder() + .setArguments(outlineMarker.inValues()) + .setIsInterface(false) + .setMethod(outline.getMaterializedOutlineMethod()) + .setPosition(throwInstruction) + .build(); instructionIterator.add(invoke); - Value returnOrThrowValue = addReturnOrThrowValue(code, instructionIterator); + Value returnValue = addReturnOrThrowValue(code, instructionIterator); // Replace the throw instruction by a normal return, but throw null in initializers. if (code.context().getDefinition().isInstanceInitializer()) { - outlineEnd.replaceValue(0, returnOrThrowValue); + throwInstruction.replaceValue(0, returnValue); } else { Return returnInstruction = Return.builder() .setPositionForNonThrowingInstruction( - outlineEnd.getPosition(), appView.options()) - .setReturnValue(returnOrThrowValue) + throwInstruction.getPosition(), appView.options()) + .setReturnValue(returnValue) .build(); block.replaceLastInstruction(returnInstruction); - outlineEnd = returnInstruction; } - } - // Remove all outlined instructions bottom up. - instructionIterator = block.listIterator(invoke); - for (Instruction outlinedInstruction = instructionIterator.previous(); - outlinedInstruction != outlineMarker; - outlinedInstruction = instructionIterator.previous()) { - assert !outlinedInstruction.isThrowBlockOutlineMarker(); - Value outValue = outlinedInstruction.outValue(); - if (outValue == null || !outValue.hasNonDebugUsers()) { - // Remove all debug users of the out-value. - if (outValue != null && outValue.hasDebugUsers()) { - for (Instruction debugUser : outValue.debugUsers()) { - debugUser.getDebugValues().remove(outValue); - if (debugUser.isDebugLocalRead() && debugUser.getDebugValues().isEmpty()) { - debugUser.remove(); + // Remove all outlined instructions bottom up. + instructionIterator = block.listIterator(invoke); + for (Instruction instruction = instructionIterator.previous(); + instruction != outlineMarker; + instruction = instructionIterator.previous()) { + Value outValue = instruction.outValue(); + if (outValue == null || !outValue.hasNonDebugUsers()) { + // Remove all debug users of the out-value. + if (outValue != null && outValue.hasDebugUsers()) { + for (Instruction debugUser : outValue.debugUsers()) { + debugUser.getDebugValues().remove(outValue); + if (debugUser.isDebugLocalRead() && debugUser.getDebugValues().isEmpty()) { + debugUser.remove(); + } } + outValue.clearDebugUsers(); } - outValue.clearDebugUsers(); - } - // We are not using `removeOrReplaceByDebugLocalRead` here due to the backwards - // iteration. - if (outlinedInstruction.getDebugValues().isEmpty()) { - outlinedInstruction.remove(); - } else { - DebugLocalRead replacement = new DebugLocalRead(); - outlinedInstruction.replace(replacement); - Instruction previous = instructionIterator.previous(); - assert previous == replacement; + // We are not using `removeOrReplaceByDebugLocalRead` here due to the backwards + // iteration. + if (instruction.getDebugValues().isEmpty()) { + instruction.remove(); + } else { + DebugLocalRead replacement = new DebugLocalRead(); + instruction.replace(replacement); + Instruction previous = instructionIterator.previous(); + assert previous == replacement; + } } } } + + // Finally delete the outline marker. + outlineMarker.removeOrReplaceByDebugLocalRead(); + + // Blocks cannot start with DebugLocalRead. + while (block.entry().isDebugLocalRead()) { + block.entry().moveDebugValues(block.entry().getNext()); + block.entry().remove(); + } } - - // Finally delete the outline marker. - outlineMarker.removeOrReplaceByDebugLocalRead(); - - // Blocks cannot start with DebugLocalRead. - while (block.entry().isDebugLocalRead()) { - block.entry().moveDebugValues(block.entry().getNext()); - block.entry().remove(); - } - - // Continue searching for outline markers from the end of the current outline. - outlineMarker = outlineEnd.nextUntilExclusive(Instruction::isThrowBlockOutlineMarker); } assert block.streamInstructions().noneMatch(Instruction::isThrowBlockOutlineMarker); } @@ -230,15 +216,4 @@ return null; } } - - private Instruction getOutlineEnd( - BasicBlock block, ThrowBlockOutline outline, ThrowBlockOutlineMarker outlineMarker) { - if (outline.isThrowOutline()) { - return block.exit(); - } else { - // The end of a StringBuilder#toString outline is the call to StringBuilder#toString. - return outlineMarker.nextUntilExclusive( - i -> ThrowBlockOutlinerScanner.isStringBuilderToString(i, factory)); - } - } }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutlinerOptions.java b/src/main/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutlinerOptions.java index ce48c06..7f6ea7d 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutlinerOptions.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutlinerOptions.java
@@ -15,10 +15,6 @@ SystemPropertyUtils.parseSystemPropertyOrDefault( "com.android.tools.r8.throwblockoutliner.enable", false); - public boolean enableStringBuilderOutlining = - SystemPropertyUtils.parseSystemPropertyOrDefault( - "com.android.tools.r8.throwblockoutliner.enablestringbuilder", false); - public final int costInBytesForTesting = SystemPropertyUtils.parseSystemPropertyOrDefault( "com.android.tools.r8.throwblockoutliner.cost", -1);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutlinerScanner.java b/src/main/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutlinerScanner.java index 2fda1c4..582fc97 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutlinerScanner.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutlinerScanner.java
@@ -42,7 +42,6 @@ import com.android.tools.r8.ir.code.NumberGenerator; import com.android.tools.r8.ir.code.Position; import com.android.tools.r8.ir.code.Position.SyntheticPosition; -import com.android.tools.r8.ir.code.Return; import com.android.tools.r8.ir.code.Throw; import com.android.tools.r8.ir.code.ThrowBlockOutlineMarker; import com.android.tools.r8.ir.code.Value; @@ -93,28 +92,6 @@ if (block.exit().isThrow()) { new ThrowBlockOutlinerScannerForThrow(code, block).tryBuildOutline(); } - if (!appView.options().getThrowBlockOutlinerOptions().enableStringBuilderOutlining) { - continue; - } - Instruction previousOutlineEnd = null; - for (Instruction instruction : block.getInstructions()) { - // If we encounter an outline marker it is because we were able to outline the tail of the - // block above. Since the remainder of the block has been outlined, abort further outlining. - if (instruction.isThrowBlockOutlineMarker()) { - assert block.exit().isThrow(); - break; - } - if (isStringBuilderToString(instruction, factory)) { - InvokeVirtual invoke = instruction.asInvokeVirtual(); - ThrowBlockOutline outline = - new ThrowBlockOutlinerScannerForStringBuilderToString( - code, block, invoke, previousOutlineEnd) - .tryBuildOutline(); - if (outline != null) { - previousOutlineEnd = instruction; - } - } - } } if (code.metadata().mayHaveThrowBlockOutlineMarker()) { if (appView.enableWholeProgramOptimizations()) { @@ -136,68 +113,55 @@ return outlines.getOutlines(); } - static boolean isStringBuilderToString(Instruction instruction, DexItemFactory factory) { - InvokeVirtual invoke = instruction.asInvokeVirtual(); - return invoke != null - && invoke.getInvokedMethod().match(factory.objectMembers.toString) - && invoke.getReceiver().getType().isClassType(factory.stringBuilderType) - && invoke.hasOutValue(); - } - private abstract class ThrowBlockOutlinerScannerForInstruction { final IRCode code; final BasicBlock block; - final Instruction previousOutlineEnd; private boolean hasRunPrefixer; - ThrowBlockOutlinerScannerForInstruction( - IRCode code, BasicBlock block, Instruction previousOutlineEnd) { + ThrowBlockOutlinerScannerForInstruction(IRCode code, BasicBlock block) { this.code = code; this.block = block; - this.previousOutlineEnd = previousOutlineEnd; } void processInstruction(Instruction instruction, Consumer<OutlineBuilder> continuation) { - if (instruction != previousOutlineEnd) { - switch (instruction.opcode()) { - case ASSUME: - processAssume(instruction.asAssume(), continuation); + switch (instruction.opcode()) { + case ASSUME: + processAssume(instruction.asAssume(), continuation); + return; + case CONST_NUMBER: + case CONST_STRING: + processConstInstruction(instruction.asConstInstruction(), continuation); + return; + case DEBUG_LOCAL_READ: + case DEBUG_POSITION: + processNonMaterializingDebugInstruction(instruction, continuation); + return; + case INVOKE_DIRECT: + if (instruction.isInvokeConstructor(factory)) { + processStringBuilderConstructorCall(instruction.asInvokeDirect(), continuation); return; - case CONST_NUMBER: - case CONST_STRING: - processConstInstruction(instruction.asConstInstruction(), continuation); + } + break; + case INVOKE_STATIC: + processStringFormatOrValueOf(instruction.asInvokeStatic(), continuation); + return; + case INVOKE_VIRTUAL: + processStringBuilderAppendOrToString(instruction.asInvokeVirtual(), continuation); + return; + case MOVE: + if (instruction.isDebugLocalWrite()) { + processDebugLocalWrite(instruction.asDebugLocalWrite(), continuation); return; - case DEBUG_LOCAL_READ: - case DEBUG_POSITION: - processNonMaterializingDebugInstruction(instruction, continuation); - return; - case INVOKE_DIRECT: - if (instruction.isInvokeConstructor(factory)) { - processStringBuilderConstructorCall(instruction.asInvokeDirect(), continuation); - return; - } - break; - case INVOKE_STATIC: - processStringFormatOrValueOf(instruction.asInvokeStatic(), continuation); - return; - case INVOKE_VIRTUAL: - processStringBuilderAppendOrToString(instruction.asInvokeVirtual(), continuation); - return; - case MOVE: - if (instruction.isDebugLocalWrite()) { - processDebugLocalWrite(instruction.asDebugLocalWrite(), continuation); - return; - } - assert false; - break; - case NEW_INSTANCE: - processNewInstanceInstruction(instruction.asNewInstance(), continuation); - return; - default: - break; - } + } + assert false; + break; + case NEW_INSTANCE: + processNewInstanceInstruction(instruction.asNewInstance(), continuation); + return; + default: + break; } // Unhandled instruction. Start the outline at the successor instruction. startOutline(instruction.getNext(), continuation); @@ -336,7 +300,7 @@ }); } - void processStringBuilderAppendOrToString( + private void processStringBuilderAppendOrToString( InvokeVirtual invoke, Consumer<OutlineBuilder> continuation) { DexMethod invokedMethod = invoke.getInvokedMethod(); if (!factory.stringBuilderMethods.isAppendMethod(invokedMethod) @@ -393,75 +357,12 @@ } } - private class ThrowBlockOutlinerScannerForStringBuilderToString - extends ThrowBlockOutlinerScannerForInstruction { - - private final InvokeVirtual stringBuilderToStringInstruction; - - private ThrowBlockOutline outline; - - ThrowBlockOutlinerScannerForStringBuilderToString( - IRCode code, - BasicBlock block, - InvokeVirtual stringBuilderToStringInstruction, - Instruction previousOutlineEnd) { - super(code, block, previousOutlineEnd); - this.stringBuilderToStringInstruction = stringBuilderToStringInstruction; - } - - ThrowBlockOutline tryBuildOutline() { - // Recursively build up the outline method. On successful outline creation, the resulting - // LirCode is passed to the continuation function. - processStringBuilderToString( - outlineBuilder -> { - // On successful outline creation, store the outline for later processing. - DexProto proto = outlineBuilder.getProto(appView, factory.stringType); - if (proto == null) { - return; - } - LirCode<?> lirCode = outlineBuilder.buildLirCode(appView, code.context()); - outline = outlines.add(lirCode, proto, code.context()); - assert proto.isIdenticalTo(outline.getProto()); - List<Value> arguments = outlineBuilder.buildArguments(); - outline.addUser(code.reference(), arguments, getAbstractValueFactory()); - - // Insert a synthetic marker instruction that references the outline so that we know - // where to materialize the outline call. - Instruction insertionPoint = outlineBuilder.getFirstOutlinedInstruction(); - assert insertionPoint.getBlock() == block; - ThrowBlockOutlineMarker marker = - ThrowBlockOutlineMarker.builder() - .setArguments(arguments) - .setOutline(outline) - .setPosition(Position.none()) - .build(); - block.listIterator(insertionPoint).add(marker); - }); - return outline; - } - - private void processStringBuilderToString(Consumer<OutlineBuilder> continuation) { - processStringBuilderAppendOrToString( - stringBuilderToStringInstruction, - outlineBuilder -> { - Value outlinedStringValue = - outlineBuilder.getOutlinedValue(stringBuilderToStringInstruction.outValue()); - outlineBuilder.add( - Return.builder() - .setPosition(Position.syntheticNone()) - .setReturnValue(outlinedStringValue) - .build()); - continuation.accept(outlineBuilder); - }); - } - } - private class ThrowBlockOutlinerScannerForThrow extends ThrowBlockOutlinerScannerForInstruction { private final Throw throwInstruction; ThrowBlockOutlinerScannerForThrow(IRCode code, BasicBlock block) { - super(code, block, null); + super(code, block); this.throwInstruction = block.exit().asThrow(); }
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java index 3ad5193..a3de438 100644 --- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java +++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
@@ -322,7 +322,7 @@ assertEquals(Collections.emptyList(), synthesizedJavaLambdaClasses); assertEquals( - Collections.emptySet(), + Collections.singleton("java.lang.StringBuilder"), collectTypes(clazz.uniqueMethodWithOriginalName("testStatelessLambda"))); assertEquals(
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutlinerConstArgumentTest.java b/src/test/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutlinerConstArgumentTest.java index 87e57c2..2badee6 100644 --- a/src/test/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutlinerConstArgumentTest.java +++ b/src/test/java/com/android/tools/r8/ir/optimize/outliner/exceptions/ThrowBlockOutlinerConstArgumentTest.java
@@ -22,8 +22,6 @@ import it.unimi.dsi.fastutil.ints.IntArraySet; import it.unimi.dsi.fastutil.ints.IntSet; import java.util.Collection; -import java.util.List; -import java.util.stream.Collectors; import org.junit.Test; public class ThrowBlockOutlinerConstArgumentTest extends ThrowBlockOutlinerTestBase { @@ -64,12 +62,10 @@ @Override public void inspectOutlines(Collection<ThrowBlockOutline> outlines, DexItemFactory factory) { - // Verify that we have two throw block outlines with one and three users, respectively. - List<ThrowBlockOutline> throwOutlines = - outlines.stream().filter(ThrowBlockOutline::isThrowOutline).collect(Collectors.toList()); - assertEquals(2, throwOutlines.size()); + // Verify that we have two outlines with one and three users, respectively. + assertEquals(2, outlines.size()); IntSet numberOfUsers = new IntArraySet(); - for (ThrowBlockOutline outline : throwOutlines) { + for (ThrowBlockOutline outline : outlines) { numberOfUsers.add(outline.getNumberOfUsers()); } assertTrue(numberOfUsers.contains(1));
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/outliner/stringbuilders/StringBuilderOutlinerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/outliner/stringbuilders/StringBuilderOutlinerTest.java deleted file mode 100644 index b64c5ba..0000000 --- a/src/test/java/com/android/tools/r8/ir/optimize/outliner/stringbuilders/StringBuilderOutlinerTest.java +++ /dev/null
@@ -1,71 +0,0 @@ -// Copyright (c) 2025, 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.outliner.stringbuilders; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - -import com.android.tools.r8.SingleTestRunResult; -import com.android.tools.r8.TestCompilerBuilder; -import com.android.tools.r8.graph.DexItemFactory; -import com.android.tools.r8.ir.optimize.outliner.exceptions.ThrowBlockOutline; -import com.android.tools.r8.ir.optimize.outliner.exceptions.ThrowBlockOutlinerTestBase; -import com.android.tools.r8.utils.codeinspector.CodeInspector; -import com.android.tools.r8.utils.codeinspector.InstructionSubject; -import com.android.tools.r8.utils.codeinspector.MethodSubject; -import java.util.Collection; -import org.junit.Test; - -public class StringBuilderOutlinerTest extends ThrowBlockOutlinerTestBase { - - @Test - public void testD8() throws Exception { - runTest(testForD8(parameters)); - } - - @Test - public void testR8() throws Exception { - assumeRelease(); - runTest(testForR8(parameters).addKeepMainRule(Main.class)); - } - - private void runTest( - TestCompilerBuilder<?, ?, ?, ? extends SingleTestRunResult<?>, ?> testBuilder) - throws Exception { - testBuilder - .addInnerClasses(getClass()) - .apply(this::configure) - .compile() - .inspect(this::inspectOutput) - .run(parameters.getRuntime(), Main.class, "Hel", "lo", ", world", "!") - .assertSuccessWithOutputLines("Hello, world!"); - } - - @Override - public void inspectOutlines(Collection<ThrowBlockOutline> outlines, DexItemFactory factory) { - assertEquals(1, outlines.size()); - ThrowBlockOutline outline = outlines.iterator().next(); - assertEquals(2, outline.getNumberOfUsers()); - } - - private void inspectOutput(CodeInspector inspector) { - MethodSubject mainMethod = inspector.clazz(Main.class).mainMethod(); - assertTrue(mainMethod.streamInstructions().noneMatch(InstructionSubject::isNewInstance)); - } - - @Override - public boolean shouldOutline(ThrowBlockOutline outline) { - return true; - } - - static class Main { - - public static void main(String[] args) { - String s1 = new StringBuilder().append(args[0]).append(args[1]).toString(); - String s2 = new StringBuilder().append(args[2]).append(args[3]).toString(); - System.out.print(s1); - System.out.println(s2); - } - } -}
diff --git a/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java b/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java index ef9fece..9646cc8 100644 --- a/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java +++ b/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java
@@ -6,10 +6,12 @@ import static com.android.tools.r8.KotlinCompilerTool.KotlinCompilerVersion.KOTLINC_1_3_72; import static com.android.tools.r8.KotlinCompilerTool.KotlinCompilerVersion.KOTLINC_1_5_0; +import static com.android.tools.r8.KotlinCompilerTool.KotlinCompilerVersion.KOTLINC_1_6_0; import static com.android.tools.r8.KotlinCompilerTool.KotlinCompilerVersion.KOTLINC_1_9_21; import static com.android.tools.r8.KotlinCompilerTool.KotlinCompilerVersion.KOTLINC_2_0_20; import static com.android.tools.r8.KotlinCompilerTool.KotlinCompilerVersion.KOTLINC_2_1_10; import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent; +import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsentIf; import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent; import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentIf; import static org.hamcrest.MatcherAssert.assertThat; @@ -58,7 +60,7 @@ } @Test - public void testJStyleLambdasNoClassInlining() throws Exception { + public void testJStyleLambdas() throws Exception { // SAM interfaces lambdas are implemented by invoke dynamic in kotlin 1.5 unlike 1.4 where a // class is generated for each. In CF we leave invokeDynamic but for DEX we desugar the classes // and merge them. @@ -85,18 +87,14 @@ "class_inliner_lambda_j_style.MainKt$testStateful3$1"); } else if (testParameters.isDexRuntime()) { Set<Set<DexType>> mergeGroups = inspector.getMergeGroups(); - assertEquals(2, mergeGroups.size()); - inspector - .assertIsCompleteMergeGroup( - "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda1", - "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda3", - "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda4", - "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda5", - "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda6", - "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda7") - .assertIsCompleteMergeGroup( - "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda2", - "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticThrowBlockOutline0"); + assertEquals(1, mergeGroups.size()); + inspector.assertIsCompleteMergeGroup( + "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda0", + "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda2", + "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda4", + "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda3", + "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda5", + "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda6"); } inspector.assertNoOtherClassesMerged(); }) @@ -105,16 +103,25 @@ inspector -> { if (testParameters.isCfRuntime() && !hasKotlinCGeneratedLambdaClasses) { assertEquals(5, inspector.allClasses().size()); + } else if (!hasKotlinCGeneratedLambdaClasses) { + assertThat( + inspector.clazz( + "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda1"), + isPresent()); + assertThat( + inspector.clazz( + "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda2"), + isAbsent()); } else { - assertEquals(7, inspector.allClasses().size()); + assertThat( + inspector.clazz("class_inliner_lambda_j_style.MainKt$testStateless$1"), + isAbsent()); + assertThat( + inspector.clazz("class_inliner_lambda_j_style.MainKt$testStateful$1"), + isPresent()); } }); - } - @Test - public void testJStyleLambdas() throws Exception { - boolean hasKotlinCGeneratedLambdaClasses = kotlinParameters.isOlderThan(KOTLINC_1_5_0); - String mainClassName = "class_inliner_lambda_j_style.MainKt"; runTest( "class_inliner_lambda_j_style", mainClassName, @@ -129,14 +136,39 @@ inspector -> { if (testParameters.isCfRuntime() && !hasKotlinCGeneratedLambdaClasses) { assertEquals(5, inspector.allClasses().size()); + return; + } + if (!hasKotlinCGeneratedLambdaClasses) { + // Kotlin 1.6.20 and later do not create intrinsics.stringPlus for two argument + // string concatination. That allow R8's stringbuilder optimization to reduce the + // size of strings and therefore inline the synthetic lambda. + assertThat( + inspector.clazz( + "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda1"), + isAbsentIf(kotlinParameters.isNewerThan(KOTLINC_1_6_0))); } else { - assertEquals(7, inspector.allClasses().size()); + assertThat( + inspector.clazz("class_inliner_lambda_j_style.MainKt$testStateless$1"), + isAbsent()); + } + + if (hasKotlinCGeneratedLambdaClasses) { + assertThat( + testParameters.isCfRuntime() + ? inspector.clazz("class_inliner_lambda_j_style.MainKt$testStateful2$1") + : inspector.clazz("class_inliner_lambda_j_style.MainKt$testStateful$2"), + isPresent()); + } else { + assertThat( + inspector.clazz( + "class_inliner_lambda_j_style.MainKt$$ExternalSyntheticLambda2"), + isAbsent()); } }); } @Test - public void testKStyleLambdasNoClassInlining() throws Exception { + public void testKStyleLambdas() throws Exception { String mainClassName = "class_inliner_lambda_k_style.MainKt"; runTest( "class_inliner_lambda_k_style", @@ -232,11 +264,7 @@ isPresent()); } }); - } - @Test - public void testKStyleLambdas() throws Exception { - String mainClassName = "class_inliner_lambda_k_style.MainKt"; runTest( "class_inliner_lambda_k_style", mainClassName,
diff --git a/src/test/java/com/android/tools/r8/kotlin/lambda/KotlinLambdaMergingCapturesKotlinStyleTest.java b/src/test/java/com/android/tools/r8/kotlin/lambda/KotlinLambdaMergingCapturesKotlinStyleTest.java index b02f228..c9467fa 100644 --- a/src/test/java/com/android/tools/r8/kotlin/lambda/KotlinLambdaMergingCapturesKotlinStyleTest.java +++ b/src/test/java/com/android/tools/r8/kotlin/lambda/KotlinLambdaMergingCapturesKotlinStyleTest.java
@@ -13,11 +13,10 @@ import com.android.tools.r8.KotlinTestParameters; import com.android.tools.r8.TestParameters; import com.android.tools.r8.references.ClassReference; -import com.android.tools.r8.references.Reference; -import com.android.tools.r8.synthesis.SyntheticItemsTestUtils; import com.android.tools.r8.utils.BooleanUtils; import com.android.tools.r8.utils.StringUtils; import com.android.tools.r8.utils.codeinspector.CodeInspector; +import com.android.tools.r8.utils.codeinspector.HorizontallyMergedClassesInspector; import com.google.common.collect.ImmutableList; import java.nio.file.Path; import java.util.ArrayList; @@ -82,20 +81,7 @@ .addProgramFiles(getProgramFiles()) .addKeepMainRule(getMainClassName()) .addHorizontallyMergedClassesInspector( - inspector -> { - if (parameters.isDexRuntime() - && kotlinParameters.getLambdaGeneration().isInvokeDynamic()) { - inspector - .assertIsCompleteMergeGroup( - SyntheticItemsTestUtils.syntheticThrowBlockOutlineClass( - getMainClassReference(), 0), - SyntheticItemsTestUtils.syntheticThrowBlockOutlineClass( - getMainClassReference(), 1)) - .assertNoOtherClassesMerged(); - } else { - inspector.assertNoClassesMerged(); - } - }) + HorizontallyMergedClassesInspector::assertNoClassesMerged) .allowAccessModification(allowAccessModification) .setMinApi(parameters) .compile() @@ -152,10 +138,6 @@ return getTestName() + ".MainKt"; } - private ClassReference getMainClassReference() { - return Reference.classFromTypeName(getMainClassName()); - } - private List<Path> getProgramFiles() { Path kotlinJarFile = getCompileMemoizer(getKotlinFilesInResource(getTestName()), getTestName())
diff --git a/src/test/java/com/android/tools/r8/kotlin/lambda/KotlinLambdaMergingKeepAttributesKotlinStyleTest.java b/src/test/java/com/android/tools/r8/kotlin/lambda/KotlinLambdaMergingKeepAttributesKotlinStyleTest.java index 56178bd..da524c4 100644 --- a/src/test/java/com/android/tools/r8/kotlin/lambda/KotlinLambdaMergingKeepAttributesKotlinStyleTest.java +++ b/src/test/java/com/android/tools/r8/kotlin/lambda/KotlinLambdaMergingKeepAttributesKotlinStyleTest.java
@@ -4,6 +4,7 @@ package com.android.tools.r8.kotlin.lambda; +import static com.android.tools.r8.KotlinCompilerTool.KotlinCompilerVersion.KOTLINC_1_9_21; import static com.android.tools.r8.shaking.ProguardKeepAttributes.ENCLOSING_METHOD; import static com.android.tools.r8.shaking.ProguardKeepAttributes.INNER_CLASSES; import static com.android.tools.r8.shaking.ProguardKeepAttributes.SIGNATURE; @@ -188,6 +189,21 @@ ClassReference mainKt = Reference.classFromTypeName(getMainClassName()); List<ClassReference> mergeGroup = ImmutableList.of( + SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 9), + SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 10), + SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 11), + SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 12), + SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 13), + SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 14), + SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 19), + SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 18), + SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 20), + SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 21), + SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 22), + SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 23)); + List<ClassReference> otherMergeGroup = + ImmutableList.of( + SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 0), SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 1), SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 2), SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 3), @@ -196,24 +212,24 @@ SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 6), SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 7), SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 8), - SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 9), - SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 10), - SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 11), - SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 12), - SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 13), - SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 14), SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 15), SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 16), - SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 17), - SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 18), - SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 19), - SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 20), - SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 21), - SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 22), - SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 23), - SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 24), - SyntheticItemsTestUtils.syntheticThrowBlockOutlineClass(mainKt, 0)); - inspector.assertIsCompleteMergeGroup(mergeGroup).assertNoOtherClassesMerged(); + SyntheticItemsTestUtils.syntheticLambdaClass(mainKt, 17)); + inspector + .applyIf( + kotlinc.getCompilerVersion().isGreaterThanOrEqualTo(KOTLINC_1_9_21) + && parameters.isDexRuntime() + && lambdaGeneration.isInvokeDynamic(), + i -> + i.assertIsCompleteMergeGroup( + ImmutableList.<ClassReference>builder() + .addAll(mergeGroup) + .addAll(otherMergeGroup) + .build()), + i -> + i.assertIsCompleteMergeGroup(mergeGroup) + .assertIsCompleteMergeGroup(otherMergeGroup)) + .assertNoOtherClassesMerged(); } } }
diff --git a/src/test/java/com/android/tools/r8/kotlin/lambda/KotlinLambdaMergingTrivialJavaStyleTest.java b/src/test/java/com/android/tools/r8/kotlin/lambda/KotlinLambdaMergingTrivialJavaStyleTest.java index ca2d90c..ae02a7f 100644 --- a/src/test/java/com/android/tools/r8/kotlin/lambda/KotlinLambdaMergingTrivialJavaStyleTest.java +++ b/src/test/java/com/android/tools/r8/kotlin/lambda/KotlinLambdaMergingTrivialJavaStyleTest.java
@@ -173,24 +173,13 @@ }, i -> { i.assertIsCompleteMergeGroup( - SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 3), - SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 4), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 0), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 1), SyntheticItemsTestUtils.syntheticLambdaClass(innerClassReference, 0)) .assertIsCompleteMergeGroup( - SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 7), - SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 8), - SyntheticItemsTestUtils.syntheticLambdaClass(innerClassReference, 2)) - .assertIsCompleteMergeGroup( - SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 5), - SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 6), - SyntheticItemsTestUtils.syntheticLambdaClass(innerClassReference, 1)) - .assertIsCompleteMergeGroup( - SyntheticItemsTestUtils.syntheticThrowBlockOutlineClass( - mainClassReference, 0), - SyntheticItemsTestUtils.syntheticThrowBlockOutlineClass( - mainClassReference, 1), - SyntheticItemsTestUtils.syntheticThrowBlockOutlineClass( - mainClassReference, 2)); + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 2), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 3), + SyntheticItemsTestUtils.syntheticLambdaClass(innerClassReference, 1)); for (int id = 4; id < 30; id++) { inspector.assertClassReferencesNotMerged( SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, id)); @@ -200,8 +189,48 @@ SyntheticItemsTestUtils.syntheticLambdaClass(innerClassReference, 2), SyntheticItemsTestUtils.syntheticLambdaClass(innerClassReference, 3)); } else { - assertEquals(10, inspector.getMergeGroups().size()); - assertEquals(34, inspector.getSources().size()); + inspector + .assertIsCompleteMergeGroup( + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 1), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 2), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 3), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 4)) + .assertIsCompleteMergeGroup( + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 0), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 9), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 10), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 11), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 12), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 21), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 22), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 23), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 24), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 25), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 26), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 27), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 28)) + .assertIsCompleteMergeGroup( + SyntheticItemsTestUtils.syntheticLambdaClass(innerClassReference, 0), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 13), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 14)) + .assertIsCompleteMergeGroup( + SyntheticItemsTestUtils.syntheticLambdaClass(innerClassReference, 1), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 15), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 16)) + .assertIsCompleteMergeGroup( + SyntheticItemsTestUtils.syntheticLambdaClass(innerClassReference, 2), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 17), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 18)) + .assertIsCompleteMergeGroup( + SyntheticItemsTestUtils.syntheticLambdaClass(innerClassReference, 3), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 19), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 20)) + .assertIsCompleteMergeGroup( + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 5), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 6)) + .assertIsCompleteMergeGroup( + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 7), + SyntheticItemsTestUtils.syntheticLambdaClass(mainClassReference, 8)); } }
diff --git a/src/test/testbase/java/com/android/tools/r8/TestCompilerBuilder.java b/src/test/testbase/java/com/android/tools/r8/TestCompilerBuilder.java index 86abea7..25efdbf 100644 --- a/src/test/testbase/java/com/android/tools/r8/TestCompilerBuilder.java +++ b/src/test/testbase/java/com/android/tools/r8/TestCompilerBuilder.java
@@ -65,7 +65,6 @@ options -> { options.testing.enableTestAssertions = true; options.getThrowBlockOutlinerOptions().enable = true; - options.getThrowBlockOutlinerOptions().enableStringBuilderOutlining = true; }; public static final Consumer<InternalOptions> DEFAULT_D8_OPTIONS = DEFAULT_OPTIONS;
diff --git a/src/test/testbase/java/com/android/tools/r8/synthesis/SyntheticItemsTestUtils.java b/src/test/testbase/java/com/android/tools/r8/synthesis/SyntheticItemsTestUtils.java index e70142c..9b5bb95 100644 --- a/src/test/testbase/java/com/android/tools/r8/synthesis/SyntheticItemsTestUtils.java +++ b/src/test/testbase/java/com/android/tools/r8/synthesis/SyntheticItemsTestUtils.java
@@ -105,10 +105,6 @@ } public static ClassReference syntheticThrowBlockOutlineClass(Class<?> clazz, int id) { - return syntheticThrowBlockOutlineClass(Reference.classFromClass(clazz), id); - } - - public static ClassReference syntheticThrowBlockOutlineClass(ClassReference clazz, int id) { return syntheticClass(clazz, naming.THROW_BLOCK_OUTLINE, id); }