[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
2022-04-13 21:03:33 +03:00
parent 677ec12b50
commit 57f16e801f
6 changed files with 258 additions and 26 deletions
@@ -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<IrReturn>()
+
+    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
-    }
+    }
@@ -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 {
@@ -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 {
@@ -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<Unit>(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"
+}
@@ -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"
+}
@@ -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