Fix initClass with position and pop in cf back-end
- initClass in cf back-end must be followed by a pop.
Bug: b/290565182
Change-Id: I0672675a4a48ade99e35395ed5eb4c4dcd7269b7
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 dd732b4..c46e899 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
@@ -412,11 +412,6 @@
*/
public abstract boolean identicalNonValueNonPositionParts(Instruction other);
- public boolean identicalNonValueParts(Instruction other) {
- assert getClass() == other.getClass();
- return position.equals(other.position) && identicalNonValueNonPositionParts(other);
- }
-
private boolean identicalInputAfterRegisterAllocation(
Value a,
int aInstrNumber,
@@ -504,18 +499,29 @@
return a.outType() == b.outType();
}
+ protected boolean identicalNonValueParts(Instruction other, RegisterAllocator allocator) {
+ assert getClass() == other.getClass();
+ if (!identicalNonValueNonPositionParts(other)) {
+ return false;
+ }
+ return identicalPosition(other, allocator);
+ }
+
+ protected final boolean identicalPosition(Instruction other, RegisterAllocator allocator) {
+ // In debug mode or if the instruction can throw we must account for positions, in release mode
+ // we do want to share non-throwing instructions even if their positions differ.
+ if (instructionTypeCanThrow() || allocator.options().debug) {
+ return position.equals(other.position);
+ }
+ return true;
+ }
+
public boolean identicalAfterRegisterAllocation(
Instruction other, RegisterAllocator allocator, MethodConversionOptions conversionOptions) {
if (other.getClass() != getClass()) {
return false;
}
- // In debug mode or if the instruction can throw we must account for positions, in release mode
- // we do want to share non-throwing instructions even if their positions differ.
- if (instructionTypeCanThrow() || allocator.options().debug) {
- if (!identicalNonValueParts(other)) {
- return false;
- }
- } else if (!identicalNonValueNonPositionParts(other)) {
+ if (!identicalNonValueParts(other, allocator)) {
return false;
}
if (isInvokeDirect() && !asInvokeDirect().sameConstructorReceiverValue(other.asInvoke())) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/Pop.java b/src/main/java/com/android/tools/r8/ir/code/Pop.java
index 5839b47..00554e8 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Pop.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Pop.java
@@ -13,7 +13,9 @@
import com.android.tools.r8.ir.optimize.DeadCodeRemover.DeadInstructionResult;
import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget;
import com.android.tools.r8.ir.optimize.InliningConstraints;
+import com.android.tools.r8.ir.regalloc.RegisterAllocator;
import com.android.tools.r8.lightir.LirBuilder;
+import java.util.function.BiPredicate;
public class Pop extends Instruction {
@@ -53,20 +55,38 @@
}
@Override
+ protected boolean identicalNonValueParts(Instruction other, RegisterAllocator allocator) {
+ assert other.isPop();
+ if (!identicalPop(
+ other.asPop(),
+ (initClass, otherInitClass) ->
+ initClass.identicalNonValueParts(otherInitClass, allocator))) {
+ return false;
+ }
+ assert identicalNonValueNonPositionParts(other);
+ return identicalPosition(other, allocator);
+ }
+
+ @Override
public boolean identicalNonValueNonPositionParts(Instruction other) {
if (!other.isPop()) {
return false;
}
- Pop pop = other.asPop();
+ return identicalPop(other.asPop(), InitClass::identicalNonValueNonPositionParts);
+ }
+
+ // Pops are identical except if immediately following an InitClass, in which case they are
+ // identical only if the corresponding InitClass are.
+ private boolean identicalPop(Pop other, BiPredicate<InitClass, InitClass> predicate) {
if (getFirstOperand().isDefinedByInstructionSatisfying(Instruction::isInitClass)) {
InitClass initClass = getFirstOperand().getDefinition().asInitClass();
- if (!pop.getFirstOperand().isDefinedByInstructionSatisfying(Instruction::isInitClass)) {
+ if (!other.getFirstOperand().isDefinedByInstructionSatisfying(Instruction::isInitClass)) {
return false;
}
- InitClass otherInitClass = pop.getFirstOperand().getDefinition().asInitClass();
- return initClass.getClassValue() == otherInitClass.getClassValue();
+ InitClass otherInitClass = other.getFirstOperand().getDefinition().asInitClass();
+ return predicate.test(initClass, otherInitClass);
}
- return !pop.getFirstOperand().isDefinedByInstructionSatisfying(Instruction::isInitClass);
+ return !other.getFirstOperand().isDefinedByInstructionSatisfying(Instruction::isInitClass);
}
@Override
diff --git a/src/test/java/com/android/tools/r8/debug/classinit/InitClassPopCfTest.java b/src/test/java/com/android/tools/r8/debug/classinit/InitClassPopCfTest.java
new file mode 100644
index 0000000..86ff2c4
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/debug/classinit/InitClassPopCfTest.java
@@ -0,0 +1,132 @@
+// Copyright (c) 2023, 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.debug.classinit;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.AlwaysInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.InstructionIterator;
+import com.android.tools.r8.ir.code.Position;
+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.Iterator;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class InitClassPopCfTest extends TestBase {
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withCfRuntimes().build();
+ }
+
+ private final TestParameters parameters;
+
+ public InitClassPopCfTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testInitClass() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .enableInliningAnnotations()
+ .enableAlwaysInliningAnnotations()
+ .addOptionsModification(
+ internalOptions -> internalOptions.testing.irModifier = this::modifyIR)
+ .addKeepRules("-keepattributes LineNumberTable")
+ .addKeepRules("-keep class * { static java.lang.Object f; }")
+ .addKeepMainRule(Main.class)
+ .compile()
+ .inspect(InitClassPopCfTest::verifyInitClassFollowedByPopInCf);
+ }
+
+ private void modifyIR(IRCode irCode, AppView<?> appView) {
+ InstructionIterator instructionIterator = irCode.instructionIterator();
+ Position pos = null;
+ while (instructionIterator.hasNext()) {
+ Instruction next = instructionIterator.next();
+ // Modify the position so code can be shared except for initClass and invoke-static that
+ // will be transformed into initClass.
+ if (!next.isInitClass()
+ && !(next.isInvokeStatic()
+ && next.asInvokeStatic().getInvokedMethod().getName().toString().equals("nop"))) {
+ if (pos == null) {
+ pos = next.getPosition();
+ }
+ next.forceOverwritePosition(pos);
+ }
+ }
+ }
+
+ private static void verifyInitClassFollowedByPopInCf(CodeInspector i) {
+ MethodSubject main = i.clazz(Main.class).mainMethod();
+ Iterator<InstructionSubject> iterator = main.iterateInstructions();
+ int index = 0;
+ while (iterator.hasNext()) {
+ InstructionSubject next = iterator.next();
+ // All static gets on non System are init class.
+ if (next.isStaticGet()
+ && !next.getField().getHolderType().toString().equals("java.lang.System")) {
+ assertTrue(iterator.next().isPop());
+ index++;
+ }
+ }
+ assertEquals(4, index);
+ }
+
+ public static class A {
+ static {
+ System.out.println("clinit A");
+ }
+
+ static Object f;
+
+ @AlwaysInline
+ static void nop() {}
+ }
+
+ public static class Main {
+
+ public static void main(String[] args) {
+ int i = System.currentTimeMillis() > 0 ? 5 : 3;
+ if (System.currentTimeMillis() > 0) {
+ System.out.println("0");
+ A.nop();
+ print((((i << 2) + 1 << 3) + 4) << 5);
+ } else if (System.currentTimeMillis() > 1) {
+ System.out.println("1");
+ A.nop();
+ print((((i << 2) + 1 << 3) + 4) << 5);
+ } else if (System.currentTimeMillis() > 2) {
+ System.out.println("2");
+ A.nop();
+ print((((i << 2) + 1 << 3) + 4) << 5);
+ } else {
+ System.out.println("3");
+ A.nop();
+ print((((i << 2) + 1 << 3) + 4) << 5);
+ }
+ System.out.println(i);
+ }
+
+ @NeverInline
+ private static void print(int k) {
+ System.out.println(k);
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
index 79425a3..989b9a6 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
@@ -33,6 +33,7 @@
import com.android.tools.r8.cf.code.CfReturn;
import com.android.tools.r8.cf.code.CfReturnVoid;
import com.android.tools.r8.cf.code.CfStackInstruction;
+import com.android.tools.r8.cf.code.CfStackInstruction.Opcode;
import com.android.tools.r8.cf.code.CfStore;
import com.android.tools.r8.cf.code.CfSwitch;
import com.android.tools.r8.cf.code.CfThrow;
@@ -139,6 +140,12 @@
}
@Override
+ public boolean isPop() {
+ return instruction instanceof CfStackInstruction
+ && ((CfStackInstruction) instruction).getOpcode() == Opcode.Pop;
+ }
+
+ @Override
public DexMethod getMethod() {
assert isInvokeMethod();
return ((CfInvoke) instruction).getMethod();
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
index bd68422..e7e5bc3 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
@@ -392,6 +392,11 @@
}
@Override
+ public boolean isPop() {
+ return false;
+ }
+
+ @Override
public boolean isIfNez() {
return instruction instanceof DexIfNez;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
index b77029f..b07a466 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
@@ -94,6 +94,8 @@
boolean isGoto();
+ boolean isPop();
+
boolean isIfNez();
boolean isIfEq();
