From 65c153a722bc8c3a089ae99fda133f6439e08ad8 Mon Sep 17 00:00:00 2001 From: pyos Date: Thu, 20 Oct 2022 10:20:11 +0200 Subject: [PATCH] JVM: consider casts of null to be redundant These should only have ever been necessary for field type inference in coroutines, which should have been fixed by 0fc676a20c. Unlike 903a5d69a4, this time the change is in an optimization pass. Advantage: optimization passes have a closer to the JVM view of the bytecode, which makes this change significantly more likely to be correct. Disadvantage: technically we don't really guarantee optimization passes other than FixStack will run at all. For KT-54581 this is 100% fine since the problem itself is caused by redundant checkcast elimination in the first place (otherwise there would've been a redundant cast to String), but for KT-53146 this means the fix is somewhat incidental and not necessarily guaranteed. ^KT-53146 Fixed ^KT-54581 Fixed --- .../RedundantCheckCastElimination.kt | 6 +++--- .../FirBlackBoxCodegenTestGenerated.java | 12 ++++++++++++ compiler/testData/codegen/box/casts/kt53146.kt | 18 ++++++++++++++++++ compiler/testData/codegen/box/casts/kt54581.kt | 17 +++++++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 6 ++++++ .../IrBlackBoxCodegenTestGenerated.java | 12 ++++++++++++ .../LightAnalysisModeTestGenerated.java | 5 +++++ 7 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 compiler/testData/codegen/box/casts/kt53146.kt create mode 100644 compiler/testData/codegen/box/casts/kt54581.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/RedundantCheckCastElimination.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/RedundantCheckCastElimination.kt index 9dd001af82d..766ee31e099 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/RedundantCheckCastElimination.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/RedundantCheckCastElimination.kt @@ -20,6 +20,7 @@ import org.jetbrains.kotlin.codegen.inline.ReifiedTypeInliner import org.jetbrains.kotlin.codegen.optimization.common.FastMethodAnalyzer import org.jetbrains.kotlin.codegen.optimization.common.InstructionLivenessAnalyzer import org.jetbrains.kotlin.codegen.optimization.common.OptimizationBasicInterpreter +import org.jetbrains.kotlin.codegen.optimization.common.StrictBasicValue import org.jetbrains.kotlin.codegen.optimization.fixStack.top import org.jetbrains.kotlin.codegen.optimization.transformer.MethodTransformer import org.jetbrains.org.objectweb.asm.Opcodes @@ -52,13 +53,12 @@ class RedundantCheckCastEliminationMethodTransformer : MethodTransformer() { val frames = FastMethodAnalyzer(internalClassName, methodNode, interpreter, pruneExceptionEdges = true).analyze() for (i in insns.indices) { - val valueType = frames[i]?.top()?.type ?: continue val insn = insns[i] - if (insn.opcode == Opcodes.CHECKCAST) { + val value = frames[i]?.top() ?: continue val typeInsn = insn as TypeInsnNode val insnType = Type.getObjectType(typeInsn.desc) - if (!isTrivialSubtype(insnType, valueType)) continue + if (value !== StrictBasicValue.NULL_VALUE && !isTrivialSubtype(insnType, value.type)) continue //Keep casts to multiarray types cause dex doesn't recognize ANEWARRAY [Ljava/lang/Object; as Object [][], but Object [] type //It's not clear is it bug in dex or not and maybe best to distinguish such types from MULTINEWARRRAY ones in method analyzer diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java index 8ab1131b9fc..8a29a6b077b 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java @@ -4787,6 +4787,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/casts/kt50577.kt"); } + @Test + @TestMetadata("kt53146.kt") + public void testKt53146() throws Exception { + runTest("compiler/testData/codegen/box/casts/kt53146.kt"); + } + @Test @TestMetadata("kt53677.kt") public void testKt53677() throws Exception { @@ -4799,6 +4805,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/casts/kt54318.kt"); } + @Test + @TestMetadata("kt54581.kt") + public void testKt54581() throws Exception { + runTest("compiler/testData/codegen/box/casts/kt54581.kt"); + } + @Test @TestMetadata("lambdaToUnitCast.kt") public void testLambdaToUnitCast() throws Exception { diff --git a/compiler/testData/codegen/box/casts/kt53146.kt b/compiler/testData/codegen/box/casts/kt53146.kt new file mode 100644 index 00000000000..13d90fcff45 --- /dev/null +++ b/compiler/testData/codegen/box/casts/kt53146.kt @@ -0,0 +1,18 @@ +// TARGET_BACKEND: JVM_IR +// WITH_STDLIB +// FULL_JDK +// CHECK_BYTECODE_TEXT + +class A + +fun box(): String { + val a = try { + A() + } catch (e: NoClassDefFoundError) { + null + } + + return "OK" +} + +// 0 CHECKCAST diff --git a/compiler/testData/codegen/box/casts/kt54581.kt b/compiler/testData/codegen/box/casts/kt54581.kt new file mode 100644 index 00000000000..dc1533e528d --- /dev/null +++ b/compiler/testData/codegen/box/casts/kt54581.kt @@ -0,0 +1,17 @@ +// TARGET_BACKEND: JVM + +fun box(): String { + val k = tryOrNull { "K" } + return "O$k" +} + +private inline fun tryOrNull(action: () -> T): T? = + try { + action() + } catch (e: Throwable) { + when { + else -> { + null + } + } + } \ No newline at end of file diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java index 6406c24f264..bce4fc07ca5 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java @@ -4673,6 +4673,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/casts/kt50577.kt"); } + @Test + @TestMetadata("kt54581.kt") + public void testKt54581() throws Exception { + runTest("compiler/testData/codegen/box/casts/kt54581.kt"); + } + @Test @TestMetadata("lambdaToUnitCast.kt") public void testLambdaToUnitCast() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java index 569da439324..c8f0da0f234 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java @@ -4787,6 +4787,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/casts/kt50577.kt"); } + @Test + @TestMetadata("kt53146.kt") + public void testKt53146() throws Exception { + runTest("compiler/testData/codegen/box/casts/kt53146.kt"); + } + @Test @TestMetadata("kt53677.kt") public void testKt53677() throws Exception { @@ -4799,6 +4805,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/casts/kt54318.kt"); } + @Test + @TestMetadata("kt54581.kt") + public void testKt54581() throws Exception { + runTest("compiler/testData/codegen/box/casts/kt54581.kt"); + } + @Test @TestMetadata("lambdaToUnitCast.kt") public void testLambdaToUnitCast() throws Exception { diff --git a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index cdbbeb3a5d5..78fd52cc640 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -4076,6 +4076,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/casts/kt50577.kt"); } + @TestMetadata("kt54581.kt") + public void testKt54581() throws Exception { + runTest("compiler/testData/codegen/box/casts/kt54581.kt"); + } + @TestMetadata("lambdaToUnitCast.kt") public void testLambdaToUnitCast() throws Exception { runTest("compiler/testData/codegen/box/casts/lambdaToUnitCast.kt");