[stubs] Port FastFunctionBindStub to TF

src/builtins/builtins-function.cc

Index: src/builtins/builtins-function.cc
diff --git a/src/builtins/builtins-function.cc b/src/builtins/builtins-function.cc
index 6cd1b283ec6f1e144bc1fe8cff3b5a5783da185c..4daabb4638608c51bbd616204135924e8afaf2da 100644
--- a/src/builtins/builtins-function.cc
+++ b/src/builtins/builtins-function.cc
@@ -5,6 +5,7 @@
 #include "src/builtins/builtins.h"
 #include "src/builtins/builtins-utils.h"
 
+#include "src/code-factory.h"
 #include "src/compiler.h"
 #include "src/string-builder.h"
 
@@ -255,6 +256,215 @@ Object* DoFunctionBind(Isolate* isolate, BuiltinArguments args) {
 // ES6 section 19.2.3.2 Function.prototype.bind ( thisArg, ...args )
 BUILTIN(FunctionPrototypeBind) { return DoFunctionBind(isolate, args); }
 
+void Builtins::Generate_FastFunctionPrototypeBind(
+    compiler::CodeAssemblerState* state) {
+  using compiler::Node;
+  typedef CodeStubAssembler::Label Label;
+  typedef CodeStubAssembler::Variable Variable;
+
+  CodeStubAssembler assembler(state);
+  Label slow(&assembler);
+
+  Node* argc = assembler.Parameter(1);

[Igor Sheludko, 2016/12/01 11:50:41]

1) BuiltinDescriptor::kArgumentsCount?

2) argc is Int32, but the code below assumes that it's IntPtr. Probably the same
applies to all the other builtins that use BuiltinDescriptor.

Maybe CodeStubArguments should be the bottleneck which gets
BuiltinsDescriptor::kArgumentsCount parameter and converts it to intptr.

[danno, 2016/12/01 15:43:33]

Done.

+  Node* context = assembler.Parameter(2);

[Igor Sheludko, 2016/12/01 11:50:40]

BuiltinDescriptor::kContext

[danno, 2016/12/01 15:43:33]

Done.

+  Node* new_target = assembler.Parameter(0);

[Igor Sheludko, 2016/12/01 11:50:40]

BuiltinDescriptor::kNewTarget

[danno, 2016/12/01 15:43:33]

Done.

+
+  CodeStubArguments args(&assembler, argc);
+
+  // Check that receiver has instance type of JS_FUNCTION_TYPE
+  Node* receiver = args.GetReceiver();
+  assembler.GotoIf(assembler.TaggedIsSmi(receiver), &slow);
+
+  Node* receiver_map = assembler.LoadMap(receiver);
+  Node* instance_type = assembler.LoadMapInstanceType(receiver_map);
+  assembler.GotoIf(
+      assembler.Word32NotEqual(instance_type,
+                               assembler.Int32Constant(JS_FUNCTION_TYPE)),
+      &slow);
+
+  // Disallow binding of slow-mode functions. We need to figure out whether the
+  // length and name property are in the original state.
+  assembler.Comment("Disallow binding of slow-mode functions");
+  Node* bit_field3 = assembler.LoadObjectField(
+      receiver_map, Map::kBitField3Offset, MachineType::Uint32());
+  int mask = static_cast<int>(Map::DictionaryMap::kMask);
+  Node* mask_node = assembler.Int32Constant(mask);
+  Node* test = assembler.Word32And(bit_field3, mask_node);
+  assembler.GotoIf(assembler.Word32Equal(test, mask_node), &slow);

[Igor Sheludko, 2016/12/01 11:50:40]

You can use IsDictionaryMap(receiver_map) here. 

And FYI: we have CSA::IsSet family of helpers.

[danno, 2016/12/01 15:43:33]

Done.

+
+  // Check whether the length and name properties are still present as
+  // AccessorInfo objects. In that case, their value can be recomputed even if
+  // the actual value on the object changes.
+  assembler.Comment("Check descriptor array length");
+  Node* descriptors =
+      assembler.LoadObjectField(receiver_map, Map::kDescriptorsOffset);
+  CodeStubAssembler::ParameterMode mode = assembler.OptimalParameterMode();

[Igor Sheludko, 2016/12/01 11:50:40]

