Revert "[builtins] don't inline calls for common Promise ops in async builtins"

This reverts commit 9461fe24.

Reason for revert: Breaks a test in Node.js: 
 parallel/test-util-inspect

=== release test-util-inspect ===                                              
Path: parallel/test-util-inspect
#
# Fatal error in , line 0
# unreachable code
#

==== C stack trace ===============================


Original change's description:
> [builtins] don't inline calls for common Promise ops in async builtins
> 
> InternalResolvePromise, InternalPromiseReject and
> InternalPerformPromiseThen generate quite a lot of code.
> 
> This change adds 3 new TF stubs which inline calls to these builtins.
> These stubs are invoked rather than inlining those operations listed
> above directly. This is done for Async Iteration builtins, as well as
> Async Function builtins. Promise builtins are left as they were, and
> continue to inline these calls.
> 
> This results in a roughly 99kb reduction in snapshot_blob.bin on an x64
> release build.
> 
> BUG=v8:5855
> R=​gsathya@chromium.org, jgruber@chromium.org
> 
> Change-Id: I3349d0f0353a72270ae40b974312d64d1c8a9e46
> Reviewed-on: https://chromium-review.googlesource.com/461269
> Commit-Queue: Caitlin Potter <caitp@igalia.com>
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Reviewed-by: Sathya Gunasekaran (ooo until April 10) <gsathya@chromium.org>
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#44445}

TBR=mstarzinger@chromium.org,gsathya@chromium.org,caitp@igalia.com,jgruber@chromium.org,v8-reviews@googlegroups.com,bmeurer@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5855

Change-Id: Iabcdf8b025cc9b053a858f8e74389638ac000ba0
Reviewed-on: https://chromium-review.googlesource.com/469946Reviewed-by: Franziska Hinkelmann <franzih@chromium.org>
Commit-Queue: Franziska Hinkelmann <franzih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44448}

src/builtins/builtins-async-gen.cc

@@ -27,7 +27,7 @@ Node* AsyncBuiltinsAssembler::Await(
   Node* const wrapped_value = AllocateAndInitJSPromise(context);
 
   // Perform ! Call(promiseCapability.[[Resolve]], undefined, « promise »).
-  CallBuiltin(Builtins::kResolveNativePromise, context, wrapped_value, value);
+  InternalResolvePromise(context, wrapped_value, value);
 
   Node* const native_context = LoadNativeContext(context);
 
@@ -88,8 +88,9 @@ Node* AsyncBuiltinsAssembler::Await(
 
   Goto(&do_perform_promise_then);
   BIND(&do_perform_promise_then);
-  CallBuiltin(Builtins::kPerformNativePromiseThen, context, wrapped_value,
-              on_resolve, on_reject, throwaway_promise);
+  InternalPerformPromiseThen(context, wrapped_value, on_resolve, on_reject,
+                             throwaway_promise, UndefinedConstant(),
+                             UndefinedConstant());
 
   return wrapped_value;
 }

src/builtins/builtins-async-generator-gen.cc

@@ -184,8 +184,7 @@ void AsyncGeneratorBuiltinsAssembler::AsyncGeneratorEnqueue(
         MakeTypeError(MessageTemplate::kIncompatibleMethodReceiver, context,
                       CStringConstant(method_name), generator);
 
-    CallBuiltin(Builtins::kRejectNativePromise, context, promise, error,
-                TrueConstant());
+    InternalPromiseReject(context, promise, error, true);
     Return(promise);
   }
 }
