JVM_IR KT-47984 don't move inplace arguments with suspension points
This commit is contained in:
committed by
TeamCityServer
parent
9acdcc7590
commit
9be941def2
+40
-3
@@ -5,6 +5,7 @@
|
|||||||
|
|
||||||
package org.jetbrains.kotlin.codegen.inline
|
package org.jetbrains.kotlin.codegen.inline
|
||||||
|
|
||||||
|
import org.jetbrains.kotlin.codegen.optimization.boxing.isMethodInsnWith
|
||||||
import org.jetbrains.kotlin.codegen.optimization.common.InsnSequence
|
import org.jetbrains.kotlin.codegen.optimization.common.InsnSequence
|
||||||
import org.jetbrains.kotlin.codegen.optimization.transformer.MethodTransformer
|
import org.jetbrains.kotlin.codegen.optimization.transformer.MethodTransformer
|
||||||
import org.jetbrains.org.objectweb.asm.Label
|
import org.jetbrains.org.objectweb.asm.Label
|
||||||
@@ -19,6 +20,7 @@ class InplaceArgumentsMethodTransformer : MethodTransformer() {
|
|||||||
|
|
||||||
collectStartToEnd(methodContext)
|
collectStartToEnd(methodContext)
|
||||||
collectLvtEntryInstructions(methodContext)
|
collectLvtEntryInstructions(methodContext)
|
||||||
|
collectSuspensionPoints(methodContext)
|
||||||
|
|
||||||
transformMethod(methodContext)
|
transformMethod(methodContext)
|
||||||
updateLvtEntriesForMovedInstructions(methodContext)
|
updateLvtEntriesForMovedInstructions(methodContext)
|
||||||
@@ -36,6 +38,7 @@ class InplaceArgumentsMethodTransformer : MethodTransformer() {
|
|||||||
val startArgToEndArg = HashMap<AbstractInsnNode, AbstractInsnNode>()
|
val startArgToEndArg = HashMap<AbstractInsnNode, AbstractInsnNode>()
|
||||||
val lvtEntryForInstruction = HashMap<AbstractInsnNode, LocalVariableNode>()
|
val lvtEntryForInstruction = HashMap<AbstractInsnNode, LocalVariableNode>()
|
||||||
val varInstructionMoved = HashMap<AbstractInsnNode, CallContext>()
|
val varInstructionMoved = HashMap<AbstractInsnNode, CallContext>()
|
||||||
|
val suspensionJumpLabels = HashSet<LabelNode>()
|
||||||
}
|
}
|
||||||
|
|
||||||
private class CallContext(
|
private class CallContext(
|
||||||
@@ -185,6 +188,37 @@ class InplaceArgumentsMethodTransformer : MethodTransformer() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private fun collectSuspensionPoints(methodContext: MethodContext) {
|
||||||
|
val insnList = methodContext.methodNode.instructions
|
||||||
|
var insn = insnList.first
|
||||||
|
while (
|
||||||
|
!insn.isMethodInsnWith(Opcodes.INVOKESTATIC) {
|
||||||
|
owner == "kotlin/coroutines/intrinsics/IntrinsicsKt" &&
|
||||||
|
name == "getCOROUTINE_SUSPENDED" &&
|
||||||
|
desc == "()Ljava/lang/Object;"
|
||||||
|
}
|
||||||
|
) {
|
||||||
|
insn = insn.next ?: return
|
||||||
|
}
|
||||||
|
|
||||||
|
// Find a first TABLESWITCH and record its jump destinations
|
||||||
|
while (insn != null) {
|
||||||
|
if (insn.opcode != Opcodes.TABLESWITCH || insn.previous.opcode != Opcodes.GETFIELD) {
|
||||||
|
insn = insn.next
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
val getFiendInsn = insn.previous as FieldInsnNode
|
||||||
|
if (getFiendInsn.name != "label" || getFiendInsn.desc != "I") {
|
||||||
|
insn = insn.next
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
val tableSwitchInsn = insn as TableSwitchInsnNode
|
||||||
|
methodContext.suspensionJumpLabels.addAll(tableSwitchInsn.labels)
|
||||||
|
methodContext.suspensionJumpLabels.add(tableSwitchInsn.dflt)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private fun transformMethod(methodContext: MethodContext) {
|
private fun transformMethod(methodContext: MethodContext) {
|
||||||
for (call in methodContext.calls) {
|
for (call in methodContext.calls) {
|
||||||
transformCall(methodContext, call)
|
transformCall(methodContext, call)
|
||||||
@@ -205,7 +239,7 @@ class InplaceArgumentsMethodTransformer : MethodTransformer() {
|
|||||||
// If an inplace argument contains a non-local jump,
|
// If an inplace argument contains a non-local jump,
|
||||||
// moving such argument inside inline function body can interfere with stack normalization.
|
// moving such argument inside inline function body can interfere with stack normalization.
|
||||||
// TODO investigate complex cases
|
// TODO investigate complex cases
|
||||||
if (callContext.args.any { it.hasNonLocalJump() }) {
|
if (callContext.args.any { it.isUnsafeToMove(methodContext) }) {
|
||||||
// Do not transform such call, just strip call and argument markers.
|
// Do not transform such call, just strip call and argument markers.
|
||||||
val insnList = methodContext.methodNode.instructions
|
val insnList = methodContext.methodNode.instructions
|
||||||
for (arg in callContext.args) {
|
for (arg in callContext.args) {
|
||||||
@@ -220,10 +254,13 @@ class InplaceArgumentsMethodTransformer : MethodTransformer() {
|
|||||||
moveInplaceArgumentsFromStoresToLoads(methodContext, callContext)
|
moveInplaceArgumentsFromStoresToLoads(methodContext, callContext)
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun ArgContext.hasNonLocalJump(): Boolean {
|
private fun ArgContext.isUnsafeToMove(methodContext: MethodContext): Boolean {
|
||||||
val argInsns = InsnSequence(this.argStartMarker, this.argEndMarker)
|
val argInsns = InsnSequence(this.argStartMarker, this.argEndMarker)
|
||||||
val localLabels = argInsns.filterTo(HashSet()) { it is LabelNode }
|
val localLabels = argInsns.filterTo(HashSet()) { it is LabelNode }
|
||||||
return argInsns.any { insn -> insn.opcode == Opcodes.GOTO && (insn as JumpInsnNode).label !in localLabels }
|
return argInsns.any { insn ->
|
||||||
|
insn in methodContext.suspensionJumpLabels ||
|
||||||
|
insn.opcode == Opcodes.GOTO && (insn as JumpInsnNode).label !in localLabels
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun moveInplaceArgumentsFromStoresToLoads(methodContext: MethodContext, callContext: CallContext) {
|
private fun moveInplaceArgumentsFromStoresToLoads(methodContext: MethodContext, callContext: CallContext) {
|
||||||
|
|||||||
+6
@@ -18251,6 +18251,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT
|
|||||||
public void testMutableCollectionPlusAssign() throws Exception {
|
public void testMutableCollectionPlusAssign() throws Exception {
|
||||||
runTest("compiler/testData/codegen/box/inlineArgsInPlace/mutableCollectionPlusAssign.kt");
|
runTest("compiler/testData/codegen/box/inlineArgsInPlace/mutableCollectionPlusAssign.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@TestMetadata("suspensionPointInsideArgument.kt")
|
||||||
|
public void testSuspensionPointInsideArgument() throws Exception {
|
||||||
|
runTest("compiler/testData/codegen/box/inlineArgsInPlace/suspensionPointInsideArgument.kt");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@Nested
|
@Nested
|
||||||
|
|||||||
+24
-11
@@ -39,20 +39,33 @@ class IrInlineCodegen(
|
|||||||
InlineCodegen<ExpressionCodegen>(codegen, state, signature, typeParameterMappings, sourceCompiler, reifiedTypeInliner),
|
InlineCodegen<ExpressionCodegen>(codegen, state, signature, typeParameterMappings, sourceCompiler, reifiedTypeInliner),
|
||||||
IrInlineCallGenerator {
|
IrInlineCallGenerator {
|
||||||
|
|
||||||
private val inlineArgumentsInPlace = run {
|
private val inlineArgumentsInPlace = canInlineArgumentsInPlace()
|
||||||
if (function.isInlineOnly() &&
|
|
||||||
!function.isSuspend &&
|
private fun canInlineArgumentsInPlace(): Boolean {
|
||||||
function.valueParameters.isNotEmpty() &&
|
if (!function.isInlineOnly()) return false
|
||||||
function.valueParameters.none { it.type.isFunction() || it.type.isSuspendFunction() }
|
if (function.isSuspend) return false
|
||||||
) {
|
|
||||||
val methodNode = sourceCompiler.compileInlineFunction(signature).node
|
var actualParametersCount = function.valueParameters.size
|
||||||
if (canInlineArgumentsInPlace(methodNode)) {
|
function.dispatchReceiverParameter?.let { dispatchReceiverParameter ->
|
||||||
return@run true
|
++actualParametersCount
|
||||||
}
|
if (dispatchReceiverParameter.isFunctionOrSuspendFunction())
|
||||||
|
return false
|
||||||
}
|
}
|
||||||
false
|
function.extensionReceiverParameter?.let { extensionReceiverParameter ->
|
||||||
|
++actualParametersCount
|
||||||
|
if (extensionReceiverParameter.isFunctionOrSuspendFunction())
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
if (actualParametersCount == 0)
|
||||||
|
return false
|
||||||
|
if (function.valueParameters.any { it.isFunctionOrSuspendFunction() })
|
||||||
|
return false
|
||||||
|
|
||||||
|
return canInlineArgumentsInPlace(sourceCompiler.compileInlineFunction(jvmSignature).node)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private fun IrValueParameter.isFunctionOrSuspendFunction() = type.isFunction() || type.isSuspendFunction()
|
||||||
|
|
||||||
override fun beforeCallStart() {
|
override fun beforeCallStart() {
|
||||||
if (inlineArgumentsInPlace) {
|
if (inlineArgumentsInPlace) {
|
||||||
codegen.visitor.addInplaceCallStartMarker()
|
codegen.visitor.addInplaceCallStartMarker()
|
||||||
|
|||||||
+25
@@ -0,0 +1,25 @@
|
|||||||
|
// IGNORE_BACKEND: WASM
|
||||||
|
// WITH_RUNTIME
|
||||||
|
|
||||||
|
import kotlin.coroutines.*
|
||||||
|
|
||||||
|
fun runs(f: suspend () -> String): String {
|
||||||
|
var result: String? = null
|
||||||
|
f.startCoroutine(
|
||||||
|
Continuation(EmptyCoroutineContext) {
|
||||||
|
result = it.getOrThrow()
|
||||||
|
}
|
||||||
|
)
|
||||||
|
return result ?: "Fail"
|
||||||
|
}
|
||||||
|
|
||||||
|
suspend fun suspendListOf(s: String) = listOf(s)
|
||||||
|
|
||||||
|
val strings: MutableCollection<String> = ArrayList()
|
||||||
|
|
||||||
|
fun box(): String {
|
||||||
|
return runs {
|
||||||
|
strings += suspendListOf("OK")
|
||||||
|
strings.iterator().next()
|
||||||
|
}
|
||||||
|
}
|
||||||
+6
@@ -18107,6 +18107,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
|
|||||||
public void testMutableCollectionPlusAssign() throws Exception {
|
public void testMutableCollectionPlusAssign() throws Exception {
|
||||||
runTest("compiler/testData/codegen/box/inlineArgsInPlace/mutableCollectionPlusAssign.kt");
|
runTest("compiler/testData/codegen/box/inlineArgsInPlace/mutableCollectionPlusAssign.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@TestMetadata("suspensionPointInsideArgument.kt")
|
||||||
|
public void testSuspensionPointInsideArgument() throws Exception {
|
||||||
|
runTest("compiler/testData/codegen/box/inlineArgsInPlace/suspensionPointInsideArgument.kt");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@Nested
|
@Nested
|
||||||
|
|||||||
+6
@@ -18251,6 +18251,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
|
|||||||
public void testMutableCollectionPlusAssign() throws Exception {
|
public void testMutableCollectionPlusAssign() throws Exception {
|
||||||
runTest("compiler/testData/codegen/box/inlineArgsInPlace/mutableCollectionPlusAssign.kt");
|
runTest("compiler/testData/codegen/box/inlineArgsInPlace/mutableCollectionPlusAssign.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@TestMetadata("suspensionPointInsideArgument.kt")
|
||||||
|
public void testSuspensionPointInsideArgument() throws Exception {
|
||||||
|
runTest("compiler/testData/codegen/box/inlineArgsInPlace/suspensionPointInsideArgument.kt");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@Nested
|
@Nested
|
||||||
|
|||||||
+5
@@ -15008,6 +15008,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes
|
|||||||
public void testMutableCollectionPlusAssign() throws Exception {
|
public void testMutableCollectionPlusAssign() throws Exception {
|
||||||
runTest("compiler/testData/codegen/box/inlineArgsInPlace/mutableCollectionPlusAssign.kt");
|
runTest("compiler/testData/codegen/box/inlineArgsInPlace/mutableCollectionPlusAssign.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@TestMetadata("suspensionPointInsideArgument.kt")
|
||||||
|
public void testSuspensionPointInsideArgument() throws Exception {
|
||||||
|
runTest("compiler/testData/codegen/box/inlineArgsInPlace/suspensionPointInsideArgument.kt");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@TestMetadata("compiler/testData/codegen/box/inlineClasses")
|
@TestMetadata("compiler/testData/codegen/box/inlineClasses")
|
||||||
|
|||||||
js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java
Generated
+5
@@ -13097,6 +13097,11 @@ public class IrJsCodegenBoxES6TestGenerated extends AbstractIrJsCodegenBoxES6Tes
|
|||||||
public void testMutableCollectionPlusAssign() throws Exception {
|
public void testMutableCollectionPlusAssign() throws Exception {
|
||||||
runTest("compiler/testData/codegen/box/inlineArgsInPlace/mutableCollectionPlusAssign.kt");
|
runTest("compiler/testData/codegen/box/inlineArgsInPlace/mutableCollectionPlusAssign.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@TestMetadata("suspensionPointInsideArgument.kt")
|
||||||
|
public void testSuspensionPointInsideArgument() throws Exception {
|
||||||
|
runTest("compiler/testData/codegen/box/inlineArgsInPlace/suspensionPointInsideArgument.kt");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@TestMetadata("compiler/testData/codegen/box/inlineClasses")
|
@TestMetadata("compiler/testData/codegen/box/inlineClasses")
|
||||||
|
|||||||
Generated
+5
@@ -12503,6 +12503,11 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest {
|
|||||||
public void testMutableCollectionPlusAssign() throws Exception {
|
public void testMutableCollectionPlusAssign() throws Exception {
|
||||||
runTest("compiler/testData/codegen/box/inlineArgsInPlace/mutableCollectionPlusAssign.kt");
|
runTest("compiler/testData/codegen/box/inlineArgsInPlace/mutableCollectionPlusAssign.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@TestMetadata("suspensionPointInsideArgument.kt")
|
||||||
|
public void testSuspensionPointInsideArgument() throws Exception {
|
||||||
|
runTest("compiler/testData/codegen/box/inlineArgsInPlace/suspensionPointInsideArgument.kt");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@TestMetadata("compiler/testData/codegen/box/inlineClasses")
|
@TestMetadata("compiler/testData/codegen/box/inlineClasses")
|
||||||
|
|||||||
Generated
+5
@@ -12568,6 +12568,11 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
|
|||||||
public void testMutableCollectionPlusAssign() throws Exception {
|
public void testMutableCollectionPlusAssign() throws Exception {
|
||||||
runTest("compiler/testData/codegen/box/inlineArgsInPlace/mutableCollectionPlusAssign.kt");
|
runTest("compiler/testData/codegen/box/inlineArgsInPlace/mutableCollectionPlusAssign.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@TestMetadata("suspensionPointInsideArgument.kt")
|
||||||
|
public void testSuspensionPointInsideArgument() throws Exception {
|
||||||
|
runTest("compiler/testData/codegen/box/inlineArgsInPlace/suspensionPointInsideArgument.kt");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@TestMetadata("compiler/testData/codegen/box/inlineClasses")
|
@TestMetadata("compiler/testData/codegen/box/inlineClasses")
|
||||||
|
|||||||
js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java
Generated
+5
@@ -6658,6 +6658,11 @@ public class IrCodegenBoxWasmTestGenerated extends AbstractIrCodegenBoxWasmTest
|
|||||||
public void testMutableCollectionPlusAssign() throws Exception {
|
public void testMutableCollectionPlusAssign() throws Exception {
|
||||||
runTest("compiler/testData/codegen/box/inlineArgsInPlace/mutableCollectionPlusAssign.kt");
|
runTest("compiler/testData/codegen/box/inlineArgsInPlace/mutableCollectionPlusAssign.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@TestMetadata("suspensionPointInsideArgument.kt")
|
||||||
|
public void testSuspensionPointInsideArgument() throws Exception {
|
||||||
|
runTest("compiler/testData/codegen/box/inlineArgsInPlace/suspensionPointInsideArgument.kt");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@TestMetadata("compiler/testData/codegen/box/inlineClasses")
|
@TestMetadata("compiler/testData/codegen/box/inlineClasses")
|
||||||
|
|||||||
Reference in New Issue
Block a user