Port a couple more stubs to x64.

runtime/lib/integers.cc

-// Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
+// 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.
 
 #include "vm/bootstrap_natives.h"
 
 #include "vm/bigint_operations.h"
 #include "vm/dart_entry.h"
 #include "vm/exceptions.h"
 #include "vm/native_entry.h"
 #include "vm/object.h"
@@ skipping 49 matching lines @@
     return big.raw();
   }
 }
 
 
 static bool Are64bitOperands(const Integer& op1, const Integer& op2) {
   return !op1.IsBigint() && !op2.IsBigint();
 }
 
 
+static bool Are63bitOperands(const Integer& op1, const Integer& op2) {
+  if (op1.IsBigint() || op2.IsBigint()) {
+    return false;
+  }
+  const int64_t limit = static_cast<int64_t>(1) << 62;

[Ivan Posva, 2012/01/12 22:27:31]

Please use (static_cast<int64_t>(1)) to get rid of any questions about
precedence between cast and << operators.

[regis, 2012/01/12 22:53:37]

Done.

+  const int64_t value1 = op1.AsInt64Value();
+  if ((-limit > value1) || (value1 >= limit)) {
+    return false;
+  }
+  const int64_t value2 = op2.AsInt64Value();
+  return (-limit <= value2) && (value2 < limit);
+}
+
+
 static RawInteger* IntegerBitOperation(Token::Kind kind,
                                        const Integer& op1_int,
                                        const Integer& op2_int) {
   if (op1_int.IsSmi() && op2_int.IsSmi()) {
     Smi& op1 = Smi::Handle();
     Smi& op2 = Smi::Handle();
     op1 ^= op1_int.raw();
     op2 ^= op2_int.raw();
     intptr_t result = 0;
     switch (kind) {
@@ skipping 116 matching lines @@
         left.ToCString(), right.ToCString());
   }
   Integer& result = Integer::Handle(
       IntegerBitOperation(Token::kBIT_XOR, left, right));
   arguments->SetReturn(Integer::Handle(AsInteger(result)));
 }
 
 
 // The result is invalid if it is outside the range
 // Smi::kMinValue..Smi::kMaxValue.
