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;
+  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.

[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();
     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 129 matching lines @@
 }
 
 
 // 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;
   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 =
+          (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