KT-15871 Unnecessary boxing for equality operator on inlined primitive values

Allow kotlin.jvm.internal.Intrinsics#areEqual for boxed values.
Rewrite to primitive equality.

NB we can't do that for Float and Double, because java.lang.Float#equals
and java.lang.Double#equals behave differently from primitive equality comparisons.
This commit is contained in:
Dmitry Petrov
2017-02-15 17:17:04 +03:00
parent a087ea559f
commit c46164481a
15 changed files with 193 additions and 2 deletions
@@ -66,7 +66,7 @@ class BoxedValueDescriptor(
var isSafeToRemove = true; private set
val unboxedType: Type = getUnboxedType(boxedType)
fun getAssociatedInsns() = associatedInsns.toReadOnlyList()
fun getAssociatedInsns() = associatedInsns.toList()
fun addInsn(insnNode: AbstractInsnNode) {
associatedInsns.add(insnNode)
@@ -71,6 +71,10 @@ open class BoxingInterpreter(private val insnList: InsnList) : OptimizationBasic
?: throw AssertionError("firstArg should be progression iterator")
createNewBoxing(insn, AsmUtil.boxType(progressionIterator.valuesPrimitiveType), progressionIterator)
}
insn.isAreEqualIntrinsicForSameTypedBoxedValues(values) && canValuesBeUnboxedForAreEqual(values) -> {
onAreEqual(insn, values[0] as BoxedBasicValue, values[1] as BoxedBasicValue)
value
}
else -> {
// N-ary operation should be a method call or multinewarray.
// Arguments for multinewarray could be only numeric,
@@ -122,6 +126,7 @@ open class BoxingInterpreter(private val insnList: InsnList) : OptimizationBasic
protected open fun onNewBoxedValue(value: BoxedBasicValue) {}
protected open fun onUnboxing(insn: AbstractInsnNode, value: BoxedBasicValue, resultType: Type) {}
protected open fun onAreEqual(insn: AbstractInsnNode, value1: BoxedBasicValue, value2: BoxedBasicValue) {}
protected open fun onMethodCallWithBoxedValue(value: BoxedBasicValue) {}
protected open fun onMergeFail(value: BoxedBasicValue) {}
protected open fun onMergeSuccess(v: BoxedBasicValue, w: BoxedBasicValue) {}
@@ -203,3 +208,28 @@ fun AbstractInsnNode.isIteratorMethodCallOfProgression(values: List<BasicValue>)
fun isProgressionClass(type: Type) =
RangeCodegenUtil.isRangeOrProgression(buildFqNameByInternal(type.internalName))
fun AbstractInsnNode.isAreEqualIntrinsicForSameTypedBoxedValues(values: List<BasicValue>) =
isAreEqualIntrinsic() && run {
if (values.size != 2) return false
val (v1, v2) = values
if (v1 !is BoxedBasicValue || v2 !is BoxedBasicValue) return false
val d1 = v1.descriptor
val d2 = v2.descriptor
d1.unboxedType == d2.unboxedType
}
fun AbstractInsnNode.isAreEqualIntrinsic() =
isMethodInsnWith(Opcodes.INVOKESTATIC) {
name == "areEqual" &&
owner == "kotlin/jvm/internal/Intrinsics" &&
desc == "(Ljava/lang/Object;Ljava/lang/Object;)Z"
}
fun canValuesBeUnboxedForAreEqual(values: List<BasicValue>): Boolean =
!values.any {
val unboxedType = getUnboxedType(it.type)
unboxedType == Type.DOUBLE_TYPE || unboxedType == Type.FLOAT_TYPE
}
@@ -84,6 +84,13 @@ internal class RedundantBoxingInterpreter(insnList: InsnList) : BoxingInterprete
}
}
override fun onAreEqual(insn: AbstractInsnNode, value1: BoxedBasicValue, value2: BoxedBasicValue) {
val descriptor1 = value1.descriptor
val descriptor2 = value2.descriptor
candidatesBoxedValues.merge(descriptor1, descriptor2)
descriptor1.addInsn(insn)
}
override fun onMethodCallWithBoxedValue(value: BoxedBasicValue) {
markValueAsDirty(value)
}
@@ -23,6 +23,7 @@ import org.jetbrains.annotations.NotNull;
import org.jetbrains.kotlin.codegen.optimization.common.StrictBasicValue;
import org.jetbrains.kotlin.codegen.optimization.common.UtilKt;
import org.jetbrains.kotlin.codegen.optimization.transformer.MethodTransformer;
import org.jetbrains.org.objectweb.asm.Label;
import org.jetbrains.org.objectweb.asm.Opcodes;
import org.jetbrains.org.objectweb.asm.Type;
import org.jetbrains.org.objectweb.asm.commons.InstructionAdapter;
@@ -327,9 +328,68 @@ public class RedundantBoxingMethodTransformer extends MethodTransformer {
);
node.instructions.set(insn, new InsnNode(Opcodes.ICONST_1));
break;
case Opcodes.INVOKESTATIC:
if (BoxingInterpreterKt.isAreEqualIntrinsic(insn)) {
adaptAreEqualIntrinsic(node, insn, value);
break;
}
else {
// fall-through to default
}
default:
// CHECKCAST or unboxing-method call
node.instructions.remove(insn);
}
}
private static void adaptAreEqualIntrinsic(@NotNull MethodNode node, @NotNull AbstractInsnNode insn, @NotNull BoxedValueDescriptor value) {
Type unboxedType = value.getUnboxedType();
switch (unboxedType.getSort()) {
case Type.BOOLEAN:
case Type.BYTE:
case Type.SHORT:
case Type.INT:
case Type.CHAR:
adaptAreEqualIntrinsicForInt(node, insn);
break;
case Type.LONG:
adaptAreEqualIntrinsicForLong(node, insn);
break;
case Type.OBJECT:
break;
default:
throw new AssertionError("Unexpected unboxed type kind: " + unboxedType);
}
}
private static void adaptAreEqualIntrinsicForInt(@NotNull MethodNode node, @NotNull AbstractInsnNode insn) {
LabelNode lNotEqual = new LabelNode(new Label());
LabelNode lDone = new LabelNode(new Label());
node.instructions.insertBefore(insn, new JumpInsnNode(Opcodes.IF_ICMPNE, lNotEqual));
node.instructions.insertBefore(insn, new InsnNode(Opcodes.ICONST_1));
node.instructions.insertBefore(insn, new JumpInsnNode(Opcodes.GOTO, lDone));
node.instructions.insertBefore(insn, lNotEqual);
node.instructions.insertBefore(insn, new InsnNode(Opcodes.ICONST_0));
node.instructions.insertBefore(insn, lDone);
node.instructions.remove(insn);
}
private static void adaptAreEqualIntrinsicForLong(@NotNull MethodNode node, @NotNull AbstractInsnNode insn) {
node.instructions.insertBefore(insn, new InsnNode(Opcodes.LCMP));
ifEqual1Else0(node, insn);
node.instructions.remove(insn);
}
private static void ifEqual1Else0(@NotNull MethodNode node, @NotNull AbstractInsnNode insn) {
LabelNode lNotEqual = new LabelNode(new Label());
LabelNode lDone = new LabelNode(new Label());
node.instructions.insertBefore(insn, new JumpInsnNode(Opcodes.IFNE, lNotEqual));
node.instructions.insertBefore(insn, new InsnNode(Opcodes.ICONST_1));
node.instructions.insertBefore(insn, new JumpInsnNode(Opcodes.GOTO, lDone));
node.instructions.insertBefore(insn, lNotEqual);
node.instructions.insertBefore(insn, new InsnNode(Opcodes.ICONST_0));
node.instructions.insertBefore(insn, lDone);
}
}
@@ -0,0 +1,8 @@
// IGNORE_BACKEND: JS
fun equals1(a: Double, b: Double) = a.equals(b)
fun box(): String {
if ((-0.0).equals(0.0)) return "fail 0"
return "OK"
}
@@ -0,0 +1,5 @@
fun box() =
if (getAndCheck({ 42 }, { 42 })) "OK" else "fail"
inline fun <T> getAndCheck(getFirst: () -> T, getSecond: () -> T) =
getFirst() == getSecond()
+6 -1
View File
@@ -2,6 +2,9 @@
fun box(): String {
val plusZero: Double? = 0.0
val minusZero: Double = -0.0
useBoxed(plusZero)
if (plusZero?.equals(minusZero) ?: null!!) {
return "fail 1"
}
@@ -11,4 +14,6 @@ fun box(): String {
}
return "OK"
}
}
fun useBoxed(a: Any?) {}
@@ -0,0 +1,17 @@
// https://youtrack.jetbrains.com/issue/KT-15871
// FILE: Test.kt
fun getAndCheckInt(a: Int, b: Int) =
getAndCheck({ a }, { b })
// @TestKt.class:
// 0 valueOf
// 0 Value
// 0 areEqual
// FILE: Inline.kt
inline fun <T> getAndCheck(getFirst: () -> T, getSecond: () -> T) =
getFirst() == getSecond()
@@ -0,0 +1,5 @@
@kotlin.Metadata
public final class ExplicitEqualsOnDoubleKt {
public final static @org.jetbrains.annotations.NotNull method box(): java.lang.String
public final static method equals1(p0: double, p1: double): boolean
}
@@ -0,0 +1,5 @@
@kotlin.Metadata
public final class Kt15871Kt {
public final static @org.jetbrains.annotations.NotNull method box(): java.lang.String
public final static method getAndCheck(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function0, @org.jetbrains.annotations.NotNull p1: kotlin.jvm.functions.Function0): boolean
}
@@ -1,4 +1,5 @@
@kotlin.Metadata
public final class SafeCallKt {
public final static @org.jetbrains.annotations.NotNull method box(): java.lang.String
public final static method useBoxed(@org.jetbrains.annotations.Nullable p0: java.lang.Object): void
}
@@ -884,6 +884,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
doTest(fileName);
}
@TestMetadata("explicitEqualsOnDouble.kt")
public void testExplicitEqualsOnDouble() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/explicitEqualsOnDouble.kt");
doTest(fileName);
}
@TestMetadata("fold.kt")
public void testFold() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/fold.kt");
@@ -896,6 +902,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
doTest(fileName);
}
@TestMetadata("kt15871.kt")
public void testKt15871() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt15871.kt");
doTest(fileName);
}
@TestMetadata("kt5493.kt")
public void testKt5493() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt5493.kt");
@@ -884,6 +884,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
doTest(fileName);
}
@TestMetadata("explicitEqualsOnDouble.kt")
public void testExplicitEqualsOnDouble() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/explicitEqualsOnDouble.kt");
doTest(fileName);
}
@TestMetadata("fold.kt")
public void testFold() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/fold.kt");
@@ -896,6 +902,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
doTest(fileName);
}
@TestMetadata("kt15871.kt")
public void testKt15871() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt15871.kt");
doTest(fileName);
}
@TestMetadata("kt5493.kt")
public void testKt5493() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt5493.kt");
@@ -455,6 +455,12 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/bytecodeText/boxingOptimization"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true);
}
@TestMetadata("boxingAndEquals.kt")
public void testBoxingAndEquals() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/boxingOptimization/boxingAndEquals.kt");
doTest(fileName);
}
@TestMetadata("casts.kt")
public void testCasts() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/boxingOptimization/casts.kt");
@@ -1094,6 +1094,18 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that.");
}
@TestMetadata("explicitEqualsOnDouble.kt")
public void testExplicitEqualsOnDouble() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/explicitEqualsOnDouble.kt");
try {
doTest(fileName);
}
catch (Throwable ignore) {
return;
}
throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that.");
}
@TestMetadata("fold.kt")
public void testFold() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/fold.kt");
@@ -1106,6 +1118,12 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
doTest(fileName);
}
@TestMetadata("kt15871.kt")
public void testKt15871() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt15871.kt");
doTest(fileName);
}
@TestMetadata("kt5493.kt")
public void testKt5493() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt5493.kt");