From 4dd100122b186abe12c78ca0a59f052259044814 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Wed, 11 Jan 2017 17:11:41 +0300 Subject: [PATCH] Explicitly remove NOPs inserted for bytecode analysis in post-conditional loops. Remove redundant NOPs during bytecode optimization. NOP instruction is required iff one of the following is true: (a) it is a first bytecode instruction in a try-catch block (JVM BE assumption); (b) it is a sole bytecode instruction in a source code line (breakpoints on that line will not work). All other NOP instructions can be removed. Note that it doesn't really affect the performance for mature JVM implementations. However, the perceived quality of the generated code is somewhat improved :). Related: KT-15609 --- .../OptimizationMethodVisitor.java | 3 +- .../RedundantNopsCleanupMethodTransformer.kt | 102 ++++++++++++++++++ .../fixStack/AnalyzeTryCatchBlocks.kt | 2 +- .../optimization/fixStack/FixStackAnalyzer.kt | 14 +-- .../optimization/fixStack/FixStackContext.kt | 2 + .../fixStack/FixStackMethodTransformer.kt | 4 + .../nopInlineFuns.kt | 2 +- ...ForNoParametersArgumentCallInExpression.kt | 2 +- .../codegen/bytecodeText/nopsInDoWhile.kt | 10 ++ .../custom/beforeGotoToWhileStart.kt | 2 +- .../codegen/BytecodeTextTestGenerated.java | 6 ++ .../semantics/JsCodegenBoxTestGenerated.java | 8 +- 12 files changed, 142 insertions(+), 15 deletions(-) create mode 100644 compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/RedundantNopsCleanupMethodTransformer.kt create mode 100644 compiler/testData/codegen/bytecodeText/nopsInDoWhile.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/OptimizationMethodVisitor.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/OptimizationMethodVisitor.java index f055261691f..13e067f51d6 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/OptimizationMethodVisitor.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/OptimizationMethodVisitor.java @@ -37,7 +37,8 @@ public class OptimizationMethodVisitor extends TransformationMethodVisitor { new RedundantBoxingMethodTransformer(), new RedundantCoercionToUnitTransformer(), new DeadCodeEliminationMethodTransformer(), - new RedundantGotoMethodTransformer() + new RedundantGotoMethodTransformer(), + new RedundantNopsCleanupMethodTransformer() }; private final boolean disableOptimization; diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/RedundantNopsCleanupMethodTransformer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/RedundantNopsCleanupMethodTransformer.kt new file mode 100644 index 00000000000..b2e7142d5ef --- /dev/null +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/RedundantNopsCleanupMethodTransformer.kt @@ -0,0 +1,102 @@ +/* + * Copyright 2010-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.kotlin.codegen.optimization + +import org.jetbrains.kotlin.codegen.optimization.common.findNextOrNull +import org.jetbrains.kotlin.codegen.optimization.common.isMeaningful +import org.jetbrains.kotlin.codegen.optimization.transformer.MethodTransformer +import org.jetbrains.kotlin.utils.addIfNotNull +import org.jetbrains.org.objectweb.asm.Opcodes +import org.jetbrains.org.objectweb.asm.tree.AbstractInsnNode +import org.jetbrains.org.objectweb.asm.tree.LineNumberNode +import org.jetbrains.org.objectweb.asm.tree.MethodNode + +class RedundantNopsCleanupMethodTransformer : MethodTransformer() { + override fun transform(internalClassName: String, methodNode: MethodNode) { + // NOP instruction is required, iff one of the following conditions is true: + // (a) it is a sole bytecode instruction in a try-catch block (TCB) + // (b) it is a sole bytecode instruction is a source code line + + val requiredNops = HashSet() + + recordNopsRequiredForSourceCodeLines(methodNode.instructions.first, requiredNops) + recordNopsRequiredForTryCatchBlocks(methodNode, requiredNops) + + var current: AbstractInsnNode? = methodNode.instructions.first + while (current != null) { + if (current.opcode == Opcodes.NOP && !requiredNops.contains(current)) { + val toRemove = current + current = current.next + methodNode.instructions.remove(toRemove) + } + else { + current = current.next + } + } + } + + private fun recordNopsRequiredForSourceCodeLines(first: AbstractInsnNode, requiredNops: MutableSet) { + var current: AbstractInsnNode? = first + while (current != null) { + if (current is LineNumberNode) { + val nextLineNumberNode = current.getNextLineNumberNode() + requiredNops.addIfNotNull(getRequiredNopInRange(current, nextLineNumberNode)) + current = nextLineNumberNode + } + else { + current = current.next + } + } + } + + private fun recordNopsRequiredForTryCatchBlocks(methodNode: MethodNode, requiredNops: MutableSet) { + for (tcb in methodNode.tryCatchBlocks) { + val nop = tcb.start.findNextOrNull { it.isMeaningful } + if (nop?.opcode == Opcodes.NOP) { + requiredNops.add(nop) + } + } + } +} + + +internal fun LineNumberNode.getNextLineNumberNode(): LineNumberNode? { + var current: AbstractInsnNode? = this + while (current != null) { + if (current is LineNumberNode && current.line != this.line) { + return current + } + current = current.next + } + return null +} + +internal fun getRequiredNopInRange(firstInclusive: AbstractInsnNode, lastExclusive: AbstractInsnNode?): AbstractInsnNode? { + var lastNop: AbstractInsnNode? = null + var current: AbstractInsnNode? = firstInclusive + while (current != null && current != lastExclusive) { + if (current.isMeaningful && current.opcode != Opcodes.NOP) { + return null + } + else if (current.opcode == Opcodes.NOP) { + lastNop = current + } + current = current.next + } + + return lastNop +} diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/AnalyzeTryCatchBlocks.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/AnalyzeTryCatchBlocks.kt index 300b7ab9aa0..ce3f1ef91be 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/AnalyzeTryCatchBlocks.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/AnalyzeTryCatchBlocks.kt @@ -16,11 +16,11 @@ package org.jetbrains.kotlin.codegen.optimization.fixStack -import com.sun.xml.internal.ws.org.objectweb.asm.Opcodes import org.jetbrains.kotlin.codegen.optimization.common.findNextOrNull import org.jetbrains.kotlin.codegen.optimization.common.hasOpcode import org.jetbrains.kotlin.codegen.pseudoInsns.PseudoInsn import org.jetbrains.org.objectweb.asm.Label +import org.jetbrains.org.objectweb.asm.Opcodes import org.jetbrains.org.objectweb.asm.tree.AbstractInsnNode import org.jetbrains.org.objectweb.asm.tree.LabelNode import org.jetbrains.org.objectweb.asm.tree.MethodNode diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackAnalyzer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackAnalyzer.kt index 80c1f9ed3ae..8c480c81ef7 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackAnalyzer.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackAnalyzer.kt @@ -55,19 +55,15 @@ internal class FixStackAnalyzer( } private fun preprocess() { - var current: AbstractInsnNode? = method.instructions.first - while (current != null) { - val next = current.next - if (PseudoInsn.FAKE_ALWAYS_FALSE_IFEQ.isa(current) && next is JumpInsnNode) { + for (marker in context.fakeAlwaysFalseIfeqMarkers) { + val next = marker.next + if (next is JumpInsnNode) { val nop = InsnNode(Opcodes.NOP) expectedStackNode[next.label] = nop method.instructions.insert(next, nop) - method.instructions.remove(current) + method.instructions.remove(marker) method.instructions.remove(next) - current = nop.next - } - else { - current = current.next + context.nodesToRemoveOnCleanup.add(nop) } } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackContext.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackContext.kt index 71986dbadde..31b414f4919 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackContext.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackContext.kt @@ -39,6 +39,8 @@ internal class FixStackContext(val methodNode: MethodNode) { val openingInlineMethodMarker = hashMapOf() var consistentInlineMarkers: Boolean = true; private set + val nodesToRemoveOnCleanup = arrayListOf() + init { saveStackMarkerForRestoreMarker = insertTryCatchBlocksMarkers(methodNode) isThereAnyTryCatch = saveStackMarkerForRestoreMarker.isNotEmpty() diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackMethodTransformer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackMethodTransformer.kt index a2545583e5a..ddbb1a724e3 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackMethodTransformer.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackMethodTransformer.kt @@ -60,6 +60,10 @@ class FixStackMethodTransformer : MethodTransformer() { context.fakeAlwaysFalseIfeqMarkers.forEach { marker -> removeAlwaysFalseIfeq(methodNode, marker) } + + context.nodesToRemoveOnCleanup.forEach { + methodNode.instructions.remove(it) + } } private fun transformBreakContinueGotos( diff --git a/compiler/testData/codegen/bytecodeText/coercionToUnitOptimization/nopInlineFuns.kt b/compiler/testData/codegen/bytecodeText/coercionToUnitOptimization/nopInlineFuns.kt index fd0484340e0..6bd370efa41 100644 --- a/compiler/testData/codegen/bytecodeText/coercionToUnitOptimization/nopInlineFuns.kt +++ b/compiler/testData/codegen/bytecodeText/coercionToUnitOptimization/nopInlineFuns.kt @@ -23,4 +23,4 @@ fun simpleFunVoid(f: () -> Unit): Unit { return f() } -// 5 NOP +// 4 NOP diff --git a/compiler/testData/codegen/bytecodeText/inline/linenumberForNoParametersArgumentCallInExpression.kt b/compiler/testData/codegen/bytecodeText/inline/linenumberForNoParametersArgumentCallInExpression.kt index 5506819e5e4..9cac096c941 100644 --- a/compiler/testData/codegen/bytecodeText/inline/linenumberForNoParametersArgumentCallInExpression.kt +++ b/compiler/testData/codegen/bytecodeText/inline/linenumberForNoParametersArgumentCallInExpression.kt @@ -11,4 +11,4 @@ inline fun lookAtMe(f: () -> Int): Int { } // TODO: Less NOPs is better -// 2 NOP \ No newline at end of file +// 1 NOP \ No newline at end of file diff --git a/compiler/testData/codegen/bytecodeText/nopsInDoWhile.kt b/compiler/testData/codegen/bytecodeText/nopsInDoWhile.kt new file mode 100644 index 00000000000..b15d9c8f9cb --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/nopsInDoWhile.kt @@ -0,0 +1,10 @@ +fun test() { + var i = 0 + do { + i++ + if (i > 5) break + if (i < 3) continue + } while (true) +} + +// 0 NOP \ No newline at end of file diff --git a/compiler/testData/lineNumber/custom/beforeGotoToWhileStart.kt b/compiler/testData/lineNumber/custom/beforeGotoToWhileStart.kt index 0cb7ee0dfd4..6427ee7de86 100644 --- a/compiler/testData/lineNumber/custom/beforeGotoToWhileStart.kt +++ b/compiler/testData/lineNumber/custom/beforeGotoToWhileStart.kt @@ -10,4 +10,4 @@ fun testSome(): Boolean { return false } -// 2 3 4 2 7 10 \ No newline at end of file +// 2 +3 4 2 7 10 \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index f7142d72e3d..8f827a84260 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -300,6 +300,12 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { doTest(fileName); } + @TestMetadata("nopsInDoWhile.kt") + public void testNopsInDoWhile() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/nopsInDoWhile.kt"); + doTest(fileName); + } + @TestMetadata("partMembersCall.kt") public void testPartMembersCall() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/partMembersCall.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index 28ff1a03716..2724c8fc4af 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -8441,7 +8441,13 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { @TestMetadata("kt15112.kt") public void testKt15112() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/fullJdk/regressions/kt15112.kt"); - doTest(fileName); + try { + doTest(fileName); + } + catch (Throwable ignore) { + return; + } + throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that."); } @TestMetadata("kt1770.kt")