JVM_IR: Generate better code for null checks.

Simplify ifs when branches have condition true/false.

Simplify blocks containing only a variable declaration
and a variable get of the same variable. Simplify to
just the condition.

Do not introduce temporary variables for constants for
null checks. Constants have no side-effects and can be
reloaded freely instead of going through a local.

This simplifies code such as "42.toLong()!!" so that the
resulting code has no branches and uses no locals. The
simplifications happen as follows:

```
block
  temp = 42.toLong()
  when
    (temp == null) throw NPE
    (true) load temp

---> null test simplification

block
  temp = 42.toLong()
  when
    (false) throw NPE
    (true) load temp

---> when simplification

block
  temp = 42.toLong()
  load temp

---> block simplification

42.toLong()
```
This commit is contained in:
Mads Ager
2019-01-21 08:15:24 +01:00
committed by max-kammerer
parent d8309f3bd7
commit fb6eafddf1
15 changed files with 192 additions and 16 deletions
@@ -8,17 +8,19 @@ package org.jetbrains.kotlin.backend.jvm.lower
import org.jetbrains.kotlin.backend.common.FileLoweringPass
import org.jetbrains.kotlin.backend.jvm.JvmBackendContext
import org.jetbrains.kotlin.codegen.intrinsics.Not
import org.jetbrains.kotlin.ir.IrStatement
import org.jetbrains.kotlin.ir.declarations.IrDeclarationOrigin
import org.jetbrains.kotlin.ir.declarations.IrFile
import org.jetbrains.kotlin.ir.expressions.IrCall
import org.jetbrains.kotlin.ir.expressions.IrConst
import org.jetbrains.kotlin.ir.expressions.IrExpression
import org.jetbrains.kotlin.ir.expressions.IrGetValue
import org.jetbrains.kotlin.ir.declarations.IrVariable
import org.jetbrains.kotlin.ir.expressions.*
import org.jetbrains.kotlin.ir.expressions.impl.IrBlockImpl
import org.jetbrains.kotlin.ir.expressions.impl.IrConstImpl
import org.jetbrains.kotlin.ir.types.isPrimitiveType
import org.jetbrains.kotlin.ir.types.toKotlinType
import org.jetbrains.kotlin.ir.util.coerceToUnitIfNeeded
import org.jetbrains.kotlin.ir.util.isFalseConst
import org.jetbrains.kotlin.ir.util.isNullConst
import org.jetbrains.kotlin.ir.util.isTrueConst
import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid
import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid
@@ -57,11 +59,12 @@ class JvmBuiltinOptimizationLowering(val context: JvmBackendContext) : FileLower
return false
}
private fun isNullCheckOfNullConstant(call: IrCall, context: JvmBackendContext): Boolean {
private fun isNullCheckOfConstant(call: IrCall, context: JvmBackendContext): Boolean {
if (call.symbol == context.irBuiltIns.eqeqSymbol) {
val left = call.getValueArgument(0)!!
val right = call.getValueArgument(1)!!
return right.isNullConst() && left.isNullConst()
return (right.isNullConst() && left is IrConst<*>)
|| (left.isNullConst() && right is IrConst<*>)
}
return false
}
@@ -87,12 +90,127 @@ class JvmBuiltinOptimizationLowering(val context: JvmBackendContext) : FileLower
statements.add(constFalse)
}
}
} else if (isNullCheckOfNullConstant(expression, context)) {
IrConstImpl.constTrue(expression.startOffset, expression.endOffset, context.irBuiltIns.booleanType)
} else if (isNullCheckOfConstant(expression, context)) {
if (expression.getValueArgument(0)!!.isNullConst() && expression.getValueArgument(1)!!.isNullConst()) {
IrConstImpl.constTrue(expression.startOffset, expression.endOffset, context.irBuiltIns.booleanType)
} else {
IrConstImpl.constFalse(expression.startOffset, expression.endOffset, context.irBuiltIns.booleanType)
}
} else {
expression
}
}
override fun visitWhen(expression: IrWhen): IrExpression {
expression.transformChildrenVoid(this)
// Remove all branches with constant false condition.
expression.branches.removeIf() {
it.condition.isFalseConst()
}
// If the only condition that is left has a constant true condition remove the
// when in favor of the result. If there are no conditions left, remove the when
// entirely and replace it with an empty block.
return if (expression.branches.size == 0) {
IrBlockImpl(expression.startOffset, expression.endOffset, context.irBuiltIns.unitType)
} else {
expression.branches.first().takeIf { it.condition.isTrueConst() }?.result ?: expression
}
}
private fun isImmutableTemporaryVariableWithConstantValue(statement: IrStatement): Boolean {
return statement is IrVariable &&
statement.origin == IrDeclarationOrigin.IR_TEMPORARY_VARIABLE &&
!statement.isVar &&
statement.initializer is IrConst<*>
}
override fun visitBlock(expression: IrBlock): IrExpression {
expression.transformChildrenVoid(this)
// Remove declarations of immutable temporary variables with constant values.
// IrGetValue operations for such temporary variables are replaced
// by the initializer IrConst. This makes sure that we do not load and
// store constants in/from locals. For example
//
// "StringConstant"!!
//
// introduces a temporary variable for the string constant and generates
// a null check
//
// block
// temp = "StringConstant"
// when (eq(temp, null))
// (true) -> throwNpe()
// (false) -> temp
//
// When generating code, this stores the string constant in a local and loads
// it from there. The removal of the temporary and the replacement of the loads
// of the temporary (see visitGetValue) with the constant avoid generating local
// loads and stores by turning this into
//
// block
// when (eq("StringConstant", null))
// (true) -> throwNpe()
// (false) -> "StringConstant"
//
// which allows the equality check to be simplified away and we end up with
// just a const string load.
expression.statements.removeIf {
isImmutableTemporaryVariableWithConstantValue(it)
}
// Remove a block that contains only two statements: the declaration of a temporary
// variable and a load of the value of that temporary variable with just the initializer
// for the temporary variable. We only perform this transformation for compiler generated
// temporary variables. Local variables can be changed at runtime and therefore eliminating
// an actual local variable changes debugging behavior.
//
// This helps avoid temporary variables even for side-effecting expressions when they are
// not needed. Having a temporary variable leads to local loads and stores in the
// generated java bytecode which are not necessary. For example
//
// 42.toLong()!!
//
// introduces a temporary variable for the toLong() call and a null check
// block
// temp = 42.toLong()
// when (eq(temp, null))
// (true) -> throwNep()
// (false) -> temp
//
// the when is simplified because long is a primitive type, which leaves us with
//
// block
// temp = 42.toLong()
// temp
//
// which can be simplified to simply
//
// block
// 42.toLong()
//
// Doing so we avoid local loads and stores.
if (expression.statements.size == 2) {
val first = expression.statements[0]
val second = expression.statements[1]
if (first is IrVariable
&& first.origin == IrDeclarationOrigin.IR_TEMPORARY_VARIABLE
&& second is IrGetValue
&& first.symbol == second.symbol) {
expression.statements.clear()
first.initializer?.let { expression.statements.add(it) }
}
}
return expression
}
override fun visitGetValue(expression: IrGetValue): IrExpression {
// Replace IrGetValue of an immutable temporary variable with a constant
// initializer with the constant initializer.
val variable = expression.symbol.owner
return if (isImmutableTemporaryVariableWithConstantValue(variable))
(variable as IrVariable).initializer!!
else
expression
}
})
}
}
}
@@ -144,6 +144,9 @@ fun IrMemberAccessExpression.addArguments(args: List<Pair<ParameterDescriptor, I
fun IrExpression.isNullConst() = this is IrConst<*> && this.kind == IrConstKind.Null
fun IrExpression.isTrueConst() = this is IrConst<*> && this.kind == IrConstKind.Boolean && this.value == true
fun IrExpression.isFalseConst() = this is IrConst<*> && this.kind == IrConstKind.Boolean && this.value == false
fun IrExpression.coerceToUnitIfNeeded(valueType: KotlinType, irBuiltIns: IrBuiltIns): IrExpression {
return if (KotlinTypeChecker.DEFAULT.isSubtypeOf(valueType, irBuiltIns.unitType.toKotlinType()))
-1
View File
@@ -1,4 +1,3 @@
// IGNORE_BACKEND: JVM_IR
fun test1() : Boolean {
try {
return true
@@ -1,4 +1,3 @@
// IGNORE_BACKEND: JVM_IR
fun shouldReturnFalse() : Boolean {
try {
return true
@@ -1,4 +1,3 @@
// IGNORE_BACKEND: JVM_IR
fun box(): String {
if (false) {
try {
@@ -1,4 +1,3 @@
// IGNORE_BACKEND: JVM_IR
class MyString {
var s = ""
operator fun plus(x : String) : MyString {
@@ -1,4 +1,3 @@
// IGNORE_BACKEND: JVM_IR
// FILE: 1.kt
package test
@@ -1,4 +1,3 @@
// IGNORE_BACKEND: JVM_IR
fun test() {
if (false) {
try {
@@ -0,0 +1,9 @@
fun box(): String {
42!!
42.toLong()!!
return "OK"!!
}
// 0 LOAD
// 0 STORE
// 0 IF
@@ -1,4 +1,3 @@
// IGNORE_BACKEND: JVM_IR
fun test(a: Any?, b: Any?, c: Any?) {
when (null) {
a -> throw IllegalArgumentException("a is null")
@@ -0,0 +1,7 @@
fun box(): String {
42!!
42.toLong()!!
return "OK"!!
}
// 2 3 4
@@ -1459,6 +1459,24 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest {
}
}
@TestMetadata("compiler/testData/codegen/bytecodeText/exclExcl")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class ExclExcl extends AbstractBytecodeTextTest {
private void runTest(String testDataFilePath) throws Exception {
KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath);
}
public void testAllFilesPresentInExclExcl() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/bytecodeText/exclExcl"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true);
}
@TestMetadata("primitive.kt")
public void testPrimitive() throws Exception {
runTest("compiler/testData/codegen/bytecodeText/exclExcl/primitive.kt");
}
}
@TestMetadata("compiler/testData/codegen/bytecodeText/forLoop")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
@@ -1459,6 +1459,24 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest {
}
}
@TestMetadata("compiler/testData/codegen/bytecodeText/exclExcl")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class ExclExcl extends AbstractIrBytecodeTextTest {
private void runTest(String testDataFilePath) throws Exception {
KotlinTestUtils.runTest(this::doTest, TargetBackend.JVM_IR, testDataFilePath);
}
public void testAllFilesPresentInExclExcl() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/bytecodeText/exclExcl"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM_IR, true);
}
@TestMetadata("primitive.kt")
public void testPrimitive() throws Exception {
runTest("compiler/testData/codegen/bytecodeText/exclExcl/primitive.kt");
}
}
@TestMetadata("compiler/testData/codegen/bytecodeText/forLoop")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
@@ -201,6 +201,11 @@ public class LineNumberTestGenerated extends AbstractLineNumberTest {
runTest("compiler/testData/lineNumber/custom/noParametersArgumentCallInExpression.kt");
}
@TestMetadata("primitiveNullChecks.kt")
public void testPrimitiveNullChecks() throws Exception {
runTest("compiler/testData/lineNumber/custom/primitiveNullChecks.kt");
}
@TestMetadata("smapInlineAsArgument.kt")
public void testSmapInlineAsArgument() throws Exception {
runTest("compiler/testData/lineNumber/custom/smapInlineAsArgument.kt");
@@ -201,6 +201,11 @@ public class IrLineNumberTestGenerated extends AbstractIrLineNumberTest {
runTest("compiler/testData/lineNumber/custom/noParametersArgumentCallInExpression.kt");
}
@TestMetadata("primitiveNullChecks.kt")
public void testPrimitiveNullChecks() throws Exception {
runTest("compiler/testData/lineNumber/custom/primitiveNullChecks.kt");
}
@TestMetadata("smapInlineAsArgument.kt")
public void testSmapInlineAsArgument() throws Exception {
runTest("compiler/testData/lineNumber/custom/smapInlineAsArgument.kt");