[stubs] Port FastFunctionBindStub to TF

src/builtins/builtins-function.cc

 // Copyright 2016 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
 
 namespace v8 {
 namespace internal {
 
 namespace {
 
 // ES6 section 19.2.1.1.1 CreateDynamicFunction
 MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,
@@ skipping 230 matching lines @@
                                     &it, name, it.property_attributes()));
   }
   return *function;
 }
 
 }  // namespace
 
 // 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) {
   DCHECK_EQ(2, args.length());
   Arguments* incoming = reinterpret_cast<Arguments*>(args[0]);
   // Rewrap the arguments as builtins arguments.
   int argc = incoming->length() + BuiltinArguments::kNumExtraArgsWithReceiver;
   BuiltinArguments caller_args(argc, incoming->arguments() + 1);
   return DoFunctionBind(isolate, caller_args);
 }
@@ skipping 21 matching lines @@
 
   Node* f = assembler.Parameter(0);
   Node* v = assembler.Parameter(1);
   Node* context = assembler.Parameter(4);
   Node* result = assembler.OrdinaryHasInstance(context, f, v);
   assembler.Return(result);
 }
 
 }  // namespace internal
 }  // namespace v8