Reproduce spill to fresh local in presence of unused argument Bug: b/375142715 Change-Id: Ic7fc89b42bb559fb15124c25eb81678419139e4f
diff --git a/src/test/java/com/android/tools/r8/ir/regalloc/SpillToHighUnusedArgumentRegisterTest.java b/src/test/java/com/android/tools/r8/ir/regalloc/SpillToHighUnusedArgumentRegisterTest.java new file mode 100644 index 0000000..f88127a --- /dev/null +++ b/src/test/java/com/android/tools/r8/ir/regalloc/SpillToHighUnusedArgumentRegisterTest.java
@@ -0,0 +1,108 @@ +// Copyright (c) 2024, 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.regalloc; + +import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; + +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.DexMoveFrom16; +import com.android.tools.r8.dex.code.DexMoveResult; +import com.android.tools.r8.graph.DexCode; +import com.android.tools.r8.utils.codeinspector.InstructionSubject; +import com.android.tools.r8.utils.codeinspector.MethodSubject; +import com.google.common.collect.MoreCollectors; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; + +@RunWith(Parameterized.class) +public class SpillToHighUnusedArgumentRegisterTest extends TestBase { + + @Parameter(0) + public TestParameters parameters; + + @Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withDefaultDexRuntime().withMaximumApiLevel().build(); + } + + @Test + public void testD8() throws Exception { + testForD8() + .addInnerClasses(getClass()) + .addOptionsModification( + options -> options.getTestingOptions().enableRegisterAllocation8BitRefinement = true) + .release() + .setMinApi(parameters) + .compile() + .inspect( + inspector -> { + MethodSubject testMethodSubject = + inspector.clazz(Main.class).uniqueMethodWithOriginalName("test"); + assertThat(testMethodSubject, isPresent()); + + DexCode code = testMethodSubject.getMethod().getCode().asDexCode(); + DexMoveResult moveResult = + testMethodSubject + .streamInstructions() + .filter(InstructionSubject::isMoveResult) + .collect(MoreCollectors.onlyElement()) + .asDexInstruction() + .getInstruction(); + + DexMoveFrom16 spillMove = + testMethodSubject + .streamInstructions() + .filter(i -> i.isMoveFrom(moveResult.AA)) + .collect(MoreCollectors.onlyElement()) + .asDexInstruction() + .getInstruction(); + int firstArgumentRegister = code.registerSize - code.incomingRegisterSize; + // TODO(b/375142715): We could have spilled this value to the unused argument + // register, which would have lead to fewer registers being used. + assertEquals(firstArgumentRegister - 1, spillMove.AA); + }); + } + + static class Main { + + static void test(long a, long b, long c, long d, long e, long f, long g, long h, int unused) { + int i = getInt(); + forceIntoLowRegister(i, i); + forceIntoLowRegister(a, a); + forceIntoLowRegister(b, b); + forceIntoLowRegister(c, c); + forceIntoLowRegister(d, d); + forceIntoLowRegister(e, e); + forceIntoLowRegister(f, f); + forceIntoLowRegister(g, g); + forceIntoLowRegister(h, h); + unconstrainedUse(i); + forceIntoLowRegister(a, a); + forceIntoLowRegister(b, b); + forceIntoLowRegister(c, c); + forceIntoLowRegister(d, d); + forceIntoLowRegister(e, e); + forceIntoLowRegister(f, f); + forceIntoLowRegister(g, g); + forceIntoLowRegister(h, h); + } + + static int getInt() { + return 0; + } + + static void forceIntoLowRegister(int a, int b) {} + + static void forceIntoLowRegister(long a, long b) {} + + static void unconstrainedUse(int a) {} + } +}
diff --git a/src/test/testbase/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java b/src/test/testbase/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java index 2c24643..6cc23c2 100644 --- a/src/test/testbase/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java +++ b/src/test/testbase/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
@@ -402,6 +402,16 @@ } @Override + public boolean isMoveFrom(int register) { + return false; + } + + @Override + public boolean isMoveResult() { + return false; + } + + @Override public boolean isFilledNewArray() { return false; }
diff --git a/src/test/testbase/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java b/src/test/testbase/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java index f4e6a57..8c1d4a2 100644 --- a/src/test/testbase/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java +++ b/src/test/testbase/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
@@ -101,6 +101,9 @@ import com.android.tools.r8.dex.code.DexMoveObject; import com.android.tools.r8.dex.code.DexMoveObject16; import com.android.tools.r8.dex.code.DexMoveObjectFrom16; +import com.android.tools.r8.dex.code.DexMoveResult; +import com.android.tools.r8.dex.code.DexMoveResultObject; +import com.android.tools.r8.dex.code.DexMoveResultWide; import com.android.tools.r8.dex.code.DexMoveWide; import com.android.tools.r8.dex.code.DexMoveWide16; import com.android.tools.r8.dex.code.DexMoveWideFrom16; @@ -664,6 +667,47 @@ } @Override + public boolean isMoveFrom(int register) { + if (instruction instanceof DexMove) { + DexMove move = getInstruction(); + return move.B == register; + } else if (instruction instanceof DexMove16) { + DexMove16 move = getInstruction(); + return move.BBBB == register; + } else if (instruction instanceof DexMoveFrom16) { + DexMoveFrom16 move = getInstruction(); + return move.BBBB == register; + } else if (instruction instanceof DexMoveObject) { + DexMoveObject move = getInstruction(); + return move.B == register; + } else if (instruction instanceof DexMoveObject16) { + DexMoveObject16 move = getInstruction(); + return move.BBBB == register; + } else if (instruction instanceof DexMoveObjectFrom16) { + DexMoveObjectFrom16 move = getInstruction(); + return move.BBBB == register; + } else if (instruction instanceof DexMoveWide) { + DexMoveWide move = getInstruction(); + return move.B == register; + } else if (instruction instanceof DexMoveWide16) { + DexMoveWide16 move = getInstruction(); + return move.BBBB == register; + } else if (instruction instanceof DexMoveWideFrom16) { + DexMoveWideFrom16 move = getInstruction(); + return move.BBBB == register; + } else { + return false; + } + } + + @Override + public boolean isMoveResult() { + return instruction instanceof DexMoveResult + || instruction instanceof DexMoveResultObject + || instruction instanceof DexMoveResultWide; + } + + @Override public boolean isFilledNewArray() { return instruction instanceof DexFilledNewArray || instruction instanceof DexFilledNewArrayRange; @@ -705,7 +749,8 @@ return instruction.toString(); } - public DexInstruction getInstruction() { - return instruction; + @SuppressWarnings("unchecked") + public <T extends DexInstruction> T getInstruction() { + return (T) instruction; } }
diff --git a/src/test/testbase/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java b/src/test/testbase/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java index 1852710..b23719d 100644 --- a/src/test/testbase/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java +++ b/src/test/testbase/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
@@ -166,6 +166,10 @@ boolean isMove(); + boolean isMoveFrom(int register); + + boolean isMoveResult(); + boolean isFilledNewArray(); boolean isFilledNewArrayData();