descriptors_length is not used anywhere else so you can just do
SmiLessThanOrEqual comparison below and avoid unnecessary untagging.

[danno, 2016/12/01 15:43:32]

Done.

+  Node* descriptors_length = assembler.UntagParameter(
+      assembler.LoadFixedArrayBaseLength(descriptors), mode);
+  assembler.GotoIf(
+      assembler.IntPtrLessThanOrEqual(descriptors_length,

[Igor Sheludko, 2016/12/01 11:50:40]

I prefer to use unsigned comparisons for lengths.

[danno, 2016/12/01 15:43:33]

Acknowledged.

+                                      assembler.IntPtrOrSmiConstant(1, mode)),
+      &slow);
+
+  // Check whether the length and name properties are still present as
+  // AccessorInfo objects. In that case, their value can be recomputed even if
+  // the actual value on the object changes.
+  assembler.Comment("Check name and length properties");
+  const int length_index = JSFunction::kLengthDescriptorIndex;
+  Node* maybe_length = assembler.LoadFixedArrayElement(
+      descriptors,
+      assembler.Int32Constant(DescriptorArray::ToKeyIndex(length_index)));

[Igor Sheludko, 2016/12/01 11:50:40]

Use IntPtrConstant() and pass INTPTR_PARAMETERS mode.

[danno, 2016/12/01 15:43:33]

Done.

+  assembler.GotoIf(
+      assembler.WordNotEqual(
+          maybe_length, assembler.HeapConstant(

[Igor Sheludko, 2016/12/01 11:50:40]

LoadRoot

[danno, 2016/12/01 15:43:33]

Done.

+                            assembler.isolate()->factory()->length_string())),
+      &slow);
+
+  Node* maybe_length_accessor = assembler.LoadFixedArrayElement(
+      descriptors,
+      assembler.Int32Constant(DescriptorArray::ToValueIndex(length_index)));
+  assembler.GotoIf(assembler.TaggedIsSmi(maybe_length_accessor), &slow);
+  Node* length_value_map = assembler.LoadMap(maybe_length_accessor);
+  assembler.GotoIf(

[Igor Sheludko, 2016/12/01 11:50:40]

GotoUnless(IsAccessorInfoMap(length_value_map), ...)

[danno, 2016/12/01 15:43:33]

Done.

+      assembler.WordNotEqual(
+          length_value_map,
+          assembler.HeapConstant(
+              assembler.isolate()->factory()->accessor_info_map())),
+      &slow);
+
+  const int name_index = JSFunction::kNameDescriptorIndex;
+  Node* maybe_name = assembler.LoadFixedArrayElement(
+      descriptors,
+      assembler.Int32Constant(DescriptorArray::ToKeyIndex(name_index)));

[Igor Sheludko, 2016/12/01 11:50:40]

Same here.

[danno, 2016/12/01 15:43:33]

Done.

+  assembler.GotoIf(
+      assembler.WordNotEqual(
+          maybe_name, assembler.HeapConstant(

[Igor Sheludko, 2016/12/01 11:50:40]

LoadRoot.

[danno, 2016/12/01 15:43:33]

Done.

+                          assembler.isolate()->factory()->name_string())),
+      &slow);
+
+  Node* maybe_name_accessor = assembler.LoadFixedArrayElement(
+      descriptors,
+      assembler.Int32Constant(DescriptorArray::ToValueIndex(name_index)));

[Igor Sheludko, 2016/12/01 11:50:40]

Same here.

[danno, 2016/12/01 15:43:33]

Done.

