From 5570a5fe74b12dcd74f99447f5511534ca6963f3 Mon Sep 17 00:00:00 2001 From: Mads Ager Date: Thu, 26 Mar 2020 15:03:49 +0100 Subject: [PATCH] [JVM_IR] Use iinc for incrementing Int variables. Fix line number generation for assignments where the right-hand side of the assignment is not on the same line. Fix line number generation for intrinsics functions where the function is not on the same line as the last argument. Be careful to not break stepping behavior with the iinc optimizations. --- .../backend/jvm/codegen/ExpressionCodegen.kt | 74 ++++++++++++++++++- .../backend/jvm/intrinsics/IntrinsicMethod.kt | 4 +- .../intrinsics/IrIllegalArgumentException.kt | 11 ++- .../jvm/intrinsics/IrIntrinsicFunction.kt | 8 +- .../kotlin/backend/jvm/intrinsics/RangeTo.kt | 10 ++- .../jvm/lower/JvmOptimizationLowering.kt | 4 - .../codegen/bytecodeText/iincGeneration.kt | 41 ++++++++++ .../bytecodeText/prefixIntVarIncrement.kt | 3 - .../testData/debug/stepping/iincStepping.kt | 47 ++++++++++++ .../codegen/BytecodeTextTestGenerated.java | 5 ++ .../IrSteppingTestGenerated.java | 6 ++ .../SteppingTestGenerated.java | 6 ++ .../ir/IrBytecodeTextTestGenerated.java | 5 ++ 13 files changed, 205 insertions(+), 19 deletions(-) create mode 100644 compiler/testData/codegen/bytecodeText/iincGeneration.kt create mode 100644 compiler/testData/debug/stepping/iincStepping.kt 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 f474f26aef3..dbe80d3de74 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 @@ -149,13 +149,15 @@ class ExpressionCodegen( private fun markNewLabel() = Label().apply { mv.visitLabel(this) } + private fun getLineNumberForOffset(offset: Int): Int = fileEntry?.getLineNumber(offset)?.plus(1) ?: -1 + private fun IrElement.markLineNumber(startOffset: Boolean) { val offset = if (startOffset) this.startOffset else endOffset if (offset < 0) { return } if (fileEntry != null) { - val lineNumber = fileEntry.getLineNumber(offset) + 1 + val lineNumber = getLineNumberForOffset(offset) assert(lineNumber > 0) if (lastLineNumber != lineNumber) { lastLineNumber = lineNumber @@ -164,6 +166,8 @@ class ExpressionCodegen( } } + fun markLineNumber(element: IrElement) = element.markLineNumber(true) + // TODO remove fun gen(expression: IrExpression, type: Type, irType: IrType, data: BlockInfo): StackValue { if (expression.attributeOwnerId === context.fakeContinuation) { @@ -591,9 +595,73 @@ class ExpressionCodegen( throw AssertionError("Non-mapped local declaration: $dump\n in ${irFunction.dump()}") } + private fun handlePlusMinus(expression: IrSetVariable, value: IrExpression?, isMinus: Boolean): Boolean { + if (value is IrConst<*> && value.kind == IrConstKind.Int) { + val delta = (value as IrConst).value + val upperBound = Byte.MAX_VALUE.toInt() + (if (isMinus) 1 else 0) + val lowerBound = Byte.MIN_VALUE.toInt() + (if (isMinus) 1 else 0) + if (delta in lowerBound..upperBound) { + expression.markLineNumber(startOffset = true) + mv.iinc(findLocalIndex(expression.symbol), if (isMinus) -delta else delta) + return true + } + } + return false + } + + private fun hasSameLineNumber(element0: IrElement, element1: IrElement): Boolean = + getLineNumberForOffset(element0.startOffset) == getLineNumberForOffset(element1.startOffset) + + // Use iinc for all for the set var int special cases where we can. + // Be careful to make sure that debugging behavior does not change and + // only perform the optimization if that can be done without losing + // line number information. + private fun handleIntVariableSpecialCases(expression: IrSetVariable): Boolean { + if (expression.symbol.owner.type.isInt()) { + when (expression.origin) { + IrStatementOrigin.PREFIX_INCR, IrStatementOrigin.PREFIX_DECR -> { + expression.markLineNumber(startOffset = true) + mv.iinc(findLocalIndex(expression.symbol), if (expression.origin == IrStatementOrigin.PREFIX_INCR) 1 else -1) + return true + } + IrStatementOrigin.PLUSEQ, IrStatementOrigin.MINUSEQ -> { + val argument = (expression.value as IrCall).getValueArgument(0)!! + if (!hasSameLineNumber(argument, expression)) { + return false + } + return handlePlusMinus(expression, argument, expression.origin is IrStatementOrigin.MINUSEQ) + } + IrStatementOrigin.EQ -> { + val value = expression.value + if (!hasSameLineNumber(value, expression)) { + return false + } + if (value is IrCall) { + val receiver = value.dispatchReceiver ?: return false + val symbol = expression.symbol + if (!hasSameLineNumber(receiver, expression)) { + return false + } + if (value.origin == IrStatementOrigin.PLUS || value.origin == IrStatementOrigin.MINUS) { + val argument = value.getValueArgument(0)!! + if (receiver is IrGetValue && receiver.symbol == symbol && hasSameLineNumber(argument, expression)) { + return handlePlusMinus(expression, argument, value.origin == IrStatementOrigin.MINUS) + } + } + } + } + } + } + return false + } + override fun visitSetVariable(expression: IrSetVariable, data: BlockInfo): PromisedValue { - expression.markLineNumber(startOffset = true) - setVariable(expression.symbol, expression.value, data) + if (!handleIntVariableSpecialCases(expression)) { + expression.value.markLineNumber(startOffset = true) + expression.value.accept(this, data).materializeAt(expression.symbol.owner.type) + expression.markLineNumber(startOffset = true) + mv.store(findLocalIndex(expression.symbol), expression.symbol.owner.asmType) + } return unitValue } diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IntrinsicMethod.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IntrinsicMethod.kt index a70f3243ae8..542e7b3b5f7 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IntrinsicMethod.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IntrinsicMethod.kt @@ -7,11 +7,9 @@ package org.jetbrains.kotlin.backend.jvm.intrinsics import org.jetbrains.kotlin.backend.jvm.JvmBackendContext import org.jetbrains.kotlin.backend.jvm.codegen.* -import org.jetbrains.kotlin.codegen.AsmUtil import org.jetbrains.kotlin.ir.expressions.IrExpression import org.jetbrains.kotlin.ir.expressions.IrFunctionAccessExpression import org.jetbrains.kotlin.ir.expressions.IrMemberAccessExpression -import org.jetbrains.kotlin.ir.types.IrType import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodSignature import org.jetbrains.org.objectweb.asm.Type import org.jetbrains.org.objectweb.asm.commons.Method @@ -26,7 +24,7 @@ abstract class IntrinsicMethod { open fun invoke(expression: IrFunctionAccessExpression, codegen: ExpressionCodegen, data: BlockInfo): PromisedValue? = with(codegen) { val descriptor = methodSignatureMapper.mapSignatureSkipGeneric(expression.symbol.owner) - val stackValue = toCallable(expression, descriptor, context).invoke(mv, codegen, data) + val stackValue = toCallable(expression, descriptor, context).invoke(mv, codegen, data, expression) stackValue.put(mv) return MaterialValue(this, stackValue.type, expression.type) } diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IrIllegalArgumentException.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IrIllegalArgumentException.kt index d6411de5b6e..a393f328c27 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IrIllegalArgumentException.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IrIllegalArgumentException.kt @@ -21,7 +21,6 @@ import org.jetbrains.kotlin.backend.jvm.codegen.BlockInfo import org.jetbrains.kotlin.backend.jvm.codegen.ExpressionCodegen import org.jetbrains.kotlin.codegen.StackValue import org.jetbrains.kotlin.ir.expressions.IrFunctionAccessExpression -import org.jetbrains.kotlin.ir.expressions.IrMemberAccessExpression import org.jetbrains.kotlin.resolve.jvm.AsmTypes.JAVA_STRING_TYPE import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodSignature import org.jetbrains.org.objectweb.asm.Type @@ -46,10 +45,16 @@ object IrIllegalArgumentException : IntrinsicMethod() { v.athrow() } - override fun invoke(v: InstructionAdapter, codegen: ExpressionCodegen, data: BlockInfo): StackValue { + override fun invoke( + v: InstructionAdapter, + codegen: ExpressionCodegen, + data: BlockInfo, + expression: IrFunctionAccessExpression + ): StackValue { + codegen.markLineNumber(expression) v.anew(exceptionTypeDescriptor) v.dup() - return super.invoke(v, codegen, data) + return super.invoke(v, codegen, data, expression) } } } diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IrIntrinsicFunction.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IrIntrinsicFunction.kt index 6648cadf4e3..f52d7d385dc 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IrIntrinsicFunction.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IrIntrinsicFunction.kt @@ -65,8 +65,14 @@ open class IrIntrinsicFunction( return returnType } - open fun invoke(v: InstructionAdapter, codegen: ExpressionCodegen, data: BlockInfo): StackValue { + open fun invoke( + v: InstructionAdapter, + codegen: ExpressionCodegen, + data: BlockInfo, + expression: IrFunctionAccessExpression + ): StackValue { loadArguments(codegen, data) + codegen.markLineNumber(expression) return StackValue.onStack(genInvokeInstructionWithResult(v)) } diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/RangeTo.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/RangeTo.kt index abf50a13f36..097e988190c 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/RangeTo.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/RangeTo.kt @@ -50,10 +50,16 @@ object RangeTo : IntrinsicMethod() { v.invokespecial(signature.returnType.internalName, "", Type.getMethodDescriptor(Type.VOID_TYPE, argType, argType), false) } - override fun invoke(v: InstructionAdapter, codegen: ExpressionCodegen, data: BlockInfo): StackValue { + override fun invoke( + v: InstructionAdapter, + codegen: ExpressionCodegen, + data: BlockInfo, + expression: IrFunctionAccessExpression + ): StackValue { + codegen.markLineNumber(expression) v.anew(returnType) v.dup() - return super.invoke(v, codegen, data) + return super.invoke(v, codegen, data, expression) } } } 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 6419a705e56..b166677e6ee 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 @@ -40,10 +40,6 @@ class JvmOptimizationLowering(val context: JvmBackendContext) : FileLoweringPass context.state.intrinsics.getIntrinsic(expression.symbol.descriptor) is Not } - private fun hasNoSideEffectsForNullCompare(expression: IrExpression): Boolean { - return expression.type.isPrimitiveType() && (expression is IrConst<*> || expression is IrGetValue) - } - private val IrFunction.isObjectEquals get() = name.asString() == "equals" && valueParameters.count() == 1 && diff --git a/compiler/testData/codegen/bytecodeText/iincGeneration.kt b/compiler/testData/codegen/bytecodeText/iincGeneration.kt new file mode 100644 index 00000000000..fa06254c327 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/iincGeneration.kt @@ -0,0 +1,41 @@ +fun main(args: Array) { + var i = 10 + + // -- 4 of these fit in iinc instructions + i += 1 + i += 127 + i += 128 + i += -1 + i += -128 + i += -129 + + // -- 4 of these fit in iinc instructions + i -= 1 + i -= 128 + i -= 129 + i -= -1 + i -= -127 + i -= -128 + + // -- 4 of these fit in iinc instructions + i = i + 1 + i = i + 127 + i = i + 128 + i = i + -1 + i = i + -128 + i = i + -129 + + // -- 4 of these fit in iinc instructions + i = i - 1 + i = i - 128 + i = i - 129 + i = i - -1 + i = i - -127 + i = i - -128 +} + +// JVM_IR_TEMPLATES +// 16 IINC + +// JVM_TEMPLATES +// 0 IINC \ No newline at end of file diff --git a/compiler/testData/codegen/bytecodeText/prefixIntVarIncrement.kt b/compiler/testData/codegen/bytecodeText/prefixIntVarIncrement.kt index eb1c0a903fa..13e5070405f 100644 --- a/compiler/testData/codegen/bytecodeText/prefixIntVarIncrement.kt +++ b/compiler/testData/codegen/bytecodeText/prefixIntVarIncrement.kt @@ -1,6 +1,3 @@ -// IGNORE_BACKEND: JVM_IR -// KT-36641 TODO Generate IINC instruction for prefix increment in JVM_IR - fun main(args: Array) { var i = 10 ++i diff --git a/compiler/testData/debug/stepping/iincStepping.kt b/compiler/testData/debug/stepping/iincStepping.kt new file mode 100644 index 00000000000..a852908e654 --- /dev/null +++ b/compiler/testData/debug/stepping/iincStepping.kt @@ -0,0 +1,47 @@ +//FILE: test.kt +fun box() { + var i = 0 + ++i + i += 1 + i += + 1 + i -= 1 + i -= + 1 + i = i + 1 + i = + i + 1 + i = + i + + 1 +} + +// IGNORE_BACKEND: JVM +// The current backend has strange stepping behavior for assignments. +// It generates the line number for the assignment first, and then +// the evaluation of the right hand side with line numbers. +// That leads to the line number with the assignment typically +// not being hit at all as it has no instructions. Also, stepping +// through the evaluation of the right hand side and then hitting +// the line number for the actual assignment makes more sense as +// that is the actual evaluation order. + +// LINENUMBERS +// test.kt:3 +// test.kt:4 +// test.kt:5 +// test.kt:6 +// test.kt:7 +// test.kt:6 +// test.kt:8 +// test.kt:9 +// test.kt:10 +// test.kt:9 +// test.kt:11 +// test.kt:13 +// test.kt:12 +// test.kt:15 +// test.kt:16 +// test.kt:15 +// test.kt:14 +// test.kt:17 \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index 80c7092df18..cb221da1e6a 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -124,6 +124,11 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/flagsInMultiFileInherit.kt"); } + @TestMetadata("iincGeneration.kt") + public void testIincGeneration() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/iincGeneration.kt"); + } + @TestMetadata("inheritedPropertyAnnotations.kt") public void testInheritedPropertyAnnotations() throws Exception { runTest("compiler/testData/codegen/bytecodeText/inheritedPropertyAnnotations.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/IrSteppingTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/IrSteppingTestGenerated.java index 34370919897..ca2ba98ced9 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/IrSteppingTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/IrSteppingTestGenerated.java @@ -67,6 +67,12 @@ public class IrSteppingTestGenerated extends AbstractIrSteppingTest { runTest("compiler/testData/debug/stepping/IfTrueThenFalse.kt"); } + @Test + @TestMetadata("iincStepping.kt") + public void testIincStepping() throws Exception { + runTest("compiler/testData/debug/stepping/iincStepping.kt"); + } + @Test @TestMetadata("inlineCallableReference.kt") public void testInlineCallableReference() throws Exception { diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/SteppingTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/SteppingTestGenerated.java index 9e9dabbc9d7..33416e44881 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/SteppingTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/debugInformation/SteppingTestGenerated.java @@ -67,6 +67,12 @@ public class SteppingTestGenerated extends AbstractSteppingTest { runTest("compiler/testData/debug/stepping/IfTrueThenFalse.kt"); } + @Test + @TestMetadata("iincStepping.kt") + public void testIincStepping() throws Exception { + runTest("compiler/testData/debug/stepping/iincStepping.kt"); + } + @Test @TestMetadata("inlineCallableReference.kt") public void testInlineCallableReference() throws Exception { diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java index 6ef7b66cc28..9f939ea2892 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java @@ -124,6 +124,11 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/flagsInMultiFileInherit.kt"); } + @TestMetadata("iincGeneration.kt") + public void testIincGeneration() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/iincGeneration.kt"); + } + @TestMetadata("inheritedPropertyAnnotations.kt") public void testInheritedPropertyAnnotations() throws Exception { runTest("compiler/testData/codegen/bytecodeText/inheritedPropertyAnnotations.kt");