From 8676ca34d76d2fcebd42f49b4dc8de297e2758a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mika=C3=ABl=20Peltier?= Date: Fri, 2 Feb 2018 13:48:41 +0100 Subject: [PATCH] KT-22334 Specialized loops using range such as integer..array.size-1 - Into PrimitiveNumberRangeLiteralRangeValue modifies how bounded value are created by checking if the high bound range can be modified to be exclusive instead of inclusive such as the generated code will be optimized. - Add black box tests checking that the specialization does not fail on any kind of arrays. - Add a bytecode test checking that the code is correctly optimized. Fix of https://youtrack.jetbrains.com/issue/KT-22334 --- .../PrimitiveNumberRangeLiteralRangeValue.kt | 54 ++++++- .../forInArraySpecializedToUntil.kt | 143 ++++++++++++++++++ .../forLoop/forInRangeSpecializedToUntil.kt | 12 ++ .../ir/IrBlackBoxCodegenTestGenerated.java | 6 + .../codegen/BlackBoxCodegenTestGenerated.java | 6 + .../codegen/BytecodeTextTestGenerated.java | 6 + .../LightAnalysisModeTestGenerated.java | 6 + .../semantics/JsCodegenBoxTestGenerated.java | 6 + 8 files changed, 236 insertions(+), 3 deletions(-) create mode 100644 compiler/testData/codegen/box/controlStructures/forInArray/forInArraySpecializedToUntil.kt create mode 100644 compiler/testData/codegen/bytecodeText/forLoop/forInRangeSpecializedToUntil.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/range/PrimitiveNumberRangeLiteralRangeValue.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/range/PrimitiveNumberRangeLiteralRangeValue.kt index c23113b4163..db7f4c2e678 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/range/PrimitiveNumberRangeLiteralRangeValue.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/range/PrimitiveNumberRangeLiteralRangeValue.kt @@ -22,18 +22,33 @@ import org.jetbrains.kotlin.codegen.generateCallSingleArgument import org.jetbrains.kotlin.codegen.range.forLoop.ForInSimpleProgressionLoopGenerator import org.jetbrains.kotlin.codegen.range.forLoop.ForLoopGenerator import org.jetbrains.kotlin.descriptors.CallableDescriptor -import org.jetbrains.kotlin.psi.KtForExpression +import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.resolve.calls.callUtil.getFirstArgumentExpression import org.jetbrains.kotlin.resolve.calls.callUtil.getReceiverExpression import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall +import org.jetbrains.kotlin.resolve.constants.IntegerValueConstant +import org.jetbrains.kotlin.utils.addToStdlib.safeAs +import org.jetbrains.org.objectweb.asm.Type class PrimitiveNumberRangeLiteralRangeValue( rangeCall: ResolvedCall ) : PrimitiveNumberRangeIntrinsicRangeValue(rangeCall), ReversableRangeValue { - override fun getBoundedValue(codegen: ExpressionCodegen) = - SimpleBoundedValue(codegen, rangeCall) + override fun getBoundedValue(codegen: ExpressionCodegen): SimpleBoundedValue { + if (codegen.canBeSpecializedByExcludingHighBound(rangeCall)) { + val highBound = (rangeCall.getFirstArgumentExpression() as KtBinaryExpression).left + return SimpleBoundedValue( + codegen, + rangeCall, + codegen.generateCallReceiver(rangeCall), + true, + codegen.gen(highBound), + false + ) + } + return SimpleBoundedValue(codegen, rangeCall) + } override fun createForLoopGenerator(codegen: ExpressionCodegen, forExpression: KtForExpression): ForLoopGenerator = createConstBoundedForInRangeLiteralGenerator(codegen, forExpression) @@ -71,4 +86,37 @@ class PrimitiveNumberRangeLiteralRangeValue( -1 ) } +} + +private fun ExpressionCodegen.canBeSpecializedByExcludingHighBound(rangeCall: ResolvedCall): Boolean { + // Currently only "cst...size-1" can be specialized to "cst until .size" + return isArraySizeMinusOne(rangeCall.getFirstArgumentExpression()!!) +} + +private fun ExpressionCodegen.isArraySizeMinusOne(expression: KtExpression): Boolean { + return when { + expression is KtBinaryExpression -> { + isArraySizeAccess(expression.left!!) && + expression.getOperationToken() === org.jetbrains.kotlin.lexer.KtTokens.MINUS && + isConstantOne(expression.right!!) + } + else -> false + } +} + +private fun ExpressionCodegen.isConstantOne(expression: KtExpression): Boolean { + val constantValue = getCompileTimeConstant(expression).safeAs>() ?: return false + return constantValue.value == 1 +} + +private fun ExpressionCodegen.isArraySizeAccess(expression: KtExpression): Boolean { + return when { + expression is KtDotQualifiedExpression -> { + val selector = expression.selectorExpression + asmType(bindingContext.getType(expression.receiverExpression)!!).sort == Type.ARRAY && + selector is KtNameReferenceExpression && + selector.text == "size" + } + else -> false + } } \ No newline at end of file diff --git a/compiler/testData/codegen/box/controlStructures/forInArray/forInArraySpecializedToUntil.kt b/compiler/testData/codegen/box/controlStructures/forInArray/forInArraySpecializedToUntil.kt new file mode 100644 index 00000000000..d761122ded9 --- /dev/null +++ b/compiler/testData/codegen/box/controlStructures/forInArray/forInArraySpecializedToUntil.kt @@ -0,0 +1,143 @@ +// WITH_RUNTIME + +fun checkByteArray():Boolean { + val byteArray = byteArrayOf(1, 2, 3) + var sum = 0 + for (i in 0..byteArray.size-1) { + sum += byteArray[i] + } + if (sum != 6) return false + return true +} + +fun checkShortArray():Boolean { + val shortArray = shortArrayOf(1, 2, 3) + var sum = 0 + for (i in 0..shortArray.size-1) { + sum += shortArray[i] + } + if (sum != 6) return false + return true +} + +fun checkCharArray():Boolean { + val charArray = charArrayOf('1', '2', '3') + var sum = "" + for (i in 0..charArray.size-1) { + sum += charArray[i] + } + if (sum != "123") return false + return true +} + +fun checkIntArray():Boolean { + val intArray = intArrayOf(1, 2, 3) + var sum = 0 + for (i in 0..intArray.size-1) { + sum += intArray[i] + } + if (sum != 6) return false + return true +} + +fun checkLongArray():Boolean { + val longArray = longArrayOf(1L, 2L, 3L) + var sum = 0L + for (i in 0..longArray.size-1) { + sum += longArray[i] + } + if (sum != 6L) return false + return true +} + +fun checkFloatArray():Boolean { + val floatArray = floatArrayOf(1.1f, 2.2f, 3.3f) + var sum = 0f + for (i in 0..floatArray.size-1) { + sum += floatArray[i] + } + if (sum != (1.1f + 2.2f + 3.3f)) return false + return true +} + +fun checkDoubleArray():Boolean { + val doubleArray = doubleArrayOf(1.1, 2.2, 3.3) + var sum = 0.0 + for (i in 0..doubleArray.size-1) { + sum += doubleArray[i] + } + if (sum != 6.6) return false + return true +} + +fun checkBooleanArray():Boolean { + val booleanArray = booleanArrayOf(false, false, true) + var result = false + for (i in 0..booleanArray.size-1) { + result = booleanArray[i] + } + return result +} + +class Value(val value: Int) {} + +fun checkObjectArray():Boolean { + val objectArray = arrayOf(Value(1), Value(2), Value(3)) + var sum = 0 + for (i in 0..objectArray.size-1) { + sum += objectArray[i].value + } + if (sum != 6) return false + return true +} + +fun checkWithArrayUpdate():Boolean { + var intArray = intArrayOf(1, 2, 3) + var sum = 0 + for (i in 0..intArray.size-1) { + sum += intArray[i] + intArray = intArrayOf(4, 5, 6, 7) + } + if (sum != 12) return false + return true +} + +fun checkIntArrayMinusArbitraryConstant():Boolean { + val intArray = intArrayOf(1, 2, 3) + var sum = 0 + for (i in 0..intArray.size-2) { + sum += intArray[i] + } + if (sum != 3) return false + return true +} + +fun checkReversedIntArray(): Boolean { + val intArray = intArrayOf(1, 2, 3) + var start = 0 + var sum = 0 + for (i in (start..intArray.size-1).reversed()) { + sum += intArray[i] + } + if (sum != 6) return false + return true +} + +fun box() : String { + // Check that the specialization of 'for (i in 0..array.size-1)' to 'for (i in 0 until array.size)' does not fail on + // any kind of arrays. + if (!checkByteArray()) return "Failure" + if (!checkShortArray()) return "Failure" + if (!checkCharArray()) return "Failure" + if (!checkIntArray()) return "Failure" + if (!checkLongArray()) return "Failure" + if (!checkFloatArray()) return "Failure" + if (!checkDoubleArray()) return "Failure" + if (!checkBooleanArray()) return "Failure" + if (!checkObjectArray()) return "Failure" + if (!checkWithArrayUpdate()) return "Failure" + if (!checkIntArrayMinusArbitraryConstant()) return "Failure" + if (!checkReversedIntArray()) return "Failure" + + return "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/bytecodeText/forLoop/forInRangeSpecializedToUntil.kt b/compiler/testData/codegen/bytecodeText/forLoop/forInRangeSpecializedToUntil.kt new file mode 100644 index 00000000000..14fffd3f5b8 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/forLoop/forInRangeSpecializedToUntil.kt @@ -0,0 +1,12 @@ +fun test(): Int { + val intArray = intArrayOf(1, 2, 3) + var sum = 0 + for (i in 0..intArray.size-1) { + sum += intArray[i] + } + return sum +} + +// 1 IF_ICMPGE +// 0 IF_ICMPGT +// 0 IF_ICMPEQ diff --git a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 98fc1ecf941..1ae8505c59e 100644 --- a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -5147,6 +5147,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/controlStructures/forInArray"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("forInArraySpecializedToUntil.kt") + public void testForInArraySpecializedToUntil() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/forInArray/forInArraySpecializedToUntil.kt"); + doTest(fileName); + } + @TestMetadata("forInArrayWithArrayPropertyUpdatedInLoopBody.kt") public void testForInArrayWithArrayPropertyUpdatedInLoopBody() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/forInArray/forInArrayWithArrayPropertyUpdatedInLoopBody.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 3ed5125b0e1..2c17a06c03f 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -5147,6 +5147,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/controlStructures/forInArray"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("forInArraySpecializedToUntil.kt") + public void testForInArraySpecializedToUntil() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/forInArray/forInArraySpecializedToUntil.kt"); + doTest(fileName); + } + @TestMetadata("forInArrayWithArrayPropertyUpdatedInLoopBody.kt") public void testForInArrayWithArrayPropertyUpdatedInLoopBody() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/forInArray/forInArrayWithArrayPropertyUpdatedInLoopBody.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index 31e6a8a6522..563ed82b107 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -1305,6 +1305,12 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { doTest(fileName); } + @TestMetadata("forInRangeSpecializedToUntil.kt") + public void testForInRangeSpecializedToUntil() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/forLoop/forInRangeSpecializedToUntil.kt"); + doTest(fileName); + } + @TestMetadata("forInRangeToCharConst.kt") public void testForInRangeToCharConst() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/forLoop/forInRangeToCharConst.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 4acb535a818..41988133f24 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -5147,6 +5147,12 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/controlStructures/forInArray"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("forInArraySpecializedToUntil.kt") + public void testForInArraySpecializedToUntil() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/forInArray/forInArraySpecializedToUntil.kt"); + doTest(fileName); + } + @TestMetadata("forInArrayWithArrayPropertyUpdatedInLoopBody.kt") public void testForInArrayWithArrayPropertyUpdatedInLoopBody() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/forInArray/forInArrayWithArrayPropertyUpdatedInLoopBody.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index f6554a9c2ba..2f2b5d44e31 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -5717,6 +5717,12 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/controlStructures/forInArray"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JS, true); } + @TestMetadata("forInArraySpecializedToUntil.kt") + public void testForInArraySpecializedToUntil() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/forInArray/forInArraySpecializedToUntil.kt"); + doTest(fileName); + } + @TestMetadata("forInArrayWithArrayPropertyUpdatedInLoopBody.kt") public void testForInArrayWithArrayPropertyUpdatedInLoopBody() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/forInArray/forInArrayWithArrayPropertyUpdatedInLoopBody.kt");