From 7b1c564b871adff2492c12b59cb1ea0ed184f318 Mon Sep 17 00:00:00 2001 From: Ilmir Usmanov Date: Fri, 14 Sep 2018 21:53:38 +0300 Subject: [PATCH] Replace ALOAD 0 with fake continuation if suspension point is default method #KT-25922 Fixed #KT-26608 Fixed #KT-26658 Fixed --- .../kotlin/codegen/inline/MethodInliner.kt | 81 ++++++-- .../codegen/boxInline/suspend/kt26658.kt | 189 ++++++++++++++++++ .../BlackBoxInlineCodegenTestGenerated.java | 5 + ...otlinAgainstInlineKotlinTestGenerated.java | 5 + .../IrBlackBoxInlineCodegenTestGenerated.java | 5 + .../InlineSuspendTestsGenerated.java | 5 + 6 files changed, 276 insertions(+), 14 deletions(-) create mode 100644 compiler/testData/codegen/boxInline/suspend/kt26658.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt index dbe2fa63ed8..f186d46a232 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt @@ -15,6 +15,7 @@ import org.jetbrains.kotlin.codegen.inline.FieldRemapper.Companion.foldName import org.jetbrains.kotlin.codegen.intrinsics.IntrinsicMethods import org.jetbrains.kotlin.codegen.optimization.ApiVersionCallsPreprocessingMethodTransformer import org.jetbrains.kotlin.codegen.optimization.FixStackWithLabelNormalizationMethodTransformer +import org.jetbrains.kotlin.codegen.optimization.common.ControlFlowGraph import org.jetbrains.kotlin.codegen.optimization.common.InsnSequence import org.jetbrains.kotlin.codegen.optimization.common.asSequence import org.jetbrains.kotlin.codegen.optimization.common.isMeaningful @@ -23,7 +24,7 @@ import org.jetbrains.kotlin.codegen.optimization.fixStack.top import org.jetbrains.kotlin.descriptors.FunctionDescriptor import org.jetbrains.kotlin.descriptors.ParameterDescriptor import org.jetbrains.kotlin.descriptors.ValueParameterDescriptor -import org.jetbrains.kotlin.resolve.isInlineClassType +import org.jetbrains.kotlin.load.java.JvmAbi import org.jetbrains.kotlin.resolve.jvm.AsmTypes import org.jetbrains.kotlin.resolve.jvm.AsmTypes.OBJECT_TYPE import org.jetbrains.kotlin.types.KotlinType @@ -640,26 +641,78 @@ class MethodInliner( private fun replaceContinuationAccessesWithFakeContinuationsIfNeeded(processingNode: MethodNode) { val lambdaInfo = inliningContext.lambdaInfo ?: return if (!lambdaInfo.invokeMethodDescriptor.isSuspend) return + val sources = analyzeMethodNodeBeforeInline(processingNode) + val cfg = ControlFlowGraph.build(processingNode) val aload0s = processingNode.instructions.asSequence().filter { it.opcode == Opcodes.ALOAD && it.safeAs()?.`var` == 0 } - // Expected pattern here: - // ALOAD 0 - // ICONST_0 - // INVOKESTATIC InlineMarker.mark - // INVOKE* suspendingFunction(..., Continuation;)Ljava/lang/Object; - val continuationAsParameterAload0s = - aload0s.filter { it.next?.next?.let(::isBeforeSuspendMarker) == true && isSuspendCall(it.next?.next?.next) } - replaceContinuationsWithFakeOnes(continuationAsParameterAload0s, processingNode) + + val visited = hashSetOf() + fun findMeaningfulSuccs(insn: AbstractInsnNode): Collection { + if (!visited.add(insn)) return emptySet() + val res = hashSetOf() + for (succIndex in cfg.getSuccessorsIndices(insn)) { + val succ = processingNode.instructions[succIndex] + if (succ.isMeaningful) res.add(succ) + else res.addAll(findMeaningfulSuccs(succ)) + } + return res + } + + // After inlining suspendCoroutineUninterceptedOrReturn there will be suspension point, which is not a MethodInsnNode. + // So, it is incorrect to expect MethodInsnNodes only + val suspensionPoints = processingNode.instructions.asSequence() + .filter { isBeforeSuspendMarker(it) } + .flatMap { findMeaningfulSuccs(it).asSequence() } + .filter { it is MethodInsnNode } + + val toReplace = hashSetOf() + for (suspensionPoint in suspensionPoints) { + assert(suspensionPoint is MethodInsnNode) { + "suspensionPoint shall be MethodInsnNode, but instead $suspensionPoint" + } + suspensionPoint as MethodInsnNode + assert(Type.getReturnType(suspensionPoint.desc) == OBJECT_TYPE) { + "suspensionPoint shall return $OBJECT_TYPE, but returns ${Type.getReturnType(suspensionPoint.desc)}" + } + val frame = sources[processingNode.instructions.indexOf(suspensionPoint)] ?: continue + val paramTypes = Type.getArgumentTypes(suspensionPoint.desc) + if (suspensionPoint.name.endsWith(JvmAbi.DEFAULT_PARAMS_IMPL_SUFFIX)) { + // Expected pattern here: + // ALOAD 0 + // (ICONST or other integers creating instruction) + // (ACONST_NULL or ALOAD) + // ICONST_0 + // INVOKESTATIC InlineMarker.mark + // INVOKE* suspendingFunction$default(..., Continuation;ILjava/lang/Object)Ljava/lang/Object; + assert(paramTypes.size >= 3) { + "${suspensionPoint.name}${suspensionPoint.desc} shall have 3+ parameters" + } + } else { + // Expected pattern here: + // ALOAD 0 + // ICONST_0 + // INVOKESTATIC InlineMarker.mark + // INVOKE* suspendingFunction(..., Continuation;)Ljava/lang/Object; + assert(paramTypes.isNotEmpty()) { + "${suspensionPoint.name}${suspensionPoint.desc} shall have 1+ parameters" + } + } + paramTypes.reversed().asSequence().withIndex() + .filter { it.value == languageVersionSettings.continuationAsmType() || it.value == OBJECT_TYPE } + .flatMap { frame.getStack(frame.stackSize - it.index - 1).insns.asSequence() } + .filter { it in aload0s }.let { toReplace.addAll(it) } + } + // Expected pattern here: // ALOAD 0 // ASTORE N // This pattern may occur after multiple inlines - val continuationToStoreAload0s = aload0s.filter { it.next?.opcode == Opcodes.ASTORE } - replaceContinuationsWithFakeOnes(continuationToStoreAload0s, processingNode) + // Note, that this is not a suspension point, thus we check it separately + toReplace.addAll(aload0s.filter { it.next?.opcode == Opcodes.ASTORE }) // Expected pattern here: // ALOAD 0 // INVOKEINTERFACE kotlin/jvm/functions/FunctionN.invoke (...,Ljava/lang/Object;)Ljava/lang/Object; - val continuationAsLambdaParameterAload0s = aload0s.filter { isLambdaCall(it.next) } - replaceContinuationsWithFakeOnes(continuationAsLambdaParameterAload0s, processingNode) + toReplace.addAll(aload0s.filter { isLambdaCall(it.next) }) + replaceContinuationsWithFakeOnes(toReplace, processingNode) } private fun isLambdaCall(invoke: AbstractInsnNode?): Boolean { @@ -672,7 +725,7 @@ class MethodInliner( } private fun replaceContinuationsWithFakeOnes( - continuations: Sequence, + continuations: Collection, node: MethodNode ) { for (toReplace in continuations) { diff --git a/compiler/testData/codegen/boxInline/suspend/kt26658.kt b/compiler/testData/codegen/boxInline/suspend/kt26658.kt new file mode 100644 index 00000000000..379cd4b94d3 --- /dev/null +++ b/compiler/testData/codegen/boxInline/suspend/kt26658.kt @@ -0,0 +1,189 @@ +// IGNORE_BACKEND: JVM_IR +// FILE: inlined.kt +// LANGUAGE_VERSION: 1.3 +// WITH_RUNTIME +// NO_CHECK_LAMBDA_INLINING +import kotlin.coroutines.* + +class Controller(val s: String) + +fun builder(c: suspend () -> Unit) { + c.startCoroutine(object : Continuation{ + override fun resumeWith(result: Result) { + result.getOrThrow() + } + + override val context: CoroutineContext + get() = EmptyCoroutineContext + + }) +} + +inline fun execute(crossinline action: suspend () -> Unit) { + builder { action() } +} + +fun builder(controller: Controller, c: suspend Controller.() -> Unit) { + c.startCoroutine(controller, object : Continuation{ + override fun resumeWith(result: Result) { + result.getOrThrow() + } + + override val context: CoroutineContext + get() = EmptyCoroutineContext + }) +} + +inline fun execute(controller: Controller, crossinline action: suspend Controller.() -> Unit) { + builder(controller) { action() } +} + +// FILE: inlineSite.kt +import kotlin.coroutines.* + +suspend fun withDefaultParameter(s: String, s1: String = "") = s + s1 + +fun launch(s: String = "", block: suspend () -> String): String { + var res = s + builder { res += block() } + return res +} + +inline fun launchCrossinline(s: String = "", crossinline block: suspend () -> String): String { + var res = s + builder { res += block() } + return res +} + +suspend fun Controller.withDefaultParameter(s: String, s1: String = "") = this.s + s + s1 + +fun Controller.launch(s: String = "", block: suspend Controller.() -> String): String { + var res = s + builder(this) { res += block() } + return res +} + +inline fun Controller.launchCrossinline(s: String = "", crossinline block: suspend Controller.() -> String): String { + var res = s + builder(this) { res += block() } + return res +} + +fun box(): String { + var res = "" + execute { + res = withDefaultParameter("OK") + } + if (res != "OK") return "FAIL 1: $res" + execute { + res = withDefaultParameter("O", "K") + } + if (res != "OK") return "FAIL 2: $res" + execute { + res = launch { + withDefaultParameter("OK") + } + } + if (res != "OK") return "FAIL 3: $res" + execute { + res = launch { + withDefaultParameter("O", "K") + } + } + if (res != "OK") return "FAIL 4: $res" + execute { + res = launch("O") { + withDefaultParameter("K") + } + } + if (res != "OK") return "FAIL 5: $res" + execute { + res = launch("O") { + withDefaultParameter("", "K") + } + } + if (res != "OK") return "FAIL 6: $res" + execute { + res = launchCrossinline { + withDefaultParameter("OK") + } + } + if (res != "OK") return "FAIL 7: $res" + execute { + res = launchCrossinline { + withDefaultParameter("O", "K") + } + } + if (res != "OK") return "FAIL 8: $res" + execute { + res = launchCrossinline ("O") { + withDefaultParameter("K") + } + } + if (res != "OK") return "FAIL 9: $res" + execute { + res = launchCrossinline("O") { + withDefaultParameter("", "K") + } + } + if (res != "OK") return "FAIL 10: $res" + + val controller = Controller("A") + execute(controller) { + res = withDefaultParameter("OK") + } + if (res != "AOK") return "FAIL 11: $res" + execute(controller) { + res = withDefaultParameter("O", "K") + } + if (res != "AOK") return "FAIL 12: $res" + execute(controller) { + res = launch { + withDefaultParameter("OK") + } + } + if (res != "AOK") return "FAIL 13: $res" + execute(controller) { + res = launch { + withDefaultParameter("O", "K") + } + } + if (res != "AOK") return "FAIL 14: $res" + execute(controller) { + res = launch("O") { + withDefaultParameter("K") + } + } + if (res != "OAK") return "FAIL 15: $res" + execute(controller) { + res = launch("O") { + withDefaultParameter("", "K") + } + } + if (res != "OAK") return "FAIL 16: $res" + execute(controller) { + res = launchCrossinline { + withDefaultParameter("OK") + } + } + if (res != "AOK") return "FAIL 17: $res" + execute(controller) { + res = launchCrossinline { + withDefaultParameter("O", "K") + } + } + if (res != "AOK") return "FAIL 18: $res" + execute(controller) { + res = launchCrossinline ("O") { + withDefaultParameter("K") + } + } + if (res != "OAK") return "FAIL 19: $res" + execute(controller) { + res = launchCrossinline("O") { + withDefaultParameter("", "K") + } + } + if (res != "OAK") return "FAIL 20: $res" + return "OK" +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java index 7deadb0be50..8802c09e0a3 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java @@ -3354,6 +3354,11 @@ public class BlackBoxInlineCodegenTestGenerated extends AbstractBlackBoxInlineCo runTestWithPackageReplacement("compiler/testData/codegen/boxInline/suspend/inlineSuspendOfSuspend.kt", "kotlin.coroutines"); } + @TestMetadata("kt26658.kt") + public void testKt26658() throws Exception { + runTest("compiler/testData/codegen/boxInline/suspend/kt26658.kt"); + } + @TestMetadata("multipleLocals.kt") public void testMultipleLocals_1_2() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/boxInline/suspend/multipleLocals.kt", "kotlin.coroutines.experimental"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java index 657930ba0ce..317bd2ad8a4 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java @@ -3354,6 +3354,11 @@ public class CompileKotlinAgainstInlineKotlinTestGenerated extends AbstractCompi runTestWithPackageReplacement("compiler/testData/codegen/boxInline/suspend/inlineSuspendOfSuspend.kt", "kotlin.coroutines"); } + @TestMetadata("kt26658.kt") + public void testKt26658() throws Exception { + runTest("compiler/testData/codegen/boxInline/suspend/kt26658.kt"); + } + @TestMetadata("multipleLocals.kt") public void testMultipleLocals_1_2() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/boxInline/suspend/multipleLocals.kt", "kotlin.coroutines.experimental"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java index ce8e43fb833..e8e3edcd076 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java @@ -3354,6 +3354,11 @@ public class IrBlackBoxInlineCodegenTestGenerated extends AbstractIrBlackBoxInli runTestWithPackageReplacement("compiler/testData/codegen/boxInline/suspend/inlineSuspendOfSuspend.kt", "kotlin.coroutines"); } + @TestMetadata("kt26658.kt") + public void testKt26658() throws Exception { + runTest("compiler/testData/codegen/boxInline/suspend/kt26658.kt"); + } + @TestMetadata("multipleLocals.kt") public void testMultipleLocals_1_2() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/boxInline/suspend/multipleLocals.kt", "kotlin.coroutines.experimental"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineSuspendTestsGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineSuspendTestsGenerated.java index 851d0933170..3f500344bc0 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineSuspendTestsGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineSuspendTestsGenerated.java @@ -133,6 +133,11 @@ public class InlineSuspendTestsGenerated extends AbstractInlineSuspendTests { runTestWithPackageReplacement("compiler/testData/codegen/boxInline/suspend/inlineSuspendOfSuspend.kt", "kotlin.coroutines"); } + @TestMetadata("kt26658.kt") + public void testKt26658() throws Exception { + runTest("compiler/testData/codegen/boxInline/suspend/kt26658.kt"); + } + @TestMetadata("multipleLocals.kt") public void testMultipleLocals_1_2() throws Exception { runTestWithPackageReplacement("compiler/testData/codegen/boxInline/suspend/multipleLocals.kt", "kotlin.coroutines.experimental");