From f4a1e27124f77b2ffca576f7393218373c6ae085 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 23 Aug 2021 16:59:17 +0300 Subject: [PATCH] JVM optimize unneeded temporary vals --- .../codegen/TransformationMethodVisitor.kt | 7 +--- .../optimization/common/FastMethodAnalyzer.kt | 42 +++++++++++++++++-- .../temporaryVals/FastStoreLoadAnalyzer.kt | 32 ++------------ .../temporaryVals/TemporaryVals.kt | 21 ++++++++-- ...emporaryVariablesEliminationTransformer.kt | 12 +++++- .../coroutines/doNotReassignContinuation.kt | 4 ++ .../varValueConflictsWithTableSameSort.kt | 28 ++++++++++--- .../inline/inlineSuspendReifiedNoSpilling.kt | 9 ++++ .../unreachableMarker.kt | 7 ++++ .../storeStackBeforeInline/withLambda.kt | 7 ++++ 10 files changed, 122 insertions(+), 47 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/TransformationMethodVisitor.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/TransformationMethodVisitor.kt index acbbd60fb84..c46cc10f99d 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/TransformationMethodVisitor.kt +++ b/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, diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/FastMethodAnalyzer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/FastMethodAnalyzer.kt index 5a52086427c..651026cc1c9 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/FastMethodAnalyzer.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/FastMethodAnalyzer.kt @@ -118,9 +118,17 @@ open class FastMethodAnalyzer( } } 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( 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.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") + } + } } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/temporaryVals/FastStoreLoadAnalyzer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/temporaryVals/FastStoreLoadAnalyzer.kt index b6b958556a5..9e35f4f47d1 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/temporaryVals/FastStoreLoadAnalyzer.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/temporaryVals/FastStoreLoadAnalyzer.kt @@ -101,22 +101,6 @@ class StoreLoadFrame(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( private val owner: String, @@ -175,15 +159,11 @@ class FastStoreLoadAnalyzer( } } - // No need to analyze exception handler code for instructions that just propagate data flow information forward. - if (isRelevantNode(insn, insnOpcode)) { - handlers[insn]?.forEach { tcb -> - val jump = tcb.handler.indexOf() - handler.init(f) - mergeControlFlowEdge(jump, handler) - } + handlers[insn]?.forEach { tcb -> + val jump = tcb.handler.indexOf() + 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( return frames } - private fun isRelevantNode(insn: Int, insnOpcode: Int) = - isMergeNode[insn] || - insnOpcode >= 0 && isOpcodeRelevantForStoreLoadAnalysis[insnOpcode] - private fun newFrame(maxLocals: Int) = StoreLoadFrame(maxLocals) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/temporaryVals/TemporaryVals.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/temporaryVals/TemporaryVals.kt index 8ea8cf64499..9a6ef01e4ec 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/temporaryVals/TemporaryVals.kt +++ b/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 - if (handlerFirstInsn.opcode == Opcodes.ASTORE) { + // Some coroutine transformations require exception handler to start from an ASTORE instruction. + 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, diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/temporaryVals/TemporaryVariablesEliminationTransformer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/temporaryVals/TemporaryVariablesEliminationTransformer.kt index 66e380371dc..d1f61047c66 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/temporaryVals/TemporaryVariablesEliminationTransformer.kt +++ b/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] diff --git a/compiler/testData/codegen/bytecodeText/coroutines/doNotReassignContinuation.kt b/compiler/testData/codegen/bytecodeText/coroutines/doNotReassignContinuation.kt index 239a7bf0f76..30e8dcd478b 100644 --- a/compiler/testData/codegen/bytecodeText/coroutines/doNotReassignContinuation.kt +++ b/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 diff --git a/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt b/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt index 98890b07764..796b8ffeded 100644 --- a/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt +++ b/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 ($completion) */ +// 1 ALOAD 2\s+INVOKESPECIAL kotlin/coroutines/jvm/internal/SuspendLambda\.\ +/* 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 - -// 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 +// 1 LOCALVARIABLE i Ljava/lang/String; L.* 3 // 4 LOCALVARIABLE s Ljava/lang/String; L.* 3 diff --git a/compiler/testData/codegen/bytecodeText/inline/inlineSuspendReifiedNoSpilling.kt b/compiler/testData/codegen/bytecodeText/inline/inlineSuspendReifiedNoSpilling.kt index dec6a9ac299..10c85993a8b 100644 --- a/compiler/testData/codegen/bytecodeText/inline/inlineSuspendReifiedNoSpilling.kt +++ b/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 diff --git a/compiler/testData/codegen/bytecodeText/storeStackBeforeInline/unreachableMarker.kt b/compiler/testData/codegen/bytecodeText/storeStackBeforeInline/unreachableMarker.kt index ece1e46f660..63cef9b08b1 100644 --- a/compiler/testData/codegen/bytecodeText/storeStackBeforeInline/unreachableMarker.kt +++ b/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 + diff --git a/compiler/testData/codegen/bytecodeText/storeStackBeforeInline/withLambda.kt b/compiler/testData/codegen/bytecodeText/storeStackBeforeInline/withLambda.kt index 5568b8ae2a8..0b3f37c4786 100644 --- a/compiler/testData/codegen/bytecodeText/storeStackBeforeInline/withLambda.kt +++ b/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