Skip to content

Commit

Permalink
Support additional intrinsics in null check elimination
Browse files Browse the repository at this point in the history 
1. checkExpressionValueIsNotNull implies checked value is non-null

2. throwNpe never returns

 #KT-18162 Fixed Target versions 1.1.4
 #KT-18164 Fixed Target versions 1.1.4
  • Loading branch information
@dnpetrov
dnpetrov committed May 31, 2017
1 parent d559212 commit 08885e2
Show file tree
Hide file tree
Showing 5 changed files with 196 additions and 29 deletions.
137 changes: 108 additions & 29 deletions ...rg/jetbrains/kotlin/codegen/optimization/nullCheck/RedundantNullCheckMethodTransformer.kt
View file
Expand Up @@ -21,6 +21,7 @@ import org.jetbrains.kotlin.codegen.inline.ReifiedTypeInliner
import org.jetbrains.kotlin.codegen.optimization.DeadCodeEliminationMethodTransformer
import org.jetbrains.kotlin.codegen.optimization.common.OptimizationBasicInterpreter
import org.jetbrains.kotlin.codegen.optimization.common.StrictBasicValue
import org.jetbrains.kotlin.codegen.optimization.common.debugText
import org.jetbrains.kotlin.codegen.optimization.common.isInsn
import org.jetbrains.kotlin.codegen.optimization.fixStack.peek
import org.jetbrains.kotlin.codegen.optimization.fixStack.top
Expand Down Expand Up @@ -61,7 +62,7 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() {
if (insn.isInstanceOfOrNullCheck()) {
checkedReferenceTypes[insn] = frame?.top()?.type ?: continue
}
else if (insn.isCheckParameterNotNull()) {
else if (insn.isCheckParameterIsNotNull() || insn.isCheckExpressionValueIsNotNull()) {
checkedReferenceTypes[insn] = frame?.peek(1)?.type ?: continue
}
}
Expand Down Expand Up @@ -92,14 +93,18 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() {
val insns = methodNode.instructions.toArray()
val nullabilityMap = LinkedHashMap<AbstractInsnNode, StrictBasicValue>()
for (i in insns.indices) {
val top = frames[i]?.top() as? StrictBasicValue ?: continue
val nullability = top.getNullability()
if (nullability == Nullability.NULLABLE) continue

val frame = frames[i] ?: continue
val insn = insns[i]
if (insn.isInstanceOfOrNullCheck()) {
nullabilityMap[insn] = top
}

val value = when {
insn.isInstanceOfOrNullCheck() -> frame.top()
insn.isCheckExpressionValueIsNotNull() -> frame.peek(1)
else -> null
} as? StrictBasicValue ?: continue

val nullability = value.getNullability()
if (nullability == Nullability.NULLABLE) continue
nullabilityMap[insn] = value
}
return nullabilityMap
}
Expand All @@ -111,6 +116,12 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() {
Opcodes.IFNULL -> transformTrivialNullJump(insn as JumpInsnNode, nullability == Nullability.NULL)
Opcodes.IFNONNULL -> transformTrivialNullJump(insn as JumpInsnNode, nullability == Nullability.NOT_NULL)
Opcodes.INSTANCEOF -> transformInstanceOf(insn as TypeInsnNode, nullability, value)

Opcodes.INVOKESTATIC -> {
if (insn.isCheckExpressionValueIsNotNull()) {
transformTrivialCheckExpressionValueIsNotNull(insn, nullability)
}
}
}
}
}
Expand Down Expand Up @@ -148,6 +159,16 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() {
}
}

private fun transformTrivialCheckExpressionValueIsNotNull(insn: AbstractInsnNode, nullability: Nullability) {
if (nullability != Nullability.NOT_NULL) return
val ldcInsn = insn.previous?.takeIf { it.opcode == Opcodes.LDC } ?: return
methodNode.instructions.run {
popReferenceValueBefore(ldcInsn)
remove(ldcInsn)
remove(insn)
}
}

