From 57f16e801fa67f0c80208d655f3d26de3c5f4bd7 Mon Sep 17 00:00:00 2001 From: Alexander Korepanov Date: Wed, 13 Apr 2022 21:03:33 +0300 Subject: [PATCH] [JS IR] Materialize Unit more aggressively for IrCalls Unit materialization for IrCall is required for operations with dynamic type. However it produces useless Unit_getInstance() calls, especially in return expressions. The pach also adds some heuristics for reducing the amount of Unit_getInstance() calls from return expressions: do not add Unit_getInstance() if return value has Unit type. Relaited to KT-51139 ^KT-23252 Fixed --- .../backend/js/lower/AutoboxingTransformer.kt | 84 ++++++++---- .../kotlin/js/test/BoxJsTestGenerated.java | 6 + .../js/test/ir/IrBoxJsTestGenerated.java | 18 +++ .../coercion/unitMaterializationOnAssign.kt | 51 ++++++++ .../box/coercion/unitMaterializationOnCall.kt | 123 ++++++++++++++++++ .../testData/lineNumbers/coroutine.kt | 2 +- 6 files changed, 258 insertions(+), 26 deletions(-) create mode 100644 js/js.translator/testData/box/coercion/unitMaterializationOnAssign.kt create mode 100644 js/js.translator/testData/box/coercion/unitMaterializationOnCall.kt diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/AutoboxingTransformer.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/AutoboxingTransformer.kt index 05e7544dfec..ad192a5e840 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/AutoboxingTransformer.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/AutoboxingTransformer.kt @@ -8,12 +8,18 @@ package org.jetbrains.kotlin.ir.backend.js.lower import org.jetbrains.kotlin.backend.common.BodyLoweringPass import org.jetbrains.kotlin.backend.common.compilationException import org.jetbrains.kotlin.backend.common.lower.AbstractValueUsageTransformer +import org.jetbrains.kotlin.backend.common.pop +import org.jetbrains.kotlin.backend.common.push import org.jetbrains.kotlin.ir.IrBuiltIns import org.jetbrains.kotlin.ir.backend.js.JsCommonBackendContext import org.jetbrains.kotlin.ir.backend.js.ir.JsIrBuilder import org.jetbrains.kotlin.ir.backend.js.utils.realOverrideTarget import org.jetbrains.kotlin.ir.declarations.* import org.jetbrains.kotlin.ir.expressions.* +import org.jetbrains.kotlin.ir.symbols.IrConstructorSymbol +import org.jetbrains.kotlin.ir.symbols.IrReturnTargetSymbol +import org.jetbrains.kotlin.ir.symbols.IrReturnableBlockSymbol +import org.jetbrains.kotlin.ir.symbols.IrSimpleFunctionSymbol import org.jetbrains.kotlin.ir.types.* import org.jetbrains.kotlin.ir.util.isPrimitiveArray import org.jetbrains.kotlin.ir.util.patchDeclarationParents @@ -43,35 +49,32 @@ abstract class AbstractValueUsageLowering(val context: JsCommonBackendContext) : abstract fun IrExpression.useExpressionAsType(actualType: IrType, expectedType: IrType): IrExpression - override fun IrExpression.useAs(type: IrType): IrExpression { - val actualType = when (this) { - is IrConstructorCall -> symbol.owner.returnType - is IrCall -> symbol.owner.realOverrideTarget.returnType - is IrGetField -> this.symbol.owner.type + protected fun IrExpression.getActualType() = when (this) { + is IrConstructorCall -> symbol.owner.returnType + is IrCall -> symbol.owner.realOverrideTarget.returnType + is IrGetField -> this.symbol.owner.type - is IrTypeOperatorCall -> { - if (operator == IrTypeOperator.REINTERPRET_CAST) { - this.typeOperand - } else { - this.type - } + is IrTypeOperatorCall -> { + if (operator == IrTypeOperator.REINTERPRET_CAST) { + this.typeOperand + } else { + this.type } - - is IrGetValue -> { - val value = this.symbol.owner - if (value is IrValueParameter && icUtils.shouldValueParameterBeBoxed(value)) { - irBuiltIns.anyType - } else { - this.type - } - } - - else -> this.type } - return useExpressionAsType(actualType, type) + is IrGetValue -> { + val value = this.symbol.owner + if (value is IrValueParameter && icUtils.shouldValueParameterBeBoxed(value)) { + irBuiltIns.anyType + } else { + this.type + } + } + + else -> this.type } + override fun IrExpression.useAs(type: IrType): IrExpression = useExpressionAsType(getActualType(), type) private val IrFunctionAccessExpression.target: IrFunction get() = when (this) { @@ -117,13 +120,44 @@ abstract class AbstractValueUsageLowering(val context: JsCommonBackendContext) : } class AutoboxingTransformer(context: JsCommonBackendContext) : AbstractValueUsageLowering(context) { + private var processingReturnStack = mutableListOf() + + private fun IrExpression.useReturnableExpressionAsType(expectedType: IrType): IrExpression { + val expressionType = getActualType() + if (expressionType.isUnit() && expectedType.isUnit()) { + return this + } + return useExpressionAsType(expressionType, expectedType) + } + + override fun visitReturn(expression: IrReturn): IrExpression { + processingReturnStack.push(expression) + val res = super.visitReturn(expression) + processingReturnStack.pop() + return res + } + + override fun IrExpression.useAsResult(enclosing: IrExpression): IrExpression { + if (processingReturnStack.lastOrNull()?.value == enclosing && enclosing is IrReturnableBlock) { + return useReturnableExpressionAsType(enclosing.type) + } + return this.useAs(enclosing.type) + } + + override fun IrExpression.useAsReturnValue(returnTarget: IrReturnTargetSymbol): IrExpression = when (returnTarget) { + is IrSimpleFunctionSymbol -> useReturnableExpressionAsType(returnTarget.owner.returnType) + is IrConstructorSymbol -> useReturnableExpressionAsType(irBuiltIns.unitType) + is IrReturnableBlockSymbol -> useReturnableExpressionAsType(returnTarget.owner.type) + else -> error(returnTarget) + } + override fun IrExpression.useExpressionAsType(actualType: IrType, expectedType: IrType): IrExpression { // // TODO: Default parameters are passed as nulls and they need not to be unboxed. Fix this if (actualType.makeNotNull().isNothing()) return this - if (actualType.isUnit() && !expectedType.isUnit()) { + if (actualType.isUnit() && (!expectedType.isUnit() || this is IrCall)) { // Don't materialize Unit if value is known to be proper Unit on runtime if (!this.isGetUnit(irBuiltIns)) { val unitValue = JsIrBuilder.buildGetObjectValue(actualType, context.irBuiltIns.unitClass) @@ -223,4 +257,4 @@ private tailrec fun IrExpression.isGetUnit(irBuiltIns: IrBuiltIns): Boolean = this.symbol == irBuiltIns.unitClass else -> false - } \ No newline at end of file + } diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/BoxJsTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/BoxJsTestGenerated.java index b62468148d2..aeddad4f6e0 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/BoxJsTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/BoxJsTestGenerated.java @@ -5352,6 +5352,12 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest { runTest("js/js.translator/testData/box/inlineEvaluationOrder/propertyAccessAndInitializer.kt"); } + @Test + @TestMetadata("propertyAccessNoSideEffect.kt") + public void testPropertyAccessNoSideEffect() throws Exception { + runTest("js/js.translator/testData/box/inlineEvaluationOrder/propertyAccessNoSideEffect.kt"); + } + @Test @TestMetadata("propertyAccessWithSideEffect.kt") public void testPropertyAccessWithSideEffect() 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 41e3acff9ae..1b67e18cc87 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 @@ -756,6 +756,18 @@ public class IrBoxJsTestGenerated extends AbstractIrBoxJsTest { runTest("js/js.translator/testData/box/coercion/unitMaterializationInOverriddenMethod.kt"); } + @Test + @TestMetadata("unitMaterializationOnAssign.kt") + public void testUnitMaterializationOnAssign() throws Exception { + runTest("js/js.translator/testData/box/coercion/unitMaterializationOnAssign.kt"); + } + + @Test + @TestMetadata("unitMaterializationOnCall.kt") + public void testUnitMaterializationOnCall() throws Exception { + runTest("js/js.translator/testData/box/coercion/unitMaterializationOnCall.kt"); + } + @Test @TestMetadata("unitNullCheck.kt") public void testUnitNullCheck() throws Exception { @@ -5742,6 +5754,12 @@ public class IrBoxJsTestGenerated extends AbstractIrBoxJsTest { runTest("js/js.translator/testData/box/inlineEvaluationOrder/propertyAccessAndInitializer.kt"); } + @Test + @TestMetadata("propertyAccessNoSideEffect.kt") + public void testPropertyAccessNoSideEffect() throws Exception { + runTest("js/js.translator/testData/box/inlineEvaluationOrder/propertyAccessNoSideEffect.kt"); + } + @Test @TestMetadata("propertyAccessWithSideEffect.kt") public void testPropertyAccessWithSideEffect() throws Exception { diff --git a/js/js.translator/testData/box/coercion/unitMaterializationOnAssign.kt b/js/js.translator/testData/box/coercion/unitMaterializationOnAssign.kt new file mode 100644 index 00000000000..09fa91d53aa --- /dev/null +++ b/js/js.translator/testData/box/coercion/unitMaterializationOnAssign.kt @@ -0,0 +1,51 @@ +// DONT_TARGET_EXACT_BACKEND: JS +// KJS_WITH_FULL_RUNTIME + +fun demo(): Unit {} + +fun testFunctionUsage() { + val u1 = demo() + val d1: dynamic = u1 + assertTrue(d1 == u1) + assertEquals(u1.toString(), "kotlin.Unit") +} + +fun testFunctionCall() { + fun test(u1: Unit): Boolean { + val d1: dynamic = u1 + assertEquals(u1.toString(), "kotlin.Unit") + return d1 == u1 + } + assertTrue(test(demo())) +} + +fun testObjectField() { + val o = object { val u1 = demo() } + val d1: dynamic = o.u1 + assertTrue(d1 == o.u1) + assertEquals(o.u1.toString(), "kotlin.Unit") +} + +fun testClassField() { + class Test(val u1: Unit) + val o = Test(demo()) + val d1: dynamic = o.u1 + assertTrue(d1 == o.u1) + assertEquals(o.u1.toString(), "kotlin.Unit") +} + +fun testContainer() { + val l = listOf(demo()) + val d1: dynamic = l.last() + assertTrue(d1 == l.last()) + assertEquals(l.last().toString(), "kotlin.Unit") +} + +fun box(): String { + testFunctionUsage() + testFunctionCall() + testObjectField() + testClassField() + testContainer() + return "OK" +} diff --git a/js/js.translator/testData/box/coercion/unitMaterializationOnCall.kt b/js/js.translator/testData/box/coercion/unitMaterializationOnCall.kt new file mode 100644 index 00000000000..53a280aa22b --- /dev/null +++ b/js/js.translator/testData/box/coercion/unitMaterializationOnCall.kt @@ -0,0 +1,123 @@ +// DONT_TARGET_EXACT_BACKEND: JS + +var demoCallCounter = 0 + +fun flushDemoCallCounter() = demoCallCounter.also { demoCallCounter = 0 } + +fun demo(): Unit { ++demoCallCounter } + +inline fun inlineDemo(): Unit { + demo() +} + +// CHECK_CALLED_IN_SCOPE: scope=testUsualCall function=Unit_getInstance +fun testUsualCall() { + val x = demo() + assertEquals(flushDemoCallCounter(), 1) + assertEquals(x.toString(), "kotlin.Unit") +} + +// CHECK_NOT_CALLED_IN_SCOPE: scope=testUsualCallNoVar function=Unit_getInstance +fun testUsualCallNoVar() { + demo() + assertEquals(flushDemoCallCounter(), 1) +} + +// CHECK_CALLED_IN_SCOPE: scope=testUsualCallInReturn function=Unit_getInstance +fun testUsualCallInReturn() { + // CHECK_NOT_CALLED_IN_SCOPE: scope=testUsualCallInReturn$test function=Unit_getInstance + fun test() { return demo() } + val x = test() + assertEquals(flushDemoCallCounter(), 1) + assertEquals(x.toString(), "kotlin.Unit") +} + +// CHECK_NOT_CALLED_IN_SCOPE: scope=testUsualCallInReturnNoVar function=Unit_getInstance +fun testUsualCallInReturnNoVar() { + // CHECK_NOT_CALLED_IN_SCOPE: scope=testUsualCallInReturnNoVar$test function=Unit_getInstance + fun test() { return demo() } + test() + assertEquals(flushDemoCallCounter(), 1) +} + +// CHECK_CALLED_IN_SCOPE: scope=testUsualCallInExpressionBody function=Unit_getInstance +fun testUsualCallInExpressionBody() { + // CHECK_NOT_CALLED_IN_SCOPE: scope=testUsualCallInExpressionBody$test function=Unit_getInstance + fun test() = demo() + val x = test() + assertEquals(flushDemoCallCounter(), 1) + assertEquals(x.toString(), "kotlin.Unit") +} + +// CHECK_NOT_CALLED_IN_SCOPE: scope=testUsualCallInExpressionBodyNoVar function=Unit_getInstance +fun testUsualCallInExpressionBodyNoVar() { + // CHECK_NOT_CALLED_IN_SCOPE: scope=testUsualCallInExpressionBodyNoVar$test function=Unit_getInstance + fun test() = demo() + test() + assertEquals(flushDemoCallCounter(), 1) +} + +// CHECK_CALLED_IN_SCOPE: scope=testInlineCall function=Unit_getInstance +fun testInlineCall() { + val x = inlineDemo() + assertEquals(flushDemoCallCounter(), 1) + assertEquals(x.toString(), "kotlin.Unit") +} + +// CHECK_NOT_CALLED_IN_SCOPE: scope=testUsualCallNoVar function=Unit_getInstance +fun testInlineCallNoVar() { + inlineDemo() + assertEquals(flushDemoCallCounter(), 1) +} + +// CHECK_CALLED_IN_SCOPE: scope=testInlineCallInReturn function=Unit_getInstance +fun testInlineCallInReturn() { + // CHECK_NOT_CALLED_IN_SCOPE: scope=testInlineCallInReturn$test function=Unit_getInstance + fun test() { return inlineDemo() } + val x = test() + assertEquals(flushDemoCallCounter(), 1) + assertEquals(x.toString(), "kotlin.Unit") +} + +// CHECK_NOT_CALLED_IN_SCOPE: scope=testInlineCallInReturnNoVar function=Unit_getInstance +fun testInlineCallInReturnNoVar() { + // CHECK_NOT_CALLED_IN_SCOPE: scope=testInlineCallInReturnNoVar$test function=Unit_getInstance + fun test() { return inlineDemo() } + test() + assertEquals(flushDemoCallCounter(), 1) +} + +// CHECK_CALLED_IN_SCOPE: scope=testInlineCallInExpressionBody function=Unit_getInstance +fun testInlineCallInExpressionBody() { + // CHECK_NOT_CALLED_IN_SCOPE: scope=testInlineCallInExpressionBody$test function=Unit_getInstance + fun test() = inlineDemo() + val x = test() + assertEquals(flushDemoCallCounter(), 1) + assertEquals(x.toString(), "kotlin.Unit") +} + +// CHECK_NOT_CALLED_IN_SCOPE: scope=testInlineCallInExpressionBodyNoVar function=Unit_getInstance +fun testInlineCallInExpressionBodyNoVar() { + // CHECK_NOT_CALLED_IN_SCOPE: scope=testInlineCallInExpressionBodyNoVar$test function=Unit_getInstance + fun test() = inlineDemo() + test() + assertEquals(flushDemoCallCounter(), 1) +} + +fun box(): String { + testUsualCall() + testUsualCallNoVar() + testUsualCallInReturn() + testUsualCallInReturnNoVar() + testUsualCallInExpressionBody() + testUsualCallInExpressionBodyNoVar() + + testInlineCall() + testInlineCallNoVar() + testInlineCallInReturn() + testInlineCallInReturnNoVar() + testInlineCallInExpressionBody() + testInlineCallInExpressionBodyNoVar() + + return "OK" +} diff --git a/js/js.translator/testData/lineNumbers/coroutine.kt b/js/js.translator/testData/lineNumbers/coroutine.kt index 4e54b9f27a1..8f663fd759a 100644 --- a/js/js.translator/testData/lineNumbers/coroutine.kt +++ b/js/js.translator/testData/lineNumbers/coroutine.kt @@ -15,4 +15,4 @@ suspend fun bar(): Unit { } // LINES(JS): 39 4 4 4 7 5 5 45 45 5 93 45 5 5 6 4 4 4 9 15 9 9 9 * 9 15 10 10 11 11 11 11 11 * 11 12 12 13 13 13 13 13 13 13 14 14 * 9 15 9 9 9 9 -// LINES(JS_IR): 19 19 * 50 50 93 93 3 45 45 3 45 6 6 19 * 9 9 9 9 9 9 9 9 * 9 9 * 10 10 * 11 * 11 12 12 * 13 * 13 13 14 14 +// LINES(JS_IR): 19 19 * 50 50 93 93 3 45 3 45 45 6 6 19 * 9 9 9 9 9 9 9 9 * 9 9 * 10 10 * 11 * 11 12 12 * 13 * 13 13 14 14