From 30ef5c19d08bfd2e143a68efc702af388c59d734 Mon Sep 17 00:00:00 2001 From: "floitsch@google.com" Date: Thu, 3 May 2012 12:50:21 +0000 Subject: [PATCH] Make bit-operations return a positive result. Fixes issue 2725 (as much as we can fix it). Review URL: https://chromiumcodereview.appspot.com//10343002 git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@7278 260f80e4-7a28-3924-810f-c04153c831b5 --- .../implementation/lib/js_helper.dart | 14 ++- lib/compiler/implementation/ssa/codegen.dart | 38 +++++- tests/language/language.status | 1 + .../positive_bit_operations_test.dart | 115 ++++++++++++++++++ tests/standalone/standalone.status | 1 - 5 files changed, 157 insertions(+), 12 deletions(-) create mode 100644 tests/language/positive_bit_operations_test.dart diff --git a/lib/compiler/implementation/lib/js_helper.dart b/lib/compiler/implementation/lib/js_helper.dart index f818cdba5b8..55d645e9da4 100644 --- a/lib/compiler/implementation/lib/js_helper.dart +++ b/lib/compiler/implementation/lib/js_helper.dart @@ -163,7 +163,7 @@ shl(var a, var b) { // TODO(floitsch): inputs must be integers. if (checkNumbers(a, b)) { if (b < 0) throw new IllegalArgumentException(b); - return JS('num', @'# << #', a, b); + return JS('num', @'(# << #) >>> 0', a, b); } return UNINTERCEPTED(a << b); } @@ -172,7 +172,7 @@ shr(var a, var b) { // TODO(floitsch): inputs must be integers. if (checkNumbers(a, b)) { if (b < 0) throw new IllegalArgumentException(b); - return JS('num', @'# >> #', a, b); + return JS('num', @'# >>> #', a, b); } return UNINTERCEPTED(a >> b); } @@ -180,7 +180,7 @@ shr(var a, var b) { and(var a, var b) { // TODO(floitsch): inputs must be integers. if (checkNumbers(a, b)) { - return JS('num', @'# & #', a, b); + return JS('num', @'(# & #) >>> 0', a, b); } return UNINTERCEPTED(a & b); } @@ -188,7 +188,7 @@ and(var a, var b) { or(var a, var b) { // TODO(floitsch): inputs must be integers. if (checkNumbers(a, b)) { - return JS('num', @'# | #', a, b); + return JS('num', @'(# | #) >>> 0', a, b); } return UNINTERCEPTED(a | b); } @@ -196,13 +196,15 @@ or(var a, var b) { xor(var a, var b) { // TODO(floitsch): inputs must be integers. if (checkNumbers(a, b)) { - return JS('num', @'# ^ #', a, b); + return JS('num', @'(# ^ #) >>> 0', a, b); } return UNINTERCEPTED(a ^ b); } not(var a) { - if (JS('bool', @'typeof # === "number"', a)) return JS('num', @'~#', a); + if (JS('bool', @'typeof # === "number"', a)) { + return JS('num', @'(~#) >>> 0', a); + } return UNINTERCEPTED(~a); } diff --git a/lib/compiler/implementation/ssa/codegen.dart b/lib/compiler/implementation/ssa/codegen.dart index c97a88ec2ce..31f1796a790 100644 --- a/lib/compiler/implementation/ssa/codegen.dart +++ b/lib/compiler/implementation/ssa/codegen.dart @@ -156,6 +156,7 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor { Element boolifiedEqualsNullElement; int indent = 0; int expectedPrecedence = JSPrecedence.STATEMENT_PRECEDENCE; + JSBinaryOperatorPrecedence unsignedShiftPrecedences; HGraph currentGraph; /** * Whether the code-generation should try to generate an expression @@ -194,7 +195,8 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor { logicalOperations = new Map(), breakAction = new Map(), continueAction = new Map(), - phiEquivalence = new Equivalence() { + phiEquivalence = new Equivalence(), + unsignedShiftPrecedences = JSPrecedence.binary['>>>'] { for (final name in parameterNames.getValues()) { prefixes[name] = 0; @@ -1111,6 +1113,19 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor { } } + // We want the outcome of bit-operations to be positive. We use the unsigned + // shift operator to achieve this. + visitBitInvokeBinary(HBinaryBitOp node, String op) { + if (node.builtin){ + beginExpression(unsignedShiftPrecedences.precedence); + visitInvokeBinary(node, op); + buffer.add(' >>> 0'); + endExpression(unsignedShiftPrecedences.precedence); + } else { + visitInvokeBinary(node, op); + } + } + visitInvokeUnary(HInvokeUnary node, String op) { if (node.builtin) { beginExpression(JSPrecedence.PREFIX_PRECEDENCE); @@ -1122,6 +1137,19 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor { } } + // We want the outcome of bit-operations to be positive. We use the unsigned + // shift operator to achieve this. + visitBitInvokeUnary(HInvokeUnary node, String op) { + if (node.builtin){ + beginExpression(unsignedShiftPrecedences.precedence); + visitInvokeUnary(node, op); + buffer.add(' >>> 0'); + endExpression(unsignedShiftPrecedences.precedence); + } else { + visitInvokeUnary(node, op); + } + } + visitEquals(HEquals node) { if (node.builtin) { beginExpression(JSPrecedence.EQUALITY_PRECEDENCE); @@ -1151,10 +1179,10 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor { // Modulo cannot be mapped to the native operator (different semantics). visitModulo(HModulo node) => visitInvokeStatic(node); - visitBitAnd(HBitAnd node) => visitInvokeBinary(node, '&'); - visitBitNot(HBitNot node) => visitInvokeUnary(node, '~'); - visitBitOr(HBitOr node) => visitInvokeBinary(node, '|'); - visitBitXor(HBitXor node) => visitInvokeBinary(node, '^'); + visitBitAnd(HBitAnd node) => visitBitInvokeBinary(node, '&'); + visitBitNot(HBitNot node) => visitBitInvokeUnary(node, '~'); + visitBitOr(HBitOr node) => visitBitInvokeBinary(node, '|'); + visitBitXor(HBitXor node) => visitBitInvokeBinary(node, '^'); // We need to check if the left operand is negative in order to use // the native operator. diff --git a/tests/language/language.status b/tests/language/language.status index 4c28bac8340..57cfc40d176 100644 --- a/tests/language/language.status +++ b/tests/language/language.status @@ -191,6 +191,7 @@ compile_time_constant_k_test: Fail # Constant maps are not canonicalized correct compile_time_constant_l_test: Fail # final instance fields with default values are not initialized correctly. compile_time_constant_o_test: Fail # String constants with string-interpolation. compile_time_constant_p_test/01: Fail # Implicit super calls that don't match are not caught in compile-time constants. +positive_bit_operations_test: Fail # Frog goes negative. disable_privacy_test: Skip # Issue 1882: Needs --disable_privacy support. double_to_string_as_exponential_test: Fail # fails due to -0.0. # Once the -0.0 in double_to_string_as_fixed_test is fixed it will probably need to diff --git a/tests/language/positive_bit_operations_test.dart b/tests/language/positive_bit_operations_test.dart new file mode 100644 index 00000000000..730d5cf861c --- /dev/null +++ b/tests/language/positive_bit_operations_test.dart @@ -0,0 +1,115 @@ +// Copyright (c) 2012, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +constants() { + Expect.equals(0x80000000, 0x80000000 | 0); + Expect.equals(0x80000001, 0x80000000 | 1); + Expect.equals(0x80000000, 0x80000000 | 0x80000000); + Expect.equals(0xFFFFFFFF, 0xFFFF0000 | 0xFFFF); + Expect.equals(0x80000000, 0x80000000 & 0xFFFFFFFF); + Expect.equals(0x80000000, 0x80000000 & 0x80000000); + Expect.equals(0x80000000, 0x80000000 & 0xF0000000); + Expect.equals(0x80000000, 0xFFFFFFFF & 0x80000000); + Expect.equals(0x80000000, 0x80000000 ^ 0); + Expect.equals(0xFFFFFFFF, 0x80000000 ^ 0x7FFFFFFF); + Expect.equals(0xFFFFFFFF, 0x7FFFFFFF ^ 0x80000000); + Expect.equals(0xF0000000, 0x70000000 ^ 0x80000000); + Expect.equals(0x80000000, 1 << 31); + Expect.equals(0xFFFFFFF0, 0xFFFFFFF << 4); + Expect.equals(0x7FFFFFFF, 0xFFFFFFFF >> 1); + Expect.equals(0xFFFFFFFC, + ((((((0xFFFFFFF << 4) // 0xFFFFFFF0 + >> 1) // 0x7FFFFFF8 + | 0x80000000) // 0xFFFFFFF8 + >> 2) // 0x3FFFFFFE + ^ 0x40000000) // 0x7FFFFFFE + << 1)); +} + +foo(i) { + if (i != 0) { + y--; + foo(i - 1); + y++; + } +} + +var y; + +// id returns [x] in a way that should be difficult to predict statically. +id(x) { + y = x; + foo(10); + return y; +} + +interceptors() { + Expect.equals(0x80000000, id(0x80000000) | id(0)); + Expect.equals(0x80000001, id(0x80000000) | id(1)); + Expect.equals(0x80000000, id(0x80000000) | id(0x80000000)); + Expect.equals(0xFFFFFFFF, id(0xFFFF0000) | id(0xFFFF)); + Expect.equals(0x80000000, id(0x80000000) & id(0xFFFFFFFF)); + Expect.equals(0x80000000, id(0x80000000) & id(0x80000000)); + Expect.equals(0x80000000, id(0x80000000) & id(0xF0000000)); + Expect.equals(0x80000000, id(0xFFFFFFFF) & id(0x80000000)); + Expect.equals(0x80000000, id(0x80000000) ^ id(0)); + Expect.equals(0xFFFFFFFF, id(0x80000000) ^ id(0x7FFFFFFF)); + Expect.equals(0xFFFFFFFF, id(0x7FFFFFFF) ^ id(0x80000000)); + Expect.equals(0xF0000000, id(0x70000000) ^ id(0x80000000)); + Expect.equals(0x80000000, id(1) << id(31)); + Expect.equals(0xFFFFFFF0, id(0xFFFFFFF) << id(4)); + Expect.equals(0x7FFFFFFF, id(0xFFFFFFFF) >> id(1)); + Expect.equals(0xFFFFFFFC, + ((((((id(0xFFFFFFF) << 4) // 0xFFFFFFF0 + >> 1) // 0x7FFFFFF8 + | 0x80000000) // 0xFFFFFFF8 + >> 2) // 0x3FFFFFFE + ^ 0x40000000) // 0x7FFFFFFE + << 1)); +} + +speculative() { + var a = id(0x80000000); + var b = id(0); + var c = id(1); + var d = id(0xFFFF0000); + var e = id(0xFFFF); + var f = id(0xFFFFFFFF); + var g = id(0xF0000000); + var h = id(0x7FFFFFFF); + var j = id(0x70000000); + var k = id(31); + var l = id(4); + var m = id(0xFFFFFFF); + for (int i = 0; i < 1; i++) { + Expect.equals(0x80000000, a | b); + Expect.equals(0x80000001, a | c); + Expect.equals(0x80000000, a | a); + Expect.equals(0xFFFFFFFF, d | e); + Expect.equals(0x80000000, a & f); + Expect.equals(0x80000000, a & a); + Expect.equals(0x80000000, a & g); + Expect.equals(0x80000000, f & a); + Expect.equals(0x80000000, a ^ b); + Expect.equals(0xFFFFFFFF, a ^ h); + Expect.equals(0xFFFFFFFF, h ^ a); + Expect.equals(0xF0000000, j ^ a); + Expect.equals(0x80000000, c << k); + Expect.equals(0xFFFFFFF0, m << l); + Expect.equals(0x7FFFFFFF, f >> c); + Expect.equals(0xFFFFFFFC, + ((((((m << 4) // 0xFFFFFFF0 + >> 1) // 0x7FFFFFF8 + | 0x80000000) // 0xFFFFFFF8 + >> 2) // 0x3FFFFFFE + ^ 0x40000000) // 0x7FFFFFFE + << 1)); + } +} + +main() { + constants(); + interceptors(); + speculative(); +} diff --git a/tests/standalone/standalone.status b/tests/standalone/standalone.status index 9321bbc5340..efe3a0cd079 100644 --- a/tests/standalone/standalone.status +++ b/tests/standalone/standalone.status @@ -65,7 +65,6 @@ deoptimization_test: Fail, OK # Requires bigint. out_of_memory_test: Fail, OK # d8 handles much larger arrays than Dart VM. io/http_parser_test: Fail, OK # ByteArray io/options_test: Fail, OK # Cannot pass options to d8. -crypto/sha1_test: Skip [ $compiler == dart2js && $runtime == none ] fail_test: Skip # fails only at runtime, overrides default expectation from standalone.status