From 9fc869397d57fe584d6a7a7301374b6ed35da4be Mon Sep 17 00:00:00 2001 From: Ting-Yuan Huang Date: Mon, 1 Apr 2019 17:04:58 -0700 Subject: [PATCH] IrBuiltins: refactoring for equality checks Previously, * Equals performs IEEE 754 equality check for floating points and byte-to-byte checks for other types, including references. * Ieee754Equals performs IEEE 754 for primitive types * TotalOrderEquals performs total order equals to all types, including floating points. Now it is simplified, * Equals performs total order checks for all types. * Ieee754Equals performs IEEE 754 for primitive types. * (TotalOrderEquals is removed.) --- .../org/jetbrains/kotlin/codegen/AsmUtil.java | 23 ++++++--- .../kotlin/backend/jvm/intrinsics/Equals.kt | 49 ++++++++----------- .../jvm/intrinsics/IntrinsicMethods.java | 7 +-- .../codegen/box/dataClasses/doubleParam.kt | 1 - .../codegen/box/dataClasses/floatParam.kt | 1 - .../testData/codegen/box/ieee754/dataClass.kt | 1 - 6 files changed, 39 insertions(+), 43 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/AsmUtil.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/AsmUtil.java index c637b4d579a..70b377bb2aa 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/AsmUtil.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/AsmUtil.java @@ -848,19 +848,30 @@ public class AsmUtil { return StackValue.operation(Type.BOOLEAN_TYPE, v -> { left.put(AsmTypes.OBJECT_TYPE, left.kotlinType, v); right.put(AsmTypes.OBJECT_TYPE, right.kotlinType, v); - genAreEqualCall(v); - - if (opToken == KtTokens.EXCLEQ || opToken == KtTokens.EXCLEQEQEQ) { - genInvertBoolean(v); - } - return Unit.INSTANCE; + return genAreEqualCall(v, opToken); }); } + @NotNull + public static StackValue genEqualsBoxedOnStack(@NotNull IElementType opToken) { + return StackValue.operation(Type.BOOLEAN_TYPE, v -> genAreEqualCall(v, opToken)); + } + public static void genAreEqualCall(InstructionAdapter v) { v.invokestatic(IntrinsicMethods.INTRINSICS_CLASS_NAME, "areEqual", "(Ljava/lang/Object;Ljava/lang/Object;)Z", false); } + @NotNull + private static Unit genAreEqualCall(InstructionAdapter v, @NotNull IElementType opToken) { + genAreEqualCall(v); + + if (opToken == KtTokens.EXCLEQ || opToken == KtTokens.EXCLEQEQEQ) { + genInvertBoolean(v); + } + + return Unit.INSTANCE; + } + public static void genIEEE754EqualForNullableTypesCall(InstructionAdapter v, Type left, Type right) { v.invokestatic(IntrinsicMethods.INTRINSICS_CLASS_NAME, "areEqual", "(" + left.getDescriptor() + right.getDescriptor() + ")Z", false); } 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 dfcb3cad97d..669896164b2 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 @@ -15,7 +15,6 @@ import org.jetbrains.kotlin.ir.expressions.IrMemberAccessExpression import org.jetbrains.kotlin.ir.expressions.IrStatementOrigin import org.jetbrains.kotlin.ir.types.toKotlinType import org.jetbrains.kotlin.lexer.KtTokens -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.typeUtil.isPrimitiveNumberOrNullableType @@ -49,12 +48,18 @@ class Equals(val operator: IElementType) : IntrinsicMethod(), ComparisonIntrinsi ): StackValue { val (leftType, rightType) = argumentTypes(expression, context) val opToken = expression.origin - return if (opToken === IrStatementOrigin.EQEQEQ || opToken === IrStatementOrigin.EXCLEQEQ) { - // TODO: always casting to the type of the left operand in case of primitives looks wrong - val operandType = if (isPrimitive(leftType)) leftType else OBJECT_TYPE - StackValue.cmp(operator, operandType, StackValue.onStack(leftType), StackValue.onStack(rightType)) - } else { - genEqualsForExpressionsOnStack(operator, StackValue.onStack(leftType), StackValue.onStack(rightType)) + return when { + leftType.isFloatingPoint && rightType.isFloatingPoint -> + genEqualsBoxedOnStack(operator) + + opToken === IrStatementOrigin.EQEQEQ || opToken === IrStatementOrigin.EXCLEQEQ -> { + // TODO: always casting to the type of the left operand in case of primitives looks wrong + val operandType = if (isPrimitive(leftType)) leftType else OBJECT_TYPE + StackValue.cmp(operator, operandType, StackValue.onStack(leftType), StackValue.onStack(rightType)) + } + + else -> + genEqualsForExpressionsOnStack(operator, StackValue.onStack(leftType), StackValue.onStack(rightType)) } } @@ -63,7 +68,12 @@ class Equals(val operator: IElementType) : IntrinsicMethod(), ComparisonIntrinsi signature: JvmMethodSignature, context: JvmBackendContext ): IrIntrinsicFunction { - var (leftType, rightType) = argumentTypes(expression, context) + val (leftType, rightType) = argumentTypes(expression, context).let { + if (it.first.isFloatingPoint && it.second.isFloatingPoint) + Pair(OBJECT_TYPE, OBJECT_TYPE) + else + it + } return object : IrIntrinsicFunction(expression, signature, context, listOf(leftType, rightType)) { override fun genInvokeInstruction(v: InstructionAdapter) { @@ -72,6 +82,9 @@ class Equals(val operator: IElementType) : IntrinsicMethod(), ComparisonIntrinsi } } } + + private val Type.isFloatingPoint: Boolean + get() = this == Type.DOUBLE_TYPE || this == Type.FLOAT_TYPE } @@ -128,23 +141,3 @@ class Ieee754Equals(val operandType: Type) : IntrinsicMethod() { } } } - -class TotalOrderEquals(operandType: Type) : IntrinsicMethod() { - private val boxedType = AsmUtil.boxType(operandType) - - override fun toCallable( - expression: IrMemberAccessExpression, - signature: JvmMethodSignature, - context: JvmBackendContext - ): IrIntrinsicFunction = - object : IrIntrinsicFunction(expression, signature, context, listOf(boxedType, boxedType)) { - override fun genInvokeInstruction(v: InstructionAdapter) { - v.invokestatic( - IntrinsicMethods.INTRINSICS_CLASS_NAME, - "areEqual", - Type.getMethodDescriptor(Type.BOOLEAN_TYPE, AsmTypes.OBJECT_TYPE, AsmTypes.OBJECT_TYPE), - false - ) - } - } -} diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IntrinsicMethods.java b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IntrinsicMethods.java index 3e06f232b0f..eab007c0e63 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IntrinsicMethods.java +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/intrinsics/IntrinsicMethods.java @@ -95,12 +95,7 @@ public class IntrinsicMethods { for (PrimitiveType type : PrimitiveType.values()) { FqName typeFqName = type.getTypeFqName(); Type asmPrimitiveType = AsmTypes.valueTypeForPrimitive(type); - if (asmPrimitiveType == Type.FLOAT_TYPE || asmPrimitiveType == Type.DOUBLE_TYPE) { - declareIntrinsicFunction(typeFqName, "equals", 1, new TotalOrderEquals(asmPrimitiveType)); - } - else { - declareIntrinsicFunction(typeFqName, "equals", 1, EQUALS); - } + declareIntrinsicFunction(typeFqName, "equals", 1, EQUALS); declareIntrinsicFunction(typeFqName, "hashCode", 0, HASH_CODE); declareIntrinsicFunction(typeFqName, "toString", 0, TO_STRING); diff --git a/compiler/testData/codegen/box/dataClasses/doubleParam.kt b/compiler/testData/codegen/box/dataClasses/doubleParam.kt index 130b4d79914..aa07eaa3fbc 100644 --- a/compiler/testData/codegen/box/dataClasses/doubleParam.kt +++ b/compiler/testData/codegen/box/dataClasses/doubleParam.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // IGNORE_BACKEND: JS_IR // TODO: muted automatically, investigate should it be ran for JS or not // IGNORE_BACKEND: JS diff --git a/compiler/testData/codegen/box/dataClasses/floatParam.kt b/compiler/testData/codegen/box/dataClasses/floatParam.kt index 18ec480d8ff..402b0497d6a 100644 --- a/compiler/testData/codegen/box/dataClasses/floatParam.kt +++ b/compiler/testData/codegen/box/dataClasses/floatParam.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // IGNORE_BACKEND: JS_IR // TODO: muted automatically, investigate should it be ran for JS or not // IGNORE_BACKEND: JS diff --git a/compiler/testData/codegen/box/ieee754/dataClass.kt b/compiler/testData/codegen/box/ieee754/dataClass.kt index fb661368e8e..8f05b07f453 100644 --- a/compiler/testData/codegen/box/ieee754/dataClass.kt +++ b/compiler/testData/codegen/box/ieee754/dataClass.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR data class Test(val z1: Double, val z2: Double?) fun box(): String {