implement fast ReturnValue setters

R=svenpanne@chromium.org
BUG=

Review URL: https://codereview.chromium.org/15398008

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14738 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

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,14 @@ 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, 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 +5140,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)) {
+  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 +5157,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 +5185,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 +5256,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 +5639,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) {}
 
@@ -5613,6 +5653,51 @@ void ReturnValue<T>::Set(const Handle<T> handle) {
   *value_ = *reinterpret_cast<internal::Object**>(*handle);
 }
 
+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,

src/api.cc

@@ -6207,12 +6207,19 @@ Local<Symbol> v8::Symbol::New(Isolate* isolate, const char* data, int length) {
 Local<Number> v8::Number::New(double value) {
   i::Isolate* isolate = i::Isolate::Current();
   EnsureInitializedForIsolate(isolate, "v8::Number::New()");
+  return Number::New(reinterpret_cast<Isolate*>(isolate), value);
+}
+
+
+Local<Number> v8::Number::New(Isolate* isolate, double value) {
+  i::Isolate* internal_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  ASSERT(internal_isolate->IsInitialized());
   if (std::isnan(value)) {
     // Introduce only canonical NaN value into the VM, to avoid signaling NaNs.
     value = i::OS::nan_value();
   }
-  ENTER_V8(isolate);
-  i::Handle<i::Object> result = isolate->factory()->NewNumber(value);
+  ENTER_V8(internal_isolate);
+  i::Handle<i::Object> result = internal_isolate->factory()->NewNumber(value);
   return Utils::NumberToLocal(result);
 }
 

src/objects-inl.h

@@ -1030,10 +1030,7 @@ int Smi::value() {
 
 Smi* Smi::FromInt(int value) {
   ASSERT(Smi::IsValid(value));
-  int smi_shift_bits = kSmiTagSize + kSmiShiftSize;
-  intptr_t tagged_value =
-      (static_cast<intptr_t>(value) << smi_shift_bits) | kSmiTag;
-  return reinterpret_cast<Smi*>(tagged_value);
+  return reinterpret_cast<Smi*>(Internals::IntToSmi(value));
 }
 
 
@@ -1111,28 +1108,8 @@ Failure* Failure::Construct(Type type, intptr_t value) {
 
 
 bool Smi::IsValid(intptr_t value) {
-#ifdef DEBUG
-  bool in_range = (value >= kMinValue) && (value <= kMaxValue);
-#endif
-
-#ifdef V8_TARGET_ARCH_X64
-  // To be representable as a long smi, the value must be a 32-bit integer.
-  bool result = (value == static_cast<int32_t>(value));
-#else
-  // 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.
-  bool result = (static_cast<uintptr_t>(value + 0x40000000U) < 0x80000000U);
-#endif
-  ASSERT(result == in_range);
+  bool result = Internals::IsValidSmi(value);
+  ASSERT_EQ(result, value >= kMinValue && value <= kMaxValue);
   return result;
 }
 

test/cctest/test-api.cc

@@ -1010,6 +1010,98 @@ THREADED_TEST(SimpleCallback) {
 }
 
 
+template<typename T>
+void FastReturnValueCallback(const v8::FunctionCallbackInfo<v8::Value>& info);
+
+// constant return values
+static const int32_t kFastReturnValueInt32 = 471;
+static const uint32_t kFastReturnValueUint32 = 571;
+static const double kFastReturnValueDouble = 2.7;
+// variable return values
+static bool fast_return_value_bool = false;
+static bool fast_return_value_void_is_null = false;
+
+template<>
+void FastReturnValueCallback<int32_t>(
+    const v8::FunctionCallbackInfo<v8::Value>& info) {
+  info.GetReturnValue().Set(info.GetIsolate(), kFastReturnValueInt32);
+}
+
+template<>
+void FastReturnValueCallback<uint32_t>(
+    const v8::FunctionCallbackInfo<v8::Value>& info) {
+  info.GetReturnValue().Set(info.GetIsolate(), kFastReturnValueUint32);
+}
+
+template<>
+void FastReturnValueCallback<double>(
+    const v8::FunctionCallbackInfo<v8::Value>& info) {
+  info.GetReturnValue().Set(info.GetIsolate(), kFastReturnValueDouble);
+}
+
+template<>
+void FastReturnValueCallback<bool>(
+    const v8::FunctionCallbackInfo<v8::Value>& info) {
+  info.GetReturnValue().Set(info.GetIsolate(), fast_return_value_bool);
+}
+
+template<>
+void FastReturnValueCallback<void>(
+    const v8::FunctionCallbackInfo<v8::Value>& info) {
+  if (fast_return_value_void_is_null) {
+    info.GetReturnValue().SetNull(info.GetIsolate());
+  } else {
+    info.GetReturnValue().SetUndefined(info.GetIsolate());
+  }
+}
+
+template<typename T>
+Handle<Value> TestFastReturnValues() {
+  LocalContext env;
+  v8::HandleScope scope(env->GetIsolate());
+  v8::Handle<v8::ObjectTemplate> object_template = v8::ObjectTemplate::New();
+  v8::FunctionCallback callback = &FastReturnValueCallback<T>;
+  object_template->Set("callback", v8::FunctionTemplate::New(callback));
+  v8::Local<v8::Object> object = object_template->NewInstance();
+  (*env)->Global()->Set(v8_str("callback_object"), object);
+  return scope.Close(CompileRun("callback_object.callback()"));
+}
+
+THREADED_TEST(FastReturnValues) {
+  v8::HandleScope scope(v8::Isolate::GetCurrent());
+  v8::Handle<v8::Value> value;
+  // check int_32
+  value = TestFastReturnValues<int32_t>();
+  CHECK(value->IsInt32());
+  CHECK_EQ(kFastReturnValueInt32, value->Int32Value());
+  // check uint32_t
+  value = TestFastReturnValues<uint32_t>();
+  CHECK(value->IsInt32());
+  CHECK_EQ(kFastReturnValueUint32, value->Int32Value());
+  // check double
+  value = TestFastReturnValues<double>();
+  CHECK(value->IsNumber());
+  CHECK_EQ(kFastReturnValueDouble, value->ToNumber()->Value());
+  // check bool values
+  for (int i = 0; i < 2; i++) {
+    fast_return_value_bool = i == 0;
+    value = TestFastReturnValues<bool>();
+    CHECK(value->IsBoolean());
+    CHECK_EQ(fast_return_value_bool, value->ToBoolean()->Value());
+  }
+  // check oddballs
+  for (int i = 0; i < 2; i++) {
+    fast_return_value_void_is_null = i == 0;
+    value = TestFastReturnValues<void>();
+    if (fast_return_value_void_is_null) {
+      CHECK(value->IsNull());
+    } else {
+      CHECK(value->IsUndefined());
+    }
+  }
+}
+
+
 THREADED_TEST(FunctionTemplateSetLength) {
   LocalContext env;
   v8::HandleScope scope(env->GetIsolate());