[promises] Remove runtime call from fastpath in PromiseReject

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 935da633a511b888780600707fbbfdc4b77c5da2..2141342a4b94d0ebda398ff749e4f03fdd2fc348 100644
--- a/src/builtins/builtins-promise.cc
+++ b/src/builtins/builtins-promise.cc
@@ -748,8 +748,8 @@ void PromiseBuiltinsAssembler::InternalResolvePromise(Node* context,
 
         Bind(&reject);
         // Don't cause a debug event as this case is forwarding a rejection
-        CallRuntime(Runtime::kPromiseReject, context, promise, thenable_value,
-                    FalseConstant());
+        InternalPromiseReject(context, promise, thenable_value,
+                              FalseConstant());
         PromiseSetHasHandler(result);
         Goto(&out);
       }
@@ -836,8 +836,7 @@ void PromiseBuiltinsAssembler::InternalResolvePromise(Node* context,
   // 9.a Return RejectPromise(promise, then.[[Value]]).
   Bind(&if_rejectpromise);
   {
-    CallRuntime(Runtime::kPromiseReject, context, promise, var_reason.value(),
-                TrueConstant());
+    InternalPromiseReject(context, promise, var_reason.value(), TrueConstant());
     Goto(&out);
   }
 
@@ -943,6 +942,41 @@ void PromiseBuiltinsAssembler::BranchIfAccessCheckFailed(
   Bind(&has_access);
 }
 
+// This duplicates a lot of logic from PromiseRejectEvent in

[adamk, 2017/01/13 20:31:33]

Is this a TODO? That is, is this duplication something you want to change?

Also can you put in this comment what this function actually does?
"InternalPromiseReject" exactly obvious (or put it in the header file if this
method is to remain public).

[gsathya, 2017/01/13 21:12:40]

Hmm, this isn't a TODO but more like a NOTE. I added this because if anyone
changes this, they may have to change PromiseRejectEvent as well. There are
tests that will fail so I can probably remove this, if that's better

[adamk, 2017/01/13 21:43:37]

I think it should be stronger in that case: it seems like this needs to be kept
in sync with PromiseRejectEvent.

+// runtime-promise.cc
+void PromiseBuiltinsAssembler::InternalPromiseReject(Node* context,
+                                                     Node* promise, Node* value,
+                                                     Node* debug_event) {
+  Label fulfill(this), report_unhandledpromise(this), run_debughandler(this),
+      run_promise_hook(this, Label::kDeferred);
+  Branch(IsPromiseHookEnabled(), &run_promise_hook, &run_debughandler);
+
+  Bind(&run_promise_hook);
+  {
+    CallRuntime(Runtime::kPromiseHookResolve, context, promise);
+    Goto(&run_debughandler);
+  }
+
+  Bind(&run_debughandler);
+  {
+    GotoUnless(IsDebugActive(), &report_unhandledpromise);
+    GotoUnless(WordEqual(debug_event, TrueConstant()),
+               &report_unhandledpromise);
+    CallRuntime(Runtime::kDebugPromiseReject, context, promise, value);
+    Goto(&report_unhandledpromise);
+  }
+
+  Bind(&report_unhandledpromise);
+  {
+    GotoIf(PromiseHasHandler(promise), &fulfill);
+    CallRuntime(Runtime::kReportPromiseReject, context, promise, value);
+    Goto(&fulfill);
+  }
+
+  Bind(&fulfill);
+  PromiseFulfill(context, promise, value, v8::Promise::kRejected);
+}
+
 // ES#sec-promise-reject-functions
 // Promise Reject Functions
 TF_BUILTIN(PromiseRejectClosure, PromiseBuiltinsAssembler) {
@@ -970,7 +1004,7 @@ TF_BUILTIN(PromiseRejectClosure, PromiseBuiltinsAssembler) {
   Node* const debug_event = LoadContextElement(
       context, IntPtrConstant(PromiseUtils::kDebugEventSlot));
 
-  CallRuntime(Runtime::kPromiseReject, context, promise, value, debug_event);
+  InternalPromiseReject(context, promise, value, debug_event);
   Return(UndefinedConstant());
 
   Bind(&out);
@@ -1207,8 +1241,7 @@ TF_BUILTIN(PromiseHandleReject, PromiseBuiltinsAssembler) {
 
   Bind(&if_internalhandler);
   {
-    CallRuntime(Runtime::kPromiseReject, context, promise, exception,
-                FalseConstant());
+    InternalPromiseReject(context, promise, exception, FalseConstant());
     Return(UndefinedConstant());
   }
 
@@ -1504,5 +1537,15 @@ TF_BUILTIN(PromiseReject, PromiseBuiltinsAssembler) {
   }
 }
 
+TF_BUILTIN(InternalPromiseReject, PromiseBuiltinsAssembler) {
+  Node* const promise = Parameter(1);
+  Node* const reason = Parameter(2);
+  Node* const debug_event = Parameter(3);
+  Node* const context = Parameter(6);
+
+  InternalPromiseReject(context, promise, reason, debug_event);
+  Return(UndefinedConstant());
+}
+
 }  // namespace internal
 }  // namespace v8