+  assembler.GotoIf(assembler.TaggedIsSmi(maybe_name_accessor), &slow);
+  Node* name_value_map = assembler.LoadMap(maybe_name_accessor);
+  assembler.GotoIf(

[Igor Sheludko, 2016/12/01 11:50:40]

Same here.

[danno, 2016/12/01 15:43:33]

Done.

+      assembler.WordNotEqual(
+          name_value_map,
+          assembler.HeapConstant(
+              assembler.isolate()->factory()->accessor_info_map())),
+      &slow);
+
+  // Choose the right bound function map based on whether the target is
+  // constructable.
+  assembler.Comment("Choose the right bound function map");
+  Variable bound_function_map(&assembler, MachineRepresentation::kTagged);
+  Label with_constructor(&assembler);
+  CodeStubAssembler::VariableList vars({&bound_function_map}, assembler.zone());
+  Label map_done(&assembler, vars);
+  Node* bit_field = assembler.LoadObjectField(

[Igor Sheludko, 2016/12/01 11:50:40]

LoadMapBitField

[danno, 2016/12/01 15:43:32]

Done.

+      receiver_map, Map::kBitFieldOffset, MachineType::Pointer());
+  mask = static_cast<int>(1 << Map::kIsConstructor);
+  mask_node = assembler.IntPtrConstant(mask);
+  Node* bits = assembler.WordAnd(bit_field, mask_node);
+  Node* native_context = assembler.LoadNativeContext(context);
+  assembler.GotoIf(assembler.Word32Equal(bits, mask_node), &with_constructor);

[Igor Sheludko, 2016/12/01 11:50:40]

IsSetWord32(bit_field, mask)

[danno, 2016/12/01 15:43:33]

Done.

+
+  bound_function_map.Bind(assembler.LoadContextElement(
+      native_context, Context::BOUND_FUNCTION_WITHOUT_CONSTRUCTOR_MAP_INDEX));
+  assembler.Goto(&map_done);
+
+  assembler.Bind(&with_constructor);
+  bound_function_map.Bind(assembler.LoadContextElement(
+      native_context, Context::BOUND_FUNCTION_WITH_CONSTRUCTOR_MAP_INDEX));
+  assembler.Goto(&map_done);
+
+  assembler.Bind(&map_done);
+
+  // Verify that __proto__ matches that of a the target bound function.
+  assembler.Comment("Verify that __proto__ matches target bound function");
+  Node* prototype = assembler.LoadMapPrototype(receiver_map);
+  Node* expected_prototype =
+      assembler.LoadMapPrototype(bound_function_map.value());
+  assembler.GotoIf(assembler.WordNotEqual(prototype, expected_prototype),
+                   &slow);
+
+  // Allocate the arguments array.
+  assembler.Comment("Allocate the arguments array");
+  Variable argument_array(&assembler, MachineRepresentation::kTagged);
+  Label empty_arguments(&assembler);
+  Label arguments_done(&assembler, &argument_array);
+  assembler.GotoIf(
+      assembler.IntPtrLessThanOrEqual(argc, assembler.IntPtrConstant(1)),

[Igor Sheludko, 2016/12/01 11:50:40]

I'd use unsigned comparison.

[danno, 2016/12/01 15:43:33]

Done.

+      &empty_arguments);
+  Node* one = assembler.IntPtrConstant(1);

[Igor Sheludko, 2016/12/01 11:50:40]

