From c441980c749bf0e93a33b5150b444796bbe3af07 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 25 Oct 2021 15:55:57 +0300 Subject: [PATCH] JVM_IR don't move inplace arguments with variable stores KT-49370 KT-49407 --- .../inline/InplaceArgumentsMethodTransformer.kt | 15 +++++++-------- .../codegen/FirBlackBoxCodegenTestGenerated.java | 12 ++++++++++++ .../codegen/box/inlineArgsInPlace/kt49370.kt | 8 ++++++++ .../codegen/box/inlineArgsInPlace/kt49407.kt | 6 ++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 12 ++++++++++++ .../codegen/IrBlackBoxCodegenTestGenerated.java | 12 ++++++++++++ .../codegen/LightAnalysisModeTestGenerated.java | 10 ++++++++++ .../semantics/IrJsCodegenBoxES6TestGenerated.java | 10 ++++++++++ .../ir/semantics/IrJsCodegenBoxTestGenerated.java | 10 ++++++++++ .../semantics/IrCodegenBoxWasmTestGenerated.java | 10 ++++++++++ .../js/testNew/JsCodegenBoxTestGenerated.java | 12 ++++++++++++ 11 files changed, 109 insertions(+), 8 deletions(-) create mode 100644 compiler/testData/codegen/box/inlineArgsInPlace/kt49370.kt create mode 100644 compiler/testData/codegen/box/inlineArgsInPlace/kt49407.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InplaceArgumentsMethodTransformer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InplaceArgumentsMethodTransformer.kt index db22f398999..4c33ce03851 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InplaceArgumentsMethodTransformer.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InplaceArgumentsMethodTransformer.kt @@ -6,10 +6,7 @@ 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.findNextOrNull -import org.jetbrains.kotlin.codegen.optimization.common.removeUnusedLocalVariables -import org.jetbrains.kotlin.codegen.optimization.common.updateMaxStack +import org.jetbrains.kotlin.codegen.optimization.common.* import org.jetbrains.kotlin.codegen.optimization.transformer.MethodTransformer import org.jetbrains.org.objectweb.asm.Opcodes import org.jetbrains.org.objectweb.asm.tree.* @@ -239,9 +236,6 @@ class InplaceArgumentsMethodTransformer : MethodTransformer() { transformCall(methodContext, call) } - // If an inplace argument contains a non-local jump, - // moving such argument inside inline function body can interfere with stack normalization. - // TODO investigate complex cases if (callContext.args.any { it.isUnsafeToMove(methodContext) }) { // Do not transform such call, just strip call and argument markers. val insnList = methodContext.methodNode.instructions @@ -258,10 +252,15 @@ class InplaceArgumentsMethodTransformer : MethodTransformer() { } private fun ArgContext.isUnsafeToMove(methodContext: MethodContext): Boolean { + // The following operations make inplace argument unsafe to move: + // - non-local jump (moving such argument inside inline function body can interfere with stack normalization); + // - variable store (variables defined inside argument can interfere with variables in inline function body). + // TODO investigate whether it's possible to lift these restrictions. val argInsns = InsnSequence(this.argStartMarker, this.argEndMarker) val localLabels = argInsns.filterTo(HashSet()) { it is LabelNode } return argInsns.any { insn -> - insn in methodContext.suspensionJumpLabels || + insn.isStoreOperation() || + insn in methodContext.suspensionJumpLabels || insn.opcode == Opcodes.GOTO && (insn as JumpInsnNode).label !in localLabels } } 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 46829b795f2..23ffc9417b4 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 @@ -18644,6 +18644,18 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/inlineArgsInPlace/kotlinReflect.kt"); } + @Test + @TestMetadata("kt49370.kt") + public void testKt49370() throws Exception { + runTest("compiler/testData/codegen/box/inlineArgsInPlace/kt49370.kt"); + } + + @Test + @TestMetadata("kt49407.kt") + public void testKt49407() throws Exception { + runTest("compiler/testData/codegen/box/inlineArgsInPlace/kt49407.kt"); + } + @Test @TestMetadata("mapSet.kt") public void testMapSet() throws Exception { diff --git a/compiler/testData/codegen/box/inlineArgsInPlace/kt49370.kt b/compiler/testData/codegen/box/inlineArgsInPlace/kt49370.kt new file mode 100644 index 00000000000..b96d9aa6d33 --- /dev/null +++ b/compiler/testData/codegen/box/inlineArgsInPlace/kt49370.kt @@ -0,0 +1,8 @@ +// !OPT_IN: kotlin.ExperimentalStdlibApi +// WITH_RUNTIME +// IGNORE_BACKEND: WASM + +fun box(): String { + 1L.mod("123a".indexOfAny("a".toCharArray())) + return "OK" +} diff --git a/compiler/testData/codegen/box/inlineArgsInPlace/kt49407.kt b/compiler/testData/codegen/box/inlineArgsInPlace/kt49407.kt new file mode 100644 index 00000000000..d981beb3356 --- /dev/null +++ b/compiler/testData/codegen/box/inlineArgsInPlace/kt49407.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME + +fun box(): String { + val x: Array> = arrayOf(arrayOf(0.plus(-1L).mod(5.mod(-47)).rem(1))) + return "OK" +} 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 cdb489f9651..4b7f5bfbd03 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 @@ -18518,6 +18518,18 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/inlineArgsInPlace/kotlinReflect.kt"); } + @Test + @TestMetadata("kt49370.kt") + public void testKt49370() throws Exception { + runTest("compiler/testData/codegen/box/inlineArgsInPlace/kt49370.kt"); + } + + @Test + @TestMetadata("kt49407.kt") + public void testKt49407() throws Exception { + runTest("compiler/testData/codegen/box/inlineArgsInPlace/kt49407.kt"); + } + @Test @TestMetadata("mapSet.kt") public void testMapSet() 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 7507c40e459..9697d31f2e3 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 @@ -18644,6 +18644,18 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/inlineArgsInPlace/kotlinReflect.kt"); } + @Test + @TestMetadata("kt49370.kt") + public void testKt49370() throws Exception { + runTest("compiler/testData/codegen/box/inlineArgsInPlace/kt49370.kt"); + } + + @Test + @TestMetadata("kt49407.kt") + public void testKt49407() throws Exception { + runTest("compiler/testData/codegen/box/inlineArgsInPlace/kt49407.kt"); + } + @Test @TestMetadata("mapSet.kt") public void testMapSet() 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 7e1e607bea9..1d375a4eeb8 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -15360,6 +15360,16 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/inlineArgsInPlace/kotlinReflect.kt"); } + @TestMetadata("kt49370.kt") + public void testKt49370() throws Exception { + runTest("compiler/testData/codegen/box/inlineArgsInPlace/kt49370.kt"); + } + + @TestMetadata("kt49407.kt") + public void testKt49407() throws Exception { + runTest("compiler/testData/codegen/box/inlineArgsInPlace/kt49407.kt"); + } + @TestMetadata("mapSet.kt") public void testMapSet() throws Exception { runTest("compiler/testData/codegen/box/inlineArgsInPlace/mapSet.kt"); diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java index e11fe98357a..f563c3ff750 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java @@ -13414,6 +13414,16 @@ public class IrJsCodegenBoxES6TestGenerated extends AbstractIrJsCodegenBoxES6Tes runTest("compiler/testData/codegen/box/inlineArgsInPlace/kotlinReflect.kt"); } + @TestMetadata("kt49370.kt") + public void testKt49370() throws Exception { + runTest("compiler/testData/codegen/box/inlineArgsInPlace/kt49370.kt"); + } + + @TestMetadata("kt49407.kt") + public void testKt49407() throws Exception { + runTest("compiler/testData/codegen/box/inlineArgsInPlace/kt49407.kt"); + } + @TestMetadata("mapSet.kt") public void testMapSet() throws Exception { runTest("compiler/testData/codegen/box/inlineArgsInPlace/mapSet.kt"); diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java index bfe290efc52..e5d88363fa6 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java @@ -12820,6 +12820,16 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { runTest("compiler/testData/codegen/box/inlineArgsInPlace/kotlinReflect.kt"); } + @TestMetadata("kt49370.kt") + public void testKt49370() throws Exception { + runTest("compiler/testData/codegen/box/inlineArgsInPlace/kt49370.kt"); + } + + @TestMetadata("kt49407.kt") + public void testKt49407() throws Exception { + runTest("compiler/testData/codegen/box/inlineArgsInPlace/kt49407.kt"); + } + @TestMetadata("mapSet.kt") public void testMapSet() throws Exception { runTest("compiler/testData/codegen/box/inlineArgsInPlace/mapSet.kt"); diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java index 1d7b5e6ad72..e3fa2541dd4 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java @@ -12146,6 +12146,16 @@ public class IrCodegenBoxWasmTestGenerated extends AbstractIrCodegenBoxWasmTest runTest("compiler/testData/codegen/box/inlineArgsInPlace/kotlinReflect.kt"); } + @TestMetadata("kt49370.kt") + public void testKt49370() throws Exception { + runTest("compiler/testData/codegen/box/inlineArgsInPlace/kt49370.kt"); + } + + @TestMetadata("kt49407.kt") + public void testKt49407() throws Exception { + runTest("compiler/testData/codegen/box/inlineArgsInPlace/kt49407.kt"); + } + @TestMetadata("mapSet.kt") public void testMapSet() throws Exception { runTest("compiler/testData/codegen/box/inlineArgsInPlace/mapSet.kt"); diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/testNew/JsCodegenBoxTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/testNew/JsCodegenBoxTestGenerated.java index a2bf1b9f552..c176e1f5875 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/testNew/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/testNew/JsCodegenBoxTestGenerated.java @@ -14530,6 +14530,18 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { runTest("compiler/testData/codegen/box/inlineArgsInPlace/kotlinReflect.kt"); } + @Test + @TestMetadata("kt49370.kt") + public void testKt49370() throws Exception { + runTest("compiler/testData/codegen/box/inlineArgsInPlace/kt49370.kt"); + } + + @Test + @TestMetadata("kt49407.kt") + public void testKt49407() throws Exception { + runTest("compiler/testData/codegen/box/inlineArgsInPlace/kt49407.kt"); + } + @Test @TestMetadata("mapSet.kt") public void testMapSet() throws Exception {