+// Note that the 64-bit result of adding or subtracting 63-bit signed values
+// cannot overflow.
 static int64_t BinaryOpWithTwoSmis(Token::Kind operation,
                                    const Smi& left,
                                    const Smi& right) {
   switch (operation) {
     case Token::kADD:
-      // TODO(regis): We may need an overflow check in 64-bit mode. Revisit.
       return left.Value() + right.Value();
     case Token::kSUB:
       return left.Value() - right.Value();
     case Token::kMUL: {
-      // TODO(regis): Using Bigint here may be a performance issue. Revisit.
-      const Bigint& big_left =
-          Bigint::Handle(BigintOperations::NewFromSmi(left));
-      const Bigint& big_right =
-          Bigint::Handle(BigintOperations::NewFromSmi(right));
-      const Bigint& big_result =
-          Bigint::Handle(BigintOperations::Multiply(big_left, big_right));
-      if (BigintOperations::FitsIntoInt64(big_result)) {
-        return BigintOperations::ToInt64(big_result);
-      } else {
-        // Overflow, return an invalid Smi.
-        return Smi::kMaxValue + 1;
-      }
+      ASSERT(Smi::kBits < 32);  // Do not use this code in 64-bit mode.
+      int64_t result_64 =

[Ivan Posva, 2012/01/12 22:27:31]

result_64 => result

[regis, 2012/01/12 22:53:37]

Done.

+          static_cast<int64_t>(left.Value()) *
+          static_cast<int64_t>(right.Value());
+      return result_64;
     }
     case Token::kTRUNCDIV:
       return left.Value() / right.Value();
     case Token::kMOD: {
       intptr_t remainder = left.Value() % right.Value();
       if (remainder < 0) {
         if (right.Value() < 0) {
           return remainder - right.Value();
         } else {
           return remainder + right.Value();
@@ skipping 23 matching lines @@
       return BigintOperations::Divide(left, right);
     case Token::kMOD:
       return BigintOperations::Modulo(left, right);
     default:
       UNIMPLEMENTED();
       return Bigint::null();
   }
 }
 
 
-// TODO(srdjan): Implement correct overflow checking before allowing 64 bit
-// operands.
-static bool AreBoth64bitOperands(const Integer& op1, const Integer& op2) {
-  return false;
-}
-
-
 static RawInteger* IntegerBinopHelper(Token::Kind operation,
                                       const Integer& left_int,
                                       const Integer& right_int) {
-  if (left_int.IsSmi() && right_int.IsSmi()) {
+  // The result of any operation (except multiplication in 64-bit mode) between
+  // two Smi will always fit in a 64-bit signed result (no overflow).
+  if (((Smi::kBits < 32) || (operation != Token::kMUL)) &&
+      left_int.IsSmi() && right_int.IsSmi()) {
     Smi& left_smi = Smi::Handle();
     Smi& right_smi = Smi::Handle();
     left_smi ^= left_int.raw();
     right_smi ^= right_int.raw();
     int64_t result = BinaryOpWithTwoSmis(operation, left_smi, right_smi);
-    if (Smi::IsValid64(result)) {
-      return Smi::New(result);
-    } else {
-      // TODO(regis): This is not going to work on x64. Check other operations.
-#if defined(TARGET_ARCH_X64)
-      UNIMPLEMENTED();
-      return 0;
-#else
-      // Overflow to Mint.
-      return Mint::New(result);
-#endif
-    }
-  } else if (AreBoth64bitOperands(left_int, right_int)) {
-    // TODO(srdjan): Test for overflow of result instead of operand
-    // types.
+    return Integer::New(result);
+
+  } else if ((operation != Token::kMUL) &&
+             Are63bitOperands(left_int, right_int)) {
+      // The result of any operation (except multiplication) between two 63-bit
+      // signed integers will fit in a 64-bit signed result.
     const int64_t a = left_int.AsInt64Value();
     const int64_t b = right_int.AsInt64Value();
     switch (operation) {
       case Token::kADD:
         return Integer::New(a + b);
       case Token::kSUB:
         return Integer::New(a - b);
-      case Token::kMUL:
-        return Integer::New(a * b);
       case Token::kTRUNCDIV:
         return Integer::New(a / b);
       case Token::kMOD: {
         int64_t remainder = a % b;
         int64_t c = 0;
         if (remainder < 0) {
           if (b < 0) {
             c = remainder - b;
           } else {
             c = remainder + b;
@@ skipping 127 matching lines @@
   }
   return count;
 }
 
 
 // TODO(srdjan): Clarify handling of negative right operand in a shift op.
 static RawInteger* SmiShiftOperation(Token::Kind kind,
                                      const Smi& left,
                                      const Smi& right) {
   ASSERT(right.Value() >= 0);
   intptr_t result = 0;

[Ivan Posva, 2012/01/12 22:27:31]

How about adding
intptr_t left_value = left.Value();
intptr_t right_value = right.Value();
to avoid many repetitive calls to get the value out of the Smis?

[regis, 2012/01/12 22:53:37]

Done here and in BinaryOpWithTwoSmis.

   switch (kind) {
-    case Token::kSHL:
+    case Token::kSHL: {
       if ((left.Value() == 0) || (right.Value() == 0)) {
         return left.raw();
       }
       { // Check for overflow.
         int cnt = HighestBit(left.Value());
         if ((cnt + right.Value()) >= Smi::kBits) {
           if ((cnt + right.Value()) >= Mint::kBits) {
             return BigintOperations::ShiftLeft(
                 Bigint::Handle(AsBigint(left)), right.Value());
           } else {
             int64_t left_64 = left.Value();
             return Integer::New(left_64 << right.Value());
           }
         }
       }
       result = left.Value() << right.Value();
       break;
+    }
     case Token::kSAR: {
-        int shift_amount = (right.Value() > 0x1F) ? 0x1F : right.Value();
-        result = left.Value() >> shift_amount;
-        break;
-      }
+      const int shift_amount =

[Ivan Posva, 2012/01/12 22:27:31]

intptr_t as you are assigning right.Value() to this.

[regis, 2012/01/12 22:53:37]

Done.

+          (right.Value() >= kBitsPerWord) ? (kBitsPerWord - 1) : right.Value();
+      result = left.Value() >> shift_amount;
+      break;
+    }
     default:
       UNIMPLEMENTED();
   }
   ASSERT(Smi::IsValid(result));
   return Smi::New(result);
 }
 
 
 static RawInteger* ShiftOperationHelper(Token::Kind kind,
                                         const Integer& value,
@@ skipping 94 matching lines @@
 
 DEFINE_NATIVE_ENTRY(Bigint_bitNegate, 1) {
   const Bigint& value = Bigint::CheckedHandle(arguments->At(0));
   const Bigint& result = Bigint::Handle(BigintOperations::BitNot(value));
   ASSERT(CheckInteger(value));
   ASSERT(CheckInteger(result));
   arguments->SetReturn(Integer::Handle(AsInteger(result)));
 }
 
 }  // namespace dart