Port a couple more stubs to x64.

runtime/lib/integers.cc

Index: runtime/lib/integers.cc
===================================================================
--- runtime/lib/integers.cc	(revision 3226)
+++ runtime/lib/integers.cc	(working copy)
@@ -1,4 +1,4 @@
-// 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.
 
@@ -68,6 +68,20 @@
 }
 
 
+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) {
@@ -204,29 +218,22 @@
 
 // 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();
@@ -270,37 +277,24 @@
 }
 
 
-// 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) {
@@ -308,8 +302,6 @@
         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: {
@@ -459,7 +451,7 @@
   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();
       }
@@ -477,11 +469,13 @@
       }
       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();
   }