diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBytecodeTextTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBytecodeTextTestGenerated.java index e009c93760e..17b9451c346 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBytecodeTextTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBytecodeTextTestGenerated.java @@ -3243,40 +3243,6 @@ public class FirBytecodeTextTestGenerated extends AbstractFirBytecodeTextTest { } } - @Nested - @TestMetadata("compiler/testData/codegen/bytecodeText/ifNullChain") - @TestDataPath("$PROJECT_ROOT") - public class IfNullChain { - @Test - public void testAllFilesPresentInIfNullChain() throws Exception { - KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/codegen/bytecodeText/ifNullChain"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JVM_IR, true); - } - - @Test - @TestMetadata("safeCallChain1.kt") - public void testSafeCallChain1() throws Exception { - runTest("compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChain1.kt"); - } - - @Test - @TestMetadata("safeCallChain2.kt") - public void testSafeCallChain2() throws Exception { - runTest("compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChain2.kt"); - } - - @Test - @TestMetadata("safeCallChainMemberExt1.kt") - public void testSafeCallChainMemberExt1() throws Exception { - runTest("compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChainMemberExt1.kt"); - } - - @Test - @TestMetadata("safeCallChainMemberExt2.kt") - public void testSafeCallChainMemberExt2() throws Exception { - runTest("compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChainMemberExt2.kt"); - } - } - @Nested @TestMetadata("compiler/testData/codegen/bytecodeText/inline") @TestDataPath("$PROJECT_ROOT") @@ -5413,6 +5379,46 @@ public class FirBytecodeTextTestGenerated extends AbstractFirBytecodeTextTest { } } + @Nested + @TestMetadata("compiler/testData/codegen/bytecodeText/temporaryVals") + @TestDataPath("$PROJECT_ROOT") + public class TemporaryVals { + @Test + public void testAllFilesPresentInTemporaryVals() throws Exception { + KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/codegen/bytecodeText/temporaryVals"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JVM_IR, true); + } + + @Test + @TestMetadata("arrayCompoundAssignment.kt") + public void testArrayCompoundAssignment() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/temporaryVals/arrayCompoundAssignment.kt"); + } + + @Test + @TestMetadata("safeCallChain1.kt") + public void testSafeCallChain1() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChain1.kt"); + } + + @Test + @TestMetadata("safeCallChain2.kt") + public void testSafeCallChain2() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChain2.kt"); + } + + @Test + @TestMetadata("safeCallChainMemberExt1.kt") + public void testSafeCallChainMemberExt1() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChainMemberExt1.kt"); + } + + @Test + @TestMetadata("safeCallChainMemberExt2.kt") + public void testSafeCallChainMemberExt2() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChainMemberExt2.kt"); + } + } + @Nested @TestMetadata("compiler/testData/codegen/bytecodeText/toArray") @TestDataPath("$PROJECT_ROOT") diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/inline/FunctionInlining.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/inline/FunctionInlining.kt index 4d12400abc2..18ef1c7c9df 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/inline/FunctionInlining.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/inline/FunctionInlining.kt @@ -15,9 +15,6 @@ import org.jetbrains.kotlin.backend.common.ir.isPure import org.jetbrains.kotlin.backend.common.lower.at import org.jetbrains.kotlin.backend.common.lower.createIrBuilder import org.jetbrains.kotlin.builtins.StandardNames -import org.jetbrains.kotlin.config.LanguageFeature -import org.jetbrains.kotlin.config.LanguageVersionSettings -import org.jetbrains.kotlin.config.languageVersionSettings import org.jetbrains.kotlin.ir.IrElement import org.jetbrains.kotlin.ir.IrStatement import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET @@ -314,7 +311,7 @@ class FunctionInlining( function.valueParameters.size ) else -> - kotlin.error("Unknown function kind : ${function.render()}") + error("Unknown function kind : ${function.render()}") } }.apply { for (parameter in functionParameters) { @@ -601,6 +598,9 @@ class FunctionInlining( fun withLocation(startOffset: Int, endOffset: Int) = IrGetValueImpl(startOffset, endOffset, type, symbol, origin) + + override fun copyWithOffsets(newStartOffset: Int, newEndOffset: Int): IrGetValue = + withLocation(newStartOffset, newEndOffset) } } diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmOptimizationLowering.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmOptimizationLowering.kt index e60cb6bfce1..dd2dc72c628 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmOptimizationLowering.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmOptimizationLowering.kt @@ -102,302 +102,314 @@ class JvmOptimizationLowering(val context: JvmBackendContext) : FileLoweringPass AsmUtil.isPrimitive(context.typeMapper.mapType(this)) override fun lower(irFile: IrFile) { - val transformer = object : IrElementTransformer { + irFile.transformChildren(Transformer(), null) + } - // Thread the current class through the transformations in order to replace - // final default accessor calls with direct backing field access when - // possible. - override fun visitClass(declaration: IrClass, data: IrClass?): IrStatement { - declaration.transformChildren(this, declaration) - return declaration + inner class Transformer : IrElementTransformer { + + private val dontTouchTemporaryVals = HashSet() + + // Thread the current class through the transformations in order to replace + // final default accessor calls with direct backing field access when + // possible. + override fun visitClass(declaration: IrClass, data: IrClass?): IrStatement { + declaration.transformChildren(this, declaration) + return declaration + } + + // For some functions, we clear the current class field since the code could end up + // in another class then the one it is nested under in the IR. + // TODO: Loosen this up for local functions for lambdas passed as an inline lambda + // argument to an inline function. In that case the code does end up in the current class. + override fun visitFunction(declaration: IrFunction, data: IrClass?): IrStatement { + val codeMightBeGeneratedInDifferentClass = declaration.isSuspend || + declaration.isInline || + declaration.origin == IrDeclarationOrigin.LOCAL_FUNCTION_FOR_LAMBDA + declaration.transformChildren(this, data.takeUnless { codeMightBeGeneratedInDifferentClass }) + return declaration + } + + override fun visitCall(expression: IrCall, data: IrClass?): IrExpression { + expression.transformChildren(this, data) + + if (expression.symbol.owner.origin == IrDeclarationOrigin.DEFAULT_PROPERTY_ACCESSOR) { + if (data == null) return expression + val simpleFunction = (expression.symbol.owner as? IrSimpleFunction) ?: return expression + val property = simpleFunction.correspondingPropertySymbol?.owner ?: return expression + if (property.isLateinit) return expression + return optimizePropertyAccess(expression, simpleFunction, property, data) } - // For some functions, we clear the current class field since the code could end up - // in another class then the one it is nested under in the IR. - // TODO: Loosen this up for local functions for lambdas passed as an inline lambda - // argument to an inline function. In that case the code does end up in the current class. - override fun visitFunction(declaration: IrFunction, data: IrClass?): IrStatement { - val codeMightBeGeneratedInDifferentClass = declaration.isSuspend || - declaration.isInline || - declaration.origin == IrDeclarationOrigin.LOCAL_FUNCTION_FOR_LAMBDA - declaration.transformChildren(this, data.takeUnless { codeMightBeGeneratedInDifferentClass }) - return declaration + if (isNegation(expression, context) && isNegation(expression.dispatchReceiver!!, context)) { + return (expression.dispatchReceiver as IrCall).dispatchReceiver!! } - override fun visitCall(expression: IrCall, data: IrClass?): IrExpression { - expression.transformChildren(this, data) + getOperandsIfCallToEQEQOrEquals(expression)?.let { (left, right) -> + if (left.isNullConst() && right.isNullConst()) + return IrConstImpl.constTrue(expression.startOffset, expression.endOffset, context.irBuiltIns.booleanType) - if (expression.symbol.owner.origin == IrDeclarationOrigin.DEFAULT_PROPERTY_ACCESSOR) { - if (data == null) return expression - val simpleFunction = (expression.symbol.owner as? IrSimpleFunction) ?: return expression - val property = simpleFunction.correspondingPropertySymbol?.owner ?: return expression - if (property.isLateinit) return expression - return optimizePropertyAccess(expression, simpleFunction, property, data) - } + if (left.isNullConst() && right is IrConst<*> || right.isNullConst() && left is IrConst<*>) + return IrConstImpl.constFalse(expression.startOffset, expression.endOffset, context.irBuiltIns.booleanType) - if (isNegation(expression, context) && isNegation(expression.dispatchReceiver!!, context)) { - return (expression.dispatchReceiver as IrCall).dispatchReceiver!! - } - - getOperandsIfCallToEQEQOrEquals(expression)?.let { (left, right) -> - if (left.isNullConst() && right.isNullConst()) - return IrConstImpl.constTrue(expression.startOffset, expression.endOffset, context.irBuiltIns.booleanType) - - if (left.isNullConst() && right is IrConst<*> || right.isNullConst() && left is IrConst<*>) - return IrConstImpl.constFalse(expression.startOffset, expression.endOffset, context.irBuiltIns.booleanType) - - if (expression.symbol == context.irBuiltIns.eqeqSymbol) { - if (right.type.isJvmPrimitive()) { - parseSafeCall(left)?.let { return rewriteSafeCallEqeqPrimitive(it, expression) } - } - if (left.type.isJvmPrimitive()) { - parseSafeCall(right)?.let { return rewritePrimitiveEqeqSafeCall(it, expression) } - } + if (expression.symbol == context.irBuiltIns.eqeqSymbol) { + if (right.type.isJvmPrimitive()) { + parseSafeCall(left)?.let { return rewriteSafeCallEqeqPrimitive(it, expression) } + } + if (left.type.isJvmPrimitive()) { + parseSafeCall(right)?.let { return rewritePrimitiveEqeqSafeCall(it, expression) } } } - - return expression } - private fun IrBuilderWithScope.ifSafe(safeCall: SafeCallInfo, expr: IrExpression): IrExpression = - irBlock(origin = IrStatementOrigin.SAFE_CALL) { - +safeCall.tmpVal - +irIfThenElse(expr.type, safeCall.ifNullBranch.condition, irFalse(), expr) - } + return expression + } - // Fuse safe call with primitive equality to avoid boxing the primitive. `a?.x == p`: - // { val tmp = a; if (tmp == null) null else tmp.x } == p` - // is transformed to: - // { val tmp = a; if (tmp == null) false else tmp.x == p } - // Note that the original IR implied that `p` is always evaluated, but the rewritten version - // only does so if `a` is not null. This is how the old backend does it, and it's consistent - // with `a?.x?.equals(p)`. - private fun rewriteSafeCallEqeqPrimitive(safeCall: SafeCallInfo, eqeqCall: IrCall): IrExpression = - context.createJvmIrBuilder(safeCall.scopeSymbol).run { - ifSafe(safeCall, eqeqCall.apply { putValueArgument(0, safeCall.ifNotNullBranch.result) }) - } - - // Fuse safe call with primitive equality to avoid boxing the primitive. 'p == a?.x': - // p == { val tmp = a; if (tmp == null) null else tmp.x } - // is transformed to: - // { val tmp_p = p; { val tmp = a; if (tmp == null) false else p == tmp } } - // Note that `p` is evaluated even if `a` is null, which is again consistent with both the old backend - // and `p.equals(a?.x)`. - private fun rewritePrimitiveEqeqSafeCall(safeCall: SafeCallInfo, eqeqCall: IrCall): IrExpression = - context.createJvmIrBuilder(safeCall.scopeSymbol).run { - val primitive = eqeqCall.getValueArgument(0)!! - if (primitive.isTrivial()) { - ifSafe(safeCall, eqeqCall.apply { putValueArgument(1, safeCall.ifNotNullBranch.result) }) - } else { - // The extra block for `p`'s variable is intentional as adding it into the inner block - // would make it no longer look like a safe call to `IfNullExpressionFusionLowering`. - irBlock { - +ifSafe(safeCall, eqeqCall.apply { - putValueArgument(0, irGet(irTemporary(primitive))) - putValueArgument(1, safeCall.ifNotNullBranch.result) - }) - } - } - } - - private fun optimizePropertyAccess( - expression: IrCall, - accessor: IrSimpleFunction, - property: IrProperty, - currentClass: IrClass - ): IrExpression { - if (accessor.parentAsClass == currentClass && - property.backingField?.parentAsClass == currentClass && - accessor.modality == Modality.FINAL && - !accessor.isExternal - ) { - val backingField = property.backingField!! - val receiver = expression.dispatchReceiver - return context.createIrBuilder(expression.symbol, expression.startOffset, expression.endOffset).irBlock(expression) { - if (backingField.isStatic && receiver != null && receiver !is IrGetValue) { - // If the field is static, evaluate the receiver for potential side effects. - +receiver.coerceToUnit(context.irBuiltIns, this@JvmOptimizationLowering.context.typeSystem) - } - if (accessor.valueParameters.isNotEmpty()) { - +irSetField( - receiver.takeUnless { backingField.isStatic }, - backingField, - expression.getValueArgument(expression.valueArgumentsCount - 1)!! - ) - } else { - +irGetField(receiver.takeUnless { backingField.isStatic }, backingField) - } - } - } - return expression + private fun IrBuilderWithScope.ifSafe(safeCall: SafeCallInfo, expr: IrExpression): IrExpression = + irBlock(origin = IrStatementOrigin.SAFE_CALL) { + +safeCall.tmpVal + +irIfThenElse(expr.type, safeCall.ifNullBranch.condition, irFalse(), expr) } - override fun visitWhen(expression: IrWhen, data: IrClass?): IrExpression { - val isCompilerGenerated = expression.origin == null - expression.transformChildren(this, data) - // Remove all branches with constant false condition. - expression.branches.removeIf { - it.condition.isFalseConst() && isCompilerGenerated - } - if (expression.origin == IrStatementOrigin.ANDAND) { - assert( - expression.type.isBoolean() - && expression.branches.size == 2 - && expression.branches[1].condition.isTrueConst() - && expression.branches[1].result.isFalseConst() - ) { - "ANDAND condition should have an 'if true then false' body on its second branch. " + - "Failing expression: ${expression.dump()}" - } - // Replace conjunction condition with intrinsic "and" function call - return IrCallImpl.fromSymbolOwner( - expression.startOffset, - expression.endOffset, - context.irBuiltIns.booleanType, - context.irBuiltIns.andandSymbol - ).apply { - putValueArgument(0, expression.branches[0].condition) - putValueArgument(1, expression.branches[0].result) - } - } - if (expression.origin == IrStatementOrigin.OROR) { - assert( - expression.type.isBoolean() - && expression.branches.size == 2 - && expression.branches[0].result.isTrueConst() - && expression.branches[1].condition.isTrueConst() - ) { - "OROR condition should have an 'if a then true' body on its first branch, " + - "and an 'if true then b' body on its second branch. " + - "Failing expression: ${expression.dump()}" - } - return IrCallImpl.fromSymbolOwner( - expression.startOffset, - expression.endOffset, - context.irBuiltIns.booleanType, - context.irBuiltIns.ororSymbol - ).apply { - putValueArgument(0, expression.branches[0].condition) - putValueArgument(1, expression.branches[1].result) - } - } - // If the only condition that is left has a constant true condition remove the - // when in favor of the result. If there are no conditions left, remove the when - // entirely and replace it with an empty block. - return if (expression.branches.size == 0) { - IrBlockImpl(expression.startOffset, expression.endOffset, context.irBuiltIns.unitType) + // Fuse safe call with primitive equality to avoid boxing the primitive. `a?.x == p`: + // { val tmp = a; if (tmp == null) null else tmp.x } == p` + // is transformed to: + // { val tmp = a; if (tmp == null) false else tmp.x == p } + // Note that the original IR implied that `p` is always evaluated, but the rewritten version + // only does so if `a` is not null. This is how the old backend does it, and it's consistent + // with `a?.x?.equals(p)`. + private fun rewriteSafeCallEqeqPrimitive(safeCall: SafeCallInfo, eqeqCall: IrCall): IrExpression = + context.createJvmIrBuilder(safeCall.scopeSymbol).run { + ifSafe(safeCall, eqeqCall.apply { putValueArgument(0, safeCall.ifNotNullBranch.result) }) + } + + // Fuse safe call with primitive equality to avoid boxing the primitive. 'p == a?.x': + // p == { val tmp = a; if (tmp == null) null else tmp.x } + // is transformed to: + // { val tmp_p = p; { val tmp = a; if (tmp == null) false else p == tmp } } + // Note that `p` is evaluated even if `a` is null, which is again consistent with both the old backend + // and `p.equals(a?.x)`. + private fun rewritePrimitiveEqeqSafeCall(safeCall: SafeCallInfo, eqeqCall: IrCall): IrExpression = + context.createJvmIrBuilder(safeCall.scopeSymbol).run { + val primitive = eqeqCall.getValueArgument(0)!! + if (primitive.isTrivial()) { + ifSafe(safeCall, eqeqCall.apply { putValueArgument(1, safeCall.ifNotNullBranch.result) }) } else { - expression.branches.first().takeIf { it.condition.isTrueConst() && isCompilerGenerated }?.result ?: expression - } - } - - private fun isImmutableTemporaryVariableWithConstantValue(statement: IrStatement): Boolean { - return statement is IrVariable && - statement.origin == IrDeclarationOrigin.IR_TEMPORARY_VARIABLE && - !statement.isVar && - statement.initializer is IrConst<*> - } - - private fun removeUnnecessaryTemporaryVariables(statements: MutableList) { - // Remove declarations of immutable temporary variables with constant values. - // IrGetValue operations for such temporary variables are replaced - // by the initializer IrConst. This makes sure that we do not load and - // store constants in/from locals. For example - // - // "StringConstant"!! - // - // introduces a temporary variable for the string constant and generates - // a null check - // - // block - // temp = "StringConstant" - // when (eq(temp, null)) - // (true) -> throwNpe() - // (false) -> temp - // - // When generating code, this stores the string constant in a local and loads - // it from there. The removal of the temporary and the replacement of the loads - // of the temporary (see visitGetValue) with the constant avoid generating local - // loads and stores by turning this into - // - // block - // when (eq("StringConstant", null)) - // (true) -> throwNpe() - // (false) -> "StringConstant" - // - // which allows the equality check to be simplified away and we end up with - // just a const string load. - statements.removeIf { - isImmutableTemporaryVariableWithConstantValue(it) - } - - // Remove a block that contains only two statements: the declaration of a temporary - // variable and a load of the value of that temporary variable with just the initializer - // for the temporary variable. We only perform this transformation for compiler generated - // temporary variables. Local variables can be changed at runtime and therefore eliminating - // an actual local variable changes debugging behavior. - // - // This helps avoid temporary variables even for side-effecting expressions when they are - // not needed. Having a temporary variable leads to local loads and stores in the - // generated java bytecode which are not necessary. For example - // - // 42.toLong()!! - // - // introduces a temporary variable for the toLong() call and a null check - // block - // temp = 42.toLong() - // when (eq(temp, null)) - // (true) -> throwNep() - // (false) -> temp - // - // the when is simplified because long is a primitive type, which leaves us with - // - // block - // temp = 42.toLong() - // temp - // - // which can be simplified to simply - // - // block - // 42.toLong() - // - // Doing so we avoid local loads and stores. - if (statements.size == 2) { - val first = statements[0] - val second = statements[1] - if (first is IrVariable - && first.origin == IrDeclarationOrigin.IR_TEMPORARY_VARIABLE - && second is IrGetValue - && first.symbol == second.symbol - ) { - statements.clear() - first.initializer?.let { statements.add(it) } + // The extra block for `p`'s variable is intentional as adding it into the inner block + // would make it no longer look like a safe call to `IfNullExpressionFusionLowering`. + irBlock { + +ifSafe(safeCall, eqeqCall.apply { + putValueArgument(0, irGet(irTemporary(primitive))) + putValueArgument(1, safeCall.ifNotNullBranch.result) + }) } } } - override fun visitBlockBody(body: IrBlockBody, data: IrClass?): IrBody { - body.transformChildren(this, data) - removeUnnecessaryTemporaryVariables(body.statements) - return body + private fun optimizePropertyAccess( + expression: IrCall, + accessor: IrSimpleFunction, + property: IrProperty, + currentClass: IrClass + ): IrExpression { + if (accessor.parentAsClass == currentClass && + property.backingField?.parentAsClass == currentClass && + accessor.modality == Modality.FINAL && + !accessor.isExternal + ) { + val backingField = property.backingField!! + val receiver = expression.dispatchReceiver + return context.createIrBuilder(expression.symbol, expression.startOffset, expression.endOffset).irBlock(expression) { + if (backingField.isStatic && receiver != null && receiver !is IrGetValue) { + // If the field is static, evaluate the receiver for potential side effects. + +receiver.coerceToUnit(context.irBuiltIns, this@JvmOptimizationLowering.context.typeSystem) + } + if (accessor.valueParameters.isNotEmpty()) { + +irSetField( + receiver.takeUnless { backingField.isStatic }, + backingField, + expression.getValueArgument(expression.valueArgumentsCount - 1)!! + ) + } else { + +irGetField(receiver.takeUnless { backingField.isStatic }, backingField) + } + } + } + return expression + } + + override fun visitWhen(expression: IrWhen, data: IrClass?): IrExpression { + val isCompilerGenerated = expression.origin == null + expression.transformChildren(this, data) + // Remove all branches with constant false condition. + expression.branches.removeIf { + it.condition.isFalseConst() && isCompilerGenerated + } + if (expression.origin == IrStatementOrigin.ANDAND) { + assert( + expression.type.isBoolean() + && expression.branches.size == 2 + && expression.branches[1].condition.isTrueConst() + && expression.branches[1].result.isFalseConst() + ) { + "ANDAND condition should have an 'if true then false' body on its second branch. " + + "Failing expression: ${expression.dump()}" + } + // Replace conjunction condition with intrinsic "and" function call + return IrCallImpl.fromSymbolOwner( + expression.startOffset, + expression.endOffset, + context.irBuiltIns.booleanType, + context.irBuiltIns.andandSymbol + ).apply { + putValueArgument(0, expression.branches[0].condition) + putValueArgument(1, expression.branches[0].result) + } + } + if (expression.origin == IrStatementOrigin.OROR) { + assert( + expression.type.isBoolean() + && expression.branches.size == 2 + && expression.branches[0].result.isTrueConst() + && expression.branches[1].condition.isTrueConst() + ) { + "OROR condition should have an 'if a then true' body on its first branch, " + + "and an 'if true then b' body on its second branch. " + + "Failing expression: ${expression.dump()}" + } + return IrCallImpl.fromSymbolOwner( + expression.startOffset, + expression.endOffset, + context.irBuiltIns.booleanType, + context.irBuiltIns.ororSymbol + ).apply { + putValueArgument(0, expression.branches[0].condition) + putValueArgument(1, expression.branches[1].result) + } } - override fun visitContainerExpression(expression: IrContainerExpression, data: IrClass?): IrExpression { - expression.transformChildren(this, data) - removeUnnecessaryTemporaryVariables(expression.statements) - return expression + // If there are no conditions left, remove the 'when' entirely and replace it with an empty block. + if (expression.branches.size == 0) { + return IrBlockImpl(expression.startOffset, expression.endOffset, context.irBuiltIns.unitType) } - override fun visitGetValue(expression: IrGetValue, data: IrClass?): IrExpression { - // Replace IrGetValue of an immutable temporary variable with a constant - // initializer with the constant initializer. - val variable = expression.symbol.owner - return if (isImmutableTemporaryVariableWithConstantValue(variable)) - ((variable as IrVariable).initializer!! as IrConst<*>).copyWithOffsets(expression.startOffset, expression.endOffset) - else + // If the only condition that is left has a constant true condition, remove the 'when' in favor of the result. + val firstBranch = expression.branches.first() + if (firstBranch.condition.isTrueConst() && isCompilerGenerated) { + return firstBranch.result + } + + return expression + } + + private fun getInlineableValueForTemporaryVal(statement: IrStatement): IrExpression? { + val variable = statement as? IrVariable ?: return null + if (variable.origin != IrDeclarationOrigin.IR_TEMPORARY_VARIABLE || variable.isVar) return null + if (variable in dontTouchTemporaryVals) return null + + when (val initializer = variable.initializer) { + is IrConst<*> -> + return initializer + is IrGetValue -> + when (val initializerValue = initializer.symbol.owner) { + is IrVariable -> + return when { + initializerValue.isVar -> + null + initializerValue.origin == IrDeclarationOrigin.IR_TEMPORARY_VARIABLE -> + getInlineableValueForTemporaryVal(initializerValue) + ?: initializer + else -> + initializer + } + is IrValueParameter -> + return initializer + } + } + + return null + } + + private fun removeUnnecessaryTemporaryVariables(statements: MutableList) { + // Remove declarations of immutable temporary variables that can be inlined. + statements.removeIf { + getInlineableValueForTemporaryVal(it) != null + } + + // Remove a block that contains only two statements: the declaration of a temporary + // variable and a load of the value of that temporary variable with just the initializer + // for the temporary variable. We only perform this transformation for compiler generated + // temporary variables. Local variables can be changed at runtime and therefore eliminating + // an actual local variable changes debugging behavior. + // + // This helps avoid temporary variables even for side-effecting expressions when they are + // not needed. Having a temporary variable leads to local loads and stores in the + // generated java bytecode which are not necessary. For example + // + // 42.toLong()!! + // + // introduces a temporary variable for the toLong() call and a null check + // block + // temp = 42.toLong() + // when (eq(temp, null)) + // (true) -> throwNep() + // (false) -> temp + // + // the when is simplified because long is a primitive type, which leaves us with + // + // block + // temp = 42.toLong() + // temp + // + // which can be simplified to simply + // + // block + // 42.toLong() + // + // Doing so we avoid local loads and stores. + if (statements.size == 2) { + val first = statements[0] + val second = statements[1] + if (first is IrVariable + && first.origin == IrDeclarationOrigin.IR_TEMPORARY_VARIABLE + && second is IrGetValue + && first.symbol == second.symbol + ) { + statements.clear() + first.initializer?.let { statements.add(it) } + } + } + } + + override fun visitBlockBody(body: IrBlockBody, data: IrClass?): IrBody { + body.transformChildren(this, data) + removeUnnecessaryTemporaryVariables(body.statements) + return body + } + + override fun visitContainerExpression(expression: IrContainerExpression, data: IrClass?): IrExpression { + val safeCall = parseSafeCall(expression) + if (safeCall != null) { + // Don't optimize out temporary values for safe calls (yet), so that safe call-based equality checks can be optimized. + dontTouchTemporaryVals.add(safeCall.tmpVal) + } + + expression.transformChildren(this, data) + removeUnnecessaryTemporaryVariables(expression.statements) + return expression + } + + override fun visitGetValue(expression: IrGetValue, data: IrClass?): IrExpression { + // Replace IrGetValue of an immutable temporary variable with a constant + // initializer with the constant initializer. + val variable = expression.symbol.owner + return when (val replacement = getInlineableValueForTemporaryVal(variable)) { + is IrConst<*> -> + replacement.copyWithOffsets(expression.startOffset, expression.endOffset) + is IrGetValue -> + replacement.copyWithOffsets(expression.startOffset, expression.endOffset) + else -> expression } } - irFile.transformChildren(transformer, null) } } diff --git a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/expressions/IrValueAccessExpression.kt b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/expressions/IrValueAccessExpression.kt index b30f24dbad5..dd206877663 100644 --- a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/expressions/IrValueAccessExpression.kt +++ b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/expressions/IrValueAccessExpression.kt @@ -16,16 +16,16 @@ package org.jetbrains.kotlin.ir.expressions -import org.jetbrains.kotlin.ir.symbols.IrValueParameterSymbol import org.jetbrains.kotlin.ir.symbols.IrValueSymbol -import org.jetbrains.kotlin.ir.symbols.IrVariableSymbol abstract class IrValueAccessExpression : IrDeclarationReference() { abstract override val symbol: IrValueSymbol abstract val origin: IrStatementOrigin? } -abstract class IrGetValue : IrValueAccessExpression() +abstract class IrGetValue : IrValueAccessExpression() { + abstract fun copyWithOffsets(newStartOffset: Int, newEndOffset: Int): IrGetValue +} abstract class IrSetValue : IrValueAccessExpression() { abstract override val symbol: IrValueSymbol diff --git a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/expressions/impl/IrGetValueImpl.kt b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/expressions/impl/IrGetValueImpl.kt index e8c2b4c0bb4..e8f40498ae7 100644 --- a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/expressions/impl/IrGetValueImpl.kt +++ b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/expressions/impl/IrGetValueImpl.kt @@ -27,4 +27,7 @@ class IrGetValueImpl( override fun accept(visitor: IrElementVisitor, data: D): R = visitor.visitGetValue(this, data) + + override fun copyWithOffsets(newStartOffset: Int, newEndOffset: Int): IrGetValue = + IrGetValueImpl(newStartOffset, newEndOffset, type, symbol, origin) } diff --git a/compiler/testData/codegen/bytecodeText/temporaryVals/arrayCompoundAssignment.kt b/compiler/testData/codegen/bytecodeText/temporaryVals/arrayCompoundAssignment.kt new file mode 100644 index 00000000000..e651f964854 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/temporaryVals/arrayCompoundAssignment.kt @@ -0,0 +1,9 @@ +fun test(xs: IntArray, dx: Int) { + for (i in xs.indices) { + xs[i] += dx + } +} + +// JVM_IR_TEMPLATES +// 5 ALOAD +// 6 ILOAD diff --git a/compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChain1.kt b/compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChain1.kt similarity index 100% rename from compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChain1.kt rename to compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChain1.kt diff --git a/compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChain2.kt b/compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChain2.kt similarity index 100% rename from compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChain2.kt rename to compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChain2.kt diff --git a/compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChainMemberExt1.kt b/compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChainMemberExt1.kt similarity index 100% rename from compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChainMemberExt1.kt rename to compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChainMemberExt1.kt diff --git a/compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChainMemberExt2.kt b/compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChainMemberExt2.kt similarity index 100% rename from compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChainMemberExt2.kt rename to compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChainMemberExt2.kt diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BytecodeTextTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BytecodeTextTestGenerated.java index cfd8337098d..fc25fb0b113 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BytecodeTextTestGenerated.java @@ -3099,40 +3099,6 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { } } - @Nested - @TestMetadata("compiler/testData/codegen/bytecodeText/ifNullChain") - @TestDataPath("$PROJECT_ROOT") - public class IfNullChain { - @Test - public void testAllFilesPresentInIfNullChain() throws Exception { - KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/codegen/bytecodeText/ifNullChain"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JVM, true); - } - - @Test - @TestMetadata("safeCallChain1.kt") - public void testSafeCallChain1() throws Exception { - runTest("compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChain1.kt"); - } - - @Test - @TestMetadata("safeCallChain2.kt") - public void testSafeCallChain2() throws Exception { - runTest("compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChain2.kt"); - } - - @Test - @TestMetadata("safeCallChainMemberExt1.kt") - public void testSafeCallChainMemberExt1() throws Exception { - runTest("compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChainMemberExt1.kt"); - } - - @Test - @TestMetadata("safeCallChainMemberExt2.kt") - public void testSafeCallChainMemberExt2() throws Exception { - runTest("compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChainMemberExt2.kt"); - } - } - @Nested @TestMetadata("compiler/testData/codegen/bytecodeText/inline") @TestDataPath("$PROJECT_ROOT") @@ -5269,6 +5235,46 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { } } + @Nested + @TestMetadata("compiler/testData/codegen/bytecodeText/temporaryVals") + @TestDataPath("$PROJECT_ROOT") + public class TemporaryVals { + @Test + public void testAllFilesPresentInTemporaryVals() throws Exception { + KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/codegen/bytecodeText/temporaryVals"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JVM, true); + } + + @Test + @TestMetadata("arrayCompoundAssignment.kt") + public void testArrayCompoundAssignment() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/temporaryVals/arrayCompoundAssignment.kt"); + } + + @Test + @TestMetadata("safeCallChain1.kt") + public void testSafeCallChain1() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChain1.kt"); + } + + @Test + @TestMetadata("safeCallChain2.kt") + public void testSafeCallChain2() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChain2.kt"); + } + + @Test + @TestMetadata("safeCallChainMemberExt1.kt") + public void testSafeCallChainMemberExt1() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChainMemberExt1.kt"); + } + + @Test + @TestMetadata("safeCallChainMemberExt2.kt") + public void testSafeCallChainMemberExt2() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChainMemberExt2.kt"); + } + } + @Nested @TestMetadata("compiler/testData/codegen/bytecodeText/toArray") @TestDataPath("$PROJECT_ROOT") diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeTextTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeTextTestGenerated.java index ca7a644fa33..3e550e343fc 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeTextTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeTextTestGenerated.java @@ -3243,40 +3243,6 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest { } } - @Nested - @TestMetadata("compiler/testData/codegen/bytecodeText/ifNullChain") - @TestDataPath("$PROJECT_ROOT") - public class IfNullChain { - @Test - public void testAllFilesPresentInIfNullChain() throws Exception { - KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/codegen/bytecodeText/ifNullChain"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JVM_IR, true); - } - - @Test - @TestMetadata("safeCallChain1.kt") - public void testSafeCallChain1() throws Exception { - runTest("compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChain1.kt"); - } - - @Test - @TestMetadata("safeCallChain2.kt") - public void testSafeCallChain2() throws Exception { - runTest("compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChain2.kt"); - } - - @Test - @TestMetadata("safeCallChainMemberExt1.kt") - public void testSafeCallChainMemberExt1() throws Exception { - runTest("compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChainMemberExt1.kt"); - } - - @Test - @TestMetadata("safeCallChainMemberExt2.kt") - public void testSafeCallChainMemberExt2() throws Exception { - runTest("compiler/testData/codegen/bytecodeText/ifNullChain/safeCallChainMemberExt2.kt"); - } - } - @Nested @TestMetadata("compiler/testData/codegen/bytecodeText/inline") @TestDataPath("$PROJECT_ROOT") @@ -5413,6 +5379,46 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest { } } + @Nested + @TestMetadata("compiler/testData/codegen/bytecodeText/temporaryVals") + @TestDataPath("$PROJECT_ROOT") + public class TemporaryVals { + @Test + public void testAllFilesPresentInTemporaryVals() throws Exception { + KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/codegen/bytecodeText/temporaryVals"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JVM_IR, true); + } + + @Test + @TestMetadata("arrayCompoundAssignment.kt") + public void testArrayCompoundAssignment() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/temporaryVals/arrayCompoundAssignment.kt"); + } + + @Test + @TestMetadata("safeCallChain1.kt") + public void testSafeCallChain1() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChain1.kt"); + } + + @Test + @TestMetadata("safeCallChain2.kt") + public void testSafeCallChain2() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChain2.kt"); + } + + @Test + @TestMetadata("safeCallChainMemberExt1.kt") + public void testSafeCallChainMemberExt1() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChainMemberExt1.kt"); + } + + @Test + @TestMetadata("safeCallChainMemberExt2.kt") + public void testSafeCallChainMemberExt2() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/temporaryVals/safeCallChainMemberExt2.kt"); + } + } + @Nested @TestMetadata("compiler/testData/codegen/bytecodeText/toArray") @TestDataPath("$PROJECT_ROOT")