JVM optimize unneeded temporary vals

compiler/backend/src/org/jetbrains/kotlin/codegen/TransformationMethodVisitor.kt

@@ -16,17 +16,14 @@
 
 package org.jetbrains.kotlin.codegen
 
+import org.jetbrains.kotlin.codegen.inline.nodeText
+import org.jetbrains.kotlin.codegen.inline.wrapWithMaxLocalCalc
 import org.jetbrains.org.objectweb.asm.MethodVisitor
 import org.jetbrains.org.objectweb.asm.Opcodes
 import org.jetbrains.org.objectweb.asm.tree.MethodNode
 import org.jetbrains.org.objectweb.asm.util.Textifier
 import org.jetbrains.org.objectweb.asm.util.TraceMethodVisitor
 
-import java.util.ArrayList
-
-import org.jetbrains.kotlin.codegen.inline.nodeText
-import org.jetbrains.kotlin.codegen.inline.wrapWithMaxLocalCalc
-
 abstract class TransformationMethodVisitor(
     private val delegate: MethodVisitor,
     access: Int,

compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/FastMethodAnalyzer.kt

@@ -118,9 +118,17 @@ open class FastMethodAnalyzer<V : Value>(
                 }
 
             } catch (e: AnalyzerException) {
-                throw AnalyzerException(e.node, "Error at instruction #$insn ${insnNode.insnText(method.instructions)}: ${e.message}", e)
+                throw AnalyzerException(
+                    e.node,
+                    "Error at instruction #$insn ${insnNode.insnText(method.instructions)}: ${e.message}\ncurrent: ${current.dump()}",
+                    e
+                )
             } catch (e: Exception) {
-                throw AnalyzerException(insnNode, "Error at instruction #$insn ${insnNode.insnText(method.instructions)}: ${e.message}", e)
+                throw AnalyzerException(
+                    insnNode,
+                    "Error at instruction #$insn ${insnNode.insnText(method.instructions)}: ${e.message}\ncurrent: ${current.dump()}",
+                    e
+                )
             }
 
         }
@@ -245,11 +253,39 @@ open class FastMethodAnalyzer<V : Value>(
                 true
             }
             else ->
-                oldFrame.merge(frame, interpreter)
+                try {
+                    oldFrame.merge(frame, interpreter)
+                } catch (e: AnalyzerException) {
+                    throw AnalyzerException(null, "${e.message}\nframe: ${frame.dump()}\noldFrame: ${oldFrame.dump()}")
+                }
         }
         if (changes && !queued[dest]) {
             queued[dest] = true
             queue[top++] = dest
         }
     }
+
+    private fun Frame<V>.dump(): String {
+        return buildString {
+            append("{\n")
+            append("  locals: [\n")
+            for (i in 0 until method.maxLocals) {
+                append("    #$i: ${this@dump.getLocal(i)}\n")
+            }
+            append("  ]\n")
+            val stackSize = this@dump.stackSize
+            append("  stack: size=")
+            append(stackSize)
+            if (stackSize == 0) {
+                append(" []\n")
+            } else {
+                append(" [\n")
+                for (i in 0 until stackSize) {
+                    append("    #$i: ${this@dump.getStack(i)}\n")
+                }
+                append("  ]\n")
+            }
+            append("}\n")
+        }
+    }
 }

compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/temporaryVals/FastStoreLoadAnalyzer.kt

@@ -101,22 +101,6 @@ class StoreLoadFrame<V : StoreLoadValue>(val maxLocals: Int) {
     }
 }
 
-
-internal val isOpcodeRelevantForStoreLoadAnalysis = BooleanArray(256).also { a ->
-    for (i in Opcodes.ILOAD..Opcodes.ALOAD) a[i] = true
-    for (i in Opcodes.ISTORE..Opcodes.ASTORE) a[i] = true
-
-    // Relevant JumpInsnNode opcodes
-    a[Opcodes.GOTO] = true
-    a[Opcodes.IFNONNULL] = true
-    a[Opcodes.IFNULL] = true
-    for (i in Opcodes.IFEQ..Opcodes.IF_ACMPNE) a[i] = true
-
-    a[Opcodes.LOOKUPSWITCH] = true
-    a[Opcodes.TABLESWITCH] = true
-}
-
-
 @Suppress("DuplicatedCode")
 class FastStoreLoadAnalyzer<V : StoreLoadValue>(
     private val owner: String,
@@ -175,15 +159,11 @@ class FastStoreLoadAnalyzer<V : StoreLoadValue>(
                     }
                 }
 
