From 45b89de9f40cd0fd9d20d92cd67eb3519eebced4 Mon Sep 17 00:00:00 2001 From: Ivan Kylchik Date: Wed, 10 Aug 2022 17:06:03 +0300 Subject: [PATCH] Optimize ir interpretation of `Enum.name` There is no need to create honest object in case of such simple calculation. Furthermore, it can be harmful if enum class has non-constant initializer or property. #KT-53480 Fixed --- .../FirBlackBoxCodegenTestGenerated.java | 6 ++++++ .../ir/interpreter/InstructionsUnfolder.kt | 17 +++++++++++++-- .../kotlin/ir/interpreter/IrInterpreter.kt | 21 +++---------------- .../jetbrains/kotlin/ir/interpreter/Utils.kt | 19 +++++++++++++++++ .../intrinsics/IntrinsicEvaluator.kt | 1 - .../intrinsics/IntrinsicImplementations.kt | 17 +++++++-------- .../testData/codegen/box/constants/kt53480.kt | 15 +++++++++++++ compiler/testData/ir/interpreter/kt53480.kt | 12 +++++++++++ .../IrBlackBoxCodegenTestGenerated.java | 6 ++++++ ...IrInterpreterAfterFir2IrTestGenerated.java | 6 ++++++ ...IrInterpreterAfterPsi2IrTestGenerated.java | 6 ++++++ 11 files changed, 95 insertions(+), 31 deletions(-) create mode 100644 compiler/testData/codegen/box/constants/kt53480.kt create mode 100644 compiler/testData/ir/interpreter/kt53480.kt 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 d357fe5d48f..ae05c45c8d9 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 @@ -8133,6 +8133,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/constants/foldingBinaryOpsUnsignedConst.kt"); } + @Test + @TestMetadata("kt53480.kt") + public void testKt53480() throws Exception { + runTest("compiler/testData/codegen/box/constants/kt53480.kt"); + } + @Test @TestMetadata("kt9532.kt") public void testKt9532() throws Exception { diff --git a/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/InstructionsUnfolder.kt b/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/InstructionsUnfolder.kt index 4a4a2b0d1fa..b42fda7c66b 100644 --- a/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/InstructionsUnfolder.kt +++ b/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/InstructionsUnfolder.kt @@ -90,7 +90,8 @@ private fun unfoldFunction(function: IrSimpleFunction, environment: IrInterprete private fun unfoldConstructor(constructor: IrConstructor, callStack: CallStack) { when (constructor.fqName) { - "kotlin.Enum.", "kotlin.Throwable." -> { + "kotlin.Enum." -> return // all properties already initialized in `interpretEnumEntry` + "kotlin.Throwable." -> { val irClass = constructor.parentAsClass val receiverSymbol = irClass.thisReceiver!!.symbol val receiverState = callStack.loadState(receiverSymbol) @@ -196,11 +197,15 @@ private fun unfoldEnumConstructorCall(element: IrEnumConstructorCall, environmen private fun unfoldInstanceInitializerCall(instanceInitializerCall: IrInstanceInitializerCall, callStack: CallStack) { val irClass = instanceInitializerCall.classSymbol.owner val toInitialize = irClass.declarations.filter { it is IrProperty || it is IrAnonymousInitializer } + val state = irClass.thisReceiver?.symbol?.let { callStack.loadState(it) } // try to avoid recalculation of properties toInitialize.reversed().forEach { when { it is IrAnonymousInitializer -> callStack.pushCompoundInstruction(it.body) - it is IrProperty && it.backingField?.initializer?.expression != null -> callStack.pushCompoundInstruction(it.backingField) + + it is IrProperty && it.backingField?.initializer?.expression != null && state?.getField(it.symbol) == null -> { + callStack.pushCompoundInstruction(it.backingField) + } } } } @@ -272,6 +277,14 @@ private fun unfoldGetEnumValue(expression: IrGetEnumValue, environment: IrInterp val callStack = environment.callStack environment.mapOfEnums[expression.symbol]?.let { return callStack.pushState(it) } + val frameOwner = callStack.currentFrameOwner + if (frameOwner is IrCall && frameOwner.dispatchReceiver is IrGetEnumValue && frameOwner.symbol.owner.name.asString() == "") { + // this optimization allow us to avoid creation of enum object when we try to interpret simple `name` call; see KT-53480 + val enumEntry = (frameOwner.dispatchReceiver as IrGetEnumValue).symbol.owner + callStack.pushState(enumEntry.toState(environment.irBuiltIns)) + return + } + callStack.pushSimpleInstruction(expression) val enumEntry = expression.symbol.owner val enumClass = enumEntry.symbol.owner.parentAsClass diff --git a/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/IrInterpreter.kt b/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/IrInterpreter.kt index e3881b92f88..0aa85dab5f0 100644 --- a/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/IrInterpreter.kt +++ b/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/IrInterpreter.kt @@ -206,7 +206,6 @@ class IrInterpreter(internal val environment: IrInterpreterEnvironment, internal if (callStack.peekState() == null) callStack.pushState(getUnitState()) // implicit Unit result } - @Suppress("UNUSED_PARAMETER") private fun interpretConstructor(constructor: IrConstructor) { callStack.pushState(callStack.loadState(constructor.parentAsClass.thisReceiver!!.symbol)) callStack.dropFrameAndCopyResult() @@ -390,28 +389,14 @@ class IrInterpreter(internal val environment: IrInterpreterEnvironment, internal private fun interpretEnumEntry(enumEntry: IrEnumEntry) { callInterceptor.interceptEnumEntry(enumEntry) { - val enumClass = enumEntry.symbol.owner.parentAsClass - val enumEntries = enumClass.declarations.filterIsInstance() + val enumClassObject = enumEntry.toState(irBuiltIns) + environment.mapOfEnums[enumEntry.symbol] = enumClassObject + val enumInitializer = enumEntry.initializerExpression?.expression ?: throw InterpreterError("Initializer at enum entry ${enumEntry.fqName} is null") val enumConstructorCall = enumInitializer as? IrEnumConstructorCall ?: (enumInitializer as IrBlock).statements.filterIsInstance().single() - val enumClassObject = Common(enumEntry.correspondingClass ?: enumClass) - environment.mapOfEnums[enumEntry.symbol] = enumClassObject - - if (enumEntries.isNotEmpty()) { - // these fields will be evaluated during interpretation of constructor call - // but we need them right away to create correct call to Enum's constructor - val valueArguments = listOf( - Primitive(enumEntry.name.asString(), irBuiltIns.stringType), - Primitive(enumEntries.indexOf(enumEntry), irBuiltIns.intType) - ) - irBuiltIns.enumClass.owner.declarations.filterIsInstance().zip(valueArguments).forEach { (property, argument) -> - enumClassObject.setField(property.symbol, argument) - } - } - callStack.newSubFrame(enumEntry) callStack.pushCompoundInstruction(enumEntry) // not really a compound instruction callStack.pushCompoundInstruction(enumInitializer) diff --git a/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/Utils.kt b/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/Utils.kt index bd1103bdea9..4719013ad98 100644 --- a/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/Utils.kt +++ b/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/Utils.kt @@ -9,6 +9,7 @@ import org.jetbrains.kotlin.builtins.PrimitiveType import org.jetbrains.kotlin.builtins.StandardNames import org.jetbrains.kotlin.descriptors.ClassKind import org.jetbrains.kotlin.descriptors.Modality +import org.jetbrains.kotlin.ir.IrBuiltIns import org.jetbrains.kotlin.ir.IrElement import org.jetbrains.kotlin.ir.declarations.* import org.jetbrains.kotlin.ir.expressions.* @@ -312,3 +313,21 @@ internal fun State.unsignedToString(): String { else -> (value as Number).toLong().toULong().toString() } } + +internal fun IrEnumEntry.toState(irBuiltIns: IrBuiltIns): Common { + val enumClass = this.symbol.owner.parentAsClass + val enumEntries = enumClass.declarations.filterIsInstance() + val enumClassObject = Common(this.correspondingClass ?: enumClass) + + if (enumEntries.isNotEmpty()) { + val valueArguments = listOf( + Primitive(this.name.asString(), irBuiltIns.stringType), + Primitive(enumEntries.indexOf(this), irBuiltIns.intType) + ) + irBuiltIns.enumClass.owner.declarations.filterIsInstance().zip(valueArguments).forEach { (property, argument) -> + enumClassObject.setField(property.symbol, argument) + } + } + + return enumClassObject +} diff --git a/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/intrinsics/IntrinsicEvaluator.kt b/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/intrinsics/IntrinsicEvaluator.kt index 0e44f5f0d95..d53cdf95848 100644 --- a/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/intrinsics/IntrinsicEvaluator.kt +++ b/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/intrinsics/IntrinsicEvaluator.kt @@ -11,7 +11,6 @@ import org.jetbrains.kotlin.ir.interpreter.IrInterpreterEnvironment import org.jetbrains.kotlin.ir.interpreter.fqName internal object IntrinsicEvaluator { - @OptIn(ExperimentalStdlibApi::class) private val fqNameToHandler: Map = buildMap { listOf( EmptyArray, ArrayOf, ArrayOfNulls, ArrayConstructor, EnumValues, EnumValueOf, diff --git a/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/intrinsics/IntrinsicImplementations.kt b/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/intrinsics/IntrinsicImplementations.kt index 8e18346f0a4..a88e4398976 100644 --- a/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/intrinsics/IntrinsicImplementations.kt +++ b/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/intrinsics/IntrinsicImplementations.kt @@ -7,9 +7,6 @@ package org.jetbrains.kotlin.ir.interpreter.intrinsics import org.jetbrains.kotlin.ir.declarations.* import org.jetbrains.kotlin.ir.interpreter.* -import org.jetbrains.kotlin.ir.interpreter.createCall -import org.jetbrains.kotlin.ir.interpreter.createGetValue -import org.jetbrains.kotlin.ir.interpreter.toIrConst import org.jetbrains.kotlin.ir.interpreter.exceptions.handleUserException import org.jetbrains.kotlin.ir.interpreter.exceptions.stop import org.jetbrains.kotlin.ir.interpreter.exceptions.withExceptionHandler @@ -30,7 +27,7 @@ internal sealed class IntrinsicBase { return listOf(customEvaluateInstruction(irFunction, environment)) } - fun customEvaluateInstruction(irFunction: IrFunction, environment: IrInterpreterEnvironment): CustomInstruction { + private fun customEvaluateInstruction(irFunction: IrFunction, environment: IrInterpreterEnvironment): CustomInstruction { return CustomInstruction { withExceptionHandler(environment) { // Exception handling is used only for indent actions; TODO: drop later evaluate(irFunction, environment) @@ -140,7 +137,7 @@ internal object EnumValueOf : IntrinsicBase() { override fun unwind(irFunction: IrFunction, environment: IrInterpreterEnvironment): List { val enumEntry = getEnumEntryByName(irFunction, environment) ?: return emptyList() - return listOf(customEvaluateInstruction(irFunction, environment), SimpleInstruction(enumEntry)) + return super.unwind(irFunction, environment) + SimpleInstruction(enumEntry) } override fun evaluate(irFunction: IrFunction, environment: IrInterpreterEnvironment) { @@ -158,7 +155,7 @@ internal object EnumIntrinsics : IntrinsicBase() { fun canHandleFunctionWithName(fqName: String, origin: IrDeclarationOrigin): Boolean { if (origin == IrDeclarationOrigin.ENUM_CLASS_SPECIAL_MEMBER) return true - return fqName.startsWith("kotlin.Enum.") + return fqName.startsWith("kotlin.Enum.") && fqName != "kotlin.Enum." } override fun unwind(irFunction: IrFunction, environment: IrInterpreterEnvironment): List { @@ -229,9 +226,9 @@ internal object ArrayConstructor : IntrinsicBase() { } override fun unwind(irFunction: IrFunction, environment: IrInterpreterEnvironment): List { - if (irFunction.valueParameters.size == 1) return listOf(customEvaluateInstruction(irFunction, environment)) + if (irFunction.valueParameters.size == 1) return super.unwind(irFunction, environment) val callStack = environment.callStack - val instructions = mutableListOf(customEvaluateInstruction(irFunction, environment)) + val instructions = super.unwind(irFunction, environment).toMutableList() val sizeSymbol = irFunction.valueParameters[0].symbol val size = callStack.loadState(sizeSymbol).asInt() @@ -298,14 +295,14 @@ internal object AssertIntrinsic : IntrinsicBase() { } override fun unwind(irFunction: IrFunction, environment: IrInterpreterEnvironment): List { - if (irFunction.valueParameters.size == 1) return listOf(customEvaluateInstruction(irFunction, environment)) + if (irFunction.valueParameters.size == 1) return super.unwind(irFunction, environment) val lambdaParameter = irFunction.valueParameters.last() val lambdaState = environment.callStack.loadState(lambdaParameter.symbol) as KFunctionState val call = (lambdaState.invokeSymbol.owner as IrSimpleFunction).createCall() call.dispatchReceiver = lambdaParameter.createGetValue() - return listOf(customEvaluateInstruction(irFunction, environment), CompoundInstruction(call)) + return super.unwind(irFunction, environment) + CompoundInstruction(call) } override fun evaluate(irFunction: IrFunction, environment: IrInterpreterEnvironment) { diff --git a/compiler/testData/codegen/box/constants/kt53480.kt b/compiler/testData/codegen/box/constants/kt53480.kt new file mode 100644 index 00000000000..7029e4e2fcf --- /dev/null +++ b/compiler/testData/codegen/box/constants/kt53480.kt @@ -0,0 +1,15 @@ +// TARGET_BACKEND: JVM_IR +// FILE: J.java +public class J { + public static int f() { return 0; } +} + +// FILE: Main.kt +enum class A { + OK; + val x = J.f() +} + +fun box(): String { + return A.OK.name +} \ No newline at end of file diff --git a/compiler/testData/ir/interpreter/kt53480.kt b/compiler/testData/ir/interpreter/kt53480.kt new file mode 100644 index 00000000000..dfb018b2185 --- /dev/null +++ b/compiler/testData/ir/interpreter/kt53480.kt @@ -0,0 +1,12 @@ +// FILE: J.java +public class J { + public static int f() { return 0; } +} + +// FILE: Main.kt +enum class A { + E; + val x = J.f() +} + +const val name = A.E.name \ No newline at end of file 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 9ceb4de8d5c..cf526510ca6 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 @@ -8133,6 +8133,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/constants/foldingBinaryOpsUnsignedConst.kt"); } + @Test + @TestMetadata("kt53480.kt") + public void testKt53480() throws Exception { + runTest("compiler/testData/codegen/box/constants/kt53480.kt"); + } + @Test @TestMetadata("kt9532.kt") public void testKt9532() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ir/interpreter/IrInterpreterAfterFir2IrTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ir/interpreter/IrInterpreterAfterFir2IrTestGenerated.java index 745ee5c8b3a..5b97113abb7 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ir/interpreter/IrInterpreterAfterFir2IrTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ir/interpreter/IrInterpreterAfterFir2IrTestGenerated.java @@ -205,6 +205,12 @@ public class IrInterpreterAfterFir2IrTestGenerated extends AbstractIrInterpreter runTest("compiler/testData/ir/interpreter/interfaceDefault.kt"); } + @Test + @TestMetadata("kt53480.kt") + public void testKt53480() throws Exception { + runTest("compiler/testData/ir/interpreter/kt53480.kt"); + } + @Test @TestMetadata("lambda.kt") public void testLambda() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ir/interpreter/IrInterpreterAfterPsi2IrTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ir/interpreter/IrInterpreterAfterPsi2IrTestGenerated.java index c937f16c495..17d82773aea 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ir/interpreter/IrInterpreterAfterPsi2IrTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ir/interpreter/IrInterpreterAfterPsi2IrTestGenerated.java @@ -205,6 +205,12 @@ public class IrInterpreterAfterPsi2IrTestGenerated extends AbstractIrInterpreter runTest("compiler/testData/ir/interpreter/interfaceDefault.kt"); } + @Test + @TestMetadata("kt53480.kt") + public void testKt53480() throws Exception { + runTest("compiler/testData/ir/interpreter/kt53480.kt"); + } + @Test @TestMetadata("lambda.kt") public void testLambda() throws Exception {