From 4695dca0562333aae0a728598c70a77c93a3ba43 Mon Sep 17 00:00:00 2001 From: Alexander Korepanov Date: Wed, 28 Jun 2023 17:20:39 +0200 Subject: [PATCH] [JS IR] Keep box and unbox intrinsics in call args after decomposing ^KT-59717 Fixed --- .../ir/backend/js/JsIrBackendContext.kt | 4 +- .../js/lower/BlockDecomposerLowering.kt | 48 +++++++++++++++++-- .../js/test/fir/FirJsBoxTestGenerated.java | 6 +++ .../js/test/ir/IrBoxJsES6TestGenerated.java | 6 +++ .../js/test/ir/IrBoxJsTestGenerated.java | 6 +++ .../testData/box/char/charNoBoxing.kt | 42 ++++++++++++++++ .../testData/box/inline/inlineClassEquals.kt | 10 ++-- 7 files changed, 112 insertions(+), 10 deletions(-) create mode 100644 js/js.translator/testData/box/char/charNoBoxing.kt diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsIrBackendContext.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsIrBackendContext.kt index ef9d1de58e3..8f6f013105c 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsIrBackendContext.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsIrBackendContext.kt @@ -97,7 +97,9 @@ class JsIrBackendContext( override fun isSideEffectFree(call: IrCall): Boolean = call.symbol in intrinsics.primitiveToLiteralConstructor.values || call.symbol == intrinsics.arrayLiteral || - call.symbol == intrinsics.arrayConcat + call.symbol == intrinsics.arrayConcat || + call.symbol == intrinsics.jsBoxIntrinsic || + call.symbol == intrinsics.jsUnboxIntrinsic val devMode = configuration[JSConfigurationKeys.DEVELOPER_MODE] ?: false val errorPolicy = configuration[JSConfigurationKeys.ERROR_TOLERANCE_POLICY] ?: ErrorTolerancePolicy.DEFAULT diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/BlockDecomposerLowering.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/BlockDecomposerLowering.kt index f67f3e0a255..8f09844825c 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/BlockDecomposerLowering.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/BlockDecomposerLowering.kt @@ -16,6 +16,7 @@ import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET import org.jetbrains.kotlin.ir.backend.js.JsCommonBackendContext import org.jetbrains.kotlin.ir.backend.js.JsIrBackendContext import org.jetbrains.kotlin.ir.backend.js.ir.JsIrBuilder +import org.jetbrains.kotlin.ir.backend.js.utils.typeArguments import org.jetbrains.kotlin.ir.builders.declarations.buildFun import org.jetbrains.kotlin.ir.declarations.* import org.jetbrains.kotlin.ir.expressions.* @@ -94,7 +95,7 @@ abstract class AbstractBlockDecomposerLowering(private val context: JsCommonBack } class BlockDecomposerTransformer( - private val context: CommonBackendContext, + private val context: JsCommonBackendContext, private val unreachableExpression: () -> IrExpression ) : IrElementTransformerVoid() { private lateinit var function: IrDeclarationParent @@ -111,6 +112,9 @@ class BlockDecomposerTransformer( private val booleanNotSymbol = context.irBuiltIns.booleanNotSymbol + private val boxIntrinsic = context.inlineClassesUtils.boxIntrinsic + private val unboxIntrinsic = context.inlineClassesUtils.unboxIntrinsic + override fun visitScript(declaration: IrScript): IrStatement { function = declaration @@ -537,9 +541,9 @@ class BlockDecomposerTransformer( value.isPure(anyVariable = false, context = context) -> value else -> { // TODO: do not wrap if value is pure (const, variable, etc) - val irVar = makeTempVar(value.type, value) - newStatements += irVar - JsIrBuilder.buildGetValue(irVar.symbol) + val (newArg, tempVar) = mapArgument(value) + newStatements += tempVar + newArg } } @@ -548,6 +552,42 @@ class BlockDecomposerTransformer( return arguments } + /** + * Move the passing argument and store it in a temporary variable. + * However, the box and unbox intrinsics should be preserved in the call. + * They can be used later for optimizations, for example, in [EqualityAndComparisonCallsTransformer]. + * Example: + * foo(boxIntrinsic()) + * should be transformed to: + * var tmp = + * foo(boxIntrinsic(tmp)) + */ + private fun mapArgument(arg: IrExpression): Pair { + var saveToTmp = arg + var rootIntrinsicCall: IrCall? = null + var lastIntrinsicCall: IrCall? = null + while (saveToTmp is IrCall && (saveToTmp.symbol == boxIntrinsic || saveToTmp.symbol == unboxIntrinsic)) { + if (lastIntrinsicCall == null) { + lastIntrinsicCall = JsIrBuilder.buildCall(saveToTmp.symbol, saveToTmp.type, saveToTmp.typeArguments.filterNotNull()) + rootIntrinsicCall = lastIntrinsicCall + } else { + val nextCall = JsIrBuilder.buildCall(saveToTmp.symbol) + lastIntrinsicCall.putValueArgument(0, nextCall) + lastIntrinsicCall = nextCall + } + saveToTmp = saveToTmp.getValueArgument(0) ?: error("expect passing 1 argument to boxing intrinsic") + } + + val irTempVar = makeTempVar(saveToTmp.type, saveToTmp) + val irGetTempVar = JsIrBuilder.buildGetValue(irTempVar.symbol) + val newArg = lastIntrinsicCall?.let { + it.putValueArgument(0, irGetTempVar) + rootIntrinsicCall + } ?: irGetTempVar + + return newArg to irTempVar + } + // TODO: remove this when vararg is lowered override fun visitVararg(expression: IrVararg): IrExpression { expression.transformChildrenVoid(expressionTransformer) diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/fir/FirJsBoxTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/fir/FirJsBoxTestGenerated.java index 59e8d3aa9b5..c7dabe06bb5 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/fir/FirJsBoxTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/fir/FirJsBoxTestGenerated.java @@ -246,6 +246,12 @@ public class FirJsBoxTestGenerated extends AbstractFirJsBoxTest { runTest("js/js.translator/testData/box/char/charIsCheck.kt"); } + @Test + @TestMetadata("charNoBoxing.kt") + public void testCharNoBoxing() throws Exception { + runTest("js/js.translator/testData/box/char/charNoBoxing.kt"); + } + @Test @TestMetadata("charRanges.kt") public void testCharRanges() throws Exception { diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsES6TestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsES6TestGenerated.java index 380587ffb95..1c5505b7eea 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsES6TestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsES6TestGenerated.java @@ -246,6 +246,12 @@ public class IrBoxJsES6TestGenerated extends AbstractIrBoxJsES6Test { runTest("js/js.translator/testData/box/char/charIsCheck.kt"); } + @Test + @TestMetadata("charNoBoxing.kt") + public void testCharNoBoxing() throws Exception { + runTest("js/js.translator/testData/box/char/charNoBoxing.kt"); + } + @Test @TestMetadata("charRanges.kt") public void testCharRanges() throws Exception { diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsTestGenerated.java index 85862ab7f1d..29ed83d6d28 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsTestGenerated.java @@ -246,6 +246,12 @@ public class IrBoxJsTestGenerated extends AbstractIrBoxJsTest { runTest("js/js.translator/testData/box/char/charIsCheck.kt"); } + @Test + @TestMetadata("charNoBoxing.kt") + public void testCharNoBoxing() throws Exception { + runTest("js/js.translator/testData/box/char/charNoBoxing.kt"); + } + @Test @TestMetadata("charRanges.kt") public void testCharRanges() throws Exception { diff --git a/js/js.translator/testData/box/char/charNoBoxing.kt b/js/js.translator/testData/box/char/charNoBoxing.kt new file mode 100644 index 00000000000..8fbde528519 --- /dev/null +++ b/js/js.translator/testData/box/char/charNoBoxing.kt @@ -0,0 +1,42 @@ +fun getC(c: Char) = c +inline fun getCInline(c: Char) = c + +// CHECK_NEW_COUNT: function=testNonInline1 count=0 +fun testNonInline1(a: Char, b: Char) : Boolean { + return getC(a) == getC(b) +} + +// CHECK_NEW_COUNT: function=testNonInline2 count=0 +fun testNonInline2(a: Char, b: Char) : Boolean { + val a1 = getC(a) + val b1 = getC(b) + return a1 == b1 +} + +// CHECK_NEW_COUNT: function=testInline1 count=0 +fun testInline1(a: Char, b: Char) : Boolean { + return getCInline(a) == getCInline(b) +} + +// CHECK_NEW_COUNT: function=testInline2 count=0 +fun testInline2(a: Char, b: Char) : Boolean { + val a1 = getCInline(a) + val b1 = getCInline(b) + return a1 == b1 +} + +fun box(): String { + if (!testNonInline1('a', 'a')) { + return "Fail testNonInline1" + } + if (!testNonInline2('b', 'b')) { + return "Fail testNonInline2" + } + if (!testInline1('c', 'c')) { + return "Fail testInline1" + } + if (!testInline2('d', 'd')) { + return "Fail testInline2" + } + return "OK" +} diff --git a/js/js.translator/testData/box/inline/inlineClassEquals.kt b/js/js.translator/testData/box/inline/inlineClassEquals.kt index 312fee341ef..20df430dd1b 100644 --- a/js/js.translator/testData/box/inline/inlineClassEquals.kt +++ b/js/js.translator/testData/box/inline/inlineClassEquals.kt @@ -141,8 +141,8 @@ fun testUnderlyingInline() { } caseJsEq() - // CHECK_CALLED_IN_SCOPE: scope=testUnderlyingInline$caseEquals function=equals - // CHECK_NEW_COUNT: function=testUnderlyingInline$caseEquals count=4 + // CHECK_NOT_CALLED_IN_SCOPE: scope=testUnderlyingInline$caseEquals function=equals + // CHECK_NEW_COUNT: function=testUnderlyingInline$caseEquals count=2 fun caseEquals() { assertTrue(c1_1 == ClassUnderlayingInline(x1!!)) assertTrue(ClassUnderlayingInline(x1!!) == c1_2) @@ -256,8 +256,8 @@ fun testNullableUnderlyingInlineClass() { } caseJsEq() - // CHECK_CALLED_IN_SCOPE: scope=testNullableUnderlyingInlineClass$caseEquals function=equals - // CHECK_NEW_COUNT: function=testNullableUnderlyingInlineClass$caseEquals count=3 + // CHECK_NOT_CALLED_IN_SCOPE: scope=testNullableUnderlyingInlineClass$caseEquals function=equals + // CHECK_NEW_COUNT: function=testNullableUnderlyingInlineClass$caseEquals count=1 fun caseEquals() { assertTrue(ClassNullableUnderlayingInline(i1_1) == ClassNullableUnderlayingInline(i1_2!!)) } @@ -265,7 +265,7 @@ fun testNullableUnderlyingInlineClass() { } // CHECK_CALLED_IN_SCOPE: scope=testInlineClassWithInterface function=equals -// CHECK_NEW_COUNT: function=testInlineClassWithInterface count=14 +// CHECK_NEW_COUNT: function=testInlineClassWithInterface count=10 fun testInlineClassWithInterface() { val xi_1_1: InterfaceForInlineClass = ClassIntWithInterface(1) val xi_1_2: InterfaceForInlineClass = ClassIntWithInterface(1)