@@ -542,15 +541,18 @@ TF_BUILTIN(AsyncGeneratorResolve, AsyncGeneratorBuiltinsAssembler) {
   Node* const promise = LoadPromiseFromAsyncGeneratorRequest(next);
 
   Node* const wrapper = AllocateAndInitJSPromise(context);
-  CallBuiltin(Builtins::kResolveNativePromise, context, wrapper, value);
+  InternalResolvePromise(context, wrapper, value);
 
   Node* const on_fulfilled =
       CreateUnwrapClosure(LoadNativeContext(context), done);
 
+  Node* const undefined = UndefinedConstant();
+  InternalPerformPromiseThen(context, wrapper, on_fulfilled, undefined, promise,
+                             undefined, undefined);
+
   // Per spec, AsyncGeneratorResolve() returns undefined. However, for the
   // benefit of %TraceExit(), return the Promise.
-  Return(CallBuiltin(Builtins::kPerformNativePromiseThen, context, wrapper,
-                     on_fulfilled, UndefinedConstant(), promise));
+  Return(promise);
 }
 
 TF_BUILTIN(AsyncGeneratorReject, AsyncGeneratorBuiltinsAssembler) {
@@ -562,8 +564,8 @@ TF_BUILTIN(AsyncGeneratorReject, AsyncGeneratorBuiltinsAssembler) {
   Node* const next = TakeFirstAsyncGeneratorRequestFromQueue(generator);
   Node* const promise = LoadPromiseFromAsyncGeneratorRequest(next);
 
-  Return(CallBuiltin(Builtins::kRejectNativePromise, context, promise, value,
-                     TrueConstant()));
+  InternalPromiseReject(context, promise, value, true);
+  Return(UndefinedConstant());
 }
 
 }  // namespace internal

src/builtins/builtins-async-iterator-gen.cc

@@ -120,7 +120,7 @@ void AsyncFromSyncBuiltinsAssembler::Generate_AsyncFromSyncIteratorMethod(
 
   // Perform ! Call(valueWrapperCapability.[[Resolve]], undefined, «
   // throwValue »).
-  CallBuiltin(Builtins::kResolveNativePromise, context, wrapper, value);
+  InternalResolvePromise(context, wrapper, value);
 
   // Let onFulfilled be a new built-in function object as defined in
   // Async Iterator Value Unwrap Functions.
@@ -129,14 +129,16 @@ void AsyncFromSyncBuiltinsAssembler::Generate_AsyncFromSyncIteratorMethod(
 
   // Perform ! PerformPromiseThen(valueWrapperCapability.[[Promise]],
   //     onFulfilled, undefined, promiseCapability).
-  Return(CallBuiltin(Builtins::kPerformNativePromiseThen, context, wrapper,
-                     on_fulfilled, UndefinedConstant(), promise));
+  Node* const undefined = UndefinedConstant();
+  InternalPerformPromiseThen(context, wrapper, on_fulfilled, undefined, promise,
+                             undefined, undefined);
+  Return(promise);
 
   BIND(&reject_promise);
   {
     Node* const exception = var_exception.value();
-    CallBuiltin(Builtins::kRejectNativePromise, context, promise, exception,
-                TrueConstant());
+    InternalPromiseReject(context, promise, exception, TrueConstant());
+
     Return(promise);
   }
 }

src/builtins/builtins-definitions.h

@@ -224,10 +224,6 @@ namespace internal {
   TFH(StoreIC_Uninitialized, BUILTIN, kNoExtraICState, StoreWithVector)        \
   TFH(StoreICStrict_Uninitialized, BUILTIN, kNoExtraICState, StoreWithVector)  \
                                                                                \
-  TFS(ResolveNativePromise, ResolveNativePromise, 1)                           \
-  TFS(RejectNativePromise, RejectNativePromise, 1)                             \
-  TFS(PerformNativePromiseThen, PerformNativePromiseThen, 1)                   \
-                                                                               \
   /* Built-in functions for Javascript */                                      \
   /* Special internal builtins */                                              \
   CPP(EmptyFunction)                                                           \
@@ -986,13 +982,10 @@ namespace internal {
   V(AsyncGeneratorResolve)                           \
   V(AsyncGeneratorAwaitCaught)                       \
   V(AsyncGeneratorAwaitUncaught)                     \
-  V(PerformNativePromiseThen)                        \
   V(PromiseConstructor)                              \
   V(PromiseHandle)                                   \
   V(PromiseResolve)                                  \
   V(PromiseResolveClosure)                           \
-  V(RejectNativePromise)                             \
-  V(ResolveNativePromise)                            \
   V(ResolvePromise)
 
 #define BUILTIN_EXCEPTION_CAUGHT_PREDICTION_LIST(V) V(PromiseHandleReject)

src/builtins/builtins-promise-gen.cc

@@ -1762,42 +1762,5 @@ TF_BUILTIN(PromiseFinally, PromiseBuiltinsAssembler) {
   }
 }
 
-TF_BUILTIN(ResolveNativePromise, PromiseBuiltinsAssembler) {
-  Node* const promise = Parameter(Descriptor::kPromise);
-  Node* const value = Parameter(Descriptor::kValue);
-  Node* const context = Parameter(Descriptor::kContext);
-
-  CSA_ASSERT(this, HasInstanceType(promise, JS_PROMISE_TYPE));
-  InternalResolvePromise(context, promise, value);
-  Return(UndefinedConstant());
-}
-
-TF_BUILTIN(RejectNativePromise, PromiseBuiltinsAssembler) {
-  Node* const promise = Parameter(Descriptor::kPromise);
-  Node* const value = Parameter(Descriptor::kValue);
-  Node* const debug_event = Parameter(Descriptor::kDebugEvent);
-  Node* const context = Parameter(Descriptor::kContext);
-
-  CSA_ASSERT(this, HasInstanceType(promise, JS_PROMISE_TYPE));
-  CSA_ASSERT(this, IsBoolean(debug_event));
-  InternalPromiseReject(context, promise, value, debug_event);
-  Return(UndefinedConstant());
-}
-
-TF_BUILTIN(PerformNativePromiseThen, PromiseBuiltinsAssembler) {
-  Node* const promise = Parameter(Descriptor::kPromise);
-  Node* const resolve_reaction = Parameter(Descriptor::kResolveReaction);
-  Node* const reject_reaction = Parameter(Descriptor::kRejectReaction);
-  Node* const result_promise = Parameter(Descriptor::kResultPromise);
-  Node* const context = Parameter(Descriptor::kContext);
-
-  CSA_ASSERT(this, HasInstanceType(result_promise, JS_PROMISE_TYPE));
-
-  InternalPerformPromiseThen(context, promise, resolve_reaction,
-                             reject_reaction, result_promise,
-                             UndefinedConstant(), UndefinedConstant());
-  Return(result_promise);
-}
-
 }  // namespace internal
 }  // namespace v8

