Make FastNewFunctionContextStub take slots parameter

src/code-stubs.cc

 // Copyright 2012 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/code-stubs.h"
 
 #include <sstream>
 
 #include "src/bootstrapper.h"
 #include "src/code-factory.h"
@@ skipping 4475 matching lines @@
   return result;
 }
 
 void FastNewClosureStub::GenerateAssembly(CodeStubAssembler* assembler) const {
   assembler->Return(
       Generate(assembler, assembler->Parameter(0), assembler->Parameter(1)));
 }
 
 void FastNewFunctionContextStub::GenerateAssembly(
     CodeStubAssembler* assembler) const {
+  typedef CodeStubAssembler::Label Label;
   typedef compiler::Node Node;
+  typedef CodeStubAssembler::Variable Variable;
 
-  int length = slots() + Context::MIN_CONTEXT_SLOTS;
-  int size = length * kPointerSize + FixedArray::kHeaderSize;
-
-  // Get the function
   Node* function =
       assembler->Parameter(FastNewFunctionContextDescriptor::kFunctionIndex);
+  Node* slots =
+      assembler->Parameter(FastNewFunctionContextDescriptor::kSlotsIndex);
   Node* context =
       assembler->Parameter(FastNewFunctionContextDescriptor::kContextIndex);
 
+  Node* min_context_slots =
+      assembler->Int32Constant(Context::MIN_CONTEXT_SLOTS);

[klaasb, 2016/07/26 17:04:52]

I've used int32 throughout, as it was used before and it works. However nothing
should ever be negative here and in the end the array stores further down use
IntPtr arithmetic anyway. Any reason why I shouldn't use Ptr here?

+  Node* length = assembler->Int32Add(slots, min_context_slots);
+  Node* size = assembler->Int32Add(
+      assembler->Int32Mul(length, assembler->Int32Constant(kPointerSize)),

[rmcilroy, 2016/07/27 11:11:38]

You could probably shift by kPointerSizeLog2 instead (although TF might already
do this optimization in instruction selection

[klaasb, 2016/07/27 17:06:31]

TF does it on the graph itself before instruction selection:
https://cs.chromium.org/chromium/src/v8/src/compiler/machine-operator-reducer...

[rmcilroy, 2016/07/28 15:04:27]

I don't think this phase is run on code generated by the code assembler though
(see Pipeline::GenerateCodeForCodeStub()), so it's probably worth doing here
explicitly.

[klaasb, 2016/07/28 15:34:47]

Done.

+      assembler->Int32Constant(FixedArray::kHeaderSize));
+
   // Create a new closure from the given function info in new space
   Node* function_context = assembler->Allocate(size);
 
   assembler->StoreMapNoWriteBarrier(
       function_context,
       assembler->HeapConstant(isolate()->factory()->function_context_map()));
-  assembler->StoreObjectFieldNoWriteBarrier(
-      function_context, Context::kLengthOffset,
-      assembler->SmiConstant(Smi::FromInt(length)));
+  assembler->StoreObjectFieldNoWriteBarrier(function_context,
+                                            Context::kLengthOffset,
+                                            assembler->SmiFromWord32(length));
 
   // Set up the fixed slots.
   assembler->StoreFixedArrayElement(
       function_context, assembler->Int32Constant(Context::CLOSURE_INDEX),
       function, SKIP_WRITE_BARRIER);
   assembler->StoreFixedArrayElement(
       function_context, assembler->Int32Constant(Context::PREVIOUS_INDEX),
       context, SKIP_WRITE_BARRIER);
   assembler->StoreFixedArrayElement(
       function_context, assembler->Int32Constant(Context::EXTENSION_INDEX),
       assembler->TheHoleConstant(), SKIP_WRITE_BARRIER);
 
   // Copy the native context from the previous context.
   Node* native_context = assembler->LoadFixedArrayElement(
       context, assembler->Int32Constant(Context::NATIVE_CONTEXT_INDEX));
   assembler->StoreFixedArrayElement(
       function_context, assembler->Int32Constant(Context::NATIVE_CONTEXT_INDEX),
       native_context, SKIP_WRITE_BARRIER);
 
   // Initialize the rest of the slots to undefined.
   Node* undefined = assembler->UndefinedConstant();
-  for (int i = Context::MIN_CONTEXT_SLOTS; i < length; ++i) {
-    assembler->StoreFixedArrayElement(function_context,
-                                      assembler->Int32Constant(i), undefined,
-                                      SKIP_WRITE_BARRIER);
+  Variable var_slot_index(assembler, MachineType::PointerRepresentation());
+  var_slot_index.Bind(min_context_slots);
+  Label loop(assembler, &var_slot_index), after_loop(assembler);
+  assembler->Goto(&loop);
+
+  assembler->Bind(&loop);
+  {
+    Node* slot_index = var_slot_index.value();
+    Label initialize_slot(assembler);
+    assembler->Branch(assembler->Int32LessThan(slot_index, length),
+                      &initialize_slot, &after_loop);

[rmcilroy, 2016/07/27 11:11:38]

You could do this check on the backward branch - you are guaranteed that there
will be at least Context::MIN_CONTEXT_SLOTS

[klaasb, 2016/07/27 17:06:31]

Done.

+
+    assembler->Bind(&initialize_slot);
+    {
+      assembler->StoreFixedArrayElement(function_context,
+                                        var_slot_index.value(), undefined,
+                                        SKIP_WRITE_BARRIER);
+      Node* one = assembler->Int32Constant(1);
+      Node* next_index = assembler->Int32Add(var_slot_index.value(), one);
+      var_slot_index.Bind(next_index);
+      assembler->Goto(&loop);
+    }
   }
+  assembler->Bind(&after_loop);
 
   assembler->Return(function_context);
 }
 
 void CreateAllocationSiteStub::GenerateAheadOfTime(Isolate* isolate) {
   CreateAllocationSiteStub stub(isolate);
   stub.GetCode();
 }
 
 
@@ skipping 310 matching lines @@
   if (type->Is(Type::UntaggedPointer())) {
     return Representation::External();
   }
 
   DCHECK(!type->Is(Type::Untagged()));
   return Representation::Tagged();
 }
 
 }  // namespace internal
 }  // namespace v8