implement fast ReturnValue setters

include/v8.h

Index: include/v8.h
diff --git a/include/v8.h b/include/v8.h
index 19a8ad8841a82d542be1ed4ad1715099111de126..0df2cf92ede8827f033513a51e2f8d0d8b8766cd 100644
--- a/include/v8.h
+++ b/include/v8.h
@@ -1879,6 +1879,7 @@ class V8EXPORT Number : public Primitive {
  public:
   double Value() const;
   static Local<Number> New(double value);
+  static Local<Number> New(Isolate* isolate, double value);
   V8_INLINE(static Number* Cast(v8::Value* obj));
  private:
   Number();
@@ -2740,16 +2741,15 @@ class V8EXPORT ReturnValue {
   // Handle setters
   V8_INLINE(void Set(const Persistent<T>& handle));
   V8_INLINE(void Set(const Handle<T> handle));
-  // TODO(dcarney): implement
   // Fast primitive setters
-//  V8_INLINE(void Set(Isolate* isolate, bool));
-//  V8_INLINE(void Set(Isolate* isolate, float i));
-//  V8_INLINE(void Set(Isolate* isolate, double i));
-//  V8_INLINE(void Set(Isolate* isolate, int32_t i));
-//  V8_INLINE(void Set(Isolate* isolate, uint32_t i));
+  V8_INLINE(void Set(Isolate* isolate, bool value));
+  V8_INLINE(void Set(Isolate* isolate, float i));
+  V8_INLINE(void Set(Isolate* isolate, double i));
+  V8_INLINE(void Set(Isolate* isolate, int32_t i));
+  V8_INLINE(void Set(Isolate* isolate, uint32_t i));
   // Fast JS primitive setters
-//  V8_INLINE(void SetNull(Isolate* isolate));
-//  V8_INLINE(void SetUndefined(Isolate* isolate));
+  V8_INLINE(void SetNull(Isolate* isolate));
+  V8_INLINE(void SetUndefined(Isolate* isolate));
  private:
   internal::Object** value_;
 };
@@ -5141,6 +5141,14 @@ const intptr_t kSmiTagMask = (1 << kSmiTagSize) - 1;
 
 template <size_t ptr_size> struct SmiTagging;
 
+template<int kSmiShiftSize>
+V8_INLINE(internal::Object* IntToSmi(int value)) {

[Sven Panne, 2013/05/21 14:14:56]

I think returning internal::Smi* (here and below) might be more handy and would
avoid some casts later. Or is there a reason why we can't do this?

[dcarney, 2013/05/21 15:14:44]

it makes casting in v8.h much worse, since we can't know that Smi inherits from
Object here

+  int smi_shift_bits = kSmiTagSize + kSmiShiftSize;
+  intptr_t tagged_value =
+      (static_cast<intptr_t>(value) << smi_shift_bits) | kSmiTag;
+  return reinterpret_cast<internal::Object*>(tagged_value);
+}
+
 // Smi constants for 32-bit systems.
 template <> struct SmiTagging<4> {
   static const int kSmiShiftSize = 0;
@@ -5150,6 +5158,23 @@ template <> struct SmiTagging<4> {
     // Throw away top 32 bits and shift down (requires >> to be sign extending).
     return static_cast<int>(reinterpret_cast<intptr_t>(value)) >> shift_bits;
   }
+  V8_INLINE(static internal::Object* IntToSmi(int value)) {
+    return internal::IntToSmi<kSmiShiftSize>(value);
+  }
+  V8_INLINE(static bool IsValidSmi(intptr_t value)) {
+    // To be representable as an tagged small integer, the two
+    // most-significant bits of 'value' must be either 00 or 11 due to
+    // sign-extension. To check this we add 01 to the two
+    // most-significant bits, and check if the most-significant bit is 0
+    //
+    // CAUTION: The original code below:
+    // bool result = ((value + 0x40000000) & 0x80000000) == 0;
+    // may lead to incorrect results according to the C language spec, and
+    // in fact doesn't work correctly with gcc4.1.1 in some cases: The
+    // compiler may produce undefined results in case of signed integer
+    // overflow. The computation must be done w/ unsigned ints.
+    return static_cast<uintptr_t>(value + 0x40000000U) < 0x80000000U;
+  }
 };
 
 // Smi constants for 64-bit systems.
@@ -5161,6 +5186,13 @@ template <> struct SmiTagging<8> {
     // Shift down and throw away top 32 bits.
     return static_cast<int>(reinterpret_cast<intptr_t>(value) >> shift_bits);
   }
+  V8_INLINE(static internal::Object* IntToSmi(int value)) {
+    return internal::IntToSmi<kSmiShiftSize>(value);
+  }
+  V8_INLINE(static bool IsValidSmi(intptr_t value)) {
+    // To be representable as a long smi, the value must be a 32-bit integer.
+    return (value == static_cast<int32_t>(value));
+  }
 };
 
 typedef SmiTagging<kApiPointerSize> PlatformSmiTagging;
