From 85b4668b7c134d44a4336ed964b65aba0d26dac1 Mon Sep 17 00:00:00 2001 From: pyos Date: Mon, 12 Apr 2021 20:10:32 +0200 Subject: [PATCH] JVM_IR: optimize more if-null chains 1. consider reads of fields from the same file "stable" just like functions, i.e. assume their nullability information is correct 2. apply if-null fusion repeatedly until the subject is no longer a nested if-null expression --- .../codegen/FirBytecodeTextTestGenerated.java | 6 ++++ .../lower/IfNullExpressionsFusionLowering.kt | 35 +++++++++++-------- .../safeCallAndElvisChains.kt | 18 ++++++++++ .../codegen/BytecodeTextTestGenerated.java | 6 ++++ .../codegen/IrBytecodeTextTestGenerated.java | 6 ++++ 5 files changed, 56 insertions(+), 15 deletions(-) create mode 100644 compiler/testData/codegen/bytecodeText/nullCheckOptimization/safeCallAndElvisChains.kt diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBytecodeTextTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBytecodeTextTestGenerated.java index 4ed00a7ad3f..6a29711e574 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBytecodeTextTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBytecodeTextTestGenerated.java @@ -4616,6 +4616,12 @@ public class FirBytecodeTextTestGenerated extends AbstractFirBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/nullCheckOptimization/reifiedNullIs.kt"); } + @Test + @TestMetadata("safeCallAndElvisChains.kt") + public void testSafeCallAndElvisChains() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/nullCheckOptimization/safeCallAndElvisChains.kt"); + } + @Test @TestMetadata("trivialInstanceOf.kt") public void testTrivialInstanceOf() throws Exception { diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/IfNullExpressionsFusionLowering.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/IfNullExpressionsFusionLowering.kt index 68f27aa6574..8ebafc7aefc 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/IfNullExpressionsFusionLowering.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/IfNullExpressionsFusionLowering.kt @@ -103,12 +103,15 @@ class IfNullExpressionsFusionLowering(val context: CommonBackendContext) : FileL if ((!outer.ifNotNullExpr.isTrivial() && innerKeepsNull != true && innerDiscardsNonNull != true) || (!outer.ifNullExpr.isTrivial() && innerKeepsNull != false && innerDiscardsNonNull != false) ) return this - return inner.createIrBuilder().irBlock { + val result = inner.createIrBuilder().irBlock { val ifNull = outer.substitute(inner.ifNullExpr, innerKeepsNull, inner.type) val ifNotNull = outer.substitute(inner.ifNotNullExpr, innerDiscardsNonNull, inner.type) +inner.subjectVar +irIfNull(outer.type, irGet(inner.subjectVar), ifNull, ifNotNull) - } + } as IrBlock + // Each `FUSE_IF_NULL` removes one level of `IfNull` from the expression being checked, + // so this eventually terminates when we no longer have `IfNull(IfNull(...), ...)`. + return result.fuseIfNull() } private fun IfNullExpr.substitute(subject: IrExpression, knownNullability: Boolean?, temporaryVarType: IrType): IrExpression = @@ -126,30 +129,32 @@ class IfNullExpressionsFusionLowering(val context: CommonBackendContext) : FileL private fun IfNullExpr.createIrBuilder() = context.createIrBuilder((subjectVar.parent as IrSymbolOwner).symbol, subjectVar.startOffset, subjectVar.endOffset) - private fun IrExpression.isNull(knownVariableSymbol: IrVariableSymbol, knownVariableIsNull: Boolean): Boolean? { + private fun IrExpression.isNull(knownVariableSymbol: IrVariableSymbol, knownVariableIsNull: Boolean): Boolean? = when (this) { - is IrConst<*> -> - return value == null - is IrGetValue -> { - if (symbol == knownVariableSymbol) return knownVariableIsNull - if (!type.isNullable()) return false + is IrConst<*> -> value == null + is IrGetValue -> when { + symbol == knownVariableSymbol -> knownVariableIsNull + !type.isNullable() -> false + else -> null } is IrConstructorCall, is IrGetSingletonValue, is IrFunctionExpression, is IrCallableReference<*>, is IrClassReference, - is IrGetClass -> - return false + is IrGetClass -> false is IrCall -> - if (!type.isNullable() && isStableCall()) return false + if (!type.isNullable() && symbol.owner.isStable()) false else null + is IrGetField -> + if (!type.isNullable() && symbol.owner.isStable()) false else null + is IrBlock -> + (statements.singleOrNull() as IrExpression?)?.takeIf { it.type == type }?.isNull(knownVariableSymbol, knownVariableIsNull) + else -> null } - return null - } // TODO make calls to the declarations within the same module "stable" - private fun IrCall.isStableCall() = - symbol.owner.fileOrNull == currentFile + private fun IrDeclaration.isStable() = + fileOrNull == currentFile } private class IfNullExpr( diff --git a/compiler/testData/codegen/bytecodeText/nullCheckOptimization/safeCallAndElvisChains.kt b/compiler/testData/codegen/bytecodeText/nullCheckOptimization/safeCallAndElvisChains.kt new file mode 100644 index 00000000000..5f083a8dd30 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/nullCheckOptimization/safeCallAndElvisChains.kt @@ -0,0 +1,18 @@ +class A(val x: String) { + fun y() = x + // Both `x` and `y()` assumed to respect their nullability information, + // so only `a` and `b` need to be checked. + fun foo1(a: A?, b: A?) = a?.x ?: b?.x ?: x // if (a == null) if (b == null) x else b.x else a.x + fun foo2(a: A?, b: A?) = a?.y() ?: b?.y() ?: y() // if (a == null) if (b == null) y() else b.y() else a.y() +} + +// JVM_TEMPLATES +// Optimization not implemented +// 8 IFNULL +// 0 IFNONNULL +// 2 ACONST_NULL + +// JVM_IR_TEMPLATES +// 0 IFNULL +// 4 IFNONNULL +// 0 ACONST_NULL diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BytecodeTextTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BytecodeTextTestGenerated.java index dc9c7c680c5..704806889cb 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BytecodeTextTestGenerated.java @@ -4484,6 +4484,12 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/nullCheckOptimization/reifiedNullIs.kt"); } + @Test + @TestMetadata("safeCallAndElvisChains.kt") + public void testSafeCallAndElvisChains() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/nullCheckOptimization/safeCallAndElvisChains.kt"); + } + @Test @TestMetadata("trivialInstanceOf.kt") public void testTrivialInstanceOf() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeTextTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeTextTestGenerated.java index 6cb19a2e3ff..1f25452602b 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeTextTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeTextTestGenerated.java @@ -4616,6 +4616,12 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/nullCheckOptimization/reifiedNullIs.kt"); } + @Test + @TestMetadata("safeCallAndElvisChains.kt") + public void testSafeCallAndElvisChains() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/nullCheckOptimization/safeCallAndElvisChains.kt"); + } + @Test @TestMetadata("trivialInstanceOf.kt") public void testTrivialInstanceOf() throws Exception {