From 7afe12123807065385ef71530a35f1c5e78443f0 Mon Sep 17 00:00:00 2001 From: Mark Punzalan Date: Tue, 26 Mar 2019 11:20:01 -0700 Subject: [PATCH] Eliminated the redundant ForLoopHeader.needsEmptinessCheck property, cast "bound" to progression type if necessary, fixed variable declaration order. --- .../common/lower/loops/ForLoopsLowering.kt | 8 +-- .../common/lower/loops/HeaderProcessor.kt | 57 +++++++++---------- .../common/lower/loops/ProgressionHandlers.kt | 33 +++++------ .../bytecodeText/forLoop/forInUntil.kt | 3 +- .../bytecodeText/forLoop/primitiveRange.kt | 3 +- 5 files changed, 45 insertions(+), 59 deletions(-) diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/ForLoopsLowering.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/ForLoopsLowering.kt index 52f41b23a47..a2442fe14ab 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/ForLoopsLowering.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/ForLoopsLowering.kt @@ -25,7 +25,6 @@ import org.jetbrains.kotlin.ir.types.toKotlinType import org.jetbrains.kotlin.ir.util.functions import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid -import org.jetbrains.kotlin.util.OperatorNameConventions val forLoopsPhase = makeIrFilePhase( ::ForLoopsLowering, @@ -171,12 +170,7 @@ private class RangeLoopTransformer( oldLoopToNewLoop[loop] = newLoop // Surround the new loop with a check for an empty loop, if necessary. - if (loopHeader.needsEmptinessCheck) { - val notEmptyCondition = loopHeader.buildNotEmptyCondition(this@with) - if (notEmptyCondition != null) - return irIfThen(notEmptyCondition, newLoop) - } - return newLoop + return loopHeader.buildNotEmptyConditionIfNecessary(this@with)?.let { irIfThen(it, newLoop) } ?: newLoop } } diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/HeaderProcessor.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/HeaderProcessor.kt index ef4798f15a1..d4dcb9ee944 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/HeaderProcessor.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/HeaderProcessor.kt @@ -30,7 +30,6 @@ internal sealed class ForLoopHeader( val last: IrVariable, val step: IrVariable, val progressionType: ProgressionType, - val needsEmptinessCheck: Boolean, var loopVariable: IrVariable? = null ) { /** Expression used to initialize the loop variable at the beginning of the loop. */ @@ -48,7 +47,7 @@ internal sealed class ForLoopHeader( * * Returns null if no check is needed for the for-loop. */ - abstract fun buildNotEmptyCondition(builder: DeclarationIrBuilder): IrExpression? + abstract fun buildNotEmptyConditionIfNecessary(builder: DeclarationIrBuilder): IrExpression? } internal class ProgressionLoopHeader( @@ -56,7 +55,7 @@ internal class ProgressionLoopHeader( inductionVariable: IrVariable, last: IrVariable, step: IrVariable -) : ForLoopHeader(inductionVariable, last, step, headerInfo.progressionType, needsEmptinessCheck = true) { +) : ForLoopHeader(inductionVariable, last, step, headerInfo.progressionType) { override fun initializeLoopVariable(symbols: Symbols, builder: DeclarationIrBuilder) = with(builder) { // loopVariable = inductionVariable @@ -64,17 +63,17 @@ internal class ProgressionLoopHeader( } override val declarations: List - get() = headerInfo.additionalVariables + listOf(inductionVariable, step, last) + get() = headerInfo.additionalVariables + listOf(inductionVariable, last, step) override fun buildInnerLoop(builder: DeclarationIrBuilder, loop: IrLoop, newBody: IrExpression?): IrLoop = with(builder) { // Condition: loopVariable != last assert(loopVariable != null) - val newCondition = irCall(context.irBuiltIns.booleanNotSymbol).apply { - putValueArgument(0, irCall(context.irBuiltIns.eqeqSymbol).apply { - putValueArgument(0, irGet(loopVariable!!)) - putValueArgument(1, irGet(last)) - }) - } + val booleanNotFun = context.irBuiltIns.booleanClass.functions.first { it.owner.name.asString() == "not" } + val newCondition = irCallOp(booleanNotFun, booleanNotFun.owner.returnType, irCall(context.irBuiltIns.eqeqSymbol).apply { + putValueArgument(0, irGet(loopVariable!!)) + putValueArgument(1, irGet(last)) + }) + // TODO: Build while loop (instead of do-while) where possible, e.g., in "until" ranges, or when "last" is known to be < MAX_VALUE IrDoWhileLoopImpl(loop.startOffset, loop.endOffset, loop.type, loop.origin).apply { @@ -84,12 +83,13 @@ internal class ProgressionLoopHeader( } } - override fun buildNotEmptyCondition(builder: DeclarationIrBuilder): IrExpression? = + override fun buildNotEmptyConditionIfNecessary(builder: DeclarationIrBuilder): IrExpression? = with(builder) { val builtIns = context.irBuiltIns val progressionKotlinType = progressionType.elementType(builtIns).toKotlinType() val lessOrEqualFun = builtIns.lessOrEqualFunByOperandType[progressionKotlinType]!! + // The default "not empty" condition depends on the direction. val notEmptyCondition = when (headerInfo.direction) { ProgressionDirection.DECREASING -> // last <= inductionVariable @@ -143,7 +143,7 @@ internal class ArrayLoopHeader( inductionVariable: IrVariable, last: IrVariable, step: IrVariable -) : ForLoopHeader(inductionVariable, last, step, ProgressionType.INT_PROGRESSION, needsEmptinessCheck = false) { +) : ForLoopHeader(inductionVariable, last, step, ProgressionType.INT_PROGRESSION) { override fun initializeLoopVariable(symbols: Symbols, builder: DeclarationIrBuilder) = with(builder) { // loopVariable = array[inductionVariable] @@ -155,7 +155,7 @@ internal class ArrayLoopHeader( } override val declarations: List - get() = listOf(headerInfo.arrayVariable, inductionVariable, step, last) + get() = listOf(headerInfo.arrayVariable, inductionVariable, last, step) override fun buildInnerLoop(builder: DeclarationIrBuilder, loop: IrLoop, newBody: IrExpression?): IrLoop = with(builder) { // Condition: loopVariable != last @@ -174,7 +174,7 @@ internal class ArrayLoopHeader( } // No surrounding emptiness check is needed with a while loop. - override fun buildNotEmptyCondition(builder: DeclarationIrBuilder): IrExpression? = null + override fun buildNotEmptyConditionIfNecessary(builder: DeclarationIrBuilder): IrExpression? = null } /** @@ -277,22 +277,21 @@ internal class HeaderProcessor( } } } +} - - private fun IrExpression.castIfNecessary(targetType: IrType, numberCastFunctionName: Name): IrExpression { - return if (type.toKotlinType() == targetType.toKotlinType()) { - this - } else { - val function = type.getClass()!!.functions.first { it.name == numberCastFunctionName } - IrCallImpl(startOffset, endOffset, function.returnType, function.symbol) - .apply { dispatchReceiver = this@castIfNecessary } - } +internal fun DeclarationIrBuilder.ensureNotNullable(expression: IrExpression) = + if (expression.type is IrSimpleType && expression.type.isNullable()) { + irImplicitCast(expression, expression.type.makeNotNull()) + } else { + expression } - private fun DeclarationIrBuilder.ensureNotNullable(expression: IrExpression) = - if (expression.type is IrSimpleType && expression.type.isNullable()) { - irImplicitCast(expression, expression.type.makeNotNull()) - } else { - expression - } +internal fun IrExpression.castIfNecessary(targetType: IrType, numberCastFunctionName: Name): IrExpression { + return if (type.toKotlinType() == targetType.toKotlinType()) { + this + } else { + val function = type.getClass()!!.functions.first { it.name == numberCastFunctionName } + IrCallImpl(startOffset, endOffset, function.returnType, function.symbol) + .apply { dispatchReceiver = this@castIfNecessary } + } } \ No newline at end of file diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/ProgressionHandlers.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/ProgressionHandlers.kt index d6e8775f4b3..7fbdf25a47d 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/ProgressionHandlers.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/ProgressionHandlers.kt @@ -81,46 +81,41 @@ internal class UntilHandler(private val context: CommonBackendContext, private v with(context.createIrBuilder(call.symbol, call.startOffset, call.endOffset)) { // `last = bound - 1` for the loop `for (i in first until bound)`. val bound = scope.createTemporaryVariable( - call.getValueArgument(0)!!, nameHint = "bound", + ensureNotNullable( + call.getValueArgument(0)!!.castIfNecessary( + data.elementType(context.irBuiltIns), + data.elementCastFunctionName + ) + ), nameHint = "bound", origin = IrDeclarationOrigin.FOR_LOOP_IMPLICIT_VARIABLE ) val decFun = data.decFun(context.irBuiltIns) - val last = irCallOp(decFun.symbol, bound.type, call.getValueArgument(0)!!) + val last = irCallOp(decFun.symbol, bound.type, irGet(bound)) - // An additional emptiness condition is required for the corner case: + // The default "not empty" check cannot be used for the required for the corner case: // - // ``` - // for (i in a until MIN_VALUE) {} - // ``` + // for (i in a until MIN_VALUE) {} // // ...which should always be considered an empty range. When the given bound is MIN_VALUE, and because `last = bound - 1`, - // "last" will underflow to MAX_VALUE, therefore the default emptiness check: + // "last" will underflow to MAX_VALUE, therefore the default "not empty" check: // - // ``` - // if (first <= last) { /* loop */ } - // ``` + // if (first <= last) { /* loop */ } // // ...will always be true and won't consider the range as empty. Therefore, we need to add an additional condition to the // emptiness check so that it becomes: // - // ``` - // if (first <= last && bound > MIN_VALUE) { /* loop */ } - // ``` - // TODO: Do not add additionalEmptinessCondition if "bound" is const and > MIN_VALUE + // if (first <= last && bound > MIN_VALUE) { /* loop */ } ProgressionHeaderInfo( data, first = call.extensionReceiver!!, last = last, step = irInt(1), additionalVariables = listOf(bound), - additionalNotEmptyCondition = buildMinValueConditionIfNecessary(data, irGet(bound)) + additionalNotEmptyCondition = buildMinValueCondition(data, irGet(bound)) ) } - private fun DeclarationIrBuilder.buildMinValueConditionIfNecessary( - progressionType: ProgressionType, - bound: IrExpression - ): IrExpression? { + private fun DeclarationIrBuilder.buildMinValueCondition(progressionType: ProgressionType, bound: IrExpression): IrExpression { val irBuiltIns = context.irBuiltIns val minConst = when (progressionType) { ProgressionType.INT_PROGRESSION -> irInt(Int.MIN_VALUE) diff --git a/compiler/testData/codegen/bytecodeText/forLoop/forInUntil.kt b/compiler/testData/codegen/bytecodeText/forLoop/forInUntil.kt index 73986f5afa6..b2c13ac872e 100644 --- a/compiler/testData/codegen/bytecodeText/forLoop/forInUntil.kt +++ b/compiler/testData/codegen/bytecodeText/forLoop/forInUntil.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR fun test(): Int { var sum = 0 for (i in 1 until 6) { @@ -12,4 +11,4 @@ fun test(): Int { // 0 getEnd // 0 getFirst // 0 getLast -// 1 IF +// 0 getStep \ No newline at end of file diff --git a/compiler/testData/codegen/bytecodeText/forLoop/primitiveRange.kt b/compiler/testData/codegen/bytecodeText/forLoop/primitiveRange.kt index e9afbe9ec09..535e629cb53 100644 --- a/compiler/testData/codegen/bytecodeText/forLoop/primitiveRange.kt +++ b/compiler/testData/codegen/bytecodeText/forLoop/primitiveRange.kt @@ -7,5 +7,4 @@ fun f(r: IntRange) { // 0 getStart // 0 getEnd // 1 getFirst -// 1 getLast -// 1 getStep \ No newline at end of file +// 1 getLast \ No newline at end of file