Please either use this in the whole function or inline (I'd do this).

[danno, 2016/12/01 15:43:33]

Done.

+  Node* elements_length = assembler.IntPtrSub(argc, one);
+  Node* elements = assembler.AllocateFixedArray(
+      FAST_ELEMENTS, elements_length, CodeStubAssembler::INTPTR_PARAMETERS);
+  Variable index(&assembler, MachineType::PointerRepresentation());
+  index.Bind(assembler.IntPtrConstant(0));
+  CodeStubAssembler::VariableList foreach_vars({&index}, assembler.zone());
+  args.ForEach(
+      foreach_vars,
+      [elements, &index](CodeStubAssembler* assembler, compiler::Node* arg) {
+        assembler->StoreFixedArrayElement(elements, index.value(), arg,
+                                          SKIP_WRITE_BARRIER, 0,

[Igor Sheludko, 2016/12/01 11:50:40]

I don't think we can skip WB here.

[danno, 2016/12/01 15:43:33]

Done.

+                                          CodeStubAssembler::INTPTR_PARAMETERS);
+        assembler->Increment(index);
+      },
+      one);
+  argument_array.Bind(elements);
+  assembler.Goto(&arguments_done);
+
+  assembler.Bind(&empty_arguments);
+  argument_array.Bind(assembler.LoadRoot(Heap::kEmptyFixedArrayRootIndex));

[Igor Sheludko, 2016/12/01 11:50:40]

EmptyFixedArrayConstant()

BTW, feel free to add more stuff to HEAP_CONSTANT_LIST.

[danno, 2016/12/01 15:43:33]

Done.

+  assembler.Goto(&arguments_done);
+
+  assembler.Bind(&arguments_done);
+
+  // Determine bound receiver.
+  assembler.Comment("Determine bound receiver");
+  Variable bound_receiver(&assembler, MachineRepresentation::kTagged);
+  Label has_receiver(&assembler);
+  Label receiver_done(&assembler, &bound_receiver);
+  assembler.GotoIf(assembler.WordNotEqual(argc, assembler.IntPtrConstant(0)),
+                   &has_receiver);
+  bound_receiver.Bind(assembler.LoadRoot(Heap::kUndefinedValueRootIndex));

[Igor Sheludko, 2016/12/01 11:50:40]

UndefinedConstant()

[danno, 2016/12/01 15:43:33]

Done.

+  assembler.Goto(&receiver_done);
+
+  assembler.Bind(&has_receiver);
+  bound_receiver.Bind(args.AtIndex(0));
+  assembler.Goto(&receiver_done);
+
+  assembler.Bind(&receiver_done);
+
+  // Allocate the resulting bound function.
+  assembler.Comment("Allocate the resulting bound function");
+  Node* bound_function = assembler.Allocate(JSBoundFunction::kSize);
+  assembler.StoreMapNoWriteBarrier(bound_function, bound_function_map.value());

[Igor Sheludko, 2016/12/01 11:50:40]

We can't skip this WB. The incremental part of the barrier should still be
triggered non-root values. 

FYI: I'm working on a CL where I'm adding SetMap which triggers map write
barrier and checking correctness of all the StoreMapNoWriteBarrier usages.

[Igor Sheludko, 2016/12/01 13:46:07]

Please ignore this. I was wrong here...

[danno, 2016/12/01 15:43:32]

Acknowledged.

[danno, 2016/12/01 15:43:33]

Done.

+  assembler.StoreObjectFieldNoWriteBarrier(

[Igor Sheludko, 2016/12/01 11:50:40]

We can't skip WB here.

[Igor Sheludko, 2016/12/01 13:46:07]

... and here ...

+      bound_function, JSBoundFunction::kBoundTargetFunctionOffset, receiver);
+  assembler.StoreObjectFieldNoWriteBarrier(bound_function,

[Igor Sheludko, 2016/12/01 11:50:40]

and here

[Igor Sheludko, 2016/12/01 13:46:07]

... and here ...

[danno, 2016/12/01 15:43:32]

Acknowledged.

+                                           JSBoundFunction::kBoundThisOffset,
+                                           bound_receiver.value());
+  assembler.StoreObjectFieldNoWriteBarrier(

[Igor Sheludko, 2016/12/01 11:50:40]

This one we could probably skip if arguments_array is still in new space but
that's not guaranteed.

[Igor Sheludko, 2016/12/01 13:46:07]

... and here.

[danno, 2016/12/01 15:43:33]

Acknowledged.

+      bound_function, JSBoundFunction::kBoundArgumentsOffset,
+      argument_array.value());
+  Node* empty_fixed_array = assembler.LoadRoot(Heap::kEmptyFixedArrayRootIndex);

[Igor Sheludko, 2016/12/01 11:50:40]

EmptyFixedArrayConstant();

[danno, 2016/12/01 15:43:33]

Done.

+  assembler.StoreObjectFieldNoWriteBarrier(
+      bound_function, JSObject::kPropertiesOffset, empty_fixed_array);
+  assembler.StoreObjectFieldNoWriteBarrier(
+      bound_function, JSObject::kElementsOffset, empty_fixed_array);
+
+  args.PopAndReturn(bound_function);
+  assembler.Bind(&slow);
+
+  Node* target = assembler.LoadFromFrame(
+      StandardFrameConstants::kFunctionOffset, MachineType::TaggedPointer());
+  assembler.TailCallStub(
+      CodeFactory::FunctionPrototypeBind(assembler.isolate()), context, target,
+      new_target, argc);

[Igor Sheludko, 2016/12/01 11:50:39]

Note: this |argc| must be Int32 again.

[danno, 2016/12/01 15:43:33]

Done.

+}
+
 // TODO(verwaest): This is a temporary helper until the FastFunctionBind stub
 // can tailcall to the builtin directly.
 RUNTIME_FUNCTION(Runtime_FunctionBind) {