From 446c4c0a3356be94f0304225e4e019735cac9b64 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Mon, 19 Sep 2016 15:13:28 +0300 Subject: [PATCH] JS: provide overflow behaviour for int binary operations (see KT-7733) --- .../js/test/semantics/BoxJsTestGenerated.java | 6 +++ .../kotlin/js/translate/context/Namer.java | 4 ++ .../PrimitiveBinaryOperationFIF.java | 37 +++++++++++++++---- .../testData/box/inline/callableReference.kt | 2 +- .../testData/box/inline/inlineChain.kt | 2 +- .../inline/inlineChainWithFewStatements.kt | 2 +- .../testData/box/inline/inlineInc.kt | 2 +- .../box/inline/inlineLambdaWithCapture.kt | 2 +- .../simpleReturnFunction.kt | 2 +- .../testData/box/number/intOverflow.kt | 21 +++++++++++ js/js.translator/testData/kotlin_lib_ecma5.js | 7 ++++ 11 files changed, 73 insertions(+), 14 deletions(-) create mode 100644 js/js.translator/testData/box/number/intOverflow.kt 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 b23e73b0b21..b4df700bcc8 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 @@ -5999,6 +5999,12 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest { doTest(fileName); } + @TestMetadata("intOverflow.kt") + public void testIntOverflow() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/number/intOverflow.kt"); + doTest(fileName); + } + @TestMetadata("kt2342.kt") public void testKt2342() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/number/kt2342.kt"); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/Namer.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/Namer.java index e025607666e..c4a708517a2 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/Namer.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/Namer.java @@ -465,6 +465,10 @@ public final class Namer { return sb.toString(); } + public static JsNameRef imul() { + return pureFqn("imul", kotlinObject()); + } + public static boolean requiresEscaping(@NotNull String name) { // TODO: remove if there is existing implementation of this method // TODO: handle JavaScript keywords diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/functions/factories/PrimitiveBinaryOperationFIF.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/functions/factories/PrimitiveBinaryOperationFIF.java index 71abb412941..598d3601c85 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/functions/factories/PrimitiveBinaryOperationFIF.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/functions/factories/PrimitiveBinaryOperationFIF.java @@ -22,12 +22,14 @@ import com.google.common.collect.ImmutableMap; import com.google.dart.compiler.backend.js.ast.JsBinaryOperation; import com.google.dart.compiler.backend.js.ast.JsBinaryOperator; import com.google.dart.compiler.backend.js.ast.JsExpression; +import com.google.dart.compiler.backend.js.ast.JsInvocation; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.builtins.KotlinBuiltIns; import org.jetbrains.kotlin.descriptors.FunctionDescriptor; import org.jetbrains.kotlin.js.patterns.DescriptorPredicate; import org.jetbrains.kotlin.js.patterns.NamePredicate; +import org.jetbrains.kotlin.js.translate.context.Namer; import org.jetbrains.kotlin.js.translate.context.TranslationContext; import org.jetbrains.kotlin.js.translate.intrinsic.functions.basic.FunctionIntrinsic; import org.jetbrains.kotlin.js.translate.intrinsic.functions.basic.RangeToIntrinsic; @@ -61,13 +63,13 @@ public enum PrimitiveBinaryOperationFIF implements FunctionIntrinsicFactory { } } - @NotNull - private static final BinaryOperationIntrinsicBase INTEGER_DIVISION_INTRINSIC = new BinaryOperationIntrinsicBase() { + private static final BinaryOperationIntrinsicBase INT_MULTIPLICATION_INTRINSIC = new BinaryOperationIntrinsicBase() { @NotNull @Override - public JsExpression doApply(@NotNull JsExpression left, @NotNull JsExpression right, @NotNull TranslationContext context) { - JsBinaryOperation div = new JsBinaryOperation(JsBinaryOperator.DIV, left, right); - return JsAstUtils.toInt32(div); + public JsExpression doApply( + @NotNull JsExpression left, @NotNull JsExpression right, @NotNull TranslationContext context + ) { + return new JsInvocation(Namer.imul(), left, right); } }; @@ -91,6 +93,10 @@ public enum PrimitiveBinaryOperationFIF implements FunctionIntrinsicFactory { @NotNull private static final NamePredicate BINARY_OPERATIONS = new NamePredicate(OperatorNameConventions.BINARY_OPERATION_NAMES); + + private static final DescriptorPredicate INT_BINARY_OPERATIONS = pattern("Int.plus|minus|div(Int)"); + private static final DescriptorPredicate SIMPLE_INT_MULTIPLICATION = pattern("Byte|Short.times(Byte|Short)"); + private static final DescriptorPredicate INT_DIVISION = pattern("Byte|Short|Int.div(Byte|Short|Int)"); private static final DescriptorPredicate PRIMITIVE_NUMBERS_BINARY_OPERATIONS = pattern(NamePredicate.PRIMITIVE_NUMBERS_MAPPED_TO_PRIMITIVE_JS, BINARY_OPERATIONS); @@ -102,6 +108,7 @@ public enum PrimitiveBinaryOperationFIF implements FunctionIntrinsicFactory { ); private static final DescriptorPredicate BOOLEAN_OPERATIONS = pattern("Boolean.or|and|xor"); private static final DescriptorPredicate STRING_PLUS = pattern("String.plus"); + private static final DescriptorPredicate INT_MULTIPLICATION = pattern("Int.times(Int)"); private static final DescriptorPredicate CHAR_RANGE_TO = pattern("Char.rangeTo(Char)"); private static final DescriptorPredicate NUMBER_RANGE_TO = pattern("Byte|Short|Int.rangeTo(Byte|Short|Int)"); @@ -139,9 +146,8 @@ public enum PrimitiveBinaryOperationFIF implements FunctionIntrinsicFactory { return null; } - - if (pattern("Int|Short|Byte.div(Int|Short|Byte)").apply(descriptor)) { - return INTEGER_DIVISION_INTRINSIC; + if (INT_MULTIPLICATION.apply(descriptor)) { + return INT_MULTIPLICATION_INTRINSIC; } if (NUMBER_RANGE_TO.apply(descriptor)) { return new RangeToIntrinsic(descriptor); @@ -153,6 +159,9 @@ public enum PrimitiveBinaryOperationFIF implements FunctionIntrinsicFactory { } } JsBinaryOperator operator = getOperator(descriptor); + if (INT_BINARY_OPERATIONS.apply(descriptor) || SIMPLE_INT_MULTIPLICATION.apply(descriptor) || INT_DIVISION.apply(descriptor)) { + return new IntBinaryOperationFunctionIntrinsic(operator); + } BinaryOperationIntrinsicBase result = new PrimitiveBinaryOperationFunctionIntrinsic(operator); if (pattern("Char.plus|minus(Int)").apply(descriptor)) { @@ -196,6 +205,18 @@ public enum PrimitiveBinaryOperationFIF implements FunctionIntrinsicFactory { } } + private static class IntBinaryOperationFunctionIntrinsic extends PrimitiveBinaryOperationFunctionIntrinsic { + private IntBinaryOperationFunctionIntrinsic(@NotNull JsBinaryOperator operator) { + super(operator); + } + + @NotNull + @Override + public JsExpression doApply(@NotNull JsExpression left, @NotNull JsExpression right, @NotNull TranslationContext context) { + return JsAstUtils.toInt32(super.doApply(left, right, context)); + } + } + private static class CharAndIntBinaryOperationFunctionIntrinsic extends BinaryOperationIntrinsicBase { @NotNull diff --git a/js/js.translator/testData/box/inline/callableReference.kt b/js/js.translator/testData/box/inline/callableReference.kt index e491082861d..67c22a7c6b2 100644 --- a/js/js.translator/testData/box/inline/callableReference.kt +++ b/js/js.translator/testData/box/inline/callableReference.kt @@ -1,6 +1,6 @@ package foo -// CHECK_CONTAINS_NO_CALLS: multiplyInline +// CHECK_CONTAINS_NO_CALLS: multiplyInline except=imul // CHECK_NOT_CALLED: runNoinline internal inline fun multiply(a: Int, b: Int) = a * b diff --git a/js/js.translator/testData/box/inline/inlineChain.kt b/js/js.translator/testData/box/inline/inlineChain.kt index 03a0275c305..f3c352bbbb6 100644 --- a/js/js.translator/testData/box/inline/inlineChain.kt +++ b/js/js.translator/testData/box/inline/inlineChain.kt @@ -1,6 +1,6 @@ package foo -// CHECK_CONTAINS_NO_CALLS: squareMultipliedByTwo +// CHECK_CONTAINS_NO_CALLS: squareMultipliedByTwo except=imul internal inline fun inline1(a: Int): Int { return a diff --git a/js/js.translator/testData/box/inline/inlineChainWithFewStatements.kt b/js/js.translator/testData/box/inline/inlineChainWithFewStatements.kt index 01d4c501dde..231157da1c9 100644 --- a/js/js.translator/testData/box/inline/inlineChainWithFewStatements.kt +++ b/js/js.translator/testData/box/inline/inlineChainWithFewStatements.kt @@ -1,6 +1,6 @@ package foo -// CHECK_CONTAINS_NO_CALLS: squareMultipliedByTwo +// CHECK_CONTAINS_NO_CALLS: squareMultipliedByTwo except=imul internal inline fun inline1(a: Int): Int { return a diff --git a/js/js.translator/testData/box/inline/inlineInc.kt b/js/js.translator/testData/box/inline/inlineInc.kt index f90c737cddf..883841349fd 100644 --- a/js/js.translator/testData/box/inline/inlineInc.kt +++ b/js/js.translator/testData/box/inline/inlineInc.kt @@ -1,6 +1,6 @@ package foo -// CHECK_CONTAINS_NO_CALLS: multiplyNoInline +// CHECK_CONTAINS_NO_CALLS: multiplyNoInline except=imul internal inline fun multiply(a: Int, b: Int): Int { return a * b diff --git a/js/js.translator/testData/box/inline/inlineLambdaWithCapture.kt b/js/js.translator/testData/box/inline/inlineLambdaWithCapture.kt index 2711c5b9728..97e7dc1185d 100644 --- a/js/js.translator/testData/box/inline/inlineLambdaWithCapture.kt +++ b/js/js.translator/testData/box/inline/inlineLambdaWithCapture.kt @@ -1,6 +1,6 @@ package foo -// CHECK_CONTAINS_NO_CALLS: maxBySquare +// CHECK_CONTAINS_NO_CALLS: maxBySquare except=imul internal data class Result(var value: Int = 0, var invocationCount: Int = 0) diff --git a/js/js.translator/testData/box/inlineSizeReduction/simpleReturnFunction.kt b/js/js.translator/testData/box/inlineSizeReduction/simpleReturnFunction.kt index ce7adf0078d..48c7cc06810 100644 --- a/js/js.translator/testData/box/inlineSizeReduction/simpleReturnFunction.kt +++ b/js/js.translator/testData/box/inlineSizeReduction/simpleReturnFunction.kt @@ -1,6 +1,6 @@ package foo -// CHECK_CONTAINS_NO_CALLS: test +// CHECK_CONTAINS_NO_CALLS: test except=imul // CHECK_VARS_COUNT: function=test count=0 // A copy of stdlib run function. diff --git a/js/js.translator/testData/box/number/intOverflow.kt b/js/js.translator/testData/box/number/intOverflow.kt new file mode 100644 index 00000000000..041d9de23c9 --- /dev/null +++ b/js/js.translator/testData/box/number/intOverflow.kt @@ -0,0 +1,21 @@ +package foo + +fun bigValue() = 0x7FFFFFFC + +fun mediumValue() = 0x12345 + +fun box(): String { + var v = bigValue() + 1 + if (v != 0x7FFFFFFD) return "fail1: $v" + + v = bigValue() + 8 + if (v != -0x7FFFFFFC) return "fail2: $v" + + v = mediumValue() * 0x23456 + if (v != -2112496338) return "fail3: $v" + + v = bigValue() * bigValue() + if (v != 16) return "fail4: $v" + + return "OK" +} \ No newline at end of file diff --git a/js/js.translator/testData/kotlin_lib_ecma5.js b/js/js.translator/testData/kotlin_lib_ecma5.js index 004b6ecbb44..32f04a6058e 100644 --- a/js/js.translator/testData/kotlin_lib_ecma5.js +++ b/js/js.translator/testData/kotlin_lib_ecma5.js @@ -313,4 +313,11 @@ Kotlin.kotlinModuleMetadata = function (abiVersion, moduleName, data) { }; + Kotlin.imul = Math.imul || function (a, b) { + var ah = (a >>> 16) & 0xffff; + var al = a & 0xffff; + var bh = (b >>> 16) & 0xffff; + var bl = b & 0xffff; + return ((al * bl) + (((ah * bl + al * bh) << 16) >>> 0) | 0); + }; })();