From b4d356c5bd7dd769e6702e0aaa0b892fecaa2e93 Mon Sep 17 00:00:00 2001 From: Ilmir Usmanov Date: Sun, 18 Jul 2021 19:44:10 +0200 Subject: [PATCH] Split LVT record for known nulls Since they are not spilled, the logic for splitting LVT records, that is applied for spilled variables, was not applied for known nulls. Fix that by applying the logic to them. #KT-47749 --- .../CoroutineTransformerMethodVisitor.kt | 66 ++++++++++--------- .../coroutines/varValueConflictsWithTable.kt | 6 +- .../varValueConflictsWithTableSameSort.kt | 8 ++- 3 files changed, 47 insertions(+), 33 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt index fde67d392f2..246550708eb 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt @@ -791,19 +791,30 @@ class CoroutineTransformerMethodVisitor( // Mutate method node fun generateSpillAndUnspill(suspension: SuspensionPoint, slot: Int, spillableVariable: SpillableVariable?) { - if (spillableVariable == null) { - with(instructions) { - insert(suspension.tryCatchBlockEndLabelAfterSuspensionCall, withInstructionAdapter { - aconst(null) - store(slot, AsmTypes.OBJECT_TYPE) - }) + fun splitLvtRecord(local: LocalVariableNode?, localRestart: LabelNode) { + // Split the local variable range for the local so that it is visible until the next state label, but is + // not visible until it has been unspilled from the continuation on the reentry path. + if (local != null) { + val previousEnd = local.end + local.end = suspension.stateLabel + // Add the local back, but end it at the next state label. + methodNode.localVariables.add(local) + // Add a new entry that starts after the local variable is restored from the continuation. + methodNode.localVariables.add( + LocalVariableNode( + local.name, + local.desc, + local.signature, + localRestart, + previousEnd, + local.index + ) + ) } - return } // Find and remove the local variable node, if any, in the local variable table corresponding to the slot that is spilled. var local: LocalVariableNode? = null - val localRestart = LabelNode().linkWithLabel() val iterator = methodNode.localVariables.listIterator() while (iterator.hasNext()) { val node = iterator.next() @@ -817,6 +828,19 @@ class CoroutineTransformerMethodVisitor( } } + if (spillableVariable == null) { + with(instructions) { + insert(suspension.tryCatchBlockEndLabelAfterSuspensionCall, withInstructionAdapter { + aconst(null) + store(slot, AsmTypes.OBJECT_TYPE) + }) + } + val newStart = suspension.tryCatchBlocksContinuationLabel.findNextOrNull { it is LabelNode } as? LabelNode ?: return + splitLvtRecord(local, newStart) + return + } + + val localRestart = LabelNode().linkWithLabel() with(instructions) { // store variable before suspension call insertBefore(suspension.suspensionCallBegin, withInstructionAdapter { @@ -846,25 +870,7 @@ class CoroutineTransformerMethodVisitor( }) } - // Split the local variable range for the local so that it is visible until the next state label, but is - // not visible until it has been unspilled from the continuation on the reentry path. - if (local != null) { - val previousEnd = local.end - local.end = suspension.stateLabel - // Add the local back, but end it at the next state label. - methodNode.localVariables.add(local) - // Add a new entry that starts after the local variable is restored from the continuation. - methodNode.localVariables.add( - LocalVariableNode( - local.name, - local.desc, - local.signature, - localRestart, - previousEnd, - local.index - ) - ) - } + splitLvtRecord(local, localRestart) } fun cleanUpField(suspension: SuspensionPoint, fieldIndex: Int) { @@ -1254,6 +1260,9 @@ internal fun replaceFakeContinuationsWithRealOnes(methodNode: MethodNode, contin } } +private fun MethodNode.nodeTextWithLiveness(liveness: List): String = + liveness.zip(this.instructions.asSequence().toList()).joinToString("\n") { (a, b) -> "$a|${b.insnText}" } + /* We do not want to spill dead variables, thus, we shrink its LVT record to region, where the variable is alive, * so, the variable will not be visible in debugger. User can still prolong life span of the variable by using it. * @@ -1288,9 +1297,6 @@ private fun updateLvtAccordingToLiveness(method: MethodNode, isForNamedFunction: fun min(a: LabelNode, b: LabelNode): LabelNode = if (method.instructions.indexOf(a) < method.instructions.indexOf(b)) a else b - fun max(a: LabelNode, b: LabelNode): LabelNode = - if (method.instructions.indexOf(a) < method.instructions.indexOf(b)) b else a - val oldLvt = arrayListOf() for (record in method.localVariables) { oldLvt += record diff --git a/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTable.kt b/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTable.kt index 9368b56d40f..b92d1eb0e52 100644 --- a/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTable.kt +++ b/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTable.kt @@ -46,7 +46,11 @@ fun box(): String { } // 1 LOCALVARIABLE i I L.* 2 -// 1 LOCALVARIABLE s Ljava/lang/String; L.* 2 // 0 PUTFIELD VarValueConflictsWithTableKt\$box\$1.I\$0 : I /* 2 loads in cycle */ // 2 ILOAD 2 + +// JVM_IR_TEMPLATES +// 1 LOCALVARIABLE s Ljava/lang/String; L.* 2 +// JVM_TEMPLATES +// 2 LOCALVARIABLE s Ljava/lang/String; L.* 2 \ No newline at end of file diff --git a/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt b/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt index c158d26224e..98890b07764 100644 --- a/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt +++ b/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt @@ -42,8 +42,6 @@ fun box(): String { } // 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. -// 3 LOCALVARIABLE s Ljava/lang/String; L.* 3 // 1 PUTFIELD VarValueConflictsWithTableSameSortKt\$box\$1.L\$0 : Ljava/lang/Object; /* 1 load in the catch (e: Throwable) { throw e } block which is implicitly wrapped around try/finally */ // 1 ALOAD 3\s+ATHROW @@ -55,3 +53,9 @@ 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 +// 4 LOCALVARIABLE s Ljava/lang/String; L.* 3