[promises] Port CreateResolvingFunctions to TF

src/code-stub-assembler.cc

Index: src/code-stub-assembler.cc
diff --git a/src/code-stub-assembler.cc b/src/code-stub-assembler.cc
index e67f81f8e1ad28afb5ac25e2ac682167e964a484..b5b2935dc4bb1c547ae42dfdf2ede673975ccd2f 100644
--- a/src/code-stub-assembler.cc
+++ b/src/code-stub-assembler.cc
@@ -5,6 +5,7 @@
 #include "src/code-factory.h"
 #include "src/frames-inl.h"
 #include "src/frames.h"
+#include "src/promise-utils.h"
 
 namespace v8 {
 namespace internal {
@@ -8222,6 +8223,75 @@ Node* CodeStubAssembler::IsPromiseHookEnabled() {
   return WordNotEqual(is_promisehook_enabled, Int32Constant(0));
 }
 
+Node* CodeStubAssembler::AllocateFunctionWithMapAndContext(Node* map,

[jgruber, 2016/12/13 10:44:04]

Wouldn't 'AllocateFunction' be simpler (since this seems to be a straight port
of Factory::NewFunction to CSA).

[gsathya, 2016/12/13 13:01:46]

Seemed like there's precedent with AllocateHeapNumberWithValue and
AllocateHeapNumber. Happy to change if you think it's worthwhile

[jgruber, 2016/12/13 13:20:52]

Ok, let's wait and see what Igor says, CSA is his baby :)

[gsathya, 2016/12/13 13:29:33]

Acknowledged.

[Igor Sheludko, 2016/12/13 23:42:34]

Let this function be :)

+                                                           Node* shared_info,
+                                                           Node* context) {
+  Node* const code =
+      LoadObjectField(shared_info, SharedFunctionInfo::kCodeOffset);
+  Node* code_entry =

[jgruber, 2016/12/13 10:44:04]

Why is this one not const? Same below - if you're going with marking nodes const
by default (which I personally like), then it probably makes sense to only
create non-const Nodes when they will actually be changed.

[gsathya, 2016/12/13 13:01:46]

I guess. Changed

+      IntPtrAdd(code, IntPtrConstant(Code::kHeaderSize - kHeapObjectTag));
+
+  Node* fun = Allocate(JSFunction::kSize);
+  StoreMapNoWriteBarrier(fun, map);
+  StoreObjectFieldRoot(fun, JSObject::kPropertiesOffset,
+                       Heap::kEmptyFixedArrayRootIndex);
+  StoreObjectFieldRoot(fun, JSObject::kElementsOffset,
+                       Heap::kEmptyFixedArrayRootIndex);
+  StoreObjectFieldRoot(fun, JSFunction::kLiteralsOffset,
+                       Heap::kEmptyLiteralsArrayRootIndex);
+  StoreObjectFieldRoot(fun, JSFunction::kPrototypeOrInitialMapOffset,
+                       Heap::kTheHoleValueRootIndex);
+  StoreObjectFieldNoWriteBarrier(fun, JSFunction::kSharedFunctionInfoOffset,
+                                 shared_info);
+  StoreObjectFieldNoWriteBarrier(fun, JSFunction::kContextOffset, context);
+  StoreObjectFieldNoWriteBarrier(fun, JSFunction::kCodeEntryOffset, code_entry);
+  StoreObjectFieldRoot(fun, JSFunction::kNextFunctionLinkOffset,
+                       Heap::kUndefinedValueRootIndex);
+
+  return fun;
+}
+
+Node* CodeStubAssembler::CreatePromiseResolvingFunctionsContext(
+    Node* promise, Node* debug_event, Node* native_context) {
+  Node* context =
+      Allocate(FixedArray::SizeFor(PromiseUtils::kPromiseContextLength));
+  Node* map = HeapConstant(isolate()->factory()->function_context_map());
+  StoreMapNoWriteBarrier(context, map);
+  StoreObjectFieldNoWriteBarrier(
+      context, FixedArray::kLengthOffset,
+      SmiConstant(PromiseUtils::kPromiseContextLength));
+
+  Node* empty_fn = LoadContextElement(native_context, Context::CLOSURE_INDEX);
+  StoreContextElement(context, Context::CLOSURE_INDEX, empty_fn);

[jgruber, 2016/12/13 10:44:04]

Perhaps this could be refactored to a more generic AllocateFunctionContext here
and the additional handling for Promise-specific slots could take place in
builtins-promise.h

[gsathya, 2016/12/13 13:01:47]

All of these are pretty specific to promise 

[jgruber, 2016/12/13 13:20:52]

Really? Isn't this almost identical to
Factory::NewFunctionContext(empty_function)?

[gsathya, 2016/12/13 13:29:33]

Can't have the FUNCTION_SCOPE check, can't set previous..

+  StoreContextElement(context, Context::PREVIOUS_INDEX, UndefinedConstant());
+  StoreContextElement(context, Context::EXTENSION_INDEX, TheHoleConstant());
+  StoreContextElement(context, Context::NATIVE_CONTEXT_INDEX, native_context);
+  StoreContextElement(context, PromiseUtils::kAlreadyVisitedSlot,
+                      SmiConstant(0));
+  StoreContextElement(context, PromiseUtils::kPromiseSlot, promise);
+  StoreContextElement(context, PromiseUtils::kDebugEventSlot, debug_event);
+  return context;
+}
+
+std::pair<Node*, Node*> CodeStubAssembler::CreatePromiseResolvingFunctions(

[jgruber, 2016/12/13 10:44:05]

This would probably be better as a member of the PromiseBuiltinsAssembler once
you get around to using the new CSA style (and as an anonymous-namespace helper
before). It's unlikely to be used outside of builtins-promises.h, right? That'd
mean you could also remove the promise-utils.h include above.

[gsathya, 2016/12/13 13:01:46]

These will be used in other builtins I think. caitp?

[Igor Sheludko, 2016/12/13 23:42:34]

+1. Maybe there should be an assembler class with stuff common to promises and
async/await.

BTW, I think it's already the time to start using TF_BUILTIN macro in
builtins-promise.cc.

+    Node* promise, Node* debug_event, Node* native_context) {
+  Node* promise_context = CreatePromiseResolvingFunctionsContext(
+      promise, debug_event, native_context);
+  Node* resolve_info =
+      LoadContextElement(native_context, Context::PROMISE_RESOLVE_SHARED_FUN);
+  Node* reject_info =
+      LoadContextElement(native_context, Context::PROMISE_REJECT_SHARED_FUN);
+
+  Node* const map = LoadContextElement(
+      native_context, Context::STRICT_FUNCTION_WITHOUT_PROTOTYPE_MAP_INDEX);
+  Node* const resolve =
+      AllocateFunctionWithMapAndContext(map, resolve_info, promise_context);
+  Node* const reject =
+      AllocateFunctionWithMapAndContext(map, reject_info, promise_context);
+
+  return std::make_pair(resolve, reject);
+}
+
 Node* CodeStubAssembler::AllocateJSPromise(Node* context) {
   Node* const native_context = LoadNativeContext(context);
   Node* const promise_fun =