private inner class NullabilityAssumptionsBuilder(val checkedReferenceTypes: Map<AbstractInsnNode, Type>) {

private val checksDependingOnVariable = HashMap<Int, MutableList<AbstractInsnNode>>()
Expand All @@ -158,25 +179,47 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() {
}

private fun collectVariableDependentChecks() {
for (insn in methodNode.instructions) {
if (insn.isInstanceOfOrNullCheck()) {
val previous = insn.previous ?: continue
if (previous.opcode == Opcodes.ALOAD) {
addDependentCheck(insn, previous as VarInsnNode)
insnLoop@ for (insn in methodNode.instructions) {
when {
insn.isInstanceOfOrNullCheck() -> {
val previous = insn.previous ?: continue@insnLoop
if (previous.opcode == Opcodes.ALOAD) {
addDependentCheck(insn, previous as VarInsnNode)
}
else if (previous.opcode == Opcodes.DUP) {
val previous2 = previous.previous ?: continue@insnLoop
if (previous2.opcode == Opcodes.ALOAD) {
addDependentCheck(insn, previous2 as VarInsnNode)
}
}
}

insn.isCheckParameterIsNotNull() -> {
val ldcInsn = insn.previous ?: continue@insnLoop
if (ldcInsn.opcode != Opcodes.LDC) continue@insnLoop
val aLoadInsn = ldcInsn.previous ?: continue@insnLoop
if (aLoadInsn.opcode != Opcodes.ALOAD) continue@insnLoop
addDependentCheck(insn, aLoadInsn as VarInsnNode)
}
else if (previous.opcode == Opcodes.DUP) {
val previous2 = previous.previous ?: continue
if (previous2.opcode == Opcodes.ALOAD) {
addDependentCheck(insn, previous2 as VarInsnNode)

insn.isCheckExpressionValueIsNotNull() -> {
val ldcInsn = insn.previous ?: continue@insnLoop
if (ldcInsn.opcode != Opcodes.LDC) continue@insnLoop
var aLoadInsn: VarInsnNode? = null
val insn1 = ldcInsn.previous ?: continue@insnLoop
if (insn1.opcode == Opcodes.ALOAD) {
aLoadInsn = insn1 as VarInsnNode
}
else if (insn1.opcode == Opcodes.DUP) {
val insn2 = insn1.previous ?: continue@insnLoop
if (insn2.opcode == Opcodes.ALOAD) {
aLoadInsn = insn2 as VarInsnNode
}
}
if (aLoadInsn == null) continue@insnLoop
addDependentCheck(insn, aLoadInsn)
}
}
else if (insn.isCheckParameterNotNull()) {
val ldcInsn = insn.previous ?: continue
if (ldcInsn.opcode != Opcodes.LDC) continue
val aLoadInsn = ldcInsn.previous ?: continue
if (aLoadInsn.opcode != Opcodes.ALOAD) continue
addDependentCheck(insn, aLoadInsn as VarInsnNode)

}
}
}
Expand All @@ -196,6 +239,11 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() {
nullabilityAssumptions.injectAssumptionsForCheck(varIndex, checkInsn, varType)
}
}
for (insn in methodNode.instructions) {
if (insn.isThrowNpeIntrinsic()) {
nullabilityAssumptions.injectCodeForThrowNpe(insn)
}
}
return nullabilityAssumptions
}

Expand All @@ -205,8 +253,10 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() {
Opcodes.IFNONNULL ->
injectAssumptionsForNullCheck(varIndex, insn as JumpInsnNode, varType)
Opcodes.INVOKESTATIC -> {
assert(insn.isCheckParameterNotNull()) { "Expected non-null parameter check @${insn.getIndex()}"}
injectAssumptionsForParameterNotNullCheck(varIndex, insn, varType)
assert(insn.isCheckParameterIsNotNull() || insn.isCheckExpressionValueIsNotNull()) {
"Expected non-null assertion: ${insn.debugText}"
}
injectAssumptionsForNotNullAssertion(varIndex, insn, varType)
}
Opcodes.INSTANCEOF ->
injectAssumptionsForInstanceOfCheck(varIndex, insn, varType)
Expand Down Expand Up @@ -249,12 +299,18 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() {
}
}

private fun NullabilityAssumptions.injectAssumptionsForParameterNotNullCheck(varIndex: Int, insn: AbstractInsnNode, varType: Type) {
private fun NullabilityAssumptions.injectAssumptionsForNotNullAssertion(varIndex: Int, insn: AbstractInsnNode, varType: Type) {
// ALOAD v
// LDC param_name
// LDC *
// INVOKESTATIC checkParameterIsNotNull
// <...> -- v is not null here (otherwise an exception was thrown)

// ALOAD v
// DUP
// LDC *
// INVOKESTATIC checkExpressionValueIsNotNull
// <...> -- v is not null here (otherwise an exception was thrown)

methodNode.instructions.insert(insn, listOfSynthetics {
anew(varType)
store(varIndex, varType)
Expand Down Expand Up @@ -300,6 +356,15 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() {
}
}

private fun NullabilityAssumptions.injectCodeForThrowNpe(insn: AbstractInsnNode) {
methodNode.instructions.run {
insert(insn, listOfSynthetics {
aconst(null)
athrow()
})
}
}

}

inner class NullabilityAssumptions {
Expand Down Expand Up @@ -336,13 +401,27 @@ internal fun AbstractInsnNode.isInstanceOfOrNullCheck() =
opcode == Opcodes.IFNULL ||
opcode == Opcodes.IFNONNULL

internal fun AbstractInsnNode.isCheckParameterNotNull() =
internal fun AbstractInsnNode.isCheckParameterIsNotNull() =
isInsn<MethodInsnNode>(Opcodes.INVOKESTATIC) {
owner == "kotlin/jvm/internal/Intrinsics" &&
name == "checkParameterIsNotNull" &&
desc == "(Ljava/lang/Object;Ljava/lang/String;)V"
}

internal fun AbstractInsnNode.isCheckExpressionValueIsNotNull() =
isInsn<MethodInsnNode>(Opcodes.INVOKESTATIC) {
owner == "kotlin/jvm/internal/Intrinsics" &&
name == "checkExpressionValueIsNotNull" &&
desc == "(Ljava/lang/Object;Ljava/lang/String;)V"
}

internal fun AbstractInsnNode.isThrowNpeIntrinsic() =
isInsn<MethodInsnNode>(Opcodes.INVOKESTATIC) {
owner == "kotlin/jvm/internal/Intrinsics" &&
name == "throwNpe" &&
desc == "()V"
}

internal fun InsnList.popReferenceValueBefore(insn: AbstractInsnNode) {
val prev = insn.previous
when (prev?.opcode) {
Expand Down
23 changes: 23 additions & 0 deletions compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNull.kt
View file
@@ -0,0 +1,23 @@
// FILE: j/J.java

package j;

public class J {
public static final String ok() { return "OK"; }
}

// FILE: k.kt
import j.J

fun foo(a: Any) {}

fun test() {
val a = J.ok()
foo(a)
if (a == null) foo("NULL-1")
}

// @KKt.class:
// 0 IFNULL
// 0 IFNONNULL
// 0 NULL-1
24 changes: 24 additions & 0 deletions ...tData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNullAfterExclExcl.kt
View file
@@ -0,0 +1,24 @@
// FILE: j/J.java

package j;

public class J {
public static final String ok() { return "OK"; }
}

// FILE: k.kt
import j.J

fun foo(a: Any) {}

fun test() {
val a = J.ok()
a!!
foo(a)
if (a == null) foo("NULL-1")
}

// @KKt.class:
// 0 IFNULL
// 1 IFNONNULL
// 0 NULL-1
23 changes: 23 additions & 0 deletions ...iler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNullTwice.kt
View file
@@ -0,0 +1,23 @@
// FILE: j/J.java

package j;

public class J {
public static final String ok() { return "OK"; }
}

// FILE: foo.kt
fun foo(a: Any) {}

// FILE: k.kt
import j.J

fun test() {
val a = J.ok()
foo(a)
foo(a)
}

// @KKt.class:
// 1 LDC "a"
// 1 checkExpressionValueIsNotNull
18 changes: 18 additions & 0 deletions compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java
View file
Expand Up @@ -1739,6 +1739,24 @@ public void testAlreadyCheckedForNull() throws Exception {
doTest(fileName);
}

@TestMetadata("expressionValueIsNotNull.kt")
public void testExpressionValueIsNotNull() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNull.kt");
doTest(fileName);
}

@TestMetadata("expressionValueIsNotNullAfterExclExcl.kt")
public void testExpressionValueIsNotNullAfterExclExcl() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNullAfterExclExcl.kt");
doTest(fileName);
}

@TestMetadata("expressionValueIsNotNullTwice.kt")
public void testExpressionValueIsNotNullTwice() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNullTwice.kt");
doTest(fileName);
}

@TestMetadata("ifNullEqualsNull.kt")
public void testIfNullEqualsNull() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/nullCheckOptimization/ifNullEqualsNull.kt");
Expand Down

0 comments on commit 08885e2

Please sign in to comment.