From 1be86e05ae987f2b247a55bd867973338e12f0ef Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Wed, 25 Oct 2017 16:36:25 +0300 Subject: [PATCH] JS: optimize range check for int and long See KT-2218 Additionally, fix temporary variable elimination for && and || --- .../ast/metadata/metadataProperties.kt | 12 +++ .../clean/TemporaryVariableElimination.kt | 5 ++ .../TemporaryVariableEliminationTest.kt | 2 + .../js/test/semantics/BoxJsTestGenerated.java | 6 ++ .../callTranslator/CallTranslator.kt | 32 +++++++ .../callTranslator/RangeCheckTranslator.kt | 84 +++++++++++++++++++ .../kotlin/js/translate/utils/JsAstUtils.java | 15 ++-- .../box/range/numberRangesOptimized.kt | 72 ++++++++++++++++ .../shortCircuit.optimized.js | 18 ++++ .../shortCircuit.original.js | 18 ++++ 10 files changed, 259 insertions(+), 5 deletions(-) create mode 100644 js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/RangeCheckTranslator.kt create mode 100644 js/js.translator/testData/box/range/numberRangesOptimized.kt create mode 100644 js/js.translator/testData/js-optimizer/temporary-variable/shortCircuit.optimized.js create mode 100644 js/js.translator/testData/js-optimizer/temporary-variable/shortCircuit.original.js diff --git a/js/js.ast/src/org/jetbrains/kotlin/js/backend/ast/metadata/metadataProperties.kt b/js/js.ast/src/org/jetbrains/kotlin/js/backend/ast/metadata/metadataProperties.kt index 53cf010ebab..71d4f4748c4 100644 --- a/js/js.ast/src/org/jetbrains/kotlin/js/backend/ast/metadata/metadataProperties.kt +++ b/js/js.ast/src/org/jetbrains/kotlin/js/backend/ast/metadata/metadataProperties.kt @@ -119,6 +119,8 @@ var JsName.imported by MetadataProperty(default = false) var JsFunction.coroutineMetadata: CoroutineMetadata? by MetadataProperty(default = null) +var JsExpression.range: Pair? by MetadataProperty(default = null) + data class CoroutineMetadata( val doResumeName: JsName, val stateName: JsName, @@ -161,4 +163,14 @@ enum class BoxingKind { NONE, BOXING, UNBOXING +} + +enum class RangeType { + INT, + LONG +} + +enum class RangeKind { + RANGE_TO, + UNTIL } \ No newline at end of file diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryVariableElimination.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryVariableElimination.kt index 6e11fb8dc9c..1ae9ae37e8c 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryVariableElimination.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryVariableElimination.kt @@ -495,6 +495,11 @@ internal class TemporaryVariableElimination(private val function: JsFunction) { accept(right) sideEffectOccurred = true } + else if (x.operator == JsBinaryOperator.AND || x.operator == JsBinaryOperator.OR) { + accept(x.arg1) + sideEffectOccurred = true + accept(x.arg2) + } else { super.visitBinaryExpression(x) } diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/optimizer/TemporaryVariableEliminationTest.kt b/js/js.tests/test/org/jetbrains/kotlin/js/test/optimizer/TemporaryVariableEliminationTest.kt index 69514cbd940..4822209b1bc 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/optimizer/TemporaryVariableEliminationTest.kt +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/optimizer/TemporaryVariableEliminationTest.kt @@ -46,4 +46,6 @@ class TemporaryVariableEliminationTest : BasicOptimizerTest("temporary-variable" @Test fun removeUnusedAndSubstitute() = box() @Test fun assignmentToOuterVar() = box() + + @Test fun shortCircuit() = box() } \ No newline at end of file diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java index ebe0aa4b8a2..830f64ed46b 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java @@ -7385,6 +7385,12 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest { doTest(fileName); } + @TestMetadata("numberRangesOptimized.kt") + public void testNumberRangesOptimized() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/range/numberRangesOptimized.kt"); + doTest(fileName); + } + @TestMetadata("rangeEquals.kt") public void testRangeEquals() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/range/rangeEquals.kt"); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/CallTranslator.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/CallTranslator.kt index 79f37af594c..f9ce14f4c2d 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/CallTranslator.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/CallTranslator.kt @@ -16,6 +16,7 @@ package org.jetbrains.kotlin.js.translate.callTranslator +import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.builtins.isFunctionTypeOrSubtype import org.jetbrains.kotlin.descriptors.CallableDescriptor import org.jetbrains.kotlin.descriptors.FunctionDescriptor @@ -28,6 +29,7 @@ import org.jetbrains.kotlin.js.translate.general.Translation import org.jetbrains.kotlin.js.translate.reference.CallArgumentTranslator import org.jetbrains.kotlin.js.translate.reference.CallExpressionTranslator import org.jetbrains.kotlin.js.translate.utils.* +import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.Call.CallType import org.jetbrains.kotlin.psi.KtExpression import org.jetbrains.kotlin.resolve.calls.callResolverUtil.isInvokeCallOnVariable @@ -35,6 +37,8 @@ import org.jetbrains.kotlin.resolve.calls.callUtil.isSafeCall import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall import org.jetbrains.kotlin.resolve.calls.model.VariableAsFunctionResolvedCall import org.jetbrains.kotlin.resolve.calls.tasks.ExplicitReceiverKind.* +import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe +import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameUnsafe import org.jetbrains.kotlin.resolve.scopes.receivers.ExpressionReceiver import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue import org.jetbrains.kotlin.types.KotlinType @@ -138,6 +142,9 @@ private fun translateFunctionCall( inlineResolvedCall: ResolvedCall, explicitReceivers: ExplicitReceivers ): JsExpression { + val rangeCheck = RangeCheckTranslator(context).translateAsRangeCheck(resolvedCall, explicitReceivers) + if (rangeCheck != null) return rangeCheck + val callExpression = context.getCallInfo(resolvedCall, explicitReceivers).translateFunctionCall() if (CallExpressionTranslator.shouldBeInlined(inlineResolvedCall.resultingDescriptor, context)) { @@ -164,9 +171,34 @@ private fun translateFunctionCall( } callExpression.type = resolvedCall.getReturnType().let { if (resolvedCall.call.isSafeCall()) it.makeNullable() else it } + mayBeMarkByRangeMetadata(resolvedCall, callExpression) return callExpression } + +private fun mayBeMarkByRangeMetadata(resolvedCall: ResolvedCall, callExpression: JsExpression) { + when (resolvedCall.resultingDescriptor.fqNameSafe) { + intRangeToFqName -> { + callExpression.range = Pair(RangeType.INT, RangeKind.RANGE_TO) + } + longRangeToFqName -> { + callExpression.range = Pair(RangeType.LONG, RangeKind.RANGE_TO) + } + untilFqName -> when (resolvedCall.resultingDescriptor.returnType?.constructor?.declarationDescriptor?.fqNameUnsafe) { + KotlinBuiltIns.FQ_NAMES.intRange -> { + callExpression.range = Pair(RangeType.INT, RangeKind.UNTIL) + } + KotlinBuiltIns.FQ_NAMES.longRange -> { + callExpression.range = Pair(RangeType.LONG, RangeKind.UNTIL) + } + } + } +} + +private val intRangeToFqName = FqName("kotlin.Int.rangeTo") +private val longRangeToFqName = FqName("kotlin.Long.rangeTo") +private val untilFqName = FqName("kotlin.ranges.until") + fun ResolvedCall.getReturnType(): KotlinType = TranslationUtils.getReturnTypeForCoercion(resultingDescriptor) private val TranslationContext.isInStateMachine diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/RangeCheckTranslator.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/RangeCheckTranslator.kt new file mode 100644 index 00000000000..c91ac80a23d --- /dev/null +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/RangeCheckTranslator.kt @@ -0,0 +1,84 @@ +/* + * Copyright 2010-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.kotlin.js.translate.callTranslator + +import org.jetbrains.kotlin.descriptors.ClassDescriptor +import org.jetbrains.kotlin.descriptors.FunctionDescriptor +import org.jetbrains.kotlin.js.backend.ast.JsExpression +import org.jetbrains.kotlin.js.backend.ast.JsInvocation +import org.jetbrains.kotlin.js.backend.ast.JsNameRef +import org.jetbrains.kotlin.js.backend.ast.metadata.* +import org.jetbrains.kotlin.js.translate.context.TranslationContext +import org.jetbrains.kotlin.js.translate.general.Translation +import org.jetbrains.kotlin.js.translate.utils.JsAstUtils +import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall + +class RangeCheckTranslator(private val context: TranslationContext) { + fun translateAsRangeCheck(resolvedCall: ResolvedCall, receivers: ExplicitReceivers): JsExpression? { + val calledFunction = resolvedCall.resultingDescriptor + if (calledFunction.name.asString() != "contains" || calledFunction.containingDeclaration !is ClassDescriptor) return null + + val invocation = (receivers.extensionOrDispatchReceiver as? JsExpression.JsExpressionHasArguments) ?: return null + val (rangeType, rangeKind) = invocation.range ?: return null + var (lower, upper) = when { + rangeKind == RangeKind.UNTIL || rangeType == RangeType.INT -> { + if (invocation.arguments.size != 2) return null + invocation.arguments + } + else -> { + if (invocation !is JsInvocation) return null + val qualifier = invocation.qualifier as? JsNameRef ?: return null + val receiver = qualifier.qualifier ?: return null + if (invocation.arguments.size != 1) return null + listOf(receiver, invocation.arguments.single()) + } + } + lower = context.cacheExpressionIfNeeded(lower) + upper = context.cacheExpressionIfNeeded(upper) + + val subjectPsi = resolvedCall.valueArguments.values.singleOrNull()?.arguments?.singleOrNull()?.getArgumentExpression() ?: return null + val subject = context.cacheExpressionIfNeeded(Translation.translateAsExpression(subjectPsi, context)) + + return when (rangeType) { + RangeType.INT -> translateAsIntRangeCheck(lower, upper, rangeKind, subject) + RangeType.LONG -> translateAsLongRangeCheck(lower, upper, rangeKind, subject) + } + } + + private fun translateAsIntRangeCheck(lower: JsExpression, upper: JsExpression, kind: RangeKind, subject: JsExpression): JsExpression { + val lowerCheck = JsAstUtils.lessThanEq(lower, subject) + val upperCheck = when (kind) { + RangeKind.RANGE_TO -> JsAstUtils.lessThanEq(subject, upper) + RangeKind.UNTIL -> JsAstUtils.lessThan(subject, upper) + } + lowerCheck.synthetic = true + upperCheck.synthetic = true + return JsAstUtils.and(lowerCheck, upperCheck).apply { synthetic = true } + } + + private fun translateAsLongRangeCheck(lower: JsExpression, upper: JsExpression, kind: RangeKind, subject: JsExpression): JsExpression { + val lowerCheck = JsAstUtils.invokeMethod(lower, "lessThanOrEqual", subject).apply { sideEffects = SideEffectKind.PURE } + val upperCheck = when (kind) { + RangeKind.RANGE_TO -> JsAstUtils.invokeMethod(subject, "lessThanOrEqual", upper) + RangeKind.UNTIL -> JsAstUtils.invokeMethod(subject, "lessThan", upper) + } + upperCheck.sideEffects = SideEffectKind.PURE + return JsAstUtils.and(lowerCheck, upperCheck) + } +} + + diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/JsAstUtils.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/JsAstUtils.java index 678fd9296c1..a534164a134 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/JsAstUtils.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/JsAstUtils.java @@ -167,26 +167,31 @@ public final class JsAstUtils { } public static JsExpression newLong(long value) { + JsExpression result; if (value < Integer.MIN_VALUE || value > Integer.MAX_VALUE) { int low = (int) value; int high = (int) (value >> 32); List args = new SmartList<>(); args.add(new JsIntLiteral(low)); args.add(new JsIntLiteral(high)); - return new JsNew(Namer.kotlinLong(), args); + result = new JsNew(Namer.kotlinLong(), args); } else { if (value == 0) { - return new JsNameRef(Namer.LONG_ZERO, Namer.kotlinLong()); + result = new JsNameRef(Namer.LONG_ZERO, Namer.kotlinLong()); } else if (value == 1) { - return new JsNameRef(Namer.LONG_ONE, Namer.kotlinLong()); + result = new JsNameRef(Namer.LONG_ONE, Namer.kotlinLong()); } else if (value == -1) { - return new JsNameRef(Namer.LONG_NEG_ONE, Namer.kotlinLong()); + result = new JsNameRef(Namer.LONG_NEG_ONE, Namer.kotlinLong()); + } + else { + result = longFromInt(new JsIntLiteral((int) value)); } - return longFromInt(new JsIntLiteral((int) value)); } + MetadataProperties.setSideEffects(result, SideEffectKind.PURE); + return result; } @NotNull diff --git a/js/js.translator/testData/box/range/numberRangesOptimized.kt b/js/js.translator/testData/box/range/numberRangesOptimized.kt new file mode 100644 index 00000000000..340f4c61dc5 --- /dev/null +++ b/js/js.translator/testData/box/range/numberRangesOptimized.kt @@ -0,0 +1,72 @@ +// EXPECTED_REACHABLE_NODES: 1135 +// CHECK_CONTAINS_NO_CALLS: inRange +// CHECK_CONTAINS_NO_CALLS: inRange2 +// CHECK_CONTAINS_NO_CALLS: inRange3 +// CHECK_CONTAINS_NO_CALLS: inRange4 + +// CHECK_CONTAINS_NO_CALLS: inLongRange except=lessThanOrEqual;lessThan;fromInt +// CHECK_CONTAINS_NO_CALLS: inLongRange2 except=lessThanOrEqual;lessThan;fromInt +// CHECK_CONTAINS_NO_CALLS: inLongRange3 except=lessThanOrEqual;lessThan;fromInt +// CHECK_CONTAINS_NO_CALLS: inLongRange4 except=lessThanOrEqual;lessThan;fromInt +// CHECK_VARS_COUNT: function=inLongRange count=0 +// CHECK_VARS_COUNT: function=inLongRange2 count=0 +// CHECK_VARS_COUNT: function=inLongRange3 count=0 +// CHECK_VARS_COUNT: function=inLongRange4 count=0 + +fun inRange(x: Int) = x in 1..10 + +fun inRange2(x: Int) = x in 1.rangeTo(10) + +fun inRange3(x: Int) = x in 1 until 10 + +fun inRange4(x: Int) = 1.rangeTo(10).contains(x) + +fun inLongRange(x: Long) = x in 1L..10L + +fun inLongRange2(x: Long) = x in 1L.rangeTo(10L) + +fun inLongRange3(x: Long) = x in 1L until 10L + +fun inLongRange4(x: Long) = 1L.rangeTo(10L).contains(x) + +fun check(x: Int, inRangeTo: Boolean, inUntil: Boolean): String? { + val p = if (inRangeTo) "!" else "" + if (inRange(x) != inRangeTo) return "fail: $x ${p}in 1..10" + if (inRange2(x) != inRangeTo) return "fail: $x ${p}in 1.rangeTo(10)" + if (inRange4(x) != inRangeTo) return "fail: ${p}1.rangeTo(10).contains($x)" + + if (inRange3(x) != inUntil) return "fail: $x ${if (inUntil) "!" else ""}in 1 until 10" + + return null +} + +fun check(x: Long, inRangeTo: Boolean, inUntil: Boolean): String? { + val p = if (inRangeTo) "!" else "" + if (inLongRange(x) != inRangeTo) return "fail: ${x}L ${p}in 1L..10L" + if (inLongRange2(x) != inRangeTo) return "fail: ${x}L ${p}in 1L.rangeTo(10L)" + if (inLongRange4(x) != inRangeTo) return "fail: ${p}1L.rangeTo(10L).contains(${x}L)" + + if (inLongRange3(x) != inUntil) return "fail: ${x}L ${if (inUntil) "!" else ""}in 1L until 10L" + + return null +} + +fun box(): String { + // Int + check(5, true, true)?.let { return it } + check(9, true, true)?.let { return it } + check(10, true, false)?.let { return it } + check(11, false, false)?.let { return it } + check(1, true, true)?.let { return it } + check(0, false, false)?.let { return it } + + // Long + check(5L, true, true)?.let { return it } + check(9L, true, true)?.let { return it } + check(10L, true, false)?.let { return it } + check(11L, false, false)?.let { return it } + check(1L, true, true)?.let { return it } + check(0L, false, false)?.let { return it } + + return "OK" +} \ No newline at end of file diff --git a/js/js.translator/testData/js-optimizer/temporary-variable/shortCircuit.optimized.js b/js/js.translator/testData/js-optimizer/temporary-variable/shortCircuit.optimized.js new file mode 100644 index 00000000000..33e5aeafb15 --- /dev/null +++ b/js/js.translator/testData/js-optimizer/temporary-variable/shortCircuit.optimized.js @@ -0,0 +1,18 @@ +var log = ""; + +function id(x) { + log += x + ";"; + return x; +} + +function box() { + var $a = id(2); + var $b = id(3); + var $c = id(4); + var $d = id(5); + + if ($a > $b || $c > $d) return "fail condition"; + if (log !== "2;3;4;5;") return "fail log: " + log; + + return "OK"; +} \ No newline at end of file diff --git a/js/js.translator/testData/js-optimizer/temporary-variable/shortCircuit.original.js b/js/js.translator/testData/js-optimizer/temporary-variable/shortCircuit.original.js new file mode 100644 index 00000000000..33e5aeafb15 --- /dev/null +++ b/js/js.translator/testData/js-optimizer/temporary-variable/shortCircuit.original.js @@ -0,0 +1,18 @@ +var log = ""; + +function id(x) { + log += x + ";"; + return x; +} + +function box() { + var $a = id(2); + var $b = id(3); + var $c = id(4); + var $d = id(5); + + if ($a > $b || $c > $d) return "fail condition"; + if (log !== "2;3;4;5;") return "fail log: " + log; + + return "OK"; +} \ No newline at end of file