From bf4f26318e7ccd934836992459a763be6347774c Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Fri, 1 Jul 2016 12:26:20 +0300 Subject: [PATCH] KT-6916: do not create Progression instances in for-in-downTo loops --- .../kotlin/codegen/ExpressionCodegen.java | 72 ++++++++++++++++--- .../kotlin/codegen/RangeCodegenUtil.java | 11 +++ .../box/ranges/forInDownTo/forIntInDownTo.kt | 13 ++++ .../forInDownTo/forIntInNonOptimizedDownTo.kt | 14 ++++ .../box/ranges/forInDownTo/forLongInDownTo.kt | 13 ++++ .../forInDownTo/forNullableIntInDownTo.kt | 13 ++++ .../bytecodeText/forLoop/forIntInDownTo.kt | 15 ++++ .../forLoop/primitiveProgression.kt | 3 +- .../codegen/BlackBoxCodegenTestGenerated.java | 33 +++++++++ .../codegen/BytecodeTextTestGenerated.java | 6 ++ 10 files changed, 183 insertions(+), 10 deletions(-) create mode 100644 compiler/testData/codegen/box/ranges/forInDownTo/forIntInDownTo.kt create mode 100644 compiler/testData/codegen/box/ranges/forInDownTo/forIntInNonOptimizedDownTo.kt create mode 100644 compiler/testData/codegen/box/ranges/forInDownTo/forLongInDownTo.kt create mode 100644 compiler/testData/codegen/box/ranges/forInDownTo/forNullableIntInDownTo.kt create mode 100644 compiler/testData/codegen/bytecodeText/forLoop/forIntInDownTo.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java index f90b73798de..19d8c93c07a 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java @@ -635,12 +635,17 @@ public class ExpressionCodegen extends KtVisitor impleme if (RangeCodegenUtil.isOptimizableRangeTo(loopRangeCallee)) { ReceiverValue from = loopRangeCall.getDispatchReceiver(); assert from != null : "Dispatch receiver should be non-null for optimizable 'rangeTo' call"; - List valueArgumentsByIndex = loopRangeCall.getValueArgumentsByIndex(); - assert valueArgumentsByIndex != null : "Value arguments should be non-null for optimizable 'rangeTo' call"; - KtExpression to = valueArgumentsByIndex.get(0).getArguments().get(0).getArgumentExpression(); - assert to != null : "1st value argument should be non-null for optimizable 'rangeTo' call"; + KtExpression to = getSingleArgumentExpression(loopRangeCall); + assert to != null : "Optimizable 'rangeTo' call should have a single expression argument"; return new ForInRangeLiteralLoopGenerator(forExpression, from, to); } + else if (RangeCodegenUtil.isOptimizableDownTo(loopRangeCallee)) { + ReceiverValue from = loopRangeCall.getExtensionReceiver(); + assert from != null : "Extension receiver should be non-null for optimizable 'downTo' call"; + KtExpression to = getSingleArgumentExpression(loopRangeCall); + assert to != null : "Optimizable 'downTo' call should have a single expression argument"; + return new ForInDownToProgressionLoopGenerator(forExpression, from, to); + } else if (RangeCodegenUtil.isArrayOrPrimitiveArrayIndices(loopRangeCallee)) { ReceiverValue extensionReceiver = loopRangeCall.getExtensionReceiver(); assert extensionReceiver != null : "Extension receiver should be non-null for optimizable 'Array.indices' call"; @@ -655,6 +660,16 @@ public class ExpressionCodegen extends KtVisitor impleme return null; } + @Nullable + private static KtExpression getSingleArgumentExpression(@NotNull ResolvedCall resolvedCall) { + List resolvedValueArguments = resolvedCall.getValueArgumentsByIndex(); + if (resolvedValueArguments == null) return null; + if (resolvedValueArguments.size() != 1) return null; + List valueArguments = resolvedValueArguments.get(0).getArguments(); + if (valueArguments.size() != 1) return null; + return valueArguments.get(0).getArgumentExpression(); + } + private OwnerKind contextKind() { return context.getContextKind(); } @@ -1044,8 +1059,16 @@ public class ExpressionCodegen extends KtVisitor impleme } private abstract class AbstractForInRangeLoopGenerator extends AbstractForInProgressionOrRangeLoopGenerator { - private AbstractForInRangeLoopGenerator(@NotNull KtForExpression forExpression) { + private final int step; + + private AbstractForInRangeLoopGenerator(@NotNull KtForExpression forExpression, int step) { super(forExpression); + this.step = step; + assert step == 1 || step == -1 : "'step' should be either 1 or -1: " + step; + } + + private AbstractForInRangeLoopGenerator(@NotNull KtForExpression forExpression) { + this(forExpression, 1); } @Override @@ -1063,10 +1086,20 @@ public class ExpressionCodegen extends KtVisitor impleme v.load(endVar, asmElementType); if (asmElementType.getSort() == Type.LONG) { v.lcmp(); - v.ifgt(loopExit); + if (step > 0) { + v.ifgt(loopExit); + } + else { + v.iflt(loopExit); + } } else { - v.ificmpgt(loopExit); + if (step > 0) { + v.ificmpgt(loopExit); + } + else { + v.ificmplt(loopExit); + } } } @@ -1079,12 +1112,12 @@ public class ExpressionCodegen extends KtVisitor impleme checkPostCondition(loopExit); if (loopParameterType == Type.INT_TYPE) { - v.iinc(loopParameterVar, 1); + v.iinc(loopParameterVar, step); } else { StackValue loopParameter = loopParameter(); loopParameter.put(asmElementType, v); - genIncrement(asmElementType, 1, v); + genIncrement(asmElementType, step, v); loopParameter.store(StackValue.onStack(asmElementType), v); } } @@ -1111,6 +1144,27 @@ public class ExpressionCodegen extends KtVisitor impleme } } + private class ForInDownToProgressionLoopGenerator extends AbstractForInRangeLoopGenerator { + private final ReceiverValue from; + private final KtExpression to; + + private ForInDownToProgressionLoopGenerator( + @NotNull KtForExpression forExpression, + @NotNull ReceiverValue from, + @NotNull KtExpression to + ) { + super(forExpression, -1); + this.from = from; + this.to = to; + } + + @Override + protected void storeRangeStartAndEnd() { + loopParameter().store(generateReceiverValue(from, false), v); + StackValue.local(endVar, asmElementType).store(gen(to), v); + } + } + private class ForInRangeInstanceLoopGenerator extends AbstractForInRangeLoopGenerator { private ForInRangeInstanceLoopGenerator(@NotNull KtForExpression forExpression) { super(forExpression); diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/RangeCodegenUtil.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/RangeCodegenUtil.java index 281b13a8a22..53125e529b9 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/RangeCodegenUtil.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/RangeCodegenUtil.java @@ -132,6 +132,17 @@ public class RangeCodegenUtil { return false; } + public static boolean isOptimizableDownTo(@NotNull CallableDescriptor descriptor) { + if (!isTopLevelInPackage(descriptor, "downTo", "kotlin.ranges")) return false; + + ReceiverParameterDescriptor extensionReceiver = descriptor.getExtensionReceiverParameter(); + if (extensionReceiver == null) return false; + ClassifierDescriptor extensionReceiverClassifier = extensionReceiver.getType().getConstructor().getDeclarationDescriptor(); + if (!isPrimitiveNumberClassDescriptor(extensionReceiverClassifier)) return false; + + return true; + } + public static boolean isArrayOrPrimitiveArrayIndices(@NotNull CallableDescriptor descriptor) { if (!isTopLevelInPackage(descriptor, "indices", "kotlin.collections")) return false; diff --git a/compiler/testData/codegen/box/ranges/forInDownTo/forIntInDownTo.kt b/compiler/testData/codegen/box/ranges/forInDownTo/forIntInDownTo.kt new file mode 100644 index 00000000000..fd046640e37 --- /dev/null +++ b/compiler/testData/codegen/box/ranges/forInDownTo/forIntInDownTo.kt @@ -0,0 +1,13 @@ +// WITH_RUNTIME + +import kotlin.test.assertEquals + +fun box(): String { + var sum = 0 + for (i in 4 downTo 1) { + sum = sum * 10 + i + } + assertEquals(4321, sum) + + return "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/ranges/forInDownTo/forIntInNonOptimizedDownTo.kt b/compiler/testData/codegen/box/ranges/forInDownTo/forIntInNonOptimizedDownTo.kt new file mode 100644 index 00000000000..45214ff7d0e --- /dev/null +++ b/compiler/testData/codegen/box/ranges/forInDownTo/forIntInNonOptimizedDownTo.kt @@ -0,0 +1,14 @@ +// WITH_RUNTIME + +import kotlin.test.assertEquals + +fun box(): String { + var sum = 0 + val dt = 4 downTo 1 + for (i in dt) { + sum = sum * 10 + i + } + assertEquals(4321, sum) + + return "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/ranges/forInDownTo/forLongInDownTo.kt b/compiler/testData/codegen/box/ranges/forInDownTo/forLongInDownTo.kt new file mode 100644 index 00000000000..4d1e9793cce --- /dev/null +++ b/compiler/testData/codegen/box/ranges/forInDownTo/forLongInDownTo.kt @@ -0,0 +1,13 @@ +// WITH_RUNTIME + +import kotlin.test.assertEquals + +fun box(): String { + var sum = 0L + for (i in 4L downTo 1L) { + sum = sum * 10L + i + } + assertEquals(4321L, sum) + + return "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/ranges/forInDownTo/forNullableIntInDownTo.kt b/compiler/testData/codegen/box/ranges/forInDownTo/forNullableIntInDownTo.kt new file mode 100644 index 00000000000..26109378c1b --- /dev/null +++ b/compiler/testData/codegen/box/ranges/forInDownTo/forNullableIntInDownTo.kt @@ -0,0 +1,13 @@ +// WITH_RUNTIME + +import kotlin.test.assertEquals + +fun box(): String { + var sum = 0 + for (i: Int? in 4 downTo 1) { + sum = sum * 10 + (i ?: 0) + } + assertEquals(4321, sum) + + return "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/bytecodeText/forLoop/forIntInDownTo.kt b/compiler/testData/codegen/bytecodeText/forLoop/forIntInDownTo.kt new file mode 100644 index 00000000000..6442e90b54d --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/forLoop/forIntInDownTo.kt @@ -0,0 +1,15 @@ +// WITH_RUNTIME + +fun test(): Int { + var sum = 0 + for (i in 4 downTo 1) { + sum = sum * 10 + i + } + return sum +} + +// 0 iterator +// 0 getStart +// 0 getEnd +// 0 getFirst +// 0 getLast \ No newline at end of file diff --git a/compiler/testData/codegen/bytecodeText/forLoop/primitiveProgression.kt b/compiler/testData/codegen/bytecodeText/forLoop/primitiveProgression.kt index ce1a5240614..ec759232526 100644 --- a/compiler/testData/codegen/bytecodeText/forLoop/primitiveProgression.kt +++ b/compiler/testData/codegen/bytecodeText/forLoop/primitiveProgression.kt @@ -1,7 +1,8 @@ fun f() { for (i in 0..5 step 2) { } - for (i in 5 downTo 1) { + + for (i in 5 downTo 1 step 1) { // suppress optimized code generation for 'for-in-downTo' } } diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index d4c0ebe046c..443eab96765 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -10801,6 +10801,39 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { } } + @TestMetadata("compiler/testData/codegen/box/ranges/forInDownTo") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class ForInDownTo extends AbstractBlackBoxCodegenTest { + public void testAllFilesPresentInForInDownTo() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/ranges/forInDownTo"), Pattern.compile("^(.+)\\.kt$"), true); + } + + @TestMetadata("forIntInDownTo.kt") + public void testForIntInDownTo() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/ranges/forInDownTo/forIntInDownTo.kt"); + doTest(fileName); + } + + @TestMetadata("forIntInNonOptimizedDownTo.kt") + public void testForIntInNonOptimizedDownTo() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/ranges/forInDownTo/forIntInNonOptimizedDownTo.kt"); + doTest(fileName); + } + + @TestMetadata("forLongInDownTo.kt") + public void testForLongInDownTo() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/ranges/forInDownTo/forLongInDownTo.kt"); + doTest(fileName); + } + + @TestMetadata("forNullableIntInDownTo.kt") + public void testForNullableIntInDownTo() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/ranges/forInDownTo/forNullableIntInDownTo.kt"); + doTest(fileName); + } + } + @TestMetadata("compiler/testData/codegen/box/ranges/forInIndices") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index c30ac648e2d..a0faa90367f 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -859,6 +859,12 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { doTest(fileName); } + @TestMetadata("forIntInDownTo.kt") + public void testForIntInDownTo() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/forLoop/forIntInDownTo.kt"); + doTest(fileName); + } + @TestMetadata("loopVarInterval.kt") public void testLoopVarInterval() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/forLoop/loopVarInterval.kt");