-                // No need to analyze exception handler code for instructions that just propagate data flow information forward.
+                handlers[insn]?.forEach { tcb ->
-                if (isRelevantNode(insn, insnOpcode)) {
+                    val jump = tcb.handler.indexOf()
-                    handlers[insn]?.forEach { tcb ->
+                    handler.init(f)
-                        val jump = tcb.handler.indexOf()
+                    mergeControlFlowEdge(jump, handler)
-                        handler.init(f)
-                        mergeControlFlowEdge(jump, handler)
-                    }
                 }
-
             } catch (e: AnalyzerException) {
                 throw AnalyzerException(e.node, "Error at instruction #$insn ${insnNode.insnText(method.instructions)}: ${e.message}", e)
             } catch (e: Exception) {
@@ -195,10 +175,6 @@ class FastStoreLoadAnalyzer<V : StoreLoadValue>(
         return frames
     }
 
-    private fun isRelevantNode(insn: Int, insnOpcode: Int) =
-        isMergeNode[insn] ||
-                insnOpcode >= 0 && isOpcodeRelevantForStoreLoadAnalysis[insnOpcode]
-
     private fun newFrame(maxLocals: Int) =
         StoreLoadFrame<V>(maxLocals)
 

compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/temporaryVals/TemporaryVals.kt

@@ -6,11 +6,15 @@
 package org.jetbrains.kotlin.codegen.optimization.temporaryVals
 
 import org.jetbrains.kotlin.codegen.optimization.OptimizationMethodVisitor
+import org.jetbrains.kotlin.codegen.optimization.common.isMeaningful
 import org.jetbrains.kotlin.codegen.optimization.common.isStoreOperation
 import org.jetbrains.kotlin.utils.SmartSet
 import org.jetbrains.org.objectweb.asm.Opcodes
 import org.jetbrains.org.objectweb.asm.Type
-import org.jetbrains.org.objectweb.asm.tree.*
+import org.jetbrains.org.objectweb.asm.tree.AbstractInsnNode
+import org.jetbrains.org.objectweb.asm.tree.IincInsnNode
+import org.jetbrains.org.objectweb.asm.tree.MethodNode
+import org.jetbrains.org.objectweb.asm.tree.VarInsnNode
 
 
 // A temporary val is a local variables that is:
@@ -61,12 +65,21 @@ class TemporaryValsAnalyzer {
             }
         }
 
