Support catch handlers in conversion to filled-array-data.
Bug: b/246971330
Change-Id: I064cb8aa0020f47b261ff245e47323db4c4bff8b
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
index a214f7c..0a79887 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
@@ -1921,8 +1921,11 @@
newBlock.getMutableSuccessors().add(this);
newBlock.getMutablePredecessors().add(predecessor);
predecessor.replaceSuccessor(this, newBlock);
- blockIterator.add(newBlock);
- assert newBlock.getNumber() >= 0 : "Number must be assigned by `onNewBlock`";
+ if (blockIterator == null) {
+ code.blocks.add(newBlock);
+ } else {
+ blockIterator.add(newBlock);
+ }
}
// Replace the blocks predecessors with the new ones.
predecessors.clear();
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionListIterator.java
index 1d40f99..afe8e0b 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionListIterator.java
@@ -129,7 +129,7 @@
}
@Override
- public void addThrowingInstructionToPossiblyThrowingBlock(
+ public BasicBlock addThrowingInstructionToPossiblyThrowingBlock(
IRCode code,
ListIterator<BasicBlock> blockIterator,
Instruction instruction,
@@ -140,11 +140,15 @@
assert !block.hasCatchHandlers();
assert splitBlock.hasCatchHandlers();
block.copyCatchHandlers(code, blockIterator, splitBlock, options);
- while (IteratorUtils.peekPrevious(blockIterator) != splitBlock) {
- blockIterator.previous();
+ if (blockIterator != null) {
+ while (IteratorUtils.peekPrevious(blockIterator) != splitBlock) {
+ blockIterator.previous();
+ }
}
+ return splitBlock;
} else {
add(instruction);
+ return null;
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java
index ae400dc..8b519ed 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java
@@ -207,12 +207,12 @@
}
@Override
- public void addThrowingInstructionToPossiblyThrowingBlock(
+ public BasicBlock addThrowingInstructionToPossiblyThrowingBlock(
IRCode code,
ListIterator<BasicBlock> blockIterator,
Instruction instruction,
InternalOptions options) {
- instructionIterator.addThrowingInstructionToPossiblyThrowingBlock(
+ return instructionIterator.addThrowingInstructionToPossiblyThrowingBlock(
code, blockIterator, instruction, options);
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
index 224b457..8a69912 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
@@ -26,7 +26,7 @@
public interface InstructionListIterator
extends InstructionIterator, ListIterator<Instruction>, PreviousUntilIterator<Instruction> {
- void addThrowingInstructionToPossiblyThrowingBlock(
+ BasicBlock addThrowingInstructionToPossiblyThrowingBlock(
IRCode code,
ListIterator<BasicBlock> blockIterator,
Instruction instruction,
diff --git a/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionListIterator.java
index ed95689..819ba43 100644
--- a/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionListIterator.java
@@ -183,12 +183,12 @@
}
@Override
- public void addThrowingInstructionToPossiblyThrowingBlock(
+ public BasicBlock addThrowingInstructionToPossiblyThrowingBlock(
IRCode code,
ListIterator<BasicBlock> blockIterator,
Instruction instruction,
InternalOptions options) {
- currentBlockIterator.addThrowingInstructionToPossiblyThrowingBlock(
+ return currentBlockIterator.addThrowingInstructionToPossiblyThrowingBlock(
code, blockIterator, instruction, options);
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index d7e5fc1..5002bac 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -2174,8 +2174,6 @@
// optimization so that we do not transform half-initialized arrays into fully initialized
// arrays on exceptional edges. If the block has no handlers it is not observable so
// we perform the rewriting.
- // TODO(b/246971330): Allow simplification when all users of the array are in the same
- // try/catch.
if (block.hasCatchHandlers() && instruction.instructionInstanceCanThrow()) {
return null;
}
@@ -2280,8 +2278,7 @@
}
DexType arrayType = newArrayEmpty.type;
boolean encodeAsFilledNewArray = canUseFilledNewArray(arrayType, size, options);
- if (!encodeAsFilledNewArray
- && !canUseFilledArrayData(arrayType, size, instruction.getBlock(), options)) {
+ if (!encodeAsFilledNewArray && !canUseFilledArrayData(arrayType, size, options)) {
return null;
}
return new FilledArrayCandidate(newArrayEmpty, size, encodeAsFilledNewArray);
@@ -2316,23 +2313,13 @@
return false;
}
- private boolean canUseFilledArrayData(
- DexType arrayType, int size, BasicBlock block, RewriteArrayOptions options) {
+ private boolean canUseFilledArrayData(DexType arrayType, int size, RewriteArrayOptions options) {
// If there is only one element it is typically smaller to generate the array put
// instruction instead of fill array data.
if (size < options.minSizeForFilledArrayData || size > options.maxSizeForFilledArrayData) {
return false;
}
- if (!arrayType.isPrimitiveArrayType()) {
- return false;
- }
- // NewArrayFilledData can throw, so creating one as done below would add a second
- // throwing instruction to the same block (the first one being NewArrayEmpty).
- // TODO(b/246971330): Support splitting the block when inserting the additional instruction.
- if (block.hasCatchHandlers()) {
- return false;
- }
- return true;
+ return arrayType.isPrimitiveArrayType();
}
/**
@@ -2394,15 +2381,16 @@
WorkList<BasicBlock> worklist = WorkList.newIdentityWorkList(code.blocks);
while (worklist.hasNext()) {
BasicBlock block = worklist.next();
- simplifyArrayConstructionBlock(block, worklist, code, options.rewriteArrayOptions());
+ simplifyArrayConstructionBlock(block, worklist, code, options);
}
}
private void simplifyArrayConstructionBlock(
- BasicBlock block, WorkList<BasicBlock> worklist, IRCode code, RewriteArrayOptions options) {
+ BasicBlock block, WorkList<BasicBlock> worklist, IRCode code, InternalOptions options) {
+ RewriteArrayOptions rewriteOptions = options.rewriteArrayOptions();
InstructionListIterator it = block.listIterator(code);
while (it.hasNext()) {
- FilledArrayCandidate candidate = computeFilledArrayCandiate(it.next(), options);
+ FilledArrayCandidate candidate = computeFilledArrayCandiate(it.next(), rewriteOptions);
if (candidate == null) {
continue;
}
@@ -2431,7 +2419,7 @@
newArrayEmpty.outValue().replaceUsers(invokeValue);
instructionsToRemove.add(newArrayEmpty);
- boolean originalAllocationPointHasHandlers = newArrayEmpty.getBlock().hasCatchHandlers();
+ boolean originalAllocationPointHasHandlers = block.hasCatchHandlers();
boolean insertionPointHasHandlers = lastArrayPut.getBlock().hasCatchHandlers();
if (!insertionPointHasHandlers && !originalAllocationPointHasHandlers) {
@@ -2462,7 +2450,11 @@
NewArrayFilledData fillArray =
new NewArrayFilledData(newArrayEmpty.outValue(), elementSize, size, contents);
fillArray.setPosition(newArrayEmpty.getPosition());
- it.add(fillArray);
+ BasicBlock newBlock =
+ it.addThrowingInstructionToPossiblyThrowingBlock(code, null, fillArray, options);
+ if (newBlock != null) {
+ worklist.addIfNotSeen(newBlock);
+ }
}
instructionsToRemove.addAll(info.arrayPutsToRemove);
diff --git a/src/test/java/com/android/tools/r8/ir/regalloc/RegisterMoveSchedulerTest.java b/src/test/java/com/android/tools/r8/ir/regalloc/RegisterMoveSchedulerTest.java
index 8983d39..4d2d2d2 100644
--- a/src/test/java/com/android/tools/r8/ir/regalloc/RegisterMoveSchedulerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/regalloc/RegisterMoveSchedulerTest.java
@@ -191,7 +191,7 @@
}
@Override
- public void addThrowingInstructionToPossiblyThrowingBlock(
+ public BasicBlock addThrowingInstructionToPossiblyThrowingBlock(
IRCode code,
ListIterator<BasicBlock> blockIterator,
Instruction instruction,
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/FilledArrayDataWithCatchHandlerTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/FilledArrayDataWithCatchHandlerTest.java
new file mode 100644
index 0000000..3e04138
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/FilledArrayDataWithCatchHandlerTest.java
@@ -0,0 +1,106 @@
+// Copyright (c) 2022, 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.rewrite.arrays;
+
+import static com.android.tools.r8.cf.methodhandles.fields.ClassFieldMethodHandleTest.Main.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeTrue;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.dex.code.DexFillArrayData;
+import com.android.tools.r8.dex.code.DexNewArray;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.InstructionOffsetSubject;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class FilledArrayDataWithCatchHandlerTest extends TestBase {
+
+ static final String EXPECTED = StringUtils.lines("1");
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimes().withAllApiLevels().build();
+ }
+
+ public FilledArrayDataWithCatchHandlerTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForRuntime(parameters)
+ .addInnerClasses(FilledArrayDataWithCatchHandlerTest.class)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ @Test
+ public void testReleaseD8() throws Exception {
+ assumeTrue(parameters.isDexRuntime());
+ testForD8(parameters.getBackend())
+ .release()
+ .setMinApi(parameters.getApiLevel())
+ .addInnerClasses(FilledArrayDataWithCatchHandlerTest.class)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED)
+ .inspect(this::checkInstructions);
+ }
+
+ private void checkInstructions(CodeInspector inspector) {
+ MethodSubject foo = inspector.clazz(TestClass.class).uniqueMethodWithFinalName("foo");
+ List<InstructionSubject> newArrays =
+ foo.streamInstructions()
+ .filter(i -> i.asDexInstruction().getInstruction() instanceof DexNewArray)
+ .collect(Collectors.toList());
+ assertEquals(1, newArrays.size());
+
+ List<InstructionSubject> fillArrays =
+ foo.streamInstructions()
+ .filter(i -> i.asDexInstruction().getInstruction() instanceof DexFillArrayData)
+ .collect(Collectors.toList());
+ assertEquals(1, fillArrays.size());
+
+ InstructionOffsetSubject offsetNew = newArrays.get(0).getOffset(foo);
+ InstructionOffsetSubject offsetFill = newArrays.get(0).getOffset(foo);
+ assertTrue(
+ foo.streamTryCatches()
+ .allMatch(r -> r.getRange().includes(offsetNew) && r.getRange().includes(offsetFill)));
+ }
+
+ static class TestClass {
+
+ public static int foo() {
+ int value = 1;
+ int[] array = null;
+ try {
+ array = new int[6];
+ } catch (RuntimeException e) {
+ return array[0];
+ }
+ array[0] = value;
+ array[1] = value;
+ array[2] = value;
+ array[3] = value;
+ array[4] = value;
+ array[5] = value;
+ return array[5];
+ }
+
+ public static void main(String[] args) {
+ System.out.println(foo());
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java
index 939753f..611ed3f 100644
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java
@@ -201,6 +201,7 @@
assertArrayTypes(arraysThatUseNewArrayEmpty, DexNewArray.class);
assertArrayTypes(intsThatUseFilledNewArray, DexFilledNewArray.class);
assertFilledArrayData(arraysThatUseFilledData);
+ assertFilledArrayData(catchHandlerNonThrowingFillArrayData);
if (compilationMode == CompilationMode.DEBUG) {
// The explicit assignments can't be collapsed without breaking the debugger's ability to
@@ -256,7 +257,6 @@
assertArrayTypes(arrayWithCorrectCountButIncompleteCoverage, DexNewArray.class);
assertArrayTypes(catchHandlerThrowing, DexNewArray.class);
- assertArrayTypes(catchHandlerNonThrowingFillArrayData, DexNewArray.class);
assertArrayTypes(catchHandlerNonThrowingFilledNewArray, DexFilledNewArray.class);
}
