From 301ac907704342b51e822a85177f0afff90734c5 Mon Sep 17 00:00:00 2001 From: Mark Punzalan Date: Tue, 19 Mar 2019 23:49:58 -0700 Subject: [PATCH] Remove "bound" and "closed" properties from HeaderInfo and ForLoopHeader, instead assuming that all ranges are closed and that the "last" property is already property calculated. This means the HeaderInfo.last for arrays, "indices" and "until" progressions are decremented from the bound. This simplifies the loop construction and moves the work to the handlers. This will also simplify the handling of "reversed()" in the future since we don't need to consider the case where the lower bound is open. --- .../backend/common/lower/loops/HeaderInfo.kt | 10 +- .../common/lower/loops/HeaderProcessor.kt | 97 ++++++--------- .../common/lower/loops/ProgressionHandlers.kt | 117 ++++++++++-------- .../kotlin/ir/builders/ExpressionHelpers.kt | 5 +- 4 files changed, 114 insertions(+), 115 deletions(-) diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/HeaderInfo.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/HeaderInfo.kt index c14e9daa177..e314a4994ae 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/HeaderInfo.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/loops/HeaderInfo.kt @@ -60,8 +60,7 @@ internal sealed class HeaderInfo( val progressionType: ProgressionType, val first: IrExpression, val last: IrExpression, - val step: IrExpression, - val closed: Boolean + val step: IrExpression ) { val direction: ProgressionDirection by lazy { // If step is a constant (either Int or Long), then we can determine the direction. @@ -81,9 +80,9 @@ internal class ProgressionHeaderInfo( first: IrExpression, last: IrExpression, step: IrExpression, - closed: Boolean = true, + val additionalVariables: List = listOf(), val additionalNotEmptyCondition: IrExpression? = null -) : HeaderInfo(progressionType, first, last, step, closed) +) : HeaderInfo(progressionType, first, last, step) internal class ArrayHeaderInfo( first: IrExpression, @@ -94,8 +93,7 @@ internal class ArrayHeaderInfo( ProgressionType.INT_PROGRESSION, first, last, - step, - closed = false + step ) internal interface HeaderInfoHandler { 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 ec7d68b7f08..fff0ec30b70 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 @@ -23,12 +23,10 @@ import org.jetbrains.kotlin.ir.symbols.IrSymbol import org.jetbrains.kotlin.ir.types.* import org.jetbrains.kotlin.ir.util.functions import org.jetbrains.kotlin.ir.util.isNullable -import org.jetbrains.kotlin.util.OperatorNameConventions /** Contains information about variables used in the loop. */ internal sealed class ForLoopHeader( val inductionVariable: IrVariable, - val bound: IrVariable, val last: IrVariable, val step: IrVariable, val progressionType: ProgressionType, @@ -47,17 +45,16 @@ internal sealed class ForLoopHeader( internal class ProgressionLoopHeader( private val headerInfo: ProgressionHeaderInfo, inductionVariable: IrVariable, - bound: IrVariable, last: IrVariable, step: IrVariable -) : ForLoopHeader(inductionVariable, bound, last, step, headerInfo.progressionType, needsEmptinessCheck = true) { +) : ForLoopHeader(inductionVariable, last, step, headerInfo.progressionType, needsEmptinessCheck = true) { override fun initializeLoopVariable(symbols: Symbols, builder: DeclarationIrBuilder) = with(builder) { irGet(inductionVariable) } override val declarations: List - get() = listOf(inductionVariable, step, bound, last) + get() = headerInfo.additionalVariables + listOf(inductionVariable, step, last) override fun buildInnerLoop(builder: DeclarationIrBuilder, loop: IrLoop, newBody: IrExpression?): IrLoop = with(builder) { assert(loopVariable != null) @@ -131,10 +128,9 @@ internal class ProgressionLoopHeader( internal class ArrayLoopHeader( private val headerInfo: ArrayHeaderInfo, inductionVariable: IrVariable, - bound: IrVariable, last: IrVariable, step: IrVariable -) : ForLoopHeader(inductionVariable, bound, last, step, ProgressionType.INT_PROGRESSION, needsEmptinessCheck = false) { +) : ForLoopHeader(inductionVariable, last, step, ProgressionType.INT_PROGRESSION, needsEmptinessCheck = false) { override fun initializeLoopVariable(symbols: Symbols, builder: DeclarationIrBuilder) = with(builder) { val arrayClass = (headerInfo.arrayVariable.type.classifierOrNull) as IrClassSymbol @@ -146,7 +142,7 @@ internal class ArrayLoopHeader( } override val declarations: List - get() = listOf(headerInfo.arrayVariable, inductionVariable, step, bound, last) + get() = listOf(headerInfo.arrayVariable, inductionVariable, step, last) override fun buildInnerLoop(builder: DeclarationIrBuilder, loop: IrLoop, newBody: IrExpression?): IrLoop = with(builder) { val builtIns = context.irBuiltIns @@ -182,23 +178,28 @@ internal class HeaderProcessor( if (!variable.type.isSubtypeOfClass(symbols.iterator)) { return null } + + // Collect loop information. + val headerInfo = headerInfoBuilder.build(variable) + ?: return null // If the iterable is not supported. + val builder = context.createIrBuilder(scopeOwnerSymbol(), variable.startOffset, variable.endOffset) - // Collect loop info and form the loop header composite. - val progressionInfo = headerInfoBuilder.build(variable) - ?: return null - if (progressionInfo is ArrayHeaderInfo) { - progressionInfo.arrayVariable.parent = variable.parent - } - with(builder) { - with(progressionInfo) { - /** - * For this loop: - * `for (i in a() .. b() step c() step d())` - * We need to call functions in the following order: a, b, c, d. - * So we call b() before step calculations and then call last element calculation function (if required). - */ - // Due to features of PSI2IR we can obtain nullable arguments here while actually - // they are non-nullable (the frontend takes care about this). So we need to cast them to non-nullable. + with(builder) builder@{ + with(headerInfo) { + // For this loop: + // + // ``` + // for (i in first()..last() step step()) + // ``` + // + // ...the functions may have side-effects so we need to call them in the following + // order: first(), last(), step(). + // + // We also need to cast them to conform to the progression type so that operations + // in the induction variable within the loop are more efficient. + // + // In the above example, if first() is a Long and last() is an Int, this creates a + // LongProgression so last(), should be cast to a Long. val inductionVariable = scope.createTemporaryVariable( first.castIfNecessary(progressionType), nameHint = "inductionVariable", @@ -206,52 +207,32 @@ internal class HeaderProcessor( origin = IrDeclarationOrigin.FOR_LOOP_IMPLICIT_VARIABLE ) - val upperBoundTmpVariable = scope.createTemporaryVariable( - ensureNotNullable(last.castIfNecessary(progressionType)), - nameHint = "upperBound", - origin = IrDeclarationOrigin.FOR_LOOP_IMPLICIT_VARIABLE - ) - - val stepExpression = ensureNotNullable(step) - - val stepValue = scope.createTemporaryVariable( - stepExpression, - nameHint = "step", - origin = IrDeclarationOrigin.FOR_LOOP_IMPLICIT_VARIABLE - ) - - // Calculate the last element of the progression - // The last element can be: - // boundValue, if step is 1 and the range is closed. - // boundValue - 1, if step is 1 and the range is open. - // getProgressionLast(inductionVariable, boundValue, step), if step != 1 and the range is closed. - // getProgressionLast(inductionVariable, boundValue - 1, step), if step != 1 and the range is open. - val lastExpression = if (closed) { - irGet(upperBoundTmpVariable) - } else { - val decrementSymbol = symbols.getUnaryOperator(OperatorNameConventions.DEC, upperBoundTmpVariable.type.toKotlinType()) - irCall(decrementSymbol.owner).apply { - dispatchReceiver = irGet(upperBoundTmpVariable) - } - } + // Due to features of PSI2IR we can obtain nullable arguments here while actually + // they are non-nullable (the frontend takes care about this). So we need to cast + // them to non-nullable. + // TODO: Confirm if casting to non-nullable is still necessary val lastValue = scope.createTemporaryVariable( - lastExpression, + ensureNotNullable(last.castIfNecessary(progressionType)), nameHint = "last", origin = IrDeclarationOrigin.FOR_LOOP_IMPLICIT_VARIABLE ) - return when (progressionInfo) { + val stepValue = scope.createTemporaryVariable( + ensureNotNullable(step), + nameHint = "step", + origin = IrDeclarationOrigin.FOR_LOOP_IMPLICIT_VARIABLE + ) + + return when (headerInfo) { is ArrayHeaderInfo -> ArrayLoopHeader( - progressionInfo, + headerInfo, inductionVariable, - upperBoundTmpVariable, lastValue, stepValue ) is ProgressionHeaderInfo -> ProgressionLoopHeader( - progressionInfo, + headerInfo, inductionVariable, - upperBoundTmpVariable, lastValue, stepValue ) 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 227f3b9e72b..6d3c7823e16 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 @@ -13,12 +13,15 @@ import org.jetbrains.kotlin.backend.common.lower.matchers.createIrCallMatcher import org.jetbrains.kotlin.backend.common.lower.matchers.singleArgumentExtension import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.ir.builders.* +import org.jetbrains.kotlin.ir.declarations.IrDeclarationOrigin +import org.jetbrains.kotlin.ir.declarations.IrFunction +import org.jetbrains.kotlin.ir.descriptors.IrBuiltIns import org.jetbrains.kotlin.ir.expressions.IrCall import org.jetbrains.kotlin.ir.expressions.IrExpression import org.jetbrains.kotlin.ir.expressions.IrStatementOrigin -import org.jetbrains.kotlin.ir.symbols.IrClassSymbol -import org.jetbrains.kotlin.ir.types.classifierOrNull +import org.jetbrains.kotlin.ir.types.getClass import org.jetbrains.kotlin.ir.types.toKotlinType +import org.jetbrains.kotlin.ir.util.functions import org.jetbrains.kotlin.ir.util.properties import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.name.Name @@ -74,7 +77,14 @@ internal class UntilHandler(private val context: CommonBackendContext, private v override fun build(call: IrCall, data: ProgressionType): HeaderInfo? = 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", + origin = IrDeclarationOrigin.FOR_LOOP_IMPLICIT_VARIABLE + ) + val decFun = data.decFun(context.irBuiltIns) + val last = irCallOp(decFun.symbol, bound.type, call.getValueArgument(0)!!) + // An additional emptiness condition is required for the corner case: // // ``` @@ -94,15 +104,14 @@ internal class UntilHandler(private val context: CommonBackendContext, private v // ``` // if (first <= last && bound > MIN_VALUE) { /* loop */ } // ``` - // // TODO: Do not add additionalEmptinessCondition if "bound" is const and > MIN_VALUE ProgressionHeaderInfo( data, first = call.extensionReceiver!!, - last = call.getValueArgument(0)!!, + last = last, step = irInt(1), - closed = false, - additionalNotEmptyCondition = buildMinValueConditionIfNecessary(data, call.getValueArgument(0)!!) + additionalVariables = listOf(bound), + additionalNotEmptyCondition = buildMinValueConditionIfNecessary(data, irGet(bound)) ) } @@ -136,68 +145,78 @@ internal class IndicesHandler(val context: CommonBackendContext) : ProgressionHa override fun build(call: IrCall, data: ProgressionType): HeaderInfo? = with(context.createIrBuilder(call.symbol, call.startOffset, call.endOffset)) { - val lowerBound = irInt(0) - val arrayClass = (call.extensionReceiver!!.type.classifierOrNull) as IrClassSymbol - val arraySizeProperty = arrayClass.owner.properties.find { it.name.toString() == "size" }!! - val upperBound = irCall(arraySizeProperty.getter!!).apply { + // `last = array.size - 1` for the loop `for (i in array.indices)`. + val arraySizeProperty = call.extensionReceiver!!.type.getClass()!!.properties.first { it.name.asString() == "size" } + val decFun = data.decFun(context.irBuiltIns) + val last = irCallOp(decFun.symbol, data.elementType(context.irBuiltIns), irCall(arraySizeProperty.getter!!).apply { dispatchReceiver = call.extensionReceiver - } + }) + ProgressionHeaderInfo( data, - lowerBound, - upperBound, - step = irInt(1), - closed = false + first = irInt(0), + last = last, + step = irInt(1) ) } - - private fun DeclarationIrBuilder.buildMinValueConditionIfNecessary( - progressionType: ProgressionType, - bound: IrExpression - ): IrExpression? { - val irBuiltIns = context.irBuiltIns - val minConst = when (progressionType) { - ProgressionType.INT_PROGRESSION -> irInt(Int.MIN_VALUE) - ProgressionType.CHAR_PROGRESSION -> irChar(Char.MIN_VALUE) - ProgressionType.LONG_PROGRESSION -> irLong(Long.MIN_VALUE) - } - val progressionKotlinType = progressionType.elementType(irBuiltIns).toKotlinType() - return irCall(irBuiltIns.greaterFunByOperandType[progressionKotlinType]!!).apply { - putValueArgument(0, bound) - putValueArgument(1, minConst) - } - } } -internal class ArrayIterationHandler(val context: CommonBackendContext) : HeaderInfoHandler { +internal class ArrayIterationHandler(private val context: CommonBackendContext) : HeaderInfoHandler { + + private val intDecFun = ProgressionType.INT_PROGRESSION.decFun(context.irBuiltIns) - // No support for rare cases like `T : IntArray` for now. override val matcher = createIrCallMatcher { origin { it == IrStatementOrigin.FOR_LOOP_ITERATOR } + // TODO: Support rare cases like `T : IntArray` dispatchReceiver { it != null && KotlinBuiltIns.isArrayOrPrimitiveArray(it.type.toKotlinType()) } } - // Consider case like `for (elem in A) { f(elem) }` - // If we lower it to `for (i in A.indices) { f(A[i]) }` - // Then we will break program behaviour if A is an expression with side-effect. - // Instead, we lower it to - // ``` - // val a = A - // for (i in a.indices) { f(a[i]) } - // ``` override fun build(call: IrCall, data: Nothing?): HeaderInfo? = with(context.createIrBuilder(call.symbol, call.startOffset, call.endOffset)) { - val arrayReference = scope.createTemporaryVariable(call.dispatchReceiver!!) - val arrayClass = (arrayReference.type.classifierOrNull) as IrClassSymbol - val arraySizeProperty = arrayClass.owner.properties.find { it.name.toString() == "size" }!! - val upperBound = irCall(arraySizeProperty.getter!!).apply { + // Consider the case like: + // + // ``` + // for (elem in A) { f(elem) }` + // ``` + // + // If we lower it to: + // + // ``` + // for (i in A.indices) { f(A[i]) } + // ``` + // + // ...then we will break program behaviour if `A` is an expression with side-effect. Instead, we lower it to: + // + // ``` + // val a = A + // for (i in a.indices) { f(a[i]) } + // ``` + val arrayReference = scope.createTemporaryVariable( + call.dispatchReceiver!!, nameHint = "array", + origin = IrDeclarationOrigin.FOR_LOOP_IMPLICIT_VARIABLE + ) + + // `last = array.size - 1` for the loop `for (i in array.indices)`. + val arraySizeProperty = arrayReference.type.getClass()!!.properties.first { it.name.asString() == "size" } + val last = irCallOp(intDecFun.symbol, context.irBuiltIns.intType, irCall(arraySizeProperty.getter!!).apply { dispatchReceiver = irGet(arrayReference) - } + }) + ArrayHeaderInfo( first = irInt(0), - last = upperBound, + last = last, step = irInt(1), arrayVariable = arrayReference ) } } + +private fun ProgressionType.decFun(builtIns: IrBuiltIns): IrFunction { + val symbol = + when (this) { + ProgressionType.INT_PROGRESSION -> builtIns.intClass + ProgressionType.LONG_PROGRESSION -> builtIns.longClass + ProgressionType.CHAR_PROGRESSION -> builtIns.charClass + } + return symbol.owner.functions.first { it.name.asString() == "dec" } +} \ No newline at end of file diff --git a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/builders/ExpressionHelpers.kt b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/builders/ExpressionHelpers.kt index b22bbc0af73..c891013ef87 100644 --- a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/builders/ExpressionHelpers.kt +++ b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/builders/ExpressionHelpers.kt @@ -249,11 +249,12 @@ fun IrBuilderWithScope.irCallOp( callee: IrFunctionSymbol, type: IrType, dispatchReceiver: IrExpression, - argument: IrExpression + argument: IrExpression? = null ): IrCall = irCall(callee, type).apply { this.dispatchReceiver = dispatchReceiver - putValueArgument(0, argument) + if (argument != null) + putValueArgument(0, argument) } fun IrBuilderWithScope.typeOperator(