From f6e74079bbabf30aaa2274cdc6154ee1dbedbcd8 Mon Sep 17 00:00:00 2001 From: Mark Punzalan Date: Wed, 20 Mar 2019 00:23:46 -0700 Subject: [PATCH] Cleanup ForLoopsLowering: Add more comments, remove descriptor usages, make code style consistent. --- .../common/lower/loops/ForLoopsLowering.kt | 153 +++++++++++------- .../backend/common/lower/loops/HeaderInfo.kt | 18 ++- .../common/lower/loops/HeaderProcessor.kt | 39 ++++- .../common/lower/loops/ProgressionHandlers.kt | 8 +- 4 files changed, 149 insertions(+), 69 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 88c119d7588..52f41b23a47 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 @@ -20,7 +20,9 @@ import org.jetbrains.kotlin.ir.declarations.IrVariable import org.jetbrains.kotlin.ir.expressions.* import org.jetbrains.kotlin.ir.expressions.impl.IrCompositeImpl import org.jetbrains.kotlin.ir.symbols.IrVariableSymbol +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.visitors.IrElementTransformerVoid import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid import org.jetbrains.kotlin.util.OperatorNameConventions @@ -34,8 +36,46 @@ val forLoopsPhase = makeIrFilePhase( /** * This lowering pass optimizes for-loops. * - * Replace iteration over ranges (X.indices, a..b, etc.) and arrays with - * simple while loop over primitive induction variable. + * Replace iteration over progressions (e.g., X.indices, a..b) and arrays with + * a simple while loop over primitive induction variable. + * + * For example, this loop: + * + * ``` + * for (i in first..last step foo) { ... } + * ``` + * + * is represented in IR in such a manner: + * + * ``` + * val it = (first..last step foo).iterator() + * while (it.hasNext()) { + * val i = it.next() + * ... + * } + * ``` + * + * We transform it into the following loop: + * + * ``` + * var it = first + * if (it <= last) { // (it >= last if the progression is decreasing) + * do { + * val i = it++ + * ... + * } while (i != last) + * } + * ``` + * + * In case of iteration over array we transform it into following: + * + * ``` + * while (it <= array.size - 1) { + * val i = array[it] + * it++ + * ... + * } + * ``` */ internal class ForLoopsLowering(val context: CommonBackendContext) : FileLoweringPass { @@ -45,6 +85,7 @@ internal class ForLoopsLowering(val context: CommonBackendContext) : FileLowerin val oldLoopToNewLoop = mutableMapOf() val transformer = RangeLoopTransformer(context, oldLoopToNewLoop, headerInfoBuilder) irFile.transformChildrenVoid(transformer) + // Update references in break/continue. irFile.transformChildrenVoid(object : IrElementTransformerVoid() { override fun visitBreakContinue(jump: IrBreakContinue): IrExpression { @@ -81,11 +122,21 @@ private class RangeLoopTransformer( } ?: super.visitVariable(declaration) } + /** + * Lowers the "header" statement that stores the iterator into the loop variable + * (e.g., `val it = someIterable.iterator()`) and gather information for building the for-loop + * (as a [ForLoopHeader]). + * + * Returns null if the for-loop cannot be lowered. + */ private fun processHeader(variable: IrVariable): IrStatement? { assert(variable.symbol !in iteratorToLoopHeader) val forLoopInfo = headerProcessor.processHeader(variable) - ?: return null + ?: return null // If the for-loop cannot be lowered. iteratorToLoopHeader[variable.symbol] = forLoopInfo + + // Lower into a composite with additional declarations (e.g., induction variable) + // used in the loop condition and body. return IrCompositeImpl( variable.startOffset, variable.endOffset, @@ -95,43 +146,15 @@ private class RangeLoopTransformer( ) } - /** - * This loop - * - * for (i in first..last step foo) { ... } - * - * is represented in IR in such a manner: - * - * val it = (first..last step foo).iterator() - * while (it.hasNext()) { - * val i = it.next() - * ... - * } - * - * We transform it into the following loop: - * - * var it = first - * if (it <= last) { // (it >= last if the progression is decreasing) - * do { - * val i = it++ - * ... - * } while (i != last) - * } - * - * In case of iteration over array we transform it into following: - * while (i <= array.size - 1) { - * val element = array[i] - * i++ - * ... - * } - */ - // 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) } with(context.createIrBuilder(getScopeOwnerSymbol(), loop.startOffset, loop.endOffset)) { + // Visit the loop body to process the "next" statement and lower nested loops. + // Processing the "next" statement is necessary for building loops that need to + // reference the loop variable in the loop condition. val newBody = loop.body?.transform(this@RangeLoopTransformer, null)?.let { if (it is IrContainerExpression && !it.isTransparentScope) { IrCompositeImpl(startOffset, endOffset, it.type, it.origin, it.statements) @@ -139,9 +162,12 @@ private class RangeLoopTransformer( it } } + val loopHeader = getLoopHeader(loop.condition) - ?: return super.visitWhileLoop(loop) + ?: return super.visitWhileLoop(loop) // If the for-loop cannot be lowered. val newLoop = loopHeader.buildInnerLoop(this, loop, newBody) + + // Update mapping from old to new loop so we can later update references in break/continue. oldLoopToNewLoop[loop] = newLoop // Surround the new loop with a check for an empty loop, if necessary. @@ -154,36 +180,55 @@ private class RangeLoopTransformer( } } - private fun getLoopHeader(oldCondition: IrExpression): ForLoopHeader? { - if (oldCondition !is IrCall || oldCondition.origin != IrStatementOrigin.FOR_LOOP_HAS_NEXT) { + private fun getLoopHeader(expression: IrExpression): ForLoopHeader? { + if (expression !is IrCall + || (expression.origin != IrStatementOrigin.FOR_LOOP_HAS_NEXT + && expression.origin != IrStatementOrigin.FOR_LOOP_NEXT) + ) { return null } - val irIteratorAccess = oldCondition.dispatchReceiver as? IrGetValue - ?: throw AssertionError() + val iterator = expression.dispatchReceiver as IrGetValue + // Return null if we didn't lower the corresponding header. - return iteratorToLoopHeader[irIteratorAccess.symbol] + return iteratorToLoopHeader[iterator.symbol] } - // Lower getting a next induction variable value. - fun processNext(variable: IrVariable): IrExpression? { + /** + * Lowers the "next" statement that stores the next element in the iterable into the + * loop variable, e.g., `val i = it.next()`. + * + * Returns null if there was no stored [ForLoopHeader] corresponding to the given "next" + * statement. + */ + private fun processNext(variable: IrVariable): IrExpression? { val initializer = variable.initializer as IrCall - - val iterator = initializer.dispatchReceiver as? IrGetValue - ?: throw AssertionError() - val forLoopInfo = iteratorToLoopHeader[iterator.symbol] - ?: return null // If we didn't lower a corresponding header. - // TODO: Use PLUS_ASSIGN to increment - val plusOperator = symbols.getBinaryOperator( - OperatorNameConventions.PLUS, - forLoopInfo.inductionVariable.type.toKotlinType(), - forLoopInfo.step.type.toKotlinType() - ) + val forLoopInfo = getLoopHeader(initializer) + ?: return null // If the for-loop cannot be lowered. forLoopInfo.loopVariable = variable + + // The "next" statement (at the top of the loop): + // + // ``` + // val i = it.next() + // ``` + // + // ...is lowered into: + // + // ``` + // val i = initialValue() // `inductionVariable` for progressions + // // `array[inductionVariable]` for arrays + // inductionVariable = inductionVariable + step + // ``` return with(context.createIrBuilder(getScopeOwnerSymbol(), initializer.startOffset, initializer.endOffset)) { variable.initializer = forLoopInfo.initializeLoopVariable(symbols, this) + val plusFun = forLoopInfo.inductionVariable.type.getClass()!!.functions.first { + it.name.asString() == "plus" && + it.valueParameters.size == 1 && + it.valueParameters[0].type.toKotlinType() == forLoopInfo.step.type.toKotlinType() + } val increment = irSetVar( forLoopInfo.inductionVariable.symbol, irCallOp( - plusOperator, plusOperator.owner.returnType, + plusFun.symbol, plusFun.returnType, irGet(forLoopInfo.inductionVariable), irGet(forLoopInfo.step) ) 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 c2dcd6199e4..257cd9bf839 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 @@ -22,9 +22,10 @@ import org.jetbrains.kotlin.utils.addToStdlib.firstNotNullResult // TODO: Handle withIndex() // TODO: Handle reversed() -// TODO: Handle direct iteration on Ranges/Progressions (e.g., NOT using rangeTo or step) // TODO: Handle Strings/CharSequences +// TODO: Handle UIntProgression, ULongProgression +/** Represents a progression type in the Kotlin stdlib. */ enum class ProgressionType(val numberCastFunctionName: Name) { INT_PROGRESSION(Name.identifier("toInt")), LONG_PROGRESSION(Name.identifier("toLong")), @@ -55,7 +56,7 @@ enum class ProgressionType(val numberCastFunctionName: Name) { internal enum class ProgressionDirection { DECREASING, INCREASING, UNKNOWN } -// Information about loop that is required by HeaderProcessor to build ForLoopHeader +/** Information about a loop that is required by [HeaderProcessor] to build a [ForLoopHeader]. */ internal sealed class HeaderInfo( val progressionType: ProgressionType, val first: IrExpression, @@ -75,6 +76,7 @@ internal sealed class HeaderInfo( } } +/** Information about a for-loop over a progression. */ internal class ProgressionHeaderInfo( progressionType: ProgressionType, first: IrExpression, @@ -84,6 +86,7 @@ internal class ProgressionHeaderInfo( val additionalNotEmptyCondition: IrExpression? = null ) : HeaderInfo(progressionType, first, last, step) +/** Information about a for-loop over an array. The internal induction variable used is an Int. */ internal class ArrayHeaderInfo( first: IrExpression, last: IrExpression, @@ -96,6 +99,7 @@ internal class ArrayHeaderInfo( step ) +/** Matches a call to `iterator()` and builds a [HeaderInfo] out of the call's context. */ internal interface HeaderInfoHandler { val matcher: IrCallMatcher @@ -109,13 +113,16 @@ internal interface HeaderInfoHandler { } internal typealias ProgressionHandler = HeaderInfoHandler -// We need to wrap builder into visitor because of `StepHandler` which has to visit its subtree -// to get information about underlying progression. +/** + * Handles a call to `iterator()` on more specialized forms of progressions, built using extension + * and member functions/properties in the stdlib (e.g., `.indices`, `downTo`). + */ private class ProgressionHeaderInfoBuilder(val context: CommonBackendContext) : IrElementVisitor { private val symbols = context.ir.symbols - private val progressionElementTypes = symbols.integerClassesTypes + symbols.char.descriptor.defaultType + // TODO: Include unsigned types + private val progressionElementTypes = symbols.integerClassesTypes + context.irBuiltIns.char private val progressionHandlers = listOf( IndicesHandler(context), @@ -127,6 +134,7 @@ private class ProgressionHeaderInfoBuilder(val context: CommonBackendContext) : override fun visitElement(element: IrElement, data: Nothing?): HeaderInfo? = null override fun visitCall(expression: IrCall, data: Nothing?): HeaderInfo? { + // Return the HeaderInfo from the first successful match. val progressionType = ProgressionType.fromIrType(expression.type, symbols) ?: return null return progressionHandlers.firstNotNullResult { it.handle(expression, progressionType) } 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 fff0ec30b70..6cd876484ab 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 @@ -18,7 +18,6 @@ import org.jetbrains.kotlin.ir.expressions.IrLoop import org.jetbrains.kotlin.ir.expressions.impl.IrCallImpl import org.jetbrains.kotlin.ir.expressions.impl.IrDoWhileLoopImpl import org.jetbrains.kotlin.ir.expressions.impl.IrWhileLoopImpl -import org.jetbrains.kotlin.ir.symbols.IrClassSymbol import org.jetbrains.kotlin.ir.symbols.IrSymbol import org.jetbrains.kotlin.ir.types.* import org.jetbrains.kotlin.ir.util.functions @@ -33,12 +32,21 @@ internal sealed class ForLoopHeader( val needsEmptinessCheck: Boolean, var loopVariable: IrVariable? = null ) { + /** Expression used to initialize the loop variable at the beginning of the loop. */ abstract fun initializeLoopVariable(symbols: Symbols, builder: DeclarationIrBuilder): IrExpression + /** Declarations used in the loop condition and body (e.g., induction variable). */ abstract val declarations: List + /** Builds a new loop from the old loop. */ abstract fun buildInnerLoop(builder: DeclarationIrBuilder, loop: IrLoop, newBody: IrExpression?): IrLoop + /** + * A condition used in a check surrounding the loop checking for an empty loop. The expression + * should evaluate to true if the loop is NOT empty. + * + * Returns null if no check is needed for the for-loop. + */ abstract fun buildNotEmptyCondition(builder: DeclarationIrBuilder): IrExpression? } @@ -50,6 +58,7 @@ internal class ProgressionLoopHeader( ) : ForLoopHeader(inductionVariable, last, step, headerInfo.progressionType, needsEmptinessCheck = true) { override fun initializeLoopVariable(symbols: Symbols, builder: DeclarationIrBuilder) = with(builder) { + // loopVariable = inductionVariable irGet(inductionVariable) } @@ -57,6 +66,7 @@ internal class ProgressionLoopHeader( get() = headerInfo.additionalVariables + listOf(inductionVariable, step, last) 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 { @@ -64,6 +74,8 @@ internal class ProgressionLoopHeader( 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 { label = loop.label condition = newCondition @@ -133,8 +145,8 @@ internal class ArrayLoopHeader( ) : 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 - val arrayGetFun = arrayClass.owner.functions.find { it.name.toString() == "get" }!! + // loopVariable = array[inductionVariable] + val arrayGetFun = headerInfo.arrayVariable.type.getClass()!!.functions.first { it.name.asString() == "get" } irCall(arrayGetFun).apply { dispatchReceiver = irGet(headerInfo.arrayVariable) putValueArgument(0, irGet(inductionVariable)) @@ -145,12 +157,14 @@ internal class ArrayLoopHeader( get() = listOf(headerInfo.arrayVariable, inductionVariable, step, last) override fun buildInnerLoop(builder: DeclarationIrBuilder, loop: IrLoop, newBody: IrExpression?): IrLoop = with(builder) { + // Condition: loopVariable != last val builtIns = context.irBuiltIns - val callee = builtIns.lessOrEqualFunByOperandType[builtIns.int]?.symbol!! + val callee = builtIns.lessOrEqualFunByOperandType[builtIns.int]!! val newCondition = irCall(callee).apply { putValueArgument(0, irGet(inductionVariable)) putValueArgument(1, irGet(last)) } + IrWhileLoopImpl(loop.startOffset, loop.endOffset, loop.type, loop.origin).apply { label = loop.label condition = newCondition @@ -162,8 +176,10 @@ internal class ArrayLoopHeader( override fun buildNotEmptyCondition(builder: DeclarationIrBuilder): IrExpression? = null } -// Given the for loop iterator variable, extract information about iterable subject -// and create ForLoopHeader from it. +/** + * Given the for-loop iterator variable, extract information about the iterable subject + * and create a [ForLoopHeader] from it. + */ internal class HeaderProcessor( private val context: CommonBackendContext, private val headerInfoBuilder: HeaderInfoBuilder, @@ -172,8 +188,15 @@ internal class HeaderProcessor( private val symbols = context.ir.symbols + /** + * Extracts information for building the for-loop (as a [ForLoopHeader]) from the given + * "header" statement that stores the iterator into the loop variable + * (e.g., `val it = someIterable.iterator()`). + * + * Returns null if the for-loop cannot be lowered. + */ fun processHeader(variable: IrVariable): ForLoopHeader? { - + // Verify the variable type is a subtype of Iterator<*>. assert(variable.origin == IrDeclarationOrigin.FOR_LOOP_ITERATOR) if (!variable.type.isSubtypeOfClass(symbols.iterator)) { return null @@ -181,7 +204,7 @@ internal class HeaderProcessor( // Collect loop information. val headerInfo = headerInfoBuilder.build(variable) - ?: return null // If the iterable is not supported. + ?: return null // If the iterable is not supported. val builder = context.createIrBuilder(scopeOwnerSymbol(), variable.startOffset, variable.endOffset) with(builder) builder@{ 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 5cd1b1c6b26..d6e8775f4b3 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 @@ -27,7 +27,7 @@ import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.types.SimpleType -// TODO: What if functions like Int.rangeTo(Char) will be added to stdlib later? +/** Builds a [HeaderInfo] for progressions built using the `rangeTo` function. */ internal class RangeToHandler(private val context: CommonBackendContext, private val progressionElementTypes: Collection) : ProgressionHandler { override val matcher = SimpleCalleeMatcher { @@ -48,6 +48,7 @@ internal class RangeToHandler(private val context: CommonBackendContext, private } } +/** Builds a [HeaderInfo] for progressions built using the `downTo` extension function. */ internal class DownToHandler(private val context: CommonBackendContext, private val progressionElementTypes: Collection) : ProgressionHandler { override val matcher = SimpleCalleeMatcher { @@ -67,6 +68,7 @@ internal class DownToHandler(private val context: CommonBackendContext, private } } +/** Builds a [HeaderInfo] for progressions built using the `until` extension function. */ internal class UntilHandler(private val context: CommonBackendContext, private val progressionElementTypes: Collection) : ProgressionHandler { override val matcher = SimpleCalleeMatcher { @@ -133,6 +135,7 @@ internal class UntilHandler(private val context: CommonBackendContext, private v } } +/** Builds a [HeaderInfo] for progressions built using the `indices` extension property. */ internal class IndicesHandler(val context: CommonBackendContext) : ProgressionHandler { override val matcher = SimpleCalleeMatcher { @@ -171,7 +174,7 @@ internal class DefaultProgressionHandler(private val context: CommonBackendConte dispatchReceiver { it != null && ProgressionType.fromIrType(it.type, symbols) != null } } - override fun build(call: IrCall, progressionType: Nothing?): HeaderInfo? = + override fun build(call: IrCall, data: Nothing?): HeaderInfo? = with(context.createIrBuilder(call.symbol, call.startOffset, call.endOffset)) { // Directly use the `first/last/step` properties of the progression. val progression = scope.createTemporaryVariable(call.dispatchReceiver!!) @@ -199,6 +202,7 @@ internal class DefaultProgressionHandler(private val context: CommonBackendConte } } +/** Builds a [HeaderInfo] for arrays. */ internal class ArrayIterationHandler(private val context: CommonBackendContext) : HeaderInfoHandler { private val intDecFun = ProgressionType.INT_PROGRESSION.decFun(context.irBuiltIns)