src/frames.cc

@@ -916,15 +916,6 @@ int StubFrame::GetNumberOfIncomingArguments() const {
   return 0;
 }
 
-int StubFrame::LookupExceptionHandlerInTable(int* stack_slots) {
-  Code* code = LookupCode();
-  DCHECK(code->is_turbofanned());
-  DCHECK_EQ(code->kind(), Code::BUILTIN);
-  HandlerTable* table = HandlerTable::cast(code->handler_table());
-  int pc_offset = static_cast<int>(pc() - code->entry());
-  *stack_slots = code->stack_slots();
-  return table->LookupReturn(pc_offset);
-}
 
 void OptimizedFrame::Iterate(ObjectVisitor* v) const {
   IterateCompiledFrame(v);

src/frames.h

@@ -1130,12 +1130,6 @@ class StubFrame : public StandardFrame {
   // Determine the code for the frame.
   Code* unchecked_code() const override;
 
-  // Lookup exception handler for current {pc}, returns -1 if none found. Only
-  // TurboFan stub frames are supported. Also returns data associated with the
-  // handler site:
-  //  - TurboFan stub: Data is the stack slot count of the entire frame.
-  int LookupExceptionHandlerInTable(int* data);
-
  protected:
   inline explicit StubFrame(StackFrameIteratorBase* iterator);
 

src/interface-descriptors.h

@@ -101,10 +101,7 @@ class PlatformInterfaceDescriptor;
   V(AsyncGeneratorResolve)                 \
   V(AsyncGeneratorReject)                  \
   V(AsyncGeneratorResumeNext)              \
-  V(WasmRuntimeCall)                       \
-  V(ResolveNativePromise)                  \
-  V(RejectNativePromise)                   \
-  V(PerformNativePromiseThen)
+  V(WasmRuntimeCall)
 
 class V8_EXPORT_PRIVATE CallInterfaceDescriptorData {
  public:
@@ -1011,28 +1008,6 @@ class AsyncGeneratorResumeNextDescriptor final
                              CallInterfaceDescriptor, kParameterCount)
 };
 
-class ResolveNativePromiseDescriptor final : public CallInterfaceDescriptor {
- public:
-  DEFINE_PARAMETERS(kPromise, kValue)
-  DECLARE_DEFAULT_DESCRIPTOR(ResolveNativePromiseDescriptor,
-                             CallInterfaceDescriptor, kParameterCount)
-};
-
-class RejectNativePromiseDescriptor final : public CallInterfaceDescriptor {
- public:
-  DEFINE_PARAMETERS(kPromise, kValue, kDebugEvent)
-  DECLARE_DEFAULT_DESCRIPTOR(RejectNativePromiseDescriptor,
-                             CallInterfaceDescriptor, kParameterCount)
-};
-
-class PerformNativePromiseThenDescriptor final
-    : public CallInterfaceDescriptor {
- public:
-  DEFINE_PARAMETERS(kPromise, kResolveReaction, kRejectReaction, kResultPromise)
-  DECLARE_DEFAULT_DESCRIPTOR(PerformNativePromiseThenDescriptor,
-                             CallInterfaceDescriptor, kParameterCount)
-};
-
 class WasmRuntimeCallDescriptor final : public CallInterfaceDescriptor {
  public:
   DEFINE_EMPTY_PARAMETERS()

src/isolate.cc

@@ -1241,29 +1241,6 @@ Object* Isolate::UnwindAndFindHandler() {
         return FoundHandler(nullptr, code, offset, return_sp, frame->fp());
       }
 
-      case StackFrame::STUB: {
-        // Some stubs are able to handle exceptions.
-        if (!catchable_by_js) break;
-        StubFrame* stub_frame = static_cast<StubFrame*>(frame);
-        Code* code = stub_frame->LookupCode();
-        if (!code->IsCode() || code->kind() != Code::BUILTIN ||
-            !code->handler_table()->length() || !code->is_turbofanned()) {
-          break;
-        }
-
-        int stack_slots = 0;  // Will contain stack slot count of frame.
-        int offset = stub_frame->LookupExceptionHandlerInTable(&stack_slots);
-        if (offset < 0) break;
-
-        // Compute the stack pointer from the frame pointer. This ensures
-        // that argument slots on the stack are dropped as returning would.
-        Address return_sp = frame->fp() +
-                            StandardFrameConstants::kFixedFrameSizeAboveFp -
-                            stack_slots * kPointerSize;
-
-        return FoundHandler(nullptr, code, offset, return_sp, frame->fp());
-      }
-
       case StackFrame::INTERPRETED: {
         // For interpreted frame we perform a range lookup in the handler table.
         if (!catchable_by_js) break;
@@ -1424,24 +1401,6 @@ Isolate::CatchType Isolate::PredictExceptionCatcher() {
         }
       } break;
 
-      case StackFrame::STUB: {
-        Handle<Code> code(frame->LookupCode());
-        if (code->kind() == Code::BUILTIN && code->is_turbofanned()) {
-          if (code->is_promise_rejection()) {
-            return CAUGHT_BY_PROMISE;
-          }
-
-          // This the exception throw in PromiseHandle which doesn't
-          // cause a promise rejection.
-          if (code->is_exception_caught()) {
-            return CAUGHT_BY_JAVASCRIPT;
-          }
-
-          // The built-in must be marked with an exception prediction.
-          UNREACHABLE();
-        }
-      }
-
       default:
         // All other types can not handle exception.
         break;