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

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;

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<AbstractInsnNode>()
+
+        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<AbstractInsnNode>) {
+        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<AbstractInsnNode>) {
+        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
+}

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

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)
             }
         }
 

compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackContext.kt

@@ -39,6 +39,8 @@ internal class FixStackContext(val methodNode: MethodNode) {
     val openingInlineMethodMarker = hashMapOf<AbstractInsnNode, AbstractInsnNode>()
     var consistentInlineMarkers: Boolean = true; private set
 
+    val nodesToRemoveOnCleanup = arrayListOf<AbstractInsnNode>()
+
     init {
         saveStackMarkerForRestoreMarker = insertTryCatchBlocksMarkers(methodNode)
         isThereAnyTryCatch = saveStackMarkerForRestoreMarker.isNotEmpty()

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(

compiler/testData/codegen/bytecodeText/coercionToUnitOptimization/nopInlineFuns.kt

@@ -23,4 +23,4 @@ fun simpleFunVoid(f: () -> Unit): Unit {
     return f()
 }
 
-// 5 NOP
+// 4 NOP

compiler/testData/codegen/bytecodeText/inline/linenumberForNoParametersArgumentCallInExpression.kt

@@ -11,4 +11,4 @@ inline fun lookAtMe(f: () -> Int): Int {
 }
 
 // TODO: Less NOPs is better
-// 2 NOP
+// 1 NOP

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

compiler/testData/lineNumber/custom/beforeGotoToWhileStart.kt

@@ -10,4 +10,4 @@ fun testSome(): Boolean {
     return false
 }
 
-// 2 3 4 2 7 10
+// 2 +3 4 2 7 10

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");

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")