diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/RedundantGotoMethodTransformer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/RedundantGotoMethodTransformer.kt index c005ef93f74..b4dd65063e2 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/RedundantGotoMethodTransformer.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/RedundantGotoMethodTransformer.kt @@ -31,6 +31,7 @@ class RedundantGotoMethodTransformer : MethodTransformer() { * (1) subsequent labels * ... * goto Label (can be removed) + * nop (any number of them, or maybe none; will be removed by RedundantNopsCleanupMethodTransformer) * Label: * ... * (2) indirect goto @@ -49,6 +50,10 @@ class RedundantGotoMethodTransformer : MethodTransformer() { var pendingGoto: JumpInsnNode? = null for (insn in insns) { + // We can remove jumps over labels, NOPs, GOTOs with the same target, and fake + // instructions used to describe the current frame state. We have to keep jumps + // over line numbers, though, as otherwise something like an `if` with an empty + // `else` will trigger a breakpoint inside the `else` even when the condition is true. when { insn is LabelNode -> { currentLabels.add(insn) @@ -62,8 +67,7 @@ class RedundantGotoMethodTransformer : MethodTransformer() { currentLabels.clear() } } - insn is LineNumberNode -> pendingGoto = null - insn.isMeaningful -> { + insn is LineNumberNode || (insn.isMeaningful && insn.opcode != Opcodes.NOP) -> { currentLabels.clear() pendingGoto = null } diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/DefaultArgumentStubGenerator.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/DefaultArgumentStubGenerator.kt index 16f3160aae6..4eefcd9fac0 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/DefaultArgumentStubGenerator.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/DefaultArgumentStubGenerator.kt @@ -108,7 +108,6 @@ open class DefaultArgumentStubGenerator( val expressionBody = valueParameter.defaultValue!! expressionBody.patchDeclarationParents(newIrFunction) - expressionBody.transformChildrenVoid(object : IrElementTransformerVoid() { override fun visitGetValue(expression: IrGetValue): IrExpression { log { "GetValue: ${expression.symbol.owner}" } @@ -117,13 +116,7 @@ open class DefaultArgumentStubGenerator( } }) - val argument = irIfThenElse( - type = parameter.type, - condition = condition, - thenPart = expressionBody.expression, - elsePart = irGet(parameter) - ) - createTmpVariable(argument, nameHint = parameter.name.asString()) + selectArgumentOrDefault(condition, parameter, expressionBody.expression) } else { parameter } @@ -157,6 +150,15 @@ open class DefaultArgumentStubGenerator( return listOf(irFunction, newIrFunction) } + protected open fun IrBlockBodyBuilder.selectArgumentOrDefault( + shouldUseDefault: IrExpression, + parameter: IrValueParameter, + default: IrExpression + ): IrValueDeclaration { + val value = irIfThenElse(parameter.type, shouldUseDefault, default, irGet(parameter)) + return createTmpVariable(value, nameHint = parameter.name.asString()) + } + private fun IrBlockBodyBuilder.dispatchToImplementation( irFunction: IrSimpleFunction, newIrFunction: IrFunction, diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt index 02d605d82dc..e93ecd690f6 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt @@ -130,8 +130,8 @@ internal val localDeclarationsPhase = makeIrFilePhase( prerequisite = setOf(callableReferencePhase, sharedVariablesPhase) ) -private val defaultArgumentStubPhase = makeIrFilePhase( - { context -> DefaultArgumentStubGenerator(context, false) }, +private val defaultArgumentStubPhase = makeIrFilePhase( + ::JvmDefaultArgumentStubGenerator, name = "DefaultArgumentsStubGenerator", description = "Generate synthetic stubs for functions with default parameter values", prerequisite = setOf(localDeclarationsPhase) diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmSymbols.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmSymbols.kt index b9cb47f5334..68f30643992 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmSymbols.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmSymbols.kt @@ -378,6 +378,18 @@ class JvmSymbols( returnType = dst.defaultType }.symbol + val reassignParameterIntrinsic: IrSimpleFunctionSymbol = + buildFun { + name = Name.special("") + origin = IrDeclarationOrigin.IR_BUILTINS_STUB + }.apply { + parent = kotlinJvmInternalPackage + val type = addTypeParameter("T", irBuiltIns.anyNType) + addValueParameter("parameter", type.defaultType) // must be IrGetValue of an IrValueParameter + addValueParameter("value", type.defaultType) + returnType = irBuiltIns.unitType + }.symbol + private val collectionToArrayClass: IrClassSymbol = createClass(FqName("kotlin.jvm.internal.CollectionToArray")) { klass -> klass.origin = JvmLoweredDeclarationOrigin.TO_ARRAY diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt index 97a7d0bc87d..90d0f911f3f 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt @@ -36,6 +36,7 @@ import org.jetbrains.kotlin.ir.declarations.* import org.jetbrains.kotlin.ir.expressions.* import org.jetbrains.kotlin.ir.symbols.IrSymbol import org.jetbrains.kotlin.ir.symbols.IrTypeParameterSymbol +import org.jetbrains.kotlin.ir.symbols.IrValueSymbol import org.jetbrains.kotlin.ir.types.* import org.jetbrains.kotlin.ir.util.* import org.jetbrains.kotlin.ir.visitors.IrElementVisitor @@ -131,7 +132,7 @@ class ExpressionCodegen( val IrExpression.asmType: Type get() = type.asmType - val IrVariable.asmType: Type + val IrValueDeclaration.asmType: Type get() = type.asmType // Assume this expression's result has already been materialized on the stack @@ -512,12 +513,16 @@ class ExpressionCodegen( override fun visitSetVariable(expression: IrSetVariable, data: BlockInfo): PromisedValue { expression.markLineNumber(startOffset = true) - expression.value.markLineNumber(startOffset = true) - expression.value.accept(this, data).coerce(expression.symbol.owner.type).materialize() - mv.store(findLocalIndex(expression.symbol), expression.symbol.owner.asmType) + setVariable(expression.symbol, expression.value, data) return defaultValue(expression.type) } + fun setVariable(symbol: IrValueSymbol, value: IrExpression, data: BlockInfo) { + value.markLineNumber(startOffset = true) + value.accept(this, data).coerce(symbol.owner.type).materialize() + mv.store(findLocalIndex(symbol), symbol.owner.asmType) + } + override fun visitConst(expression: IrConst, data: BlockInfo): PromisedValue { expression.markLineNumber(startOffset = true) when (val value = expression.value) { diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IrIntrinsicMethods.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IrIntrinsicMethods.kt index 88915f3fa44..57b0f10b14c 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IrIntrinsicMethods.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IrIntrinsicMethods.kt @@ -91,7 +91,8 @@ class IrIntrinsicMethods(val irBuiltIns: IrBuiltIns, val symbols: JvmSymbols) { irBuiltIns.andandSymbol.toKey()!! to AndAnd, irBuiltIns.ororSymbol.toKey()!! to OrOr, symbols.unsafeCoerceIntrinsic.toKey()!! to UnsafeCoerce, - symbols.signatureStringIntrinsic.toKey()!! to SignatureString + symbols.signatureStringIntrinsic.toKey()!! to SignatureString, + symbols.reassignParameterIntrinsic.toKey()!! to ReassignParameter ) + numberConversionMethods() + unaryFunForPrimitives("plus", UnaryPlus) + diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/ReassignParameter.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/ReassignParameter.kt new file mode 100644 index 00000000000..61a1be501c2 --- /dev/null +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/ReassignParameter.kt @@ -0,0 +1,33 @@ +/* + * Copyright 2010-2019 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.backend.jvm.intrinsics + +import org.jetbrains.kotlin.backend.jvm.codegen.* +import org.jetbrains.kotlin.ir.expressions.IrFunctionAccessExpression +import org.jetbrains.kotlin.ir.expressions.IrGetValue +import org.jetbrains.kotlin.ir.symbols.IrValueParameterSymbol + +// A magic intrinsic that changes the supposedly-immutable parameter referenced by its argument: +// +// fun f(x: Int) { +// (x, 1) +// return x +// } +// assert(f(2) == 1) +// +// This *only* works for parameters; any other variable should be made mutable instead. +// +// This is used to optimize default method stubs (and, more importantly, produce bytecode that +// the inliner can actually handle) in `JvmDefaultArgumentStubGenerator`. +object ReassignParameter : IntrinsicMethod() { + override fun invoke(expression: IrFunctionAccessExpression, codegen: ExpressionCodegen, data: BlockInfo): PromisedValue { + val parameterGet = expression.getValueArgument(0) as? IrGetValue + val parameter = parameterGet?.symbol as? IrValueParameterSymbol + ?: throw AssertionError("${expression.getValueArgument(0)} is not a get of a parameter") + codegen.setVariable(parameter, expression.getValueArgument(1)!!, data) + return codegen.immaterialUnitValue + } +} diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmDefaultArgumentStubGenerator.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmDefaultArgumentStubGenerator.kt new file mode 100644 index 00000000000..dbc81cb6c63 --- /dev/null +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmDefaultArgumentStubGenerator.kt @@ -0,0 +1,46 @@ +/* + * Copyright 2010-2019 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.backend.jvm.lower + +import org.jetbrains.kotlin.backend.common.lower.DefaultArgumentStubGenerator +import org.jetbrains.kotlin.backend.common.lower.irIfThen +import org.jetbrains.kotlin.backend.jvm.JvmBackendContext +import org.jetbrains.kotlin.ir.builders.IrBlockBodyBuilder +import org.jetbrains.kotlin.ir.builders.irCall +import org.jetbrains.kotlin.ir.builders.irGet +import org.jetbrains.kotlin.ir.declarations.IrValueDeclaration +import org.jetbrains.kotlin.ir.declarations.IrValueParameter +import org.jetbrains.kotlin.ir.expressions.IrExpression + +class JvmDefaultArgumentStubGenerator(override val context: JvmBackendContext) : DefaultArgumentStubGenerator(context, false, false) { + override fun IrBlockBodyBuilder.selectArgumentOrDefault( + shouldUseDefault: IrExpression, + parameter: IrValueParameter, + default: IrExpression + ): IrValueDeclaration { + // We have to generate precisely this code because that results in the bytecode the inliner expects; + // see `expandMaskConditionsAndUpdateVariableNodes`. In short, the bytecode sequence should be + // + // -- no loads of the parameter here, as after inlining its value will be uninitialized + // ILOAD + // ICONST + // IAND + // IFEQ Lx + // -- any code inserted here is removed if the call site specifies the parameter + // STORE + // -- no jumps here + // Lx + // + // This control flow limits us to an if-then (without an else), and this together with the + // restriction on loading the parameter in the default case means we cannot create any temporaries. + +irIfThen(shouldUseDefault, irCall(this@JvmDefaultArgumentStubGenerator.context.ir.symbols.reassignParameterIntrinsic).apply { + putTypeArgument(0, parameter.type) + putValueArgument(0, irGet(parameter)) + putValueArgument(1, default) + }) + return parameter + } +} diff --git a/compiler/testData/codegen/bytecodeText/argumentOrder/sameOrderWithDefault.kt b/compiler/testData/codegen/bytecodeText/argumentOrder/sameOrderWithDefault.kt index add9536e525..971d79fa3ca 100644 --- a/compiler/testData/codegen/bytecodeText/argumentOrder/sameOrderWithDefault.kt +++ b/compiler/testData/codegen/bytecodeText/argumentOrder/sameOrderWithDefault.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR class A { fun test(x: String? = "x", a: String?, b: String?, y: String? = "y") { } diff --git a/compiler/testData/codegen/bytecodeText/defaultArguments/maskCheckSequence.kt b/compiler/testData/codegen/bytecodeText/defaultArguments/maskCheckSequence.kt new file mode 100644 index 00000000000..ce2230f2ec1 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/defaultArguments/maskCheckSequence.kt @@ -0,0 +1,6 @@ +inline fun f(x: Int = 1) = x + +// MUST contain these instructions to avoid breaking the inliner. +// See `expandMaskConditionsAndUpdateVariableNodes`. +// 1 ILOAD 1\s*ICONST_1\s*IAND\s*IFEQ L1 +// 1 ICONST_1\s*ISTORE 0\s*(L\d\s*)*L1 diff --git a/compiler/testData/codegen/bytecodeText/defaultMethodBody.kt b/compiler/testData/codegen/bytecodeText/defaultMethodBody.kt index 19f9a10baed..6ce69824f90 100644 --- a/compiler/testData/codegen/bytecodeText/defaultMethodBody.kt +++ b/compiler/testData/codegen/bytecodeText/defaultMethodBody.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR fun test(a: Int, z: String = try{"1"} catch (e: Exception) {"2"}) { "Default body" } diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index 82d6c40927a..c92d9f786d1 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -1590,6 +1590,11 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/defaultArguments/maskAndArgumentElimination.kt"); } + @TestMetadata("maskCheckSequence.kt") + public void testMaskCheckSequence() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/defaultArguments/maskCheckSequence.kt"); + } + @TestMetadata("methodHandlerElimination.kt") public void testMethodHandlerElimination() throws Exception { runTest("compiler/testData/codegen/bytecodeText/defaultArguments/methodHandlerElimination.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java index 1bdb1744727..8aad27c62f3 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java @@ -1545,6 +1545,11 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/defaultArguments/maskAndArgumentElimination.kt"); } + @TestMetadata("maskCheckSequence.kt") + public void testMaskCheckSequence() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/defaultArguments/maskCheckSequence.kt"); + } + @TestMetadata("methodHandlerElimination.kt") public void testMethodHandlerElimination() throws Exception { runTest("compiler/testData/codegen/bytecodeText/defaultArguments/methodHandlerElimination.kt");