@@ -5225,6 +5257,14 @@ class Internals {
     return PlatformSmiTagging::SmiToInt(value);
   }
 
+  V8_INLINE(static internal::Object* IntToSmi(int value)) {
+    return PlatformSmiTagging::IntToSmi(value);
+  }
+
+  V8_INLINE(static bool IsValidSmi(intptr_t value)) {
+    return PlatformSmiTagging::IsValidSmi(value);
+  }
+
   V8_INLINE(static int GetInstanceType(internal::Object* obj)) {
     typedef internal::Object O;
     O* map = ReadField<O*>(obj, kHeapObjectMapOffset);
@@ -5600,6 +5640,7 @@ uint16_t Persistent<T>::WrapperClassId(Isolate* isolate) const {
   return *reinterpret_cast<uint16_t*>(addr);
 }
 
+
 template<typename T>
 ReturnValue<T>::ReturnValue(internal::Object** slot) : value_(slot) {}
 
@@ -5614,6 +5655,56 @@ void ReturnValue<T>::Set(const Handle<T> handle) {
 }
 
 template<typename T>
+void ReturnValue<T>::Set(Isolate* isolate, float i) {

[Sven Panne, 2013/05/21 14:14:56]

Why do we need this function if we convert to double, anyway?

[dcarney, 2013/05/21 15:14:44]

removed

+  Set(isolate, static_cast<double>(i));
+}
+
+template<typename T>
+void ReturnValue<T>::Set(Isolate* isolate, double i) {
+  Set(Number::New(isolate, i));
+}
+
+template<typename T>
+void ReturnValue<T>::Set(Isolate* isolate, int32_t i) {
+  typedef internal::Internals I;
+  if (V8_LIKELY(I::IsValidSmi(i))) {
+    *value_ = I::IntToSmi(i);
+    return;
+  }
+  Set(Integer::New(i, isolate));
+}
+
+template<typename T>
+void ReturnValue<T>::Set(Isolate* isolate, uint32_t i) {
+  typedef internal::Internals I;
+  if (V8_LIKELY(I::IsValidSmi(i))) {
+    *value_ = I::IntToSmi(i);
+    return;
+  }
+  Set(Integer::NewFromUnsigned(i, isolate));
+}
+
+template<typename T>
+void ReturnValue<T>::Set(Isolate* isolate, bool value) {
+  typedef internal::Internals I;
+  *value_ = *I::GetRoot(
+      isolate, value ? I::kTrueValueRootIndex : I::kFalseValueRootIndex);
+}
+
+template<typename T>
+void ReturnValue<T>::SetNull(Isolate* isolate) {
+  typedef internal::Internals I;
+  *value_ = *I::GetRoot(isolate, I::kNullValueRootIndex);
+}
+
+template<typename T>
+void ReturnValue<T>::SetUndefined(Isolate* isolate) {
+  typedef internal::Internals I;
+  *value_ = *I::GetRoot(isolate, I::kUndefinedValueRootIndex);
+}
+
+
+template<typename T>
 FunctionCallbackInfo<T>::FunctionCallbackInfo(internal::Object** implicit_args,
                                               internal::Object** values,
                                               int length,