JVM_IR: fix primitive comparison optimizations

1. the `primitive == object?.something` fusion should not apply to
    `primitive.equals(object?.something)` because it can't;

 2. coercions to Int are there for a reason - don't remove them;

 3. better optimize `primitive == object?.something` -- the result
    should be subject to if-null fusion, so it needs to have a specific
    pattern that resembles safe calls.

 #KT-47597 Fixed
This commit is contained in:
pyos
2021-07-05 12:38:34 +02:00
committed by TeamCityServer
parent 7e70c93cbf
commit c3a91efea3
15 changed files with 229 additions and 115 deletions
@@ -1478,6 +1478,18 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT
runTest("compiler/testData/codegen/box/binaryOp/eqNullableDoublesWithTP.kt");
}
@Test
@TestMetadata("eqNullableShortToShort.kt")
public void testEqNullableShortToShort() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableShortToShort.kt");
}
@Test
@TestMetadata("eqNullableToPrimitiveWithSideEffects.kt")
public void testEqNullableToPrimitiveWithSideEffects() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableToPrimitiveWithSideEffects.kt");
}
@Test
@TestMetadata("intrinsic.kt")
public void testIntrinsic() throws Exception {
@@ -1543,6 +1555,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT
public void testOverflowLong() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/overflowLong.kt");
}
@Test
@TestMetadata("primitiveEqualsSafeCall.kt")
public void testPrimitiveEqualsSafeCall() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/primitiveEqualsSafeCall.kt");
}
}
@Nested
@@ -127,8 +127,6 @@ class JvmOptimizationLowering(val context: JvmBackendContext) : FileLoweringPass
override fun visitCall(expression: IrCall, data: IrClass?): IrExpression {
expression.transformChildren(this, data)
removeIntTypeSafeCastsForEquality(expression)
if (expression.symbol.owner.origin == IrDeclarationOrigin.DEFAULT_PROPERTY_ACCESSOR) {
if (data == null) return expression
val simpleFunction = (expression.symbol.owner as? IrSimpleFunction) ?: return expression
@@ -148,121 +146,59 @@ class JvmOptimizationLowering(val context: JvmBackendContext) : FileLoweringPass
if (left.isNullConst() && right is IrConst<*> || right.isNullConst() && left is IrConst<*>)
return IrConstImpl.constFalse(expression.startOffset, expression.endOffset, context.irBuiltIns.booleanType)
val safeCallLeft = parseSafeCall(left)
if (safeCallLeft != null && right.type.isJvmPrimitive()) {
return rewriteSafeCallEqeqPrimitive(safeCallLeft, right, 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) }
}
}
val safeCallRight = parseSafeCall(right)
if (safeCallRight != null && left.type.isJvmPrimitive()) {
return rewritePrimitiveEqeqSafeCall(left, safeCallRight, expression)
}
return expression
}
return expression
}
private fun rewriteSafeCallEqeqPrimitive(safeCall: SafeCallInfo, primitive: IrExpression, eqeqCall: IrCall): IrExpression =
context.createJvmIrBuilder(safeCall.scopeSymbol).run {
// Fuze safe call with primitive equality to avoid boxing the primitive.
// 'a?.<...> == p' becomes:
// {
// val tmp = a
// when {
// tmp == null -> false
// else -> tmp == p
// }
// }
irBlock {
+safeCall.tmpVal
+irWhen(
eqeqCall.type,
listOf(
irBranch(safeCall.ifNullBranch.condition, irFalse()),
irElseBranch(
irCall(eqeqCall.symbol).apply {
putValueArgument(0, safeCall.ifNotNullBranch.result)
putValueArgument(1, primitive)
}
)
)
)
}
private fun IrBuilderWithScope.ifSafe(safeCall: SafeCallInfo, expr: IrExpression): IrExpression =
irBlock(origin = IrStatementOrigin.SAFE_CALL) {
+safeCall.tmpVal
+irIfThenElse(expr.type, safeCall.ifNullBranch.condition, irFalse(), expr)
}
private fun rewritePrimitiveEqeqSafeCall(primitive: IrExpression, safeCall: SafeCallInfo, eqeqCall: IrCall): IrExpression =
// 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 {
// Fuze safe call with primitive equality to avoid boxing the primitive.
// 'p == a?.<...>' becomes:
// {
// val tmp_p = p // should evaluate 'p' before 'a'
// val tmp = a
// when {
// tmp == null -> false
// else -> tmp_p == tmp
// }
// }
// 'tmp_p' above could be elided if 'p' is a variable or a constant.
irBlock {
val lhs =
if (primitive.isTrivial())
primitive
else {
val tmp = irTemporary(primitive)
irGet(tmp)
}
+safeCall.tmpVal
+irWhen(
eqeqCall.type,
listOf(
irBranch(safeCall.ifNullBranch.condition, irFalse()),
irElseBranch(
irCall(eqeqCall.symbol).apply {
putValueArgument(0, lhs)
putValueArgument(1, safeCall.ifNotNullBranch.result)
}
)
)
)
}
ifSafe(safeCall, eqeqCall.apply { putValueArgument(0, safeCall.ifNotNullBranch.result) })
}
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)
}
}
}
}
}
}
// 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,
@@ -0,0 +1,7 @@
class C(val x: Short)
fun box(): String {
val a: C = C(1)
val b: C? = C(1)
return if (b?.x == a.x) "OK" else "fail"
}
@@ -0,0 +1,19 @@
// IGNORE_BACKEND: JS, JS_IR, WASM, NATIVE
var result = ""
fun sideEffecting(): Int {
result += "OK"
return 123
}
class C(val x: Int)
val a: C? = C(123)
val b: C? = null
fun box(): String {
if (a?.x != sideEffecting()) return "fail cmp 1"
// RHS not evaluated because `b` is null, might be a bug:
if (b?.x == sideEffecting()) return "fail cmp 2"
return result
}
@@ -0,0 +1,7 @@
class C(val x: Short)
fun box(): String {
val a: Short = 1
val b: C? = C(1)
return if (a.equals(b?.x)) "OK" else "fail"
}
@@ -1,10 +1,25 @@
// FILE: J.java
import org.jetbrains.annotations.NotNull;
public interface J {
@NotNull
public Integer foo();
}
// FILE: test.kt
fun Long.id() = this
fun Short.id() = this
fun String.drop2() = if (length >= 2) subSequence(2, length) else null
fun doSimple1(s: String?) = s?.length == 3
fun doLongReceiver1(x: Long) = x?.id() == 3L
fun doJava1(s: String?, j: J) = s?.length == j.foo()
fun doLongReceiver1(x: Long?) = x?.id() == 3L
fun doShortReceiver1(x: Short?, y: Short) = x?.id() == y
fun doChain1(s: String?) = s?.drop2()?.length == 1
@@ -13,11 +28,16 @@ fun doIf1(s: String?) =
fun doSimple2(s: String?) = 3 == s?.length
fun doLongReceiver2(x: Long) = 3L == x?.id()
fun doJava2(s: String?, j: J) = j.foo() == s?.length
fun doLongReceiver2(x: Long?) = 3L == x?.id()
fun doShortReceiver2(x: Short?, y: Short) = y == x?.id()
fun doChain2(s: String?) = 1 == s?.drop2()?.length
fun doIf2(s: String?) =
if (1 == s?.length) "A" else "B"
// 0 valueOf
// `doJava1`/`doJava2` box `s?.length` instead of unboxing `j.foo()`:
// 2 valueOf
@@ -1,5 +1,4 @@
// !LANGUAGE: +ProperIeee754Comparisons
// IGNORE_BACKEND_FIR: 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!!
@@ -21,10 +20,9 @@ fun less5(a: Any?, b: Any?) = if (a is Byte && b is Byte) a < b else true
// 3 IF_ICMPGE
// JVM_IR_TEMPLATES
// 2 Intrinsics\.areEqual
// 0 Intrinsics\.areEqual
// 0 Intrinsics\.compare
// 4 INVOKEVIRTUAL java/lang/Byte\.byteValue \(\)B
// 8 INVOKEVIRTUAL java/lang/Byte\.byteValue \(\)B
// 4 INVOKEVIRTUAL java/lang/Number\.byteValue \(\)B
// 0 IFGE
// 3 IF_ICMPGE
@@ -1,6 +1,4 @@
// !LANGUAGE: -ProperIeee754Comparisons
// IGNORE_BACKEND_FIR: 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!!
@@ -21,9 +19,9 @@ fun less5(a: Any?, b: Any?) = if (a is Byte && b is Byte) a < b else true
// 0 IF_ICMPGE
// JVM_IR_TEMPLATES
// 2 Intrinsics\.areEqual
// 0 Intrinsics\.areEqual
// 0 Intrinsics\.compare
// 4 INVOKEVIRTUAL java/lang/Byte\.byteValue \(\)B
// 8 INVOKEVIRTUAL java/lang/Byte\.byteValue \(\)B
// 4 INVOKEVIRTUAL java/lang/Number\.byteValue \(\)B
// 0 IFGE
// 3 IF_ICMPGE
// 3 IF_ICMPGE
@@ -1478,6 +1478,18 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableDoublesWithTP.kt");
}
@Test
@TestMetadata("eqNullableShortToShort.kt")
public void testEqNullableShortToShort() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableShortToShort.kt");
}
@Test
@TestMetadata("eqNullableToPrimitiveWithSideEffects.kt")
public void testEqNullableToPrimitiveWithSideEffects() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableToPrimitiveWithSideEffects.kt");
}
@Test
@TestMetadata("intrinsic.kt")
public void testIntrinsic() throws Exception {
@@ -1543,6 +1555,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
public void testOverflowLong() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/overflowLong.kt");
}
@Test
@TestMetadata("primitiveEqualsSafeCall.kt")
public void testPrimitiveEqualsSafeCall() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/primitiveEqualsSafeCall.kt");
}
}
@Nested
@@ -1478,6 +1478,18 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
runTest("compiler/testData/codegen/box/binaryOp/eqNullableDoublesWithTP.kt");
}
@Test
@TestMetadata("eqNullableShortToShort.kt")
public void testEqNullableShortToShort() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableShortToShort.kt");
}
@Test
@TestMetadata("eqNullableToPrimitiveWithSideEffects.kt")
public void testEqNullableToPrimitiveWithSideEffects() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableToPrimitiveWithSideEffects.kt");
}
@Test
@TestMetadata("intrinsic.kt")
public void testIntrinsic() throws Exception {
@@ -1543,6 +1555,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
public void testOverflowLong() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/overflowLong.kt");
}
@Test
@TestMetadata("primitiveEqualsSafeCall.kt")
public void testPrimitiveEqualsSafeCall() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/primitiveEqualsSafeCall.kt");
}
}
@Nested
@@ -1306,6 +1306,16 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes
runTest("compiler/testData/codegen/box/binaryOp/eqNullableDoublesWithTP.kt");
}
@TestMetadata("eqNullableShortToShort.kt")
public void testEqNullableShortToShort() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableShortToShort.kt");
}
@TestMetadata("eqNullableToPrimitiveWithSideEffects.kt")
public void testEqNullableToPrimitiveWithSideEffects() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableToPrimitiveWithSideEffects.kt");
}
@TestMetadata("intrinsic.kt")
public void testIntrinsic() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/intrinsic.kt");
@@ -1360,6 +1370,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes
public void testOverflowLong() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/overflowLong.kt");
}
@TestMetadata("primitiveEqualsSafeCall.kt")
public void testPrimitiveEqualsSafeCall() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/primitiveEqualsSafeCall.kt");
}
}
@TestMetadata("compiler/testData/codegen/box/boxingOptimization")
@@ -806,6 +806,16 @@ public class IrJsCodegenBoxES6TestGenerated extends AbstractIrJsCodegenBoxES6Tes
runTest("compiler/testData/codegen/box/binaryOp/eqNullableDoublesWithTP.kt");
}
@TestMetadata("eqNullableShortToShort.kt")
public void testEqNullableShortToShort() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableShortToShort.kt");
}
@TestMetadata("eqNullableToPrimitiveWithSideEffects.kt")
public void testEqNullableToPrimitiveWithSideEffects() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableToPrimitiveWithSideEffects.kt");
}
@TestMetadata("intrinsic.kt")
public void testIntrinsic() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/intrinsic.kt");
@@ -860,6 +870,11 @@ public class IrJsCodegenBoxES6TestGenerated extends AbstractIrJsCodegenBoxES6Tes
public void testOverflowLong() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/overflowLong.kt");
}
@TestMetadata("primitiveEqualsSafeCall.kt")
public void testPrimitiveEqualsSafeCall() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/primitiveEqualsSafeCall.kt");
}
}
@TestMetadata("compiler/testData/codegen/box/boxingOptimization")
@@ -806,6 +806,16 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableDoublesWithTP.kt");
}
@TestMetadata("eqNullableShortToShort.kt")
public void testEqNullableShortToShort() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableShortToShort.kt");
}
@TestMetadata("eqNullableToPrimitiveWithSideEffects.kt")
public void testEqNullableToPrimitiveWithSideEffects() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableToPrimitiveWithSideEffects.kt");
}
@TestMetadata("intrinsic.kt")
public void testIntrinsic() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/intrinsic.kt");
@@ -860,6 +870,11 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest {
public void testOverflowLong() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/overflowLong.kt");
}
@TestMetadata("primitiveEqualsSafeCall.kt")
public void testPrimitiveEqualsSafeCall() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/primitiveEqualsSafeCall.kt");
}
}
@TestMetadata("compiler/testData/codegen/box/boxingOptimization")
@@ -806,6 +806,16 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableDoublesWithTP.kt");
}
@TestMetadata("eqNullableShortToShort.kt")
public void testEqNullableShortToShort() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableShortToShort.kt");
}
@TestMetadata("eqNullableToPrimitiveWithSideEffects.kt")
public void testEqNullableToPrimitiveWithSideEffects() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableToPrimitiveWithSideEffects.kt");
}
@TestMetadata("intrinsic.kt")
public void testIntrinsic() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/intrinsic.kt");
@@ -860,6 +870,11 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
public void testOverflowLong() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/overflowLong.kt");
}
@TestMetadata("primitiveEqualsSafeCall.kt")
public void testPrimitiveEqualsSafeCall() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/primitiveEqualsSafeCall.kt");
}
}
@TestMetadata("compiler/testData/codegen/box/boxingOptimization")
@@ -696,6 +696,16 @@ public class IrCodegenBoxWasmTestGenerated extends AbstractIrCodegenBoxWasmTest
runTest("compiler/testData/codegen/box/binaryOp/eqNullableDoublesWithTP.kt");
}
@TestMetadata("eqNullableShortToShort.kt")
public void testEqNullableShortToShort() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableShortToShort.kt");
}
@TestMetadata("eqNullableToPrimitiveWithSideEffects.kt")
public void testEqNullableToPrimitiveWithSideEffects() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/eqNullableToPrimitiveWithSideEffects.kt");
}
@TestMetadata("intrinsic.kt")
public void testIntrinsic() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/intrinsic.kt");
@@ -750,6 +760,11 @@ public class IrCodegenBoxWasmTestGenerated extends AbstractIrCodegenBoxWasmTest
public void testOverflowLong() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/overflowLong.kt");
}
@TestMetadata("primitiveEqualsSafeCall.kt")
public void testPrimitiveEqualsSafeCall() throws Exception {
runTest("compiler/testData/codegen/box/binaryOp/primitiveEqualsSafeCall.kt");
}
}
@TestMetadata("compiler/testData/codegen/box/boxingOptimization")