-        // Some coroutine transformations require exception handler to start from an ASTORE instruction.
         for (tcb in methodNode.tryCatchBlocks) {
-            val handlerFirstInsn = tcb.handler.next ?: continue
+            // Some coroutine transformations require exception handler to start from an ASTORE instruction.
-            if (handlerFirstInsn.opcode == Opcodes.ASTORE) {
+            var handlerFirstInsn: AbstractInsnNode? = tcb.handler
+            while (handlerFirstInsn != null && !handlerFirstInsn.isMeaningful) {
+                handlerFirstInsn = handlerFirstInsn.next
+            }
+            if (handlerFirstInsn != null && handlerFirstInsn.opcode == Opcodes.ASTORE) {
                 potentiallyTemporaryStores.remove(handlerFirstInsn)
             }
+            // Don't touch stack spilling at TCB start
+            var insn = tcb.start.previous
+            while (insn != null && insn.isStoreOperation()) {
+                potentiallyTemporaryStores.remove(insn)
+                insn = insn.previous
+            }
         }
 
         // If the method is big, and we couldn't eliminate enough temporary variable store candidates,

compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/temporaryVals/TemporaryVariablesEliminationTransformer.kt

@@ -151,7 +151,17 @@ class TemporaryVariablesEliminationTransformer(private val state: GenerationStat
         val insnList = cfg.methodNode.instructions
 
         for (tmp in temporaryVals) {
-            if (tmp.loadInsns.size == 1) {
+            if (tmp.loadInsns.isEmpty()) {
+                // Drop unused temporary store
+                val popOpcode = when (tmp.storeInsn.opcode) {
+                    Opcodes.ISTORE, Opcodes.FSTORE, Opcodes.ASTORE ->
+                        Opcodes.POP
+                    else ->
+                        Opcodes.POP2
+                }
+                insnList.insertBefore(tmp.storeInsn, InsnNode(popOpcode))
+                insnList.remove(tmp.storeInsn)
+            } else if (tmp.loadInsns.size == 1) {
                 val storeInsn = tmp.storeInsn
                 val loadInsn = tmp.loadInsns[0]
 

compiler/testData/codegen/bytecodeText/coroutines/doNotReassignContinuation.kt

@@ -12,4 +12,8 @@ suspend fun suspendThere(param: Int, param2: String, param3: Long): String {
     return a + b
 }
 
+// JVM_TEMPLATES
 // 1 ASTORE 4
+
+// JVM_IR_TEMPLATES
+// 0 ASTORE 4

compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt

@@ -41,8 +41,28 @@ fun box(): String {
     return result
 }
 
-// 1 LOCALVARIABLE i Ljava/lang/String; L.* 3
+
 // 1 PUTFIELD VarValueConflictsWithTableSameSortKt\$box\$1.L\$0 : Ljava/lang/Object;
+
+// We merge LVT records for two consequent branches, but we split the local over the restore code.
+// JVM_IR_TEMPLATES
+/* 1 load in VarValueConflictsWithTableSameSortKt$box$1 <init> ($completion) */
+// 1 ALOAD 2\s+INVOKESPECIAL kotlin/coroutines/jvm/internal/SuspendLambda\.\<init\>
+/* 1 load in the catch (e: Throwable) { throw e } block which is implicitly wrapped around try/finally */
+// 1 ALOAD 2\s+ATHROW
+/* 1 load in result = s */
+// 1 ALOAD 2\s+PUTFIELD kotlin/jvm/internal/Ref\$ObjectRef\.element
+/* 1 load in spill */
+// 1 ALOAD 2\s+PUTFIELD VarValueConflictsWithTableSameSortKt\$box\$1\.L\$0 : Ljava/lang/Object;
+/* 2 loads in println(s) */
+// 2 ALOAD 2\s+INVOKEVIRTUAL java/io/PrintStream.println \(Ljava/lang/Object;\)V
+/* But no further load when spilling 's' to the continuation */
+// 6 ALOAD 2
+// 1 LOCALVARIABLE i Ljava/lang/String; L.* 2
+// 3 LOCALVARIABLE s Ljava/lang/String; L.* 2
+// 1 LOCALVARIABLE \$completion Lkotlin/coroutines/Continuation; L.* 2
+
+// JVM_TEMPLATES
 /* 1 load in the catch (e: Throwable) { throw e } block which is implicitly wrapped around try/finally */
 // 1 ALOAD 3\s+ATHROW
 /* 1 load in result = s */
@@ -53,9 +73,5 @@ fun box(): String {
 // 2 ALOAD 3\s+INVOKEVIRTUAL java/io/PrintStream.println \(Ljava/lang/Object;\)V
 /* But no further load when spilling 's' to the continuation */
 // 5 ALOAD 3
-
+// 1 LOCALVARIABLE i Ljava/lang/String; L.* 3
-// We merge LVT records for two consequent branches, but we split the local over the restore code.
-// JVM_IR_TEMPLATES
-// 3 LOCALVARIABLE s Ljava/lang/String; L.* 3
-// JVM_TEMPLATES
 // 4 LOCALVARIABLE s Ljava/lang/String; L.* 3

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

@@ -17,8 +17,17 @@ suspend fun ApplicationCall.test(authenticationService: AuthenticationService) {
     respond(authenticationService.execute(receiveJSON()))
 }
 
+// JVM_TEMPLATES
 // $i$f$respond x1, $i$f$receiveJSON x2: before and after suspension point
 // 3 ISTORE 5
 // 0 ILOAD 5
 // 2 \$i\$f\$receiveJSON I .* 5
 // 1 \$i\$f\$respond I .* 5
+
+// JVM_IR_TEMPLATES
+// 5 ISTORE 3
+// 1 ISTORE 2
+// 0 ILOAD 3
+// 0 ILOAD 2
+// 1 \$i\$f\$receiveJSON I .* 2
+// 3 \$i\$f\$respond I .* 3

compiler/testData/codegen/bytecodeText/storeStackBeforeInline/unreachableMarker.kt

@@ -21,6 +21,13 @@ fun foo() : String {
     )
 }
 
+// JVM_TEMPLATES
 // 12 ALOAD
 // 2 ASTORE
 // 0 InlineMarker
+
+// JVM_IR_TEMPLATES
+// 11 ALOAD
+// 1 ASTORE
+// 0 InlineMarker
+

compiler/testData/codegen/bytecodeText/storeStackBeforeInline/withLambda.kt

@@ -11,7 +11,14 @@ fun foo() : String {
     return foobar("abc", bar("ghi") { x -> x + "jkl" }, "mno")
 }
 
+// JVM_TEMPLATES
 // 6 ASTORE
 // 18 ALOAD
 // 1 MAXLOCALS = 7
 // 0 InlineMarker
+
+// JVM_IR_TEMPLATES
+// 4 ASTORE
+// 16 ALOAD
+// 1 MAXLOCALS = 7
+// 0 InlineMarker