From 1f4a3b0d1cb326815cc18eec227d3bfd28f481b2 Mon Sep 17 00:00:00 2001 From: Mads Ager Date: Thu, 7 May 2020 15:09:25 +0200 Subject: [PATCH] [JVM_IR] Avoid safe-call conversions from Byte? and Short? to Int? for comparisons. Having those conversions leads to unnecesary boxing and null checks. This change does it only for JVM in the optimization lowering. It is unclear to me if the other backends can get away with something similar. --- .../kotlin/backend/jvm/intrinsics/Equals.kt | 5 +-- .../jvm/lower/JvmOptimizationLowering.kt | 41 +++++++++++++++++-- .../conditions/noBoxingForBoxedEqPrimitive.kt | 2 - .../conditions/noBoxingForPrimitiveEqBoxed.kt | 2 - .../intrinsicsCompare/byteSmartCast_after.kt | 12 +++++- .../intrinsicsCompare/byteSmartCast_before.kt | 8 ++-- .../intrinsicsCompare/shortSmartCast_after.kt | 11 +++++ .../shortSmartCast_before.kt | 4 +- 8 files changed, 66 insertions(+), 19 deletions(-) diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/Equals.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/Equals.kt index 77962db081d..c94a8ca34f3 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/Equals.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/Equals.kt @@ -28,7 +28,6 @@ import org.jetbrains.kotlin.ir.util.isNullConst import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi2ir.generators.hasNoSideEffects import org.jetbrains.kotlin.resolve.jvm.AsmTypes -import org.jetbrains.kotlin.resolve.jvm.AsmTypes.OBJECT_TYPE import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodSignature import org.jetbrains.kotlin.types.isNullable import org.jetbrains.kotlin.types.typeUtil.isPrimitiveNumberOrNullableType @@ -104,12 +103,12 @@ class Equals(val operator: IElementType) : IntrinsicMethod() { // could be overridden for the object. if ((opToken == IrStatementOrigin.EQEQ || opToken == IrStatementOrigin.EXCLEQ) && ((AsmUtil.isIntOrLongPrimitive(leftType) && !AsmUtil.isPrimitive(rightType)) || - (AsmUtil.isIntOrLongPrimitive(rightType) && AsmUtil.isBoxedTypeOf(leftType, rightType))) + (AsmUtil.isIntOrLongPrimitive(rightType) && AsmUtil.isBoxedPrimitiveType(leftType))) ) { val leftIsPrimitive = AsmUtil.isIntOrLongPrimitive(leftType) val primitiveType = if (leftIsPrimitive) leftType else rightType val nonPrimitiveType = if (leftIsPrimitive) rightType else leftType - val useNullCheck = AsmUtil.isBoxedTypeOf(nonPrimitiveType, primitiveType) + val useNullCheck = AsmUtil.isBoxedPrimitiveType(nonPrimitiveType) return if (leftIsPrimitive) { val loadOther = loadOther(a, leftType) val boxedValue = b.accept(codegen, data).materializedAt(rightType, b.type) 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 b166677e6ee..2bc3bdf2055 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 @@ -20,9 +20,7 @@ import org.jetbrains.kotlin.ir.expressions.* import org.jetbrains.kotlin.ir.expressions.impl.IrBlockImpl import org.jetbrains.kotlin.ir.expressions.impl.IrCallImpl import org.jetbrains.kotlin.ir.expressions.impl.IrConstImpl -import org.jetbrains.kotlin.ir.types.isBoolean -import org.jetbrains.kotlin.ir.types.isNullableAny -import org.jetbrains.kotlin.ir.types.isPrimitiveType +import org.jetbrains.kotlin.ir.types.* import org.jetbrains.kotlin.ir.util.* import org.jetbrains.kotlin.ir.visitors.IrElementTransformer @@ -91,6 +89,8 @@ class JvmOptimizationLowering(val context: JvmBackendContext) : FileLoweringPass override fun visitCall(expression: IrCall, currentClass: IrClass?): IrExpression { expression.transformChildren(this, currentClass) + removeIntTypeSafeCastsForEquality(expression) + if (expression.symbol.owner.origin == IrDeclarationOrigin.DEFAULT_PROPERTY_ACCESSOR) { if (currentClass == null) return expression val simpleFunction = (expression.symbol.owner as? IrSimpleFunction) ?: return expression @@ -118,6 +118,41 @@ class JvmOptimizationLowering(val context: JvmBackendContext) : FileLoweringPass return expression } + private fun IrType.isByteOrShort() = isByte() || isShort() + + // For `==` and `!=`, get rid of safe calls to convert `Byte?` or `Short?` to `Int?`. + // For equality, we do not need to perform such conversions as the builtin for equality + // will handle it. Having the safe call leads to unnecessary null checks and boxing. + private fun removeIntTypeSafeCastsForEquality(expression: IrCall) { + if (expression.origin == IrStatementOrigin.EQEQ || expression.origin == IrStatementOrigin.EXCLEQ) { + for (i in 0 until expression.valueArgumentsCount) { + if (expression.getValueArgument(i)!!.type.makeNotNull().isInt()) { + val argument = expression.getValueArgument(i)!! + if (argument is IrBlock && argument.origin == IrStatementOrigin.SAFE_CALL) { + if (argument.statements.size == 2) { + val variable = argument.statements[0] + if (variable is IrVariable && variable.type.makeNotNull().isByteOrShort()) { + val whenExpression = argument.statements[1] + if (whenExpression is IrWhen && whenExpression.branches.size == 2) { + val secondBranch = whenExpression.branches[1] + if (secondBranch is IrElseBranch && secondBranch.result is IrCall) { + val conversion = secondBranch.result as IrCall + if (conversion.symbol.owner.name.asString() == "toInt" && + conversion.dispatchReceiver is IrGetValue && + (conversion.dispatchReceiver as IrGetValue).symbol.owner == variable + ) { + expression.putValueArgument(i, variable.initializer) + } + } + } + } + } + } + } + } + } + } + private fun optimizePropertyAccess( expression: IrCall, accessor: IrSimpleFunction, diff --git a/compiler/testData/codegen/bytecodeText/conditions/noBoxingForBoxedEqPrimitive.kt b/compiler/testData/codegen/bytecodeText/conditions/noBoxingForBoxedEqPrimitive.kt index da8f2b0f78c..d20f0230e33 100644 --- a/compiler/testData/codegen/bytecodeText/conditions/noBoxingForBoxedEqPrimitive.kt +++ b/compiler/testData/codegen/bytecodeText/conditions/noBoxingForBoxedEqPrimitive.kt @@ -1,5 +1,3 @@ -// IGNORE_BACKEND: JVM_IR - fun testBoolean1(a: Boolean?, b: Boolean) = a == b fun testBoolean2(a: Boolean?, b: Boolean) = a != b diff --git a/compiler/testData/codegen/bytecodeText/conditions/noBoxingForPrimitiveEqBoxed.kt b/compiler/testData/codegen/bytecodeText/conditions/noBoxingForPrimitiveEqBoxed.kt index 02f4a299e82..7fb29c4428c 100644 --- a/compiler/testData/codegen/bytecodeText/conditions/noBoxingForPrimitiveEqBoxed.kt +++ b/compiler/testData/codegen/bytecodeText/conditions/noBoxingForPrimitiveEqBoxed.kt @@ -1,5 +1,3 @@ -// IGNORE_BACKEND: JVM_IR - fun testBoolean1(a: Boolean, b: Boolean?) = a == b fun testBoolean2(a: Boolean, b: Boolean?) = a != b diff --git a/compiler/testData/codegen/bytecodeText/intrinsicsCompare/byteSmartCast_after.kt b/compiler/testData/codegen/bytecodeText/intrinsicsCompare/byteSmartCast_after.kt index 4bc41a6475b..46a5d76e9b4 100644 --- a/compiler/testData/codegen/bytecodeText/intrinsicsCompare/byteSmartCast_after.kt +++ b/compiler/testData/codegen/bytecodeText/intrinsicsCompare/byteSmartCast_after.kt @@ -1,6 +1,4 @@ // !LANGUAGE: +ProperIeee754Comparisons -// IGNORE_BACKEND: JVM_IR - fun equals3(a: Byte?, b: Byte?) = a != null && b != null && a == b fun equals4(a: Byte?, b: Byte?) = if (a is Byte && b is Byte) a == b else null!! @@ -13,9 +11,19 @@ fun less4(a: Byte?, b: Byte?) = if (a is Byte && b is Byte) a < b else true fun less5(a: Any?, b: Any?) = if (a is Byte && b is Byte) a < b else true +// JVM_TEMPLATES // 3 Intrinsics\.areEqual // 0 Intrinsics\.compare // 4 INVOKEVIRTUAL java/lang/Byte\.byteValue \(\)B // 2 INVOKEVIRTUAL java/lang/Number\.intValue \(\)I // 0 IFGE // 3 IF_ICMPGE + +// JVM_IR_TEMPLATES +// 2 Intrinsics\.areEqual +// 0 Intrinsics\.compare +// 4 INVOKEVIRTUAL java/lang/Byte\.byteValue \(\)B +// 4 INVOKEVIRTUAL java/lang/Number\.byteValue \(\)B +// 0 IFGE +// 3 IF_ICMPGE + diff --git a/compiler/testData/codegen/bytecodeText/intrinsicsCompare/byteSmartCast_before.kt b/compiler/testData/codegen/bytecodeText/intrinsicsCompare/byteSmartCast_before.kt index 4feed9d39f5..25f5bc0ff17 100644 --- a/compiler/testData/codegen/bytecodeText/intrinsicsCompare/byteSmartCast_before.kt +++ b/compiler/testData/codegen/bytecodeText/intrinsicsCompare/byteSmartCast_before.kt @@ -1,5 +1,4 @@ // !LANGUAGE: -ProperIeee754Comparisons -// IGNORE_BACKEND: JVM_IR fun equals3(a: Byte?, b: Byte?) = a != null && b != null && a == b @@ -21,10 +20,9 @@ fun less5(a: Any?, b: Any?) = if (a is Byte && b is Byte) a < b else true // 0 IF_ICMPGE // JVM_IR_TEMPLATES -// 3 Intrinsics\.areEqual +// 2 Intrinsics\.areEqual // 0 Intrinsics\.compare // 4 INVOKEVIRTUAL java/lang/Byte\.byteValue \(\)B -// 2 INVOKEVIRTUAL java/lang/Number\.intValue \(\)I +// 4 INVOKEVIRTUAL java/lang/Number\.byteValue \(\)B // 0 IFGE -// 3 IF_ICMPGE -// 0 IF_ICMPNE \ No newline at end of file +// 3 IF_ICMPGE \ No newline at end of file diff --git a/compiler/testData/codegen/bytecodeText/intrinsicsCompare/shortSmartCast_after.kt b/compiler/testData/codegen/bytecodeText/intrinsicsCompare/shortSmartCast_after.kt index fb537f3d29b..d0eae9f50a5 100644 --- a/compiler/testData/codegen/bytecodeText/intrinsicsCompare/shortSmartCast_after.kt +++ b/compiler/testData/codegen/bytecodeText/intrinsicsCompare/shortSmartCast_after.kt @@ -13,6 +13,7 @@ fun less4(a: Short?, b: Short?) = if (a is Short && b is Short) a < b else true fun less5(a: Any?, b: Any?) = if (a is Short && b is Short) a < b else true +// JVM_TEMPLATES // 3 Intrinsics\.areEqual // 0 Intrinsics\.compare // 4 INVOKEVIRTUAL java/lang/Short\.shortValue \(\)S @@ -20,3 +21,13 @@ fun less5(a: Any?, b: Any?) = if (a is Short && b is Short) a < b else true // 0 IFGE // 3 IF_ICMPGE // 0 IF_ICMPNE + +// JVM_IR_TEMPLATES +// 2 Intrinsics\.areEqual +// 0 Intrinsics\.compare +// 4 INVOKEVIRTUAL java/lang/Short\.shortValue \(\)S +// 4 INVOKEVIRTUAL java/lang/Number\.shortValue \(\)S +// 0 IFGE +// 3 IF_ICMPGE +// 0 IF_ICMPNE + diff --git a/compiler/testData/codegen/bytecodeText/intrinsicsCompare/shortSmartCast_before.kt b/compiler/testData/codegen/bytecodeText/intrinsicsCompare/shortSmartCast_before.kt index d9bdf38b980..94aac9757c0 100644 --- a/compiler/testData/codegen/bytecodeText/intrinsicsCompare/shortSmartCast_before.kt +++ b/compiler/testData/codegen/bytecodeText/intrinsicsCompare/shortSmartCast_before.kt @@ -20,10 +20,10 @@ fun less5(a: Any?, b: Any?) = if (a is Short && b is Short) a < b else true // 0 IF_ICMPGE // JVM_IR_TEMPLATES -// 3 Intrinsics\.areEqual +// 2 Intrinsics\.areEqual // 0 Intrinsics\.compare // 4 INVOKEVIRTUAL java/lang/Short\.shortValue \(\)S -// 2 INVOKEVIRTUAL java/lang/Number\.intValue \(\)I +// 4 INVOKEVIRTUAL java/lang/Number\.shortValue \(\)S // 0 IFGE // 3 IF_ICMPGE // 0 IF_ICMPNE \ No newline at end of file