Changed an order of processing loops in ForLoopsLowering

2021-06-22 12:57:13 +03:00
parent 0e04c21625
commit 9503627864
11 changed files with 122 additions and 132 deletions
@@ -10,7 +10,7 @@ import org.jetbrains.kotlin.backend.common.lower.loops.ForLoopsLowering
 import org.jetbrains.kotlin.backend.common.lower.optimizations.FoldConstantLowering
 import org.jetbrains.kotlin.backend.common.lower.optimizations.PropertyAccessorInlineLowering
 import org.jetbrains.kotlin.backend.common.phaser.*
-import org.jetbrains.kotlin.backend.konan.ir.FunctionsWithoutBCGenerator
+import org.jetbrains.kotlin.backend.konan.ir.FunctionsWithoutBoundCheckGenerator
 import org.jetbrains.kotlin.backend.konan.lower.*
 import org.jetbrains.kotlin.backend.konan.lower.FinallyBlocksLowering
 import org.jetbrains.kotlin.backend.konan.lower.InitializersLowering
@@ -258,12 +258,21 @@ internal val rangeContainsLoweringPhase = makeKonanFileLoweringPhase(
        description = "Optimizes calls to contains() for ClosedRanges"
 )

+internal val functionsWithoutBoundCheck = makeKonanModuleOpPhase(
+        name = "FunctionsWithoutBoundCheckGenerator",
+        description = "Functions without bounds check generation",
+        op = { context, _ ->
+            FunctionsWithoutBoundCheckGenerator(context).generate()
+        }
+)
+
 internal val forLoopsPhase = makeKonanFileOpPhase(
        { context, irFile ->
            ForLoopsLowering(context, KonanBCEForLoopBodyTransformer()).lower(irFile)
        },
        name = "ForLoops",
-        description = "For loops lowering"
+        description = "For loops lowering",
+        prerequisite = setOf(functionsWithoutBoundCheck)
 )

 internal val dataClassesPhase = makeKonanFileLoweringPhase(
@@ -341,19 +350,11 @@ internal val interopPhase = makeKonanFileLoweringPhase(
        prerequisite = setOf(inlinePhase, localFunctionsPhase, functionReferencePhase)
 )

-internal val functionsWithoutBC = makeKonanModuleOpPhase(
-        name = "FunctionsWithoutBCGenerator",
-        description = "Functions without bounds check generation",
-        op = { context, _ ->
-            FunctionsWithoutBCGenerator(context).generate()
-        }
-)
-
 internal val varargPhase = makeKonanFileLoweringPhase(
        ::VarargInjectionLowering,
        name = "Vararg",
        description = "Vararg lowering",
-        prerequisite = setOf(functionReferencePhase, defaultParameterExtentPhase, interopPhase, functionsWithoutBC)
+        prerequisite = setOf(functionReferencePhase, defaultParameterExtentPhase, interopPhase, functionsWithoutBoundCheck)
 )

 internal val compileTimeEvaluatePhase = makeKonanFileLoweringPhase(
@@ -393,7 +393,7 @@ private val backendCodegen = namedUnitPhase(
        name = "Backend codegen",
        description = "Backend code generation",
        lower = takeFromContext<Context, Unit, IrModuleFragment> { it.irModule!! } then
-                functionsWithoutBC then
+                functionsWithoutBoundCheck then
                allLoweringsPhase then // Lower current module first.
                dependenciesLowerPhase then // Then lower all libraries in topological order.
                                            // With that we guarantee that inline functions are unlowered while being inlined.
@@ -27,7 +27,7 @@ import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.util.OperatorNameConventions

 // Generate additional functions for array set and get operators without bounds checking.
-internal class FunctionsWithoutBCGenerator(val context: KonanBackendContext) {
+internal class FunctionsWithoutBoundCheckGenerator(val context: KonanBackendContext) {
    private val symbols = context.ir.symbols

    private fun generateFunction(baseFunction: IrSimpleFunction, functionName: Name) =
@@ -55,7 +55,7 @@ internal class FunctionsWithoutBCGenerator(val context: KonanBackendContext) {
                val setWithoutBEAnnotations = baseFunction.annotations.map { annotation ->
                    annotation.deepCopyWithSymbols().also { copy ->
                        if (copy.isAnnotationWithEqualFqName(KonanFqNames.gcUnsafeCall)) {
-                            val value = "${annotation.getAnnotationStringValue("callee")}_without_BC"
+                            val value = "${annotation.getAnnotationStringValue("callee")}_without_BoundCheck"
                            copy.putValueArgument(0,
                                    IrConstImpl.string(UNDEFINED_OFFSET, UNDEFINED_OFFSET, context.irBuiltIns.stringType, value))
                        }
@@ -69,10 +69,10 @@ internal class FunctionsWithoutBCGenerator(val context: KonanBackendContext) {
        val arraysClasses = symbols.primitiveArrays.values + symbols.array
        arraysClasses.forEach { classSymbol ->
            val setFunction = classSymbol.owner.functions.single { it.name == OperatorNameConventions.SET }
-            classSymbol.owner.addMember(generateFunction(setFunction, KonanNameConventions.setWithoutBC))
+            classSymbol.owner.addMember(generateFunction(setFunction, KonanNameConventions.setWithoutBoundCheck))

            val getFunction = classSymbol.owner.functions.single { it.descriptor.name == OperatorNameConventions.GET }
-            classSymbol.owner.addMember(generateFunction(getFunction, KonanNameConventions.getWithoutBC))
+            classSymbol.owner.addMember(generateFunction(getFunction, KonanNameConventions.getWithoutBoundCheck))
        }
    }
 }
@@ -43,8 +43,8 @@ import org.jetbrains.kotlin.types.Variance
 import kotlin.properties.Delegates

 object KonanNameConventions {
-    val setWithoutBC = Name.special("<setWithoutBC>")
-    val getWithoutBC = Name.special("<getWithoutBC>")
+    val setWithoutBoundCheck = Name.special("<setWithoutBoundCheck>")
+    val getWithoutBoundCheck = Name.special("<getWithoutBoundCheck>")
 }

 // This is what Context collects about IR.
@@ -575,9 +575,9 @@ internal class KonanSymbols(
    val topLevelSuite    = getKonanTestClass("TopLevelSuite")
    val testFunctionKind = getKonanTestClass("TestFunctionKind")

-    override val getWithoutBCName: Name? = KonanNameConventions.getWithoutBC
+    override val getWithoutBoundCheckName: Name? = KonanNameConventions.getWithoutBoundCheck

-    override val setWithoutBCName: Name? = KonanNameConventions.setWithoutBC
+    override val setWithoutBoundCheckName: Name? = KonanNameConventions.setWithoutBoundCheck

    private val testFunctionKindCache = TestProcessor.FunctionKind.values().associate {
        val symbol = if (it.runtimeKindString.isEmpty())
@@ -208,7 +208,7 @@ internal class VarargInjectionLowering constructor(val context: KonanBackendCont
    abstract inner class ArrayHandle(val arraySymbol: IrClassSymbol) {
        val setMethodSymbol = with(arraySymbol.owner.functions) {
            // For unsigned types use set method.
-            singleOrNull { it.name == KonanNameConventions.setWithoutBC } ?: single { it.name == OperatorNameConventions.SET }
+            singleOrNull { it.name == KonanNameConventions.setWithoutBoundCheck } ?: single { it.name == OperatorNameConventions.SET }
        }
        val sizeGetterSymbol = arraySymbol.getPropertyGetter("size")!!
        val copyIntoSymbol = symbols.copyInto[arraySymbol.descriptor]!!
@@ -21,6 +21,7 @@ import org.jetbrains.kotlin.ir.util.IdSignature
 import org.jetbrains.kotlin.ir.util.functions
 import org.jetbrains.kotlin.ir.util.getPropertyGetter
 import org.jetbrains.kotlin.ir.util.isPrimitiveArray
+import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid
 import org.jetbrains.kotlin.util.OperatorNameConventions

 // Class contains information about analyzed loop.
@@ -30,16 +31,22 @@ internal data class BoundsCheckAnalysisResult(val boundsAreSafe: Boolean, val ar
 * Transformer for for loops bodies replacing get/set operators on analogs without bounds check where it's possible.
 */
 class KonanBCEForLoopBodyTransformer : ForLoopBodyTransformer() {
+    lateinit var mainLoopVariable: IrVariable
+    lateinit var loopHeader: ForLoopHeader
+    lateinit var loopVariableComponents: Map<Int, IrVariable>
+    lateinit var context: CommonBackendContext
+
    private var analysisResult: BoundsCheckAnalysisResult = BoundsCheckAnalysisResult(false, null)

-    override fun shouldTransform(context: CommonBackendContext): Boolean {
-        return context is Context && context.shouldOptimize()
-    }
-
-    override fun initialize(context: CommonBackendContext, loopVariable: IrVariable,
-                            forLoopHeader: ForLoopHeader, loopComponents: Map<Int, IrVariable>) {
-        super.initialize(context, loopVariable, forLoopHeader, loopComponents)
+    override fun transform(context: CommonBackendContext, irExpression: IrExpression, loopVariable: IrVariable,
+                           forLoopHeader: ForLoopHeader, loopComponents: Map<Int, IrVariable>) {
+        this.context = context
+        mainLoopVariable = loopVariable
+        loopHeader = forLoopHeader
+        loopVariableComponents = loopComponents
        analysisResult = analyzeLoopHeader(loopHeader)
+        if (analysisResult.boundsAreSafe && analysisResult.arrayInLoop != null)
+            irExpression.transformChildrenVoid(this)
    }

    private fun IrExpression.compareIntegerNumericConst(compare: (Long) -> Boolean): Boolean {
@@ -60,46 +67,45 @@ class KonanBCEForLoopBodyTransformer : ForLoopBodyTransformer() {
    private fun IrCall.dispatchReceiverIsGetSizeCall() = (dispatchReceiver as? IrCall)?.let { it.isGetSizeCall() } ?: false

    private fun lessThanSize(functionCall: IrCall): BoundsCheckAnalysisResult {
-        var result = false
-        when (functionCall.symbol.owner.name) {
+        val boundsAreSafe = when (functionCall.symbol.owner.name) {
            OperatorNameConventions.DEC ->
-                if (functionCall.dispatchReceiverIsGetSizeCall()) {
-                    result = true
-                }
+                functionCall.dispatchReceiverIsGetSizeCall()
            OperatorNameConventions.MINUS -> {
                val value = functionCall.getValueArgument(0)
-                result = functionCall.dispatchReceiverIsGetSizeCall() &&
+                functionCall.dispatchReceiverIsGetSizeCall() &&
                        value?.compareIntegerNumericConst { it > 0 } == true
            }
-            OperatorNameConventions.DIV-> {
+            OperatorNameConventions.DIV -> {
                val value = functionCall.getValueArgument(0)
-                result = functionCall.dispatchReceiverIsGetSizeCall() &&
+                functionCall.dispatchReceiverIsGetSizeCall() &&
                        value?.compareFloatNumericConst { it > 1 } == true
            }
+            else -> false
        }
        val array = ((functionCall.dispatchReceiver as? IrCall)?.dispatchReceiver as? IrGetValue)?.symbol
-        return BoundsCheckAnalysisResult(result, array)
+        return BoundsCheckAnalysisResult(boundsAreSafe, array)
    }

    private fun checkLastElement(last: IrExpression, loopHeader: ProgressionLoopHeader): BoundsCheckAnalysisResult {
-        var result = BoundsCheckAnalysisResult(false, null)
-        if (last is IrCall) {
-            result = lessThanSize(last)
+        return if (last is IrCall) {
            if (last.isGetSizeCall() && !loopHeader.headerInfo.isLastInclusive) {
-                result = BoundsCheckAnalysisResult(true, (last.dispatchReceiver as? IrGetValue)?.symbol)
+                BoundsCheckAnalysisResult(true, (last.dispatchReceiver as? IrGetValue)?.symbol)
+            } else {
+                lessThanSize(last)
            }
+        } else {
+            BoundsCheckAnalysisResult(false, null)
        }
-        return result
    }

    private fun IrExpression.isProgressionPropertyGetter(propertyName: String) =
            this is IrCall && symbol.owner.origin == IrDeclarationOrigin.DEFAULT_PROPERTY_ACCESSOR &&
                    (symbol.signature as? IdSignature.AccessorSignature)?.propertySignature?.asPublic()?.shortName == propertyName &&
-                        dispatchReceiver?.type?.getClass()?.symbol in context.ir.symbols.progressionClasses
+                    dispatchReceiver?.type?.getClass()?.symbol in context.ir.symbols.progressionClasses

    private fun analyzeLoopHeader(loopHeader: ForLoopHeader): BoundsCheckAnalysisResult {
-        analysisResult = BoundsCheckAnalysisResult(false, null)
-        when(loopHeader) {
+        var analysisResult = BoundsCheckAnalysisResult(false, null)
+        when (loopHeader) {
            is ProgressionLoopHeader ->
                when (loopHeader.headerInfo.direction) {
                    ProgressionDirection.INCREASING -> {
@@ -112,6 +118,8 @@ class KonanBCEForLoopBodyTransformer : ForLoopBodyTransformer() {
                        if (loopHeader.headerInfo.last is IrCall) {
                            val functionCall = (loopHeader.headerInfo.last as IrCall)
                            // Case of range with step - `for (i in 0..array.size - 1 step n)`.
+                            // There is a temporary variable `val nestedLast = array.size - 1`
+                            // and `last` is computed as `getProgressionLastElement(0, nestedLast, n)`
                            if (loopHeader.headerInfo.progressionType.getProgressionLastElementFunction == functionCall.symbol) {
                                val nestedLastVariable = functionCall.getValueArgument(1)
                                if (nestedLastVariable is IrGetValue && nestedLastVariable.symbol.owner is IrVariable) {
@@ -130,6 +138,8 @@ class KonanBCEForLoopBodyTransformer : ForLoopBodyTransformer() {
                        if (loopHeader.headerInfo.last is IrCall) {
                            val functionCall = (loopHeader.headerInfo.last as IrCall)
                            // Case of range with step - for (i in array.size - 1 downTo 0 step n).
+                            // There is a temporary variable `val nestedFirst = array.size - 1`
+                            // and `last` is computed as `getProgressionLastElement(nestedFirst, 0, n)`
                            if (loopHeader.headerInfo.progressionType.getProgressionLastElementFunction == functionCall.symbol) {
                                if (functionCall.getValueArgument(1)?.compareIntegerNumericConst { it >= valueToCompare } == true) {
                                    boundsAreSafe = true
@@ -140,16 +150,13 @@ class KonanBCEForLoopBodyTransformer : ForLoopBodyTransformer() {
                        }
                        if (!boundsAreSafe)
                            return analysisResult
-                        when (loopHeader.headerInfo.first) {
-                            is IrCall -> {
-                                val functionCall = (loopHeader.headerInfo.first as IrCall)
-                                analysisResult = lessThanSize(functionCall)
-                            }
-                            is IrGetValue -> {
-                                (((loopHeader.headerInfo.first as IrGetValue).symbol.owner as? IrVariable)?.initializer as? IrCall)?.let {
+                        when (val first = loopHeader.headerInfo.first) {
+                            is IrCall ->
+                                analysisResult = lessThanSize(first)
+                            is IrGetValue ->
+                                ((first.symbol.owner as? IrVariable)?.initializer as? IrCall)?.let {
                                    analysisResult = lessThanSize(it)
                                }
-                            }
                        }
                    }
                    ProgressionDirection.UNKNOWN ->
@@ -177,7 +184,7 @@ class KonanBCEForLoopBodyTransformer : ForLoopBodyTransformer() {
                }

            is WithIndexLoopHeader ->
-                when(loopHeader.nestedLoopHeader) {
+                when (loopHeader.nestedLoopHeader) {
                    is IndexedGetLoopHeader -> {
                        analysisResult = BoundsCheckAnalysisResult(true,
                                ((loopHeader.loopInitStatements[0] as? IrVariable)?.initializer as? IrGetValue)?.symbol)
@@ -190,14 +197,14 @@ class KonanBCEForLoopBodyTransformer : ForLoopBodyTransformer() {

    private fun replaceOperators(expression: IrCall, index: IrExpression, safeIndexVariables: List<IrVariable>): IrExpression {
        if (index is IrGetValue && index.symbol.owner in safeIndexVariables) {
-            val operatorWithoutBC = expression.dispatchReceiver!!.type.getClass()!!.functions.singleOrNull {
+            val operatorWithoutBoundCheck = expression.dispatchReceiver!!.type.getClass()!!.functions.singleOrNull {
                if (expression.symbol.owner.name == OperatorNameConventions.SET)
-                    it.name == KonanNameConventions.setWithoutBC
+                    it.name == KonanNameConventions.setWithoutBoundCheck
                else
-                    it.name == KonanNameConventions.getWithoutBC
+                    it.name == KonanNameConventions.getWithoutBoundCheck
            } ?: return expression
            return IrCallImpl(
-                    expression.startOffset, expression.endOffset, expression.type, operatorWithoutBC.symbol,
+                    expression.startOffset, expression.endOffset, expression.type, operatorWithoutBoundCheck.symbol,
                    typeArgumentsCount = expression.typeArgumentsCount,
                    valueArgumentsCount = expression.valueArgumentsCount).apply {
                dispatchReceiver = expression.dispatchReceiver
@@ -210,28 +217,28 @@ class KonanBCEForLoopBodyTransformer : ForLoopBodyTransformer() {
    }

    override fun visitCall(expression: IrCall): IrExpression {
-        if (!analysisResult.boundsAreSafe || analysisResult.arrayInLoop == null)
-            return expression
-        if (expression.symbol.owner.name != OperatorNameConventions.SET && expression.symbol.owner.name != OperatorNameConventions.GET) {
-            expression.transformChildrenVoid()
-            return expression
-        }
+        if (expression.symbol.owner.name != OperatorNameConventions.SET && expression.symbol.owner.name != OperatorNameConventions.GET)
+            return super.visitCall(expression)
        if (expression.dispatchReceiver?.type?.isBasicArray() != true ||
                (expression.dispatchReceiver as? IrGetValue)?.symbol != analysisResult.arrayInLoop)
-            return expression
+            return super.visitCall(expression)
        // Analyze arguments of set/get operator.
-        val index = expression.getValueArgument(0)
-        return when(loopHeader) {
+        val index = expression.getValueArgument(0)!!
+        return when (loopHeader) {
            is ProgressionLoopHeader -> with(loopHeader as ProgressionLoopHeader) {
-                replaceOperators(expression, index!!, listOf(mainLoopVariable, inductionVariable))
+                replaceOperators(expression, index, listOf(mainLoopVariable, inductionVariable))
            }

            is WithIndexLoopHeader -> with(loopHeader as WithIndexLoopHeader) {
-                when(nestedLoopHeader) {
+                when (nestedLoopHeader) {
                    is IndexedGetLoopHeader ->
-                        replaceOperators(expression, index!!, listOfNotNull(indexVariable, loopVariableComponents[1]))
+                        replaceOperators(expression, index, listOfNotNull(indexVariable, loopVariableComponents[1]))
                    is ProgressionLoopHeader ->
-                        replaceOperators(expression, index!!,
+                        // Case of `for ((index, value) in (0..array.size - 1 step n).withIndex())`.
+                        // Both `index` (progression size less than array size)
+                        // and `value` (progression start and end element are inside bounds)
+                        // are safe variables if use them in get/set operators.
+                        replaceOperators(expression, index,
                                listOfNotNull(indexVariable, loopVariableComponents[1], loopVariableComponents[2])
                        )
                    else -> expression
@@ -2,6 +2,8 @@
 * Copyright 2010-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license
 * that can be found in the LICENSE file.
 */
+package codegen.bce.arraysForLoops
+
 import kotlin.test.*

@Test fun forEachIndexedTest() {
@@ -159,7 +161,7 @@ import kotlin.test.*
@Test fun forRangeToWithStep() {
    val array = Array(10) { 0L }
    val array1 = Array(3) { 0L }
-    var j = 4
+    var j = 8

    assertFailsWith<ArrayIndexOutOfBoundsException> {
        for (i in 0..array.size - 1 step 2) {
@@ -202,7 +204,7 @@ import kotlin.test.*
@Test fun forUntilWithStep() {
    val array = CharArray(10) { '0' }
    val array1 = CharArray(3) { '0' }
-    var j = 4
+    var j = 8

    assertFailsWith<ArrayIndexOutOfBoundsException> {
        for (i in 0 until array.size step 2) {
@@ -213,7 +215,7 @@ import kotlin.test.*

    assertFailsWith<ArrayIndexOutOfBoundsException> {
        for (i in 0 until array.size step 2) {
-            array[i + 1] = '6'
+            array[i + 3] = '6'
        }
    }

@@ -239,7 +241,7 @@ import kotlin.test.*
@Test fun forDownToWithStep() {
    val array = Array(10) { 0L }
    val array1 = Array(3) { 0L }
-    var j = 4
+    var j = 8

    assertFailsWith<ArrayIndexOutOfBoundsException> {
        for (i in array.size - 1 downTo 0 step 2) {
@@ -282,7 +284,7 @@ import kotlin.test.*
@Test fun forIndiciesWithStep() {
    val array = Array(10) { 0L }
    val array1 = Array(3) { 0L }
-    var j = 4
+    var j = 8

    assertFailsWith<ArrayIndexOutOfBoundsException> {
        for (i in array.indices step 2) {
@@ -307,7 +309,7 @@ import kotlin.test.*
@Test fun forWithIndex() {
    val array = Array(10) { 100 }
    val array1 = Array(3) { 0 }
-    var j = 4
+    var j = 8

    assertFailsWith<ArrayIndexOutOfBoundsException> {
        for ((index, value) in array.withIndex()) {
@@ -329,7 +331,7 @@ import kotlin.test.*
    }

    assertFailsWith<ArrayIndexOutOfBoundsException> {
-        for ((i, v) in (0..array.size + 1 step 2).withIndex()) {
+        for ((i, v) in (0..array.size + 30 step 2).withIndex()) {
            array[i] = 6
        }
    }
@@ -344,7 +346,7 @@ import kotlin.test.*
@Test fun forReversed() {
    val array = Array(10) { 100 }
    val array1 = Array(3) { 0 }
-    var j = 4
+    var j = 8

    assertFailsWith<ArrayIndexOutOfBoundsException> {
        for (i in (0..array.size-1).reversed()) {