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 bcc3df5244d..530698279d2 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt @@ -111,13 +111,13 @@ class CoroutineTransformerMethodVisitor( addCompletionParameterToLVT(methodNode) } - val examined = ExaminedMethodNode( + val examiner = MethodNodeExaminer( languageVersionSettings, containingClassInternalName, methodNode ) - if (examined.allSuspensionPointsAreTailCalls(suspensionPoints)) { - examined.replacePopsBeforeSafeUnitInstancesWithCoroutineSuspendedChecks() + if (examiner.allSuspensionPointsAreTailCalls(suspensionPoints)) { + examiner.replacePopsBeforeSafeUnitInstancesWithCoroutineSuspendedChecks() dropSuspensionMarkers(methodNode, suspensionPoints) return } @@ -821,62 +821,80 @@ class CoroutineTransformerMethodVisitor( } // TODO Use this in variable liveness analysis -private class ExaminedMethodNode( +private class MethodNodeExaminer( val languageVersionSettings: LanguageVersionSettings, val containingClassInternalName: String, val methodNode: MethodNode ) { - // DO NOT REORDER: pops and areturns collecting depends on unit instances collecting - // Which, in turn depends on frames and cfg - val sourceFrames: Array?> = + private val sourceFrames: Array?> = MethodTransformer.analyze(containingClassInternalName, methodNode, IgnoringCopyOperationSourceInterpreter()) - val controlFlowGraph = ControlFlowGraph.build(methodNode) + private val controlFlowGraph = ControlFlowGraph.build(methodNode) - private val safeUnitInstances = collectSafeUnitInstances() - private val popsBeforeSafeUnitInstances = collectPopsBeforeSafeUnitInstances() - private val areturnsAfterSafeUnitInstances = collectAreturnsAfterSafeUnitInstances() + private val safeUnitInstances = mutableSetOf() + private val popsBeforeSafeUnitInstances = mutableSetOf() + private val areturnsAfterSafeUnitInstances = mutableSetOf() + private val meaningfulSuccessorsCache = hashMapOf>() + private val meaningfulPredecessorsCache = hashMapOf>() - private fun collectSafeUnitInstances() = methodNode.instructions.asSequence().filter { unit -> - unit.isUnitInstance() && - methodNode.instructions.asSequence().any { insn -> - insn.opcode == Opcodes.POP && insn.meaningfulSuccessors().let { succs -> - succs.all { it.isUnitInstance() } && unit in succs - } - } && unit.meaningfulSuccessors().all { it.opcode == Opcodes.ARETURN } - }.toList() - - private fun collectPopsBeforeSafeUnitInstances() = methodNode.instructions.asSequence().filter { pop -> - pop.opcode == Opcodes.POP && pop.meaningfulSuccessors().all { it.isSafeUnitInstance() } - }.toList() - - private fun collectAreturnsAfterSafeUnitInstances() = methodNode.instructions.asSequence().filter { areturn -> - areturn.opcode == Opcodes.ARETURN && sourceFrames[areturn.index()]?.top()?.insns?.all { it.isSafeUnitInstance() } == true + init { + // retrieve all POP insns + val pops = methodNode.instructions.asSequence().filter { it.opcode == Opcodes.POP } + // for each of them check that all successors are PUSH Unit + val popsBeforeUnitInstances = pops.map { it to it.meaningfulSuccessors() } + .filter { (_, succs) -> succs.all { it.isUnitInstance() } } + .map { it.first }.toList() + for (pop in popsBeforeUnitInstances) { + val units = pop.meaningfulSuccessors() + val allUnitsAreSafe = units.all { unit -> + // check no other predecessor exists + unit.meaningfulPredecessors().all { it in popsBeforeUnitInstances } && + // check they have only returns among successors + unit.meaningfulSuccessors().all { it.opcode == Opcodes.ARETURN } + } + if (!allUnitsAreSafe) continue + // save them all to the properties + popsBeforeSafeUnitInstances += pop + safeUnitInstances += units + units.flatMapTo(areturnsAfterSafeUnitInstances) { it.meaningfulSuccessors() } + } } - fun AbstractInsnNode.index() = methodNode.instructions.indexOf(this) + private fun AbstractInsnNode.index() = methodNode.instructions.indexOf(this) // GETSTATIC kotlin/Unit.INSTANCE is considered safe iff // it is part of POP, PUSH Unit, ARETURN sequence. - fun AbstractInsnNode.isSafeUnitInstance(): Boolean = this in safeUnitInstances + private fun AbstractInsnNode.isSafeUnitInstance(): Boolean = this in safeUnitInstances - fun AbstractInsnNode.isPopBeforeSafeUnitInstance(): Boolean = this in popsBeforeSafeUnitInstances - fun AbstractInsnNode.isAreturnAfterSafeUnitInstance(): Boolean = this in areturnsAfterSafeUnitInstances + private fun AbstractInsnNode.isPopBeforeSafeUnitInstance(): Boolean = this in popsBeforeSafeUnitInstances + private fun AbstractInsnNode.isAreturnAfterSafeUnitInstance(): Boolean = this in areturnsAfterSafeUnitInstances - private fun AbstractInsnNode.meaningfulSuccessors(): List { + private fun AbstractInsnNode.meaningfulSuccessors(): List = meaningfulSuccessorsCache.getOrPut(this) { + meaningfulSuccessorsOrPredecessors(true) + } + + private fun AbstractInsnNode.meaningfulPredecessors(): List = meaningfulPredecessorsCache.getOrPut(this) { + meaningfulSuccessorsOrPredecessors(false) + } + + private fun AbstractInsnNode.meaningfulSuccessorsOrPredecessors(isSuccessors: Boolean): List { fun AbstractInsnNode.isMeaningful() = isMeaningful && opcode != Opcodes.NOP && opcode != Opcodes.GOTO && this !is LineNumberNode + fun AbstractInsnNode.getIndices() = + if (isSuccessors) controlFlowGraph.getSuccessorsIndices(this) + else controlFlowGraph.getPredecessorsIndices(this) + val visited = arrayListOf() fun dfs(insn: AbstractInsnNode) { if (insn in visited) return visited += insn if (!insn.isMeaningful()) { - for (succIndex in controlFlowGraph.getSuccessorsIndices(insn)) { + for (succIndex in insn.getIndices()) { dfs(methodNode.instructions[succIndex]) } } } - for (succIndex in controlFlowGraph.getSuccessorsIndices(this)) { + for (succIndex in getIndices()) { dfs(methodNode.instructions[succIndex]) } return visited.filter { it.isMeaningful() } @@ -887,8 +905,12 @@ private class ExaminedMethodNode( basicAnalyser.analyze(containingClassInternalName, methodNode) val typedFrames = basicAnalyser.frames + val isReferenceMap = popsBeforeSafeUnitInstances + .map { it to (!isUnreachable(it.index(), sourceFrames) && typedFrames[it.index()]?.top()?.isReference == true) } + .toMap() + for (pop in popsBeforeSafeUnitInstances) { - if (!isUnreachable(pop.index(), sourceFrames) && typedFrames[pop.index()]?.top()?.isReference == true) { + if (isReferenceMap[pop] == true) { val label = Label() methodNode.instructions.insertBefore(pop, withInstructionAdapter { dup() @@ -900,6 +922,90 @@ private class ExaminedMethodNode( } } } + + fun allSuspensionPointsAreTailCalls(suspensionPoints: List): Boolean { + val safelyReachableReturns = findSafelyReachableReturns() + + val instructions = methodNode.instructions + return suspensionPoints.all { suspensionPoint -> + val beginIndex = instructions.indexOf(suspensionPoint.suspensionCallBegin) + val endIndex = instructions.indexOf(suspensionPoint.suspensionCallEnd) + + if (isUnreachable(endIndex, sourceFrames)) return@all true + + val insideTryBlock = methodNode.tryCatchBlocks.any { block -> + val tryBlockStartIndex = instructions.indexOf(block.start) + val tryBlockEndIndex = instructions.indexOf(block.end) + + beginIndex in tryBlockStartIndex..tryBlockEndIndex + } + if (insideTryBlock) return@all false + + safelyReachableReturns[endIndex + 1]?.all { returnIndex -> + sourceFrames[returnIndex]?.top().sure { + "There must be some value on stack to return" + }.insns.any { sourceInsn -> + sourceInsn?.let(instructions::indexOf) in beginIndex..endIndex + } + } ?: false + } + } + + /** + * Let's call an instruction safe if its execution is always invisible: stack modifications, branching, variable insns (invisible in debug) + * + * For some instruction `insn` define the result as following: + * - if there is a path leading to the non-safe instruction then result is `null` + * - Otherwise result contains all the reachable ARETURN indices + * + * @return indices of safely reachable returns for each instruction in the method node + */ + private fun findSafelyReachableReturns(): Array?> { + val insns = methodNode.instructions + val reachableReturnsIndices = Array?>(insns.size()) init@{ index -> + val insn = insns[index] + + if (insn.opcode == Opcodes.ARETURN && !insn.isAreturnAfterSafeUnitInstance()) { + if (isUnreachable(index, sourceFrames)) return@init null + return@init setOf(index) + } + + // Since POP, PUSH Unit, ARETURN behaves like normal return in terms of tail-call optimization, set return index to POP + if (insn.isPopBeforeSafeUnitInstance()) { + return@init setOf(index) + } + + if (!insn.isMeaningful || insn.opcode in SAFE_OPCODES || insn.isInvisibleInDebugVarInsn(methodNode) || isInlineMarker(insn) + || insn.isSafeUnitInstance() || insn.isAreturnAfterSafeUnitInstance() + ) { + setOf() + } else null + } + + var changed: Boolean + do { + changed = false + for (index in 0 until insns.size()) { + if (insns[index].opcode == Opcodes.ARETURN) continue + + @Suppress("RemoveExplicitTypeArguments") + val newResult = + controlFlowGraph + .getSuccessorsIndices(index).plus(index) + .map(reachableReturnsIndices::get) + .fold?, Set?>(mutableSetOf()) { acc, successorsResult -> + if (acc != null && successorsResult != null) acc + successorsResult else null + } + + if (newResult != reachableReturnsIndices[index]) { + reachableReturnsIndices[index] = newResult + changed = true + } + } + } while (changed) + + return reachableReturnsIndices + } } internal fun InstructionAdapter.generateContinuationConstructorCall( @@ -1031,94 +1137,10 @@ private fun getAllParameterTypes(desc: String, hasDispatchReceiver: Boolean, thi listOfNotNull(if (!hasDispatchReceiver) null else Type.getObjectType(thisName)).toTypedArray() + Type.getArgumentTypes(desc) -private fun ExaminedMethodNode.allSuspensionPointsAreTailCalls(suspensionPoints: List): Boolean { - val safelyReachableReturns = findSafelyReachableReturns() - - val instructions = methodNode.instructions - return suspensionPoints.all { suspensionPoint -> - val beginIndex = instructions.indexOf(suspensionPoint.suspensionCallBegin) - val endIndex = instructions.indexOf(suspensionPoint.suspensionCallEnd) - - if (isUnreachable(beginIndex, sourceFrames)) return@all true - - val insideTryBlock = methodNode.tryCatchBlocks.any { block -> - val tryBlockStartIndex = instructions.indexOf(block.start) - val tryBlockEndIndex = instructions.indexOf(block.end) - - beginIndex in tryBlockStartIndex..tryBlockEndIndex - } - if (insideTryBlock) return@all false - - safelyReachableReturns[endIndex + 1]?.all { returnIndex -> - sourceFrames[returnIndex]?.top().sure { - "There must be some value on stack to return" - }.insns.any { sourceInsn -> - sourceInsn?.let(instructions::indexOf) in beginIndex..endIndex - } - } ?: false - } -} - internal class IgnoringCopyOperationSourceInterpreter : SourceInterpreter(Opcodes.API_VERSION) { override fun copyOperation(insn: AbstractInsnNode?, value: SourceValue?) = value } -/** - * Let's call an instruction safe if its execution is always invisible: stack modifications, branching, variable insns (invisible in debug) - * - * For some instruction `insn` define the result as following: - * - if there is a path leading to the non-safe instruction then result is `null` - * - Otherwise result contains all the reachable ARETURN indices - * - * @return indices of safely reachable returns for each instruction in the method node - */ -private fun ExaminedMethodNode.findSafelyReachableReturns(): Array?> { - val insns = methodNode.instructions - val reachableReturnsIndices = Array?>(insns.size()) init@{ index -> - val insn = insns[index] - - if (insn.opcode == Opcodes.ARETURN && !insn.isAreturnAfterSafeUnitInstance()) { - if (isUnreachable(index, sourceFrames)) return@init null - return@init setOf(index) - } - - // Since POP, PUSH Unit, ARETURN behaves like normal return in terms of tail-call optimization, set return index to POP - if (insn.isPopBeforeSafeUnitInstance()) { - return@init setOf(index) - } - - if (!insn.isMeaningful || insn.opcode in SAFE_OPCODES || insn.isInvisibleInDebugVarInsn(methodNode) || isInlineMarker(insn) - || insn.isSafeUnitInstance() || insn.isAreturnAfterSafeUnitInstance() - ) { - setOf() - } else null - } - - var changed: Boolean - do { - changed = false - for (index in 0 until insns.size()) { - if (insns[index].opcode == Opcodes.ARETURN) continue - - @Suppress("RemoveExplicitTypeArguments") - val newResult = - controlFlowGraph - .getSuccessorsIndices(index).plus(index) - .map(reachableReturnsIndices::get) - .fold?, Set?>(mutableSetOf()) { acc, successorsResult -> - if (acc != null && successorsResult != null) acc + successorsResult else null - } - - if (newResult != reachableReturnsIndices[index]) { - reachableReturnsIndices[index] = newResult - changed = true - } - } - } while (changed) - - return reachableReturnsIndices -} - // Check whether this instruction is unreachable, i.e. there is no path leading to this instruction internal fun isUnreachable(index: Int, sourceFrames: Array?>): Boolean = sourceFrames.size <= index || sourceFrames[index] == null diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/ControlFlowGraph.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/ControlFlowGraph.kt index 2bfb3bb8230..89d50eeb3e2 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/ControlFlowGraph.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/ControlFlowGraph.kt @@ -1,17 +1,6 @@ /* - * Copyright 2010-2014 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. + * Copyright 2010-2019 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. */ package org.jetbrains.kotlin.codegen.optimization.common @@ -24,9 +13,12 @@ import org.jetbrains.org.objectweb.asm.tree.analysis.BasicValue class ControlFlowGraph private constructor(private val insns: InsnList) { private val edges: Array> = Array(insns.size()) { arrayListOf() } + private val backwardEdges: Array> = Array(insns.size()) { arrayListOf() } fun getSuccessorsIndices(insn: AbstractInsnNode): List = getSuccessorsIndices(insns.indexOf(insn)) fun getSuccessorsIndices(index: Int): List = edges[index] + fun getPredecessorsIndices(insn: AbstractInsnNode): List = getPredecessorsIndices(insns.indexOf(insn)) + fun getPredecessorsIndices(index: Int): List = backwardEdges[index] companion object { @JvmStatic @@ -35,6 +27,7 @@ class ControlFlowGraph private constructor(private val insns: InsnList) { fun addEdge(from: Int, to: Int) { graph.edges[from].add(to) + graph.backwardEdges[to].add(from) } object : MethodAnalyzer("fake", node, OptimizationBasicInterpreter()) { diff --git a/compiler/testData/codegen/box/coroutines/tailCallOptimizations/crossinline.txt b/compiler/testData/codegen/box/coroutines/tailCallOptimizations/crossinline.txt index 42984f13958..e8cb1d1f635 100644 --- a/compiler/testData/codegen/box/coroutines/tailCallOptimizations/crossinline.txt +++ b/compiler/testData/codegen/box/coroutines/tailCallOptimizations/crossinline.txt @@ -17,7 +17,12 @@ public final class CrossinlineKt$box$1$filter$$inlined$source$1$1 { } @kotlin.Metadata +@kotlin.coroutines.jvm.internal.DebugMetadata public final class CrossinlineKt$box$1$filter$$inlined$source$1$lambda$1$1 { + field L$0: java.lang.Object + field L$1: java.lang.Object + field L$2: java.lang.Object + field L$3: java.lang.Object field label: int synthetic field result: java.lang.Object synthetic final field this$0: CrossinlineKt$box$1$filter$$inlined$source$1$lambda$1 @@ -97,11 +102,28 @@ public final class CrossinlineKt$box$1$invokeSuspend$$inlined$filter$1$1 { public final @org.jetbrains.annotations.Nullable method invokeSuspend(@org.jetbrains.annotations.NotNull p0: java.lang.Object): java.lang.Object } +@kotlin.Metadata +@kotlin.coroutines.jvm.internal.DebugMetadata +public final class CrossinlineKt$box$1$invokeSuspend$$inlined$filter$1$2$1 { + field L$0: java.lang.Object + field L$1: java.lang.Object + field L$2: java.lang.Object + field L$3: java.lang.Object + field label: int + synthetic field result: java.lang.Object + synthetic final field this$0: CrossinlineKt$box$1$invokeSuspend$$inlined$filter$1$2 + inner class CrossinlineKt$box$1$invokeSuspend$$inlined$filter$1$2 + inner class CrossinlineKt$box$1$invokeSuspend$$inlined$filter$1$2$1 + public method (p0: CrossinlineKt$box$1$invokeSuspend$$inlined$filter$1$2, p1: kotlin.coroutines.Continuation): void + public final @org.jetbrains.annotations.Nullable method invokeSuspend(@org.jetbrains.annotations.NotNull p0: java.lang.Object): java.lang.Object +} + @kotlin.Metadata public final class CrossinlineKt$box$1$invokeSuspend$$inlined$filter$1$2 { synthetic final field $this_source$inlined: Sink synthetic final field this$0: CrossinlineKt$box$1$invokeSuspend$$inlined$filter$1 inner class CrossinlineKt$box$1$invokeSuspend$$inlined$filter$1$2 + inner class CrossinlineKt$box$1$invokeSuspend$$inlined$filter$1$2$1 public method (p0: Sink, p1: CrossinlineKt$box$1$invokeSuspend$$inlined$filter$1): void public method close(@org.jetbrains.annotations.Nullable p0: java.lang.Throwable): void public @org.jetbrains.annotations.Nullable method send(p0: java.lang.Object, @org.jetbrains.annotations.NotNull p1: kotlin.coroutines.Continuation): java.lang.Object @@ -183,7 +205,12 @@ public final class CrossinlineKt$filter$$inlined$source$1$1 { } @kotlin.Metadata +@kotlin.coroutines.jvm.internal.DebugMetadata public final class CrossinlineKt$filter$$inlined$source$1$lambda$1$1 { + field L$0: java.lang.Object + field L$1: java.lang.Object + field L$2: java.lang.Object + field L$3: java.lang.Object field label: int synthetic field result: java.lang.Object synthetic final field this$0: CrossinlineKt$filter$$inlined$source$1$lambda$1 diff --git a/compiler/testData/codegen/box/coroutines/tailCallOptimizations/crossinline_1_2.txt b/compiler/testData/codegen/box/coroutines/tailCallOptimizations/crossinline_1_2.txt index 9d6c0509d68..9a3c8b6af5d 100644 --- a/compiler/testData/codegen/box/coroutines/tailCallOptimizations/crossinline_1_2.txt +++ b/compiler/testData/codegen/box/coroutines/tailCallOptimizations/crossinline_1_2.txt @@ -17,11 +17,29 @@ public final class CrossinlineKt$box$1$doResume$$inlined$filter$1$1 { synthetic final method setLabel(p0: int): void } +@kotlin.Metadata +public final class CrossinlineKt$box$1$doResume$$inlined$filter$1$2$1 { + field L$0: java.lang.Object + field L$1: java.lang.Object + field L$2: java.lang.Object + field L$3: java.lang.Object + synthetic field data: java.lang.Object + synthetic field exception: java.lang.Throwable + synthetic final field this$0: CrossinlineKt$box$1$doResume$$inlined$filter$1$2 + inner class CrossinlineKt$box$1$doResume$$inlined$filter$1$2 + inner class CrossinlineKt$box$1$doResume$$inlined$filter$1$2$1 + public method (p0: CrossinlineKt$box$1$doResume$$inlined$filter$1$2, p1: kotlin.coroutines.experimental.Continuation): void + public final @org.jetbrains.annotations.Nullable method doResume(@org.jetbrains.annotations.Nullable p0: java.lang.Object, @org.jetbrains.annotations.Nullable p1: java.lang.Throwable): java.lang.Object + synthetic final method getLabel(): int + synthetic final method setLabel(p0: int): void +} + @kotlin.Metadata public final class CrossinlineKt$box$1$doResume$$inlined$filter$1$2 { synthetic final field $this_source$inlined: Sink synthetic final field this$0: CrossinlineKt$box$1$doResume$$inlined$filter$1 inner class CrossinlineKt$box$1$doResume$$inlined$filter$1$2 + inner class CrossinlineKt$box$1$doResume$$inlined$filter$1$2$1 public method (p0: Sink, p1: CrossinlineKt$box$1$doResume$$inlined$filter$1): void public method close(@org.jetbrains.annotations.Nullable p0: java.lang.Throwable): void public @org.jetbrains.annotations.Nullable method send(p0: java.lang.Object, @org.jetbrains.annotations.NotNull p1: kotlin.coroutines.experimental.Continuation): java.lang.Object @@ -66,6 +84,10 @@ public final class CrossinlineKt$box$1$filter$$inlined$source$1$1 { @kotlin.Metadata public final class CrossinlineKt$box$1$filter$$inlined$source$1$lambda$1$1 { + field L$0: java.lang.Object + field L$1: java.lang.Object + field L$2: java.lang.Object + field L$3: java.lang.Object synthetic field data: java.lang.Object synthetic field exception: java.lang.Throwable synthetic final field this$0: CrossinlineKt$box$1$filter$$inlined$source$1$lambda$1 @@ -190,6 +212,10 @@ public final class CrossinlineKt$filter$$inlined$source$1$1 { @kotlin.Metadata public final class CrossinlineKt$filter$$inlined$source$1$lambda$1$1 { + field L$0: java.lang.Object + field L$1: java.lang.Object + field L$2: java.lang.Object + field L$3: java.lang.Object synthetic field data: java.lang.Object synthetic field exception: java.lang.Throwable synthetic final field this$0: CrossinlineKt$filter$$inlined$source$1$lambda$1