KT-17110 Rewrite branches targeting other branches
- Rewrite branches targeting other branches to target directly the final destination to avoid runtime performance penalties. Fix of https://youtrack.jetbrains.com/issue/KT-17110
This commit is contained in:
committed by
Dmitry Petrov
parent
c9624ce0f9
commit
9fb0f59813
+53
-8
@@ -22,31 +22,76 @@ import org.jetbrains.org.objectweb.asm.Opcodes
|
||||
import org.jetbrains.org.objectweb.asm.tree.AbstractInsnNode
|
||||
import org.jetbrains.org.objectweb.asm.tree.JumpInsnNode
|
||||
import org.jetbrains.org.objectweb.asm.tree.LabelNode
|
||||
import org.jetbrains.org.objectweb.asm.tree.LineNumberNode
|
||||
import org.jetbrains.org.objectweb.asm.tree.MethodNode
|
||||
|
||||
class RedundantGotoMethodTransformer : MethodTransformer() {
|
||||
/**
|
||||
* Removes redundant GOTO's, i.e. to subsequent labels
|
||||
* Removes redundant GOTO's in the following cases:
|
||||
* (1) subsequent labels
|
||||
* ...
|
||||
* goto Label (can be removed)
|
||||
* Label:
|
||||
* ...
|
||||
* (2) indirect goto
|
||||
* ...
|
||||
* <branch instruction> Label (can be rewrote to <branch instruction> Label2)
|
||||
* ...
|
||||
* Label:
|
||||
* goto Label2 (must not be removed due to the previous instruction that can fallthrough on this goto)
|
||||
* ...
|
||||
*/
|
||||
override fun transform(internalClassName: String, methodNode: MethodNode) {
|
||||
val insns = methodNode.instructions.toArray().apply { reverse() }
|
||||
val insnsToRemove = arrayListOf<AbstractInsnNode>()
|
||||
|
||||
var insnsToRemove = arrayListOf<AbstractInsnNode>()
|
||||
val currentLabels = hashSetOf<LabelNode>()
|
||||
var labelsToReplace = hashMapOf<LabelNode, JumpInsnNode>()
|
||||
var pendingGoto: JumpInsnNode? = null
|
||||
|
||||
for (insn in insns) {
|
||||
when {
|
||||
insn is LabelNode ->
|
||||
insn is LabelNode -> {
|
||||
currentLabels.add(insn)
|
||||
insn.opcode == Opcodes.GOTO &&
|
||||
(insn as JumpInsnNode).label in currentLabels ->
|
||||
insnsToRemove.add(insn)
|
||||
insn.isMeaningful ->
|
||||
pendingGoto?.let { labelsToReplace[insn] = it }
|
||||
}
|
||||
insn.opcode == Opcodes.GOTO -> {
|
||||
pendingGoto = insn as JumpInsnNode
|
||||
if (insn.label in currentLabels) {
|
||||
insnsToRemove.add(insn)
|
||||
} else {
|
||||
currentLabels.clear()
|
||||
}
|
||||
}
|
||||
insn is LineNumberNode -> pendingGoto = null
|
||||
insn.isMeaningful -> {
|
||||
currentLabels.clear()
|
||||
pendingGoto = null
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Rewrite branch instructions.
|
||||
if (!labelsToReplace.isEmpty()) {
|
||||
insns.filter { it is JumpInsnNode }.forEach { rewriteLabelIfNeeded(it as JumpInsnNode, labelsToReplace) }
|
||||
}
|
||||
|
||||
for (insnToRemove in insnsToRemove) {
|
||||
methodNode.instructions.remove(insnToRemove)
|
||||
}
|
||||
}
|
||||
|
||||
private fun rewriteLabelIfNeeded(
|
||||
jumpInsn: JumpInsnNode,
|
||||
labelsToReplace: HashMap<LabelNode, JumpInsnNode>
|
||||
) {
|
||||
getLastTargetJumpInsn(jumpInsn, labelsToReplace).takeIf { it.label != jumpInsn.label }?.let {
|
||||
// Do not remove the old label because it can be used to define a local variable range.
|
||||
jumpInsn.label = it.label
|
||||
}
|
||||
}
|
||||
|
||||
private fun getLastTargetJumpInsn(jumpInsn: JumpInsnNode, labelsToReplace: HashMap<LabelNode, JumpInsnNode>): JumpInsnNode {
|
||||
labelsToReplace[jumpInsn.label]?.let { return getLastTargetJumpInsn(it, labelsToReplace) }
|
||||
return jumpInsn
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,20 @@
|
||||
|
||||
fun test(x: Int, y: Int): String {
|
||||
var result: String
|
||||
if (x == 6) {
|
||||
if (y == 6) {
|
||||
result = "a"
|
||||
} else {
|
||||
result = "b"
|
||||
}
|
||||
} else {
|
||||
result = "c"
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
fun box(): String {
|
||||
if (test(9, 10) != "c")
|
||||
return "Failures"
|
||||
return "OK"
|
||||
}
|
||||
@@ -0,0 +1,21 @@
|
||||
fun test(x: Int, y: Int): String {
|
||||
var result: String
|
||||
if (x == 6) {
|
||||
if (y == 6) {
|
||||
result = "a"
|
||||
} else {
|
||||
result = "b"
|
||||
}
|
||||
} else {
|
||||
result = "c"
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
fun box(): String {
|
||||
if (test(9, 10) != "c")
|
||||
return "Failures"
|
||||
return "OK"
|
||||
}
|
||||
|
||||
// 2 GOTO L7
|
||||
Generated
+6
@@ -4800,6 +4800,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
|
||||
doTest(fileName);
|
||||
}
|
||||
|
||||
@TestMetadata("kt17110.kt")
|
||||
public void testKt17110() throws Exception {
|
||||
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt17110.kt");
|
||||
doTest(fileName);
|
||||
}
|
||||
|
||||
@TestMetadata("kt1742.kt")
|
||||
public void testKt1742() throws Exception {
|
||||
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt1742.kt");
|
||||
|
||||
+6
@@ -4800,6 +4800,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
|
||||
doTest(fileName);
|
||||
}
|
||||
|
||||
@TestMetadata("kt17110.kt")
|
||||
public void testKt17110() throws Exception {
|
||||
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt17110.kt");
|
||||
doTest(fileName);
|
||||
}
|
||||
|
||||
@TestMetadata("kt1742.kt")
|
||||
public void testKt1742() throws Exception {
|
||||
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt1742.kt");
|
||||
|
||||
@@ -1069,6 +1069,21 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest {
|
||||
}
|
||||
}
|
||||
|
||||
@TestMetadata("compiler/testData/codegen/bytecodeText/controlStructures")
|
||||
@TestDataPath("$PROJECT_ROOT")
|
||||
@RunWith(JUnit3RunnerWithInners.class)
|
||||
public static class ControlStructures extends AbstractBytecodeTextTest {
|
||||
public void testAllFilesPresentInControlStructures() throws Exception {
|
||||
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/bytecodeText/controlStructures"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true);
|
||||
}
|
||||
|
||||
@TestMetadata("kt17110.kt")
|
||||
public void testKt17110() throws Exception {
|
||||
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/controlStructures/kt17110.kt");
|
||||
doTest(fileName);
|
||||
}
|
||||
}
|
||||
|
||||
@TestMetadata("compiler/testData/codegen/bytecodeText/coroutines")
|
||||
@TestDataPath("$PROJECT_ROOT")
|
||||
@RunWith(JUnit3RunnerWithInners.class)
|
||||
|
||||
+6
@@ -4800,6 +4800,12 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes
|
||||
doTest(fileName);
|
||||
}
|
||||
|
||||
@TestMetadata("kt17110.kt")
|
||||
public void testKt17110() throws Exception {
|
||||
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt17110.kt");
|
||||
doTest(fileName);
|
||||
}
|
||||
|
||||
@TestMetadata("kt1742.kt")
|
||||
public void testKt1742() throws Exception {
|
||||
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt1742.kt");
|
||||
|
||||
+6
@@ -5334,6 +5334,12 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
|
||||
doTest(fileName);
|
||||
}
|
||||
|
||||
@TestMetadata("kt17110.kt")
|
||||
public void testKt17110() throws Exception {
|
||||
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt17110.kt");
|
||||
doTest(fileName);
|
||||
}
|
||||
|
||||
@TestMetadata("kt1742.kt")
|
||||
public void testKt1742() throws Exception {
|
||||
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt1742.kt");
|
||||
|
||||
Reference in New Issue
Block a user