From 3ee09f05ef67141a4e1d84209aed9ea2aa523a55 Mon Sep 17 00:00:00 2001 From: Ilmir Usmanov Date: Wed, 14 Sep 2022 21:26:11 +0200 Subject: [PATCH] JVM: Check for multiple {POP, Unit} sequences in suspend function TCO We already check for {POP, Unit} sequence before ARETURN, but if the there are multiple sequence before ARETURN, the compiler assumes, that TCO misses. The fix is to check, that the instruction after the sequence is either ARETURN or another {POP, Unit} sequence. #KT-50835 #KT-54152 Fixed --- .../coroutines/TailCallOptimization.kt | 2 +- .../FirBlackBoxCodegenTestGenerated.java | 6 ++ .../lambdaParameterInlining.kt | 72 +++++++++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 6 ++ .../IrBlackBoxCodegenTestGenerated.java | 6 ++ .../LightAnalysisModeTestGenerated.java | 5 ++ 6 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 compiler/testData/codegen/box/coroutines/tailCallOptimizations/lambdaParameterInlining.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/TailCallOptimization.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/TailCallOptimization.kt index 48168c038ba..b88190874b2 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/TailCallOptimization.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/TailCallOptimization.kt @@ -87,7 +87,7 @@ private val AbstractInsnNode.nextMeaningful: AbstractInsnNode? get() = next.skipUntilMeaningful() private val AbstractInsnNode.isReturnUnit: Boolean - get() = isUnitInstance() && nextMeaningful?.opcode == Opcodes.ARETURN + get() = isUnitInstance() && nextMeaningful?.let { it.opcode == Opcodes.ARETURN || it.isPopBeforeReturnUnit } == true private val AbstractInsnNode.isPopBeforeReturnUnit: Boolean get() = opcode == Opcodes.POP && nextMeaningful?.isReturnUnit == true diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java index 413d9d6bee8..67d970ddc57 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java @@ -13026,6 +13026,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/coroutines/tailCallOptimizations/interfaceDelegation.kt"); } + @Test + @TestMetadata("lambdaParameterInlining.kt") + public void testLambdaParameterInlining() throws Exception { + runTest("compiler/testData/codegen/box/coroutines/tailCallOptimizations/lambdaParameterInlining.kt"); + } + @Test @TestMetadata("returnInlineClass.kt") public void testReturnInlineClass() throws Exception { diff --git a/compiler/testData/codegen/box/coroutines/tailCallOptimizations/lambdaParameterInlining.kt b/compiler/testData/codegen/box/coroutines/tailCallOptimizations/lambdaParameterInlining.kt new file mode 100644 index 00000000000..a8628b880d1 --- /dev/null +++ b/compiler/testData/codegen/box/coroutines/tailCallOptimizations/lambdaParameterInlining.kt @@ -0,0 +1,72 @@ +// TARGET_BACKEND: JVM +// FULL_JDK +// WITH_STDLIB +// WITH_COROUTINES +// CHECK_TAIL_CALL_OPTIMIZATION + +import helpers.* +import kotlin.coroutines.* +import kotlin.coroutines.intrinsics.* + +fun assert(value: () -> Boolean) {} + +class ChannelSegment(val id: Long) + +suspend fun suspendHere() = suspendCoroutineUninterceptedOrReturn { x -> + TailCallOptimizationChecker.saveStackTrace(x) + COROUTINE_SUSPENDED +} + +private const val RESULT_SUSPEND_NO_WAITER = 3 + +open class BufferedChannel { + private val sendSegment = ChannelSegment(0) + + suspend fun send(element: E): Unit = + sendImpl( + onClosed = {}, + onNoWaiterSuspend = { sendOnNoWaiterSuspend() } + ) + + private suspend fun sendOnNoWaiterSuspend() { + suspendHere() + } + + private fun findSegmentSend(): ChannelSegment? = null + + private fun updateCellSend(): Int = RESULT_SUSPEND_NO_WAITER + + private inline fun sendImpl( + onClosed: () -> R, + onNoWaiterSuspend: () -> R = { error("unexpected") } + ): R { + var segment = sendSegment + while (true) { + val closed = false + val id = 0L + if (segment.id != id) { + assert { segment.id < id } + // Find the required segment. + segment = null ?: + if (closed) return onClosed() else continue + } + when(updateCellSend()) { + RESULT_SUSPEND_NO_WAITER -> { + return onNoWaiterSuspend() + } + } + } + } +} + +fun builder(c: suspend () -> Unit) { + c.startCoroutine(EmptyContinuation) +} + +fun box(): String { + builder { + BufferedChannel().send(0) + } + TailCallOptimizationChecker.checkNoStateMachineIn("send") + return "OK" +} \ No newline at end of file diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java index a5e12987c1f..e73725d5eee 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java @@ -12858,6 +12858,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/coroutines/tailCallOptimizations/interfaceDelegation.kt"); } + @Test + @TestMetadata("lambdaParameterInlining.kt") + public void testLambdaParameterInlining() throws Exception { + runTest("compiler/testData/codegen/box/coroutines/tailCallOptimizations/lambdaParameterInlining.kt"); + } + @Test @TestMetadata("returnInlineClass.kt") public void testReturnInlineClass() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java index 2810440f015..bb4b4fbe6ff 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java @@ -13026,6 +13026,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/coroutines/tailCallOptimizations/interfaceDelegation.kt"); } + @Test + @TestMetadata("lambdaParameterInlining.kt") + public void testLambdaParameterInlining() throws Exception { + runTest("compiler/testData/codegen/box/coroutines/tailCallOptimizations/lambdaParameterInlining.kt"); + } + @Test @TestMetadata("returnInlineClass.kt") public void testReturnInlineClass() throws Exception { diff --git a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 07e9a265b44..ac812dd856e 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -10371,6 +10371,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/coroutines/tailCallOptimizations/interfaceDelegation.kt"); } + @TestMetadata("lambdaParameterInlining.kt") + public void testLambdaParameterInlining() throws Exception { + runTest("compiler/testData/codegen/box/coroutines/tailCallOptimizations/lambdaParameterInlining.kt"); + } + @TestMetadata("returnInlineClass.kt") public void testReturnInlineClass() throws Exception { runTest("compiler/testData/codegen/box/coroutines/tailCallOptimizations/returnInlineClass.kt");