From 42c43916548e3486db75a6bbe2a50f76c8ffbe9d Mon Sep 17 00:00:00 2001 From: Ilya Matveev Date: Fri, 21 Jul 2017 17:35:50 +0700 Subject: [PATCH] backend: Process 'until MIN_VALUE' in for loops This patch takes into account a corner case available for loops over a progression (see backend.native/tests/external/codegen/box/ ranges/forInUntil/forInUntilMinint.kt test): for (i in 0 until Int.MIN_VALUE) { ... } Here we cannot use subtraction to obtain a right bound of the loop due to an overflow. Instead we consider any loop with MIN_VALUE in it's right bound as empty loop. Such behaviour is similar to the one of 'until' method. So now the empty check for a loops with 'until' call: for (i in first until bound) { ... } is as follows: val last = getProgressionLast(first, bound, step) // Only if step==1 if (first <= last && bound > .MIN_VALUE) { do { ... } while(i != last) } Further improvements: We can generate a simpler IR for some simple frequent cases (e.g. for until calls without steps) and eliminate loops if we can prove that they are empty. --- .../jetbrains/kotlin/backend/common/ir/Ir.kt | 2 +- .../backend/konan/lower/ForLoopsLowering.kt | 138 +++++++++++++----- backend.native/tests/build.gradle | 5 + .../controlflow/for_loops_call_order.kt | 10 ++ .../codegen/controlflow/for_loops_overflow.kt | 14 ++ .../kotlin/konan/internal/RuntimeUtils.kt | 8 +- 6 files changed, 133 insertions(+), 44 deletions(-) create mode 100644 backend.native/tests/codegen/controlflow/for_loops_call_order.kt diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/common/ir/Ir.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/common/ir/Ir.kt index 18adb911cb6..72615a99108 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/common/ir/Ir.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/common/ir/Ir.kt @@ -81,7 +81,7 @@ open class Symbols(val context: T, private val symb val checkProgressionStep = context.getInternalFunctions("checkProgressionStep") .map { Pair(it.returnType, symbolTable.referenceSimpleFunction(it)) }.toMap() - val getProgressionBound = context.getInternalFunctions("getProgressionBound") + val getProgressionLast = context.getInternalFunctions("getProgressionLast") .map { Pair(it.returnType, symbolTable.referenceSimpleFunction(it)) }.toMap() val defaultConstructorMarker = symbolTable.referenceClass(context.getInternalClass("DefaultConstructorMarker")) diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/ForLoopsLowering.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/ForLoopsLowering.kt index 7436dd11d3d..92309c6889d 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/ForLoopsLowering.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/ForLoopsLowering.kt @@ -38,10 +38,8 @@ import org.jetbrains.kotlin.ir.symbols.IrVariableSymbol import org.jetbrains.kotlin.ir.visitors.* import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.name.Name -import org.jetbrains.kotlin.serialization.KonanIr import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.SimpleType -import org.jetbrains.kotlin.types.typeUtil.isBoolean import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf import org.jetbrains.kotlin.types.typeUtil.replaceArgumentsWithStarProjections import org.jetbrains.kotlin.util.OperatorNameConventions @@ -185,11 +183,11 @@ private class ForLoopsTransformer(val context: Context) : IrElementTransformerVo } to true } - private fun irGetProgressionBound(progressionType: ProgressionType, - first: IrVariableSymbol, - lastExpression: IrExpression, - step: IrVariableSymbol): IrExpression { - val symbol = symbols.getProgressionBound[progressionType.elementType] + private fun irGetProgressionLast(progressionType: ProgressionType, + first: IrVariableSymbol, + lastExpression: IrExpression, + step: IrVariableSymbol): IrExpression { + val symbol = symbols.getProgressionLast[progressionType.elementType] ?: throw IllegalArgumentException("Unknown progression element type: ${lastExpression.type}") val startOffset = lastExpression.startOffset val endOffset = lastExpression.endOffset @@ -204,7 +202,7 @@ private class ForLoopsTransformer(val context: Context) : IrElementTransformerVo //region Util classes ============================================================================================== // TODO: Replace with a cast when such support is added in the boxing lowering. private data class ProgressionType(val elementType: KotlinType, - val numberCastFunctionName: Name) { + val numberCastFunctionName: Name) { fun isIntProgression() = KotlinBuiltIns.isInt(elementType) fun isLongProgression() = KotlinBuiltIns.isLong(elementType) fun isCharProgression() = KotlinBuiltIns.isChar(elementType) @@ -213,16 +211,18 @@ private class ForLoopsTransformer(val context: Context) : IrElementTransformerVo private data class ProgressionInfo( val progressionType: ProgressionType, val first: IrExpression, - val last: IrExpression, + val bound: IrExpression, val step: IrExpression? = null, val increasing: Boolean = true, - var needBoundCalculation: Boolean = false) + var needLastCalculation: Boolean = false, + val closed: Boolean = true) /** Contains information about variables used in the loop. */ private data class ForLoopInfo( val progressionInfo: ProgressionInfo, val inductionVariable: IrVariableSymbol, val bound: IrVariableSymbol, + val last: IrVariableSymbol, val step: IrVariableSymbol, var loopVariable: IrVariableSymbol? = null) @@ -233,17 +233,15 @@ private class ForLoopsTransformer(val context: Context) : IrElementTransformerVo val CHAR_PROGRESSION = ProgressionType(context.builtIns.charType, Name.identifier("toChar")) private fun buildRangeTo(expression: IrCall, progressionType: ProgressionType) = - ProgressionInfo(progressionType, expression.dispatchReceiver!!, expression.getValueArgument(0)!!) + ProgressionInfo(progressionType, + expression.dispatchReceiver!!, + expression.getValueArgument(0)!!) - private fun buildUntil(expression: IrCall, progressionType: ProgressionType): ProgressionInfo { - val firstExpression = expression.extensionReceiver!! - val lastExpression = expression.getValueArgument(0)!! - val decrementSymbol = symbols.getUnaryOperator(OperatorNameConventions.DEC, lastExpression.type) - val decrementCall = IrCallImpl(expression.startOffset, expression.endOffset, decrementSymbol).apply { - dispatchReceiver = lastExpression - } - return ProgressionInfo(progressionType, firstExpression, decrementCall) - } + private fun buildUntil(expression: IrCall, progressionType: ProgressionType): ProgressionInfo = + ProgressionInfo(progressionType, + expression.extensionReceiver!!, + expression.getValueArgument(0)!!, + closed = false) private fun buildDownTo(expression: IrCall, progressionType: ProgressionType) = ProgressionInfo(progressionType, expression.extensionReceiver!!, expression.getValueArgument(0)!!, increasing = false) @@ -264,7 +262,7 @@ private class ForLoopsTransformer(val context: Context) : IrElementTransformerVo statements.add(newStepCheck) } } - ProgressionInfo(progressionType, it.first, it.last, step, it.increasing, needBoundCalculation) + ProgressionInfo(progressionType, it.first, it.bound, step, it.increasing, needBoundCalculation, it.closed) } override fun visitElement(element: IrElement, data: Nothing?): ProgressionInfo? = null @@ -307,28 +305,58 @@ private class ForLoopsTransformer(val context: Context) : IrElementTransformerVo with(progressionInfo) { val statements = mutableListOf() - + /** + * 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 necessary). + */ val inductionVariable = scope.createTemporaryVariable(first.castIfNecessary(progressionType), "inductionVariable", - true) - statements.add(inductionVariable) + true).also { + statements.add(it) + } + + val boundValue = scope.createTemporaryVariable(bound.castIfNecessary(progressionType),"bound") + .also { statements.add(it) } + val stepExpression = (if (increasing) step else step?.unaryMinus()) ?: defaultStep(startOffset, endOffset) - val stepValue = scope.createTemporaryVariable(stepExpression, "step") - statements.add(stepValue) - - // Don't call progression bound calculation if the step is 1. - val boundExpression = if (needBoundCalculation) { - irGetProgressionBound(progressionType, inductionVariable.symbol, last, stepValue.symbol) - } else { - last.castIfNecessary(progressionType) + val stepValue = scope.createTemporaryVariable(stepExpression, "step").also { + statements.add(it) + } + + // 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. + var lastExpression: IrExpression? = null + if (!closed) { + val decrementSymbol = symbols.getUnaryOperator(OperatorNameConventions.DEC, boundValue.descriptor.type) + lastExpression = irCall(decrementSymbol).apply { + dispatchReceiver = irGet(boundValue.symbol) + } + } + if (needLastCalculation) { + lastExpression = irGetProgressionLast(progressionType, + inductionVariable.symbol, + lastExpression ?: irGet(boundValue.symbol), + stepValue.symbol) + } + val lastValue = if (lastExpression != null) { + scope.createTemporaryVariable(lastExpression, "last").also { + statements.add(it) + } + } else { + boundValue } - val boundValue = scope.createTemporaryVariable(boundExpression, "bound") - statements.add(boundValue) iteratorToLoopInfo[symbol] = ForLoopInfo(progressionInfo, inductionVariable.symbol, boundValue.symbol, + lastValue.symbol, stepValue.symbol) return IrCompositeImpl(startOffset, endOffset, context.builtIns.unitType, null, statements) @@ -362,18 +390,49 @@ private class ForLoopsTransformer(val context: Context) : IrElementTransformerVo } } + private fun DeclarationIrBuilder.buildMinValueCondition(forLoopInfo: ForLoopInfo): IrExpression { + // Condition for a corner case: for (i in a until Int.MIN_VALUE) {}. + // Check if forLoopInfo.bound > MIN_VALUE. + val progressionType = forLoopInfo.progressionInfo.progressionType + return irCall(context.irBuiltIns.gt0Symbol).apply { + val minConst = when { + progressionType.isIntProgression() -> IrConstImpl.int(startOffset, endOffset, context.builtIns.intType, Int.MIN_VALUE) + progressionType.isCharProgression() -> IrConstImpl.char(startOffset, endOffset, context.builtIns.charType, 0.toChar()) + progressionType.isLongProgression() -> IrConstImpl.long(startOffset, endOffset, context.builtIns.longType, Long.MIN_VALUE) + else -> throw IllegalArgumentException("Unknown progression type") + } + val compareToCall = irCall(symbols.getBinaryOperator(OperatorNameConventions.COMPARE_TO, + forLoopInfo.bound.descriptor.type, + minConst.type)).apply { + dispatchReceiver = irGet(forLoopInfo.bound) + putValueArgument(0, minConst) + } + putValueArgument(0, compareToCall) + } + } + + // TODO: Eliminate the loop if we can prove that it will not be executed. private fun DeclarationIrBuilder.buildEmptyCheck(loop: IrLoop, forLoopInfo: ForLoopInfo): IrExpression { val increasing = forLoopInfo.progressionInfo.increasing val comparingBuiltIn = if (increasing) context.irBuiltIns.lteq0Symbol else context.irBuiltIns.gteq0Symbol + // Check if inductionVariable <= last. val compareTo = symbols.getBinaryOperator(OperatorNameConventions.COMPARE_TO, forLoopInfo.inductionVariable.descriptor.type, - forLoopInfo.bound.descriptor.type) + forLoopInfo.last.descriptor.type) - val check = irCall(comparingBuiltIn).apply { - putValueArgument(0, irCallOp(compareTo, irGet(forLoopInfo.inductionVariable), irGet(forLoopInfo.bound))) + val check: IrExpression = irCall(comparingBuiltIn).apply { + putValueArgument(0, irCallOp(compareTo, irGet(forLoopInfo.inductionVariable), irGet(forLoopInfo.last))) + } + + // Process closed and opened ranges in different manners. + return if (forLoopInfo.progressionInfo.closed) { + irIfThen(check, loop) // if (inductionVariable <= last) { loop } + } else { + // Take into account a corner case: for (i in a until Int.MIN_VALUE) {}. + // if (inductionVariable <= last && bound > MIN_VALUE) { loop } + return irIfThen(check, irIfThen(buildMinValueCondition(forLoopInfo), loop)) } - return irIfThen(check, loop) } private fun DeclarationIrBuilder.buildNewCondition(oldCondition: IrExpression): Pair? { @@ -389,7 +448,7 @@ private class ForLoopsTransformer(val context: Context) : IrElementTransformerVo return irCall(context.irBuiltIns.booleanNotSymbol).apply { val eqeqCall = irCall(context.irBuiltIns.eqeqSymbol).apply { putValueArgument(0, irGet(forLoopInfo.loopVariable!!)) - putValueArgument(1, irGet(forLoopInfo.bound)) + putValueArgument(1, irGet(forLoopInfo.last)) } putValueArgument(0, eqeqCall) } to forLoopInfo @@ -418,6 +477,7 @@ private class ForLoopsTransformer(val context: Context) : IrElementTransformerVo * } while (i != last) * } */ + // TODO: Lower `for (i in a until b)` to loop with precondition: for (i = a; i < b; a++); override fun visitWhileLoop(loop: IrWhileLoop): IrExpression { if (loop.origin != IrStatementOrigin.FOR_LOOP_INNER_WHILE) { return super.visitWhileLoop(loop) diff --git a/backend.native/tests/build.gradle b/backend.native/tests/build.gradle index 94d0e738fa0..ac401ae0e01 100644 --- a/backend.native/tests/build.gradle +++ b/backend.native/tests/build.gradle @@ -357,6 +357,11 @@ task codegen_controlflow_for_loops_coroutines(type: RunKonanTest) { "Got: 0 2 4 6\n" } +task codegen_controlflow_for_loops_call_order(type: RunKonanTest) { + source = "codegen/controlflow/for_loops_call_order.kt" + goldValue = "1234\n1234\n2134\n" +} + task local_variable(type: RunKonanTest) { source = "codegen/basics/local_variable.kt" } diff --git a/backend.native/tests/codegen/controlflow/for_loops_call_order.kt b/backend.native/tests/codegen/controlflow/for_loops_call_order.kt new file mode 100644 index 00000000000..12f0a6b07c7 --- /dev/null +++ b/backend.native/tests/codegen/controlflow/for_loops_call_order.kt @@ -0,0 +1,10 @@ +fun f1(): Int { print("1"); return 0 } +fun f2(): Int { print("2"); return 6 } +fun f3(): Int { print("3"); return 2 } +fun f4(): Int { print("4"); return 3 } + +fun main(args: Array) { + for (i in f1()..f2() step f3() step f4()) { }; println() + for (i in f1() until f2() step f3() step f4()) {}; println() + for (i in f2() downTo f1() step f3() step f4()) {}; println() +} \ No newline at end of file diff --git a/backend.native/tests/codegen/controlflow/for_loops_overflow.kt b/backend.native/tests/codegen/controlflow/for_loops_overflow.kt index 69bfde26aa6..688a978a11a 100644 --- a/backend.native/tests/codegen/controlflow/for_loops_overflow.kt +++ b/backend.native/tests/codegen/controlflow/for_loops_overflow.kt @@ -3,6 +3,20 @@ fun main(args: Array) { for (i in Int.MAX_VALUE - 1 until Int.MAX_VALUE) { print(i); print(' ') }; println() for (i in Int.MIN_VALUE + 1 downTo Int.MIN_VALUE) { print(i); print(' ') }; println() + // Empty loops + for (i in Byte.MIN_VALUE until Byte.MIN_VALUE) { print(i); print(' ') } + for (i in Short.MIN_VALUE until Short.MIN_VALUE) { print(i); print(' ') } + for (i in Int.MIN_VALUE until Int.MIN_VALUE) { print(i); print(' ') } + for (i in Long.MIN_VALUE until Long.MIN_VALUE) { print(i); print(' ') } + for (i in 0.toChar() until 0.toChar()) { print(i); print(' ') } + + for (i in 0 until Byte.MIN_VALUE) { print(i); print(' ') } + for (i in 0 until Short.MIN_VALUE) { print(i); print(' ') } + for (i in 0 until Int.MIN_VALUE) { print(i); print(' ') } + for (i in 0 until Long.MIN_VALUE) { print(i); print(' ') } + for (i in 'a' until 0.toChar()) { print(i); print(' ') } + + val M = Int.MAX_VALUE / 2 for (i in M + 4..M + 10 step M) { print(i); print(' ') }; println() } \ No newline at end of file diff --git a/runtime/src/main/kotlin/konan/internal/RuntimeUtils.kt b/runtime/src/main/kotlin/konan/internal/RuntimeUtils.kt index 54ccefd9871..a59bce9a82f 100644 --- a/runtime/src/main/kotlin/konan/internal/RuntimeUtils.kt +++ b/runtime/src/main/kotlin/konan/internal/RuntimeUtils.kt @@ -91,8 +91,8 @@ fun > valuesForEnum(values: Array): Array fun checkProgressionStep(step: Int) = if (step > 0) step else throw IllegalArgumentException("Step must be positive, was: $step.") fun checkProgressionStep(step: Long) = if (step > 0) step else throw IllegalArgumentException("Step must be positive, was: $step.") -fun getProgressionBound(start: Char, end: Char, step: Int): Char = - getProgressionBound(start.toInt(), end.toInt(), step).toChar() +fun getProgressionLast(start: Char, end: Char, step: Int): Char = + getProgressionLast(start.toInt(), end.toInt(), step).toChar() -fun getProgressionBound(start: Int, end: Int, step: Int): Int = getProgressionLastElement(start, end, step) -fun getProgressionBound(start: Long, end: Long, step: Long): Long = getProgressionLastElement(start, end, step) \ No newline at end of file +fun getProgressionLast(start: Int, end: Int, step: Int): Int = getProgressionLastElement(start, end, step) +fun getProgressionLast(start: Long, end: Long, step: Long): Long = getProgressionLastElement(start, end, step) \ No newline at end of file