[promises] Port ResolvePromise 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 49de29a92958c996c00d7cc22b160920622478f7..55cc04a93aaa7343ef731c7785efdde36b164862 100644
--- a/src/builtins/builtins-promise.cc
+++ b/src/builtins/builtins-promise.cc
@@ -11,30 +11,6 @@
 namespace v8 {
 namespace internal {
 
-// ES#sec-promise-resolve-functions
-// Promise Resolve Functions
-BUILTIN(PromiseResolveClosure) {
-  HandleScope scope(isolate);
-
-  Handle<Context> context(isolate->context(), isolate);
-
-  if (PromiseUtils::HasAlreadyVisited(context)) {
-    return isolate->heap()->undefined_value();
-  }
-
-  PromiseUtils::SetAlreadyVisited(context);
-  Handle<JSObject> promise = handle(PromiseUtils::GetPromise(context), isolate);
-  Handle<Object> value = args.atOrUndefined(isolate, 1);
-
-  MaybeHandle<Object> maybe_result;
-  Handle<Object> argv[] = {promise, value};
-  RETURN_FAILURE_ON_EXCEPTION(
-      isolate, Execution::Call(isolate, isolate->promise_resolve(),
-                               isolate->factory()->undefined_value(),
-                               arraysize(argv), argv));
-  return isolate->heap()->undefined_value();
-}
-
 // ES#sec-promise-reject-functions
 // Promise Reject Functions
 BUILTIN(PromiseRejectClosure) {
@@ -665,5 +641,209 @@ void Builtins::Generate_PromiseThen(compiler::CodeAssemblerState* state) {
   a.Return(result);
 }
 
+namespace {
+// RegExp fast path implementations rely on unmodified JSPromise instances.

[jgruber, 2016/12/05 09:25:17]

Promise

[gsathya, 2016/12/05 16:03:05]

Done.

+// We use a fairly coarse granularity for this and simply check whether both
+// the promise itself is unmodified (i.e. its map has not changed) and its
+// prototype is unmodified.
+// TODO(gsathya): Refactor this out to prevent code dupe with builtins-regexp
+void BranchIfFastPath(CodeStubAssembler* a, compiler::Node* context,
+                      compiler::Node* promise,
+                      CodeStubAssembler::Label* if_isunmodified,
+                      CodeStubAssembler::Label* if_ismodified) {
+  typedef compiler::Node Node;
+
+  // TODO(gsathya): Assert if promise is receiver
+  Node* const map = a->LoadMap(promise);
+  Node* const native_context = a->LoadNativeContext(context);
+  Node* const promise_fun =
+      a->LoadContextElement(native_context, Context::PROMISE_FUNCTION_INDEX);
+  Node* const initial_map =
+      a->LoadObjectField(promise_fun, JSFunction::kPrototypeOrInitialMapOffset);
+  Node* const has_initialmap = a->WordEqual(map, initial_map);
+
+  a->GotoUnless(has_initialmap, if_ismodified);
+
+  Node* const initial_proto_initial_map = a->LoadContextElement(
+      native_context, Context::PROMISE_PROTOTYPE_MAP_INDEX);
+  Node* const proto_map = a->LoadMap(a->LoadMapPrototype(map));
+  Node* const proto_has_initialmap =
+      a->WordEqual(proto_map, initial_proto_initial_map);
+
+  // TODO(ishell): Update this check once map changes for constant field

[jgruber, 2016/12/05 09:25:17]

We can remove this TODO since the mentioned changes will not be happening AFAIK
(I should also do the same in builtins-regexp).

[gsathya, 2016/12/05 16:03:05]

Done.

+  // tracking are landing.
+  a->Branch(proto_has_initialmap, if_isunmodified, if_ismodified);
+}
+}  // namespace
+
+void InternalResolvePromise(CodeStubAssembler* a, compiler::Node* context,
+                            compiler::Node* promise, compiler::Node* result) {
+  typedef CodeStubAssembler::Variable Variable;

[jgruber, 2016/12/05 09:25:17]

As mentioned offline, you might consider switching to the new TF_BUILTIN style
sooner than later. You won't need these typedefs in every function in that case.

[gsathya, 2016/12/05 16:03:05]

I'm going to wait until I finish the rest of the builtins before doing the
cleanup. I currently have 4 levels of dependent patches so I don't want to deal
with too much unnecessary churn right now.

[jgruber, 2016/12/07 12:59:38]

Acknowledged, sgtm.

+  typedef CodeStubAssembler::Label Label;
+  typedef compiler::Node Node;
+
+  Isolate* isolate = a->isolate();
+
+  Variable var_reason(a, MachineRepresentation::kTagged);
+
+  Label out(a), fulfill(a), if_cycle(a, Label::kDeferred),
+      if_rejectpromise(a, Label::kDeferred);
+
+  // 6. If SameValue(resolution, promise) is true, then
+  a->GotoIf(a->WordEqual(promise, result), &if_cycle);

[jgruber, 2016/12/05 09:25:17]

SameValue is more involved than WordEqual. You can use CSA::SameValue.

[gsathya, 2016/12/05 16:03:05]

Done.

+
+  // 7. If Type(resolution) is not Object, then
+  a->GotoIf(a->TaggedIsSmi(result), &fulfill);
+  Node* const result_instance_type = a->LoadMapInstanceType(a->LoadMap(result));
+  a->GotoUnless(a->IsJSReceiverInstanceType(result_instance_type), &fulfill);

[jgruber, 2016/12/05 09:25:17]

IsJSReceiver()

[gsathya, 2016/12/05 16:03:05]

Done.

+
+  Label if_nativepromise(a), if_notnativepromise(a, Label::kDeferred);
+  BranchIfFastPath(a, context, result, &if_nativepromise, &if_notnativepromise);
+
+  a->Bind(&if_nativepromise);
+  {
+    Node* const thenable_status =
+        a->LoadObjectField(result, JSPromise::kStatusOffset);
+    Node* const thenable_value =
+        a->LoadObjectField(result, JSPromise::kResultOffset);
+
+    Label if_isnotpending(a);
+    a->GotoUnless(a->SmiEqual(a->SmiConstant(kPromisePending), thenable_status),
+                  &if_isnotpending);
+
+    Node* const native_context = a->LoadNativeContext(context);
+    Node* const then =
+        a->LoadContextElement(native_context, Context::PROMISE_THEN_INDEX);
+
+    a->CallRuntime(Runtime::kEnqueuePromiseResolveThenableJob, context, promise,
+                   result, then);
+    a->Goto(&out);
+
+    a->Bind(&if_isnotpending);
+    {
+      Label if_fulfilled(a), if_rejected(a);
+      a->Branch(a->SmiEqual(a->SmiConstant(kPromiseFulfilled), thenable_status),
+                &if_fulfilled, &if_rejected);
+
+      a->Bind(&if_fulfilled);
+      {
+        a->CallRuntime(Runtime::kPromiseFulfill, context, promise,
+                       a->SmiConstant(kPromiseFulfilled), thenable_value);
+        a->Goto(&out);
+      }
+
+      a->Bind(&if_rejected);
+      {
+        a->CallRuntime(Runtime::kPromiseReject, context, promise,
+                       thenable_value, a->FalseConstant());
+        a->Goto(&out);
+      }
+    }
+  }
+
+  a->Bind(&if_notnativepromise);
+  {
+    // 8. Let then be Get(resolution, "then").
+    Node* const then_str = a->HeapConstant(isolate->factory()->then_string());
+    Callable getproperty_callable = CodeFactory::GetProperty(a->isolate());
+    Node* const then =
+        a->CallStub(getproperty_callable, context, result, then_str);
+
+    // 9. If then is an abrupt completion, then
+    a->GotoIfException(then, &if_rejectpromise, &var_reason);
+
+    // 11. If IsCallable(thenAction) is false, then
+    a->GotoIf(a->TaggedIsSmi(then), &fulfill);
+    Node* const then_map = a->LoadMap(then);
+    a->GotoUnless(a->IsCallableMap(then_map), &fulfill);
+
+    // 12. Perform EnqueueJob("PromiseJobs",
+    // PromiseResolveThenableJob, « promise, resolution, thenAction
+    // »).
+    a->CallRuntime(Runtime::kEnqueuePromiseResolveThenableJob, context, promise,
+                   result, then);
+    a->Goto(&out);
+  }
+
+  // 7.b Return FulfillPromise(promise, resolution).
+  a->Bind(&fulfill);
+  {
+    a->CallRuntime(Runtime::kPromiseFulfill, context, promise,
+                   a->SmiConstant(kPromiseFulfilled), result);
+    a->Goto(&out);
+  }
+
+  a->Bind(&if_cycle);
+  {
+    // 6.a Let selfResolutionError be a newly created TypeError object.
+    Node* const message_id = a->SmiConstant(MessageTemplate::kPromiseCyclic);
+    Node* const error =
+        a->CallRuntime(Runtime::kNewTypeError, context, message_id, result);
+    var_reason.Bind(error);
+
+    // 6.b Return RejectPromise(promise, selfResolutionError).
+    a->Goto(&if_rejectpromise);
+  }
+
+  // 9.a Return RejectPromise(promise, then.[[Value]]).
+  a->Bind(&if_rejectpromise);
+  {
+    a->CallRuntime(Runtime::kPromiseReject, context, promise,
+                   var_reason.value(), a->TrueConstant());
+    a->Goto(&out);
+  }
+
+  a->Bind(&out);
+  a->Return(a->UndefinedConstant());
+}
+
+// ES#sec-promise-resolve-functions
+// Promise Resolve Functions
+void Builtins::Generate_PromiseResolveClosure(
+    compiler::CodeAssemblerState* state) {
+  CodeStubAssembler a(state);
+  typedef compiler::Node Node;
+  typedef CodeStubAssembler::Label Label;
+
+  Node* const value = a.Parameter(1);
+  Node* const context = a.Parameter(4);
+  Isolate* isolate = a.isolate();
+
+  Label out(&a);
+
+  // 3. Let alreadyResolved be F.[[AlreadyResolved]].
+  Node* const has_already_visited_slot =
+      a.IntPtrConstant(PromiseUtils::kAlreadyVisitedSlot);
+
+  Node* const has_already_visited =
+      a.LoadFixedArrayElement(context, has_already_visited_slot);
+
+  // 4. If alreadyResolved.[[Value]] is true, return undefined.
+  a.GotoIf(a.SmiEqual(has_already_visited, a.SmiConstant(1)), &out);

[jgruber, 2016/12/05 09:25:17]

Is line 825 below the only place where we write into kAlreadyVisitedSlot? Would
it make sense to store True/FalseConstant instead of 0/1 Smis?

[gsathya, 2016/12/05 16:03:05]

PromiseUtils::CreateResolvingFunctions and PromiseRejectClosure also use this. I
think Smi is cheaper though anyway

[jgruber, 2016/12/07 12:59:38]

Ok. IMHO the fact that we use a Smi 0/1 encoding is not obvious and we should at
least document this well. Perhaps in promise-utils.h together with the contents
of all other custom slots? It seems kind of off though that only Promise uses
Smi 0/1 to represent true/false. Likewise for JSPromise::kHasHandlerOffset. If
you do decide to change it, a follow-up CL is also fine.

And out of interest, are Smis really cheaper? It should be a pointer comparison
against a constant in both cases.

+
+  // 5.Set alreadyResolved.[[Value]] to true.
+  a.StoreFixedArrayElement(context, has_already_visited_slot, a.SmiConstant(1));
+
+  // 2. Let promise be F.[[Promise]].
+  Node* const promise = a.LoadFixedArrayElement(
+      context, a.IntPtrConstant(PromiseUtils::kPromiseSlot));
+
+  InternalResolvePromise(&a, context, promise, value);
+
+  a.Bind(&out);
+  a.Return(a.UndefinedConstant());
+}
+
+void Builtins::Generate_ResolvePromise(compiler::CodeAssemblerState* state) {
+  CodeStubAssembler a(state);
+  typedef compiler::Node Node;
+
+  Node* const promise = a.Parameter(1);
+  Node* const result = a.Parameter(2);
+  Node* const context = a.Parameter(5);
+
+  InternalResolvePromise(&a, context, promise, result);
+}
+
 }  // namespace internal
 }  // namespace v8