Port a couple more stubs to x64.

runtime/lib/integers.cc

Index: runtime/lib/integers.cc
===================================================================
--- runtime/lib/integers.cc	(revision 3261)
+++ 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;
+  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.

[Ivan Posva, 2012/01/13 06:27:40]

I am a bit concerned how this logic is split up between the two different areas.
Either we handle all multiplication of two Smi inputs here or we leave it at the
bottom. At the moment the logic is spread into too many places.

Personally, I would prefer to handle all operations of two Smis here, but that
means that this helper function needs to change its signature and return a
RawInteger*.

Actually you might want to change the signature in such a way that left and
right are expected to be int64_t with a value that fits in 63-bits. Which would
make the code dispatching below even simpler.

[regis, 2012/01/13 18:57:31]

I have merged this helper into its caller, which makes the code cleaner and
easier to understand.

+      int64_t result_64 =
+          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;
   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 =
+          (right.Value() >= kBitsPerWord) ? (kBitsPerWord - 1) : right.Value();
+      result = left.Value() >> shift_amount;
+      break;
+    }
     default:
       UNIMPLEMENTED();
   }