[promises] Move Promise.resolve to TF

src/builtins/builtins-promise.cc

Index: src/builtins/builtins-promise.cc
diff --git a/src/builtins/builtins-promise.cc b/src/builtins/builtins-promise.cc
index 49e9d6de2ed0d4906f96c254177a1bbd4ab2db8e..068eb35fd75ad8dab7c92571455eec8ef024e30b 100644
--- a/src/builtins/builtins-promise.cc
+++ b/src/builtins/builtins-promise.cc
@@ -63,7 +63,8 @@ PromiseBuiltinsAssembler::CreatePromiseResolvingFunctions(
 }
 
 Node* PromiseBuiltinsAssembler::ThrowIfNotJSReceiver(
-    Node* context, Node* value, MessageTemplate::Template msg_template) {
+    Node* context, Node* value, MessageTemplate::Template msg_template,
+    const char* method_name) {
   Label out(this), throw_exception(this, Label::kDeferred);
   Variable var_value_map(this, MachineRepresentation::kTagged);
 
@@ -78,8 +79,15 @@ Node* PromiseBuiltinsAssembler::ThrowIfNotJSReceiver(
   // The {value} is not a compatible receiver for this method.
   Bind(&throw_exception);
   {
+    Isolate* isolate = this->isolate();
+    Node* method;
+    method = method_name == NULL
+                 ? UndefinedConstant()
+                 : HeapConstant(isolate->factory()->NewStringFromAsciiChecked(
+                       method_name));
+
     Node* const message_id = SmiConstant(msg_template);
-    CallRuntime(Runtime::kThrowTypeError, context, message_id);
+    CallRuntime(Runtime::kThrowTypeError, context, message_id, method);
     var_value_map.Bind(UndefinedConstant());
     Goto(&out);  // Never reached.
   }
@@ -1034,5 +1042,116 @@ TF_BUILTIN(PromiseCatch, PromiseBuiltinsAssembler) {
   }
 }
 
+TF_BUILTIN(PromiseResolve, PromiseBuiltinsAssembler) {
+  //  1. Let C be the this value.
+  Node* receiver = Parameter(0);
+  Node* value = Parameter(1);
+  Node* context = Parameter(4);
+  Isolate* isolate = this->isolate();
+
+  // 2. If Type(C) is not Object, throw a TypeError exception.
+  ThrowIfNotJSReceiver(context, receiver, MessageTemplate::kCalledOnNonObject,
+                       "PromiseResolve");
+
+  Label if_valueisnativepromise(this), if_valueisnotnativepromise(this),
+      if_valueisnotpromise(this);
+
+  // 3.If IsPromise(x) is true, then
+  GotoIf(TaggedIsSmi(value), &if_valueisnotpromise);
+
+  // This shortcircuits the constructor lookups.
+  GotoUnless(HasInstanceType(value, JS_PROMISE_TYPE), &if_valueisnotpromise);
+
+  // This adds a fast path as non-subclassed native promises don't have
+  // an observable constructor lookup.
+  BranchIfFastPath(context, value, &if_valueisnativepromise,
+                   &if_valueisnotnativepromise);
+
+  Bind(&if_valueisnativepromise);
+  {
+    Node* const native_context = LoadNativeContext(context);

[adamk, 2016/12/22 19:39:35]

Newbie question: Doesn't this duplicate the context load that's already being
done in BranchIfFastPath? Same for the load of PROMISE_FUNCTION_INDEX below. Not
sure what the preferred code organization strategy is (maybe factor our part of
BranchIfFastPath out into something that takes the promise constructor as an
argument?), but it seems like the fast path should be as fast as possible.

[Igor Sheludko, 2016/12/27 14:30:28]

Good point. Maybe BranchIfFastPath() should return a native context.

[gsathya, 2017/01/03 17:35:26]

Added a BranchIfFastPath that accepts a native context and promise constructor.

+    Node* const promise_fun =
+        LoadContextElement(native_context, Context::PROMISE_FUNCTION_INDEX);
+    GotoUnless(WordEqual(promise_fun, receiver), &if_valueisnotnativepromise);
+    Return(value);
+  }
+
+  // At this point, value or/and receiver are not native promises, but
+  // they could be of the same subclass.
+  Bind(&if_valueisnotnativepromise);
+  {
+    // 3.a Let xConstructor be ? Get(x, "constructor").
+    // The constructor lookup is observable.
+    Node* const constructor_str =
+        HeapConstant(isolate->factory()->constructor_string());
+    Callable getproperty_callable = CodeFactory::GetProperty(isolate);
+    Node* const constructor =
+        CallStub(getproperty_callable, context, value, constructor_str);
+
+    // 3.b If SameValue(xConstructor, C) is true, return x.
+    GotoUnless(SameValue(constructor, receiver, context),
+               &if_valueisnotpromise);
+
+    Return(value);
+  }
+
+  Bind(&if_valueisnotpromise);
+  {
+    Label if_nativepromise(this), if_notnativepromise(this);
+    BranchIfFastPath(context, receiver, &if_nativepromise,
+                     &if_notnativepromise);
+
+    // This adds a fast path for native promises that don't need to
+    // create NewPromiseCapability.
+    Bind(&if_nativepromise);
+    {
+      Label do_resolve(this);
+
+      Node* const result = AllocateJSPromise(context);
+      PromiseInit(result);
+
+      GotoUnless(IsPromiseHookEnabled(), &do_resolve);
+      CallRuntime(Runtime::kPromiseHookInit, context, result,
+                  UndefinedConstant());
+      Goto(&do_resolve);
+
+      Bind(&do_resolve);
+      InternalResolvePromise(context, result, value);
+      Return(result);
+    }
+
+    Bind(&if_notnativepromise);
+    {
+      Isolate* isolate = this->isolate();
+      Node* const native_context = LoadNativeContext(context);

[adamk, 2016/12/22 19:39:35]

Possibly another newbie question: seems like this shouldn't need to reload the
native context if you do so earlier in the code?

[gsathya, 2017/01/03 17:35:25]

Only certain code paths use the native_context so I couldn't move this to the
top of the function, otherwise they would pay the cost of loading the native
context. I could refactor this out to be a separate function that takes a
native_context or load if it's null, not sure if that's worth the savings in the
slow path?

+
+      // 4. Let promiseCapability be ? NewPromiseCapability(C).
+      // TODO(gsathya): Move this to TF.
+      Callable call_callable = CodeFactory::Call(isolate);
+      Node* const new_promise_capability = LoadContextElement(
+          native_context, Context::NEW_PROMISE_CAPABILITY_INDEX);
+      // Should debug event be false?
+      Node* const capability =
+          CallJS(call_callable, context, new_promise_capability,
+                 UndefinedConstant(), receiver, TrueConstant());
+
+      Callable getproperty_callable = CodeFactory::GetProperty(isolate);
+      // 5. Perform ? Call(promiseCapability.[[Resolve]], undefined, « x »).
+      Node* const resolve_str =
+          HeapConstant(isolate->factory()->resolve_string());
+      Node* const resolve =
+          CallStub(getproperty_callable, context, capability, resolve_str);
+      CallJS(call_callable, context, resolve, UndefinedConstant(), value);
+
+      // 6. Return promiseCapability.[[Promise]].
+      Node* const promise_str =
+          HeapConstant(isolate->factory()->promise_string());
+      Node* const result =
+          CallStub(getproperty_callable, context, capability, promise_str);
+      Return(result);
+    }
+  }
+}
+
 }  // namespace internal
 }  // namespace v8