Inline array constructor.

src/hydrogen.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 2049 matching lines @@
   ASSERT(length_node != NULL);
 
   int base_size = JSArray::kSize;
   if (mode_ == TRACK_ALLOCATION_SITE) {
     base_size += AllocationMemento::kSize;
   }
 
   STATIC_ASSERT(FixedDoubleArray::kHeaderSize == FixedArray::kHeaderSize);
   base_size += FixedArray::kHeaderSize;
 
+  if (length_node->IsConstant()) {
+    HConstant* constant_length_node = HConstant::cast(length_node);
+    if (constant_length_node->HasInteger32Value()) {
+      int size = base_size + elements_size() *
+          constant_length_node->Integer32Value();
+      HInstruction* const_size = builder()->Add<HConstant>(size);
+      return const_size;
+    }
+  }
+
   HInstruction* elements_size_value =
       builder()->Add<HConstant>(elements_size());
   HInstruction* mul = builder()->Add<HMul>(length_node, elements_size_value);
   mul->ClearFlag(HValue::kCanOverflow);
 
   HInstruction* base = builder()->Add<HConstant>(base_size);
   HInstruction* total_size = builder()->Add<HAdd>(base, mul);
   total_size->ClearFlag(HValue::kCanOverflow);
   return total_size;
 }
@@ skipping 5082 matching lines @@
 
       call = New<HCallFunction>(function, argument_count);
       Drop(argument_count + 1);
     }
   }
 
   return ast_context()->ReturnInstruction(call, expr->id());
 }
 
 
+HValue* HOptimizedGraphBuilder::BuildInlinedCallNewArray_OneArg(
+    CallNew* expr,
+    HValue* constructor,
+    HInstruction* cell_instruction) {
+  ElementsKind kind = expr->elements_kind();
+  HValue* argument = environment()->ExpressionStackAt(0);

[Toon Verwaest, 2013/11/06 16:42:44]

Top();

[mvstanton, 2013/11/07 16:34:05]

Done.

+  HValue* new_object;
+  if (argument->IsConstant()) {
+    HConstant* constant_argument = HConstant::cast(argument);
+    ASSERT(constant_argument->HasSmiValue());
+    int value = constant_argument->Integer32Value();

[Toon Verwaest, 2013/11/06 16:42:44]

Rename value to array_size.

[mvstanton, 2013/11/07 16:34:05]

Done.

+    JSArrayBuilder array_builder(this,
+                                 value == 0
+                                   ? kind
+                                   : GetHoleyElementsKind(kind),
+                                 cell_instruction,
+                                 constructor,
+                                 DISABLE_ALLOCATION_SITES);
+
+    new_object = (value == 0)
+        ? array_builder.AllocateEmptyArray()
+        : array_builder.AllocateArray(argument, argument, true);

[Toon Verwaest, 2013/11/06 16:42:44]

Can we replace that "true" with an enum indicating the meaning?

[mvstanton, 2013/11/07 16:34:05]

Done. Doing this correctly took me on a refactoring journey into
code-stubs-hydrogen because the block just below (pushing capacity and length)
was essentially duplicated.

+  } else {
+    ASSERT(IsHoleyElementsKind(kind));
+
+    // Smi check and range check on the input arg.
+    HValue* constant_zero = graph()->GetConstant0();
+    HConstant* max_alloc_length =
+        Add<HConstant>(JSObject::kInitialMaxFastElementArray);
+    const int initial_capacity = JSArray::kPreallocatedArrayElements;
+    HConstant* initial_capacity_node = Add<HConstant>(initial_capacity);
+    HInstruction* checked_arg = Add<HBoundsCheck>(argument, max_alloc_length);
+    IfBuilder if_builder(this);
+    if_builder.If<HCompareNumericAndBranch>(checked_arg, constant_zero,
+                                            Token::EQ);
+
+    if_builder.Then();
+    Push(initial_capacity_node);    // capacity
+    Push(constant_zero);            // length
+    if_builder.Else();
+    Push(checked_arg);              // capacity
+    Push(checked_arg);              // length
+    if_builder.End();
+
+    // Figure out total size
+    HValue* length = Pop();
+    HValue* capacity = Pop();
+    JSArrayBuilder array_builder(this,
+                                 kind,
+                                 cell_instruction,
+                                 constructor,
+                                 DISABLE_ALLOCATION_SITES);
+    new_object = array_builder.AllocateArray(capacity, length, true);
+  }
+
+  return new_object;
+}
+
+
+void HOptimizedGraphBuilder::BuildInlinedCallNewArray(CallNew* expr) {
+  NoObservableSideEffectsScope no_effects(this);
+
+  int argument_count = expr->arguments()->length();
+  // We should at least have the constructor on the expression stack.
+  ASSERT(!environment()->ExpressionStackIsEmpty());
+  HValue* constructor = environment()->ExpressionStackAt(argument_count);
+
+  ElementsKind kind = expr->elements_kind();
+  Handle<Cell> cell = expr->allocation_info_cell();
+  AllocationSite* site = AllocationSite::cast(cell->value());
+
+  // Register on the site for deoptimization if the cell value changes.
+  site->AddDependentCompilationInfo(AllocationSite::TRANSITIONS, top_info());
+  HInstruction* cell_instruction = Add<HConstant>(cell);
+
+  // Build the array.
+  HValue* new_object;
+  if (argument_count == 0) {
+    JSArrayBuilder array_builder(this,
+                                 kind,
+                                 cell_instruction,
+                                 constructor,
+                                 DISABLE_ALLOCATION_SITES);
+    new_object = array_builder.AllocateEmptyArray();
+  } else if (argument_count == 1) {
+    new_object = BuildInlinedCallNewArray_OneArg(expr, constructor,
+                                                 cell_instruction);
+  } else {
+    ASSERT(argument_count <= 5);

[Toon Verwaest, 2013/11/06 16:42:44]

Why 5? Can we at least make it a descriptive constant somewhere?

[mvstanton, 2013/11/07 16:34:05]

Done.

+    JSArrayBuilder array_builder(this,
+                                 kind,
+                                 cell_instruction,
+                                 constructor,
+                                 DISABLE_ALLOCATION_SITES);
+    HValue* length = Add<HConstant>(argument_count);
+    bool fill_with_hole = IsFastSmiElementsKind(kind);
+    new_object = array_builder.AllocateArray(length, length, fill_with_hole);

[Toon Verwaest, 2013/11/06 16:42:44]

Why do we only fill with holes if the kind is smi? At least add a comment.

[mvstanton, 2013/11/07 16:34:05]

Done. Bailout could occur in the HStoreKeyed if we try to initialize a smi array
with a HeapNumber too big. So we have to leave the array without garbage if that
happens.

Also added a TODO that if all the arguments are constants in smi range then we
can eliminate the hole filling.

+    HValue* elements = array_builder.GetElementsLocation();
+    for (int i = 0; i < argument_count; i++) {
+      HValue* value = environment()->ExpressionStackAt(argument_count - i - 1);
+      HValue* constant_i = Add<HConstant>(i);
+      Add<HStoreKeyed>(elements, constant_i, value, kind);
+    }
+  }
+
+  Drop(argument_count + 1);  // drop constructor and args.
+  ast_context()->ReturnValue(new_object);
+}
+
+
 // Checks whether allocation using the given constructor can be inlined.
 static bool IsAllocationInlineable(Handle<JSFunction> constructor) {
   return constructor->has_initial_map() &&
       constructor->initial_map()->instance_type() == JS_OBJECT_TYPE &&
       constructor->initial_map()->instance_size() < HAllocate::kMaxInlineSize &&
       constructor->initial_map()->InitialPropertiesLength() == 0;
 }
 
 
+bool HOptimizedGraphBuilder::IsCallNewArrayInlineable(CallNew* expr) {
+  bool inline_ok = false;
+  int argument_count = expr->arguments()->length();
+  // We should have the function plus array arguments on the environment stack.
+  ASSERT(environment()->length() >= (argument_count + 1));
+  if (argument_count <= 5) {

[Toon Verwaest, 2013/11/06 16:42:44]

Why 5?

[mvstanton, 2013/11/07 16:34:05]

Why indeed. I eliminated this as any predicate.

+    Handle<Cell> cell = expr->allocation_info_cell();
+    AllocationSite* site = AllocationSite::cast(cell->value());
+    if (!site->DoNotInlineCall()) {

[Toon Verwaest, 2013/11/06 16:42:44]

What about just CanInlineCall() rather than the double negative?

[mvstanton, 2013/11/07 16:34:05]

Done.

+      // We also want to avoid inlining in certain 1 argument scenarios.
+      if (argument_count == 1) {
+        HValue* argument = Top();
+        if (argument->IsConstant()) {
+          // Do not inline if the constant length argument is not a smi or
+          // outside the valid range for a fast array.
+          HConstant* constant_argument = HConstant::cast(argument);
+          if (constant_argument->HasSmiValue()) {
+            int value = constant_argument->Integer32Value();
+            inline_ok = value >= 0 &&
+                value < JSObject::kInitialMaxFastElementArray;
+          }
+        } else {
+          // The amount of code we'd have to generate doubles if we have a
+          // non-constant length, but packed elements_kind feedback. Don't
+          // inline this case.

[Toon Verwaest, 2013/11/06 16:42:44]

Shouldn't we just generate a check whether the argument is 0 and bail out if
not?

[mvstanton, 2013/11/07 16:34:05]

I think it's a lot of trouble for a corner case (never happens in benchmarks).
We'd bail out and then have to set the AllocationSite::DoNotInline flag for the
next time around. But setting that flag is awkward. The bailout code would have
to recognize that it was called with completely valid arguments
(FAST_SMI_ELEMENTS, 0 < index < kInitialMaxFastElementArray), and that needs to
be the signal to set the DoNotInline flag. Alternatively, the DoNotInline flag
could be set just before the bailout. It just doesn't seem worth it. 

+          inline_ok = IsHoleyElementsKind(expr->elements_kind());
+        }
+      } else {
+        inline_ok = true;
+      }
+    }
+  }
+  return inline_ok;
+}
+
+
 void HOptimizedGraphBuilder::VisitCallNew(CallNew* expr) {
   ASSERT(!HasStackOverflow());
   ASSERT(current_block() != NULL);
   ASSERT(current_block()->HasPredecessor());
   if (!FLAG_emit_opt_code_positions) SetSourcePosition(expr->position());
   int argument_count = expr->arguments()->length() + 1;  // Plus constructor.
   Factory* factory = isolate()->factory();
 
+  // The constructor function is on the stack in the unoptimized code
+  // during evaluation of the arguments.
+  CHECK_ALIVE(VisitForValue(expr->expression()));
+  HValue* function = Top();
+  CHECK_ALIVE(VisitExpressions(expr->arguments()));
+
   if (FLAG_inline_construct &&
       expr->IsMonomorphic() &&
       IsAllocationInlineable(expr->target())) {
-    // The constructor function is on the stack in the unoptimized code
-    // during evaluation of the arguments.
-    CHECK_ALIVE(VisitForValue(expr->expression()));
-    HValue* function = Top();
-    CHECK_ALIVE(VisitExpressions(expr->arguments()));
     Handle<JSFunction> constructor = expr->target();
     HValue* check = Add<HCheckValue>(function, constructor);
 
     // Force completion of inobject slack tracking before generating
     // allocation code to finalize instance size.
     if (constructor->shared()->IsInobjectSlackTrackingInProgress()) {
       constructor->shared()->CompleteInobjectSlackTracking();
     }
 
     // Calculate instance size from initial map of constructor.
@@ skipping 66 matching lines @@
     check->DeleteAndReplaceWith(NULL);
     environment()->SetExpressionStackAt(receiver_index, function);
     HInstruction* call =
       PreProcessCall(New<HCallNew>(function, argument_count));
     return ast_context()->ReturnInstruction(call, expr->id());
   } else {
     // The constructor function is both an operand to the instruction and an
     // argument to the construct call.
     Handle<JSFunction> array_function(
         isolate()->global_context()->array_function(), isolate());
-    CHECK_ALIVE(VisitArgument(expr->expression()));
-    HValue* constructor = HPushArgument::cast(Top())->argument();

[Toon Verwaest, 2013/11/06 16:42:44]

Why is it ok to merge this with the code above? The code above contains the
function on the TOS, this code contains an HPushArgument(function) on the TOS.

[mvstanton, 2013/11/07 16:34:05]

Because I changed this whole else block to work like the block above: push
function and args onto the expression stack. Then, if we decide to make the
call, we use PreProcessCall() below to remove from the expression stack and turn
into PushArguments. The reason for that is in order to decide if the array call
is eligible for inlining I need to examine the expressions.

-    CHECK_ALIVE(VisitArgumentList(expr->arguments()));
+    bool use_call_new_array = expr->target().is_identical_to(array_function);
+    Handle<Cell> cell = expr->allocation_info_cell();
+    if (use_call_new_array && IsCallNewArrayInlineable(expr)) {
+      // Verify we are still calling the array function for our native context.
+      Add<HCheckValue>(function, array_function);
+      BuildInlinedCallNewArray(expr);
+      return;
+    }
+
     HBinaryCall* call;
-    if (expr->target().is_identical_to(array_function)) {
-      Handle<Cell> cell = expr->allocation_info_cell();
-      Add<HCheckValue>(constructor, array_function);
-      call = New<HCallNewArray>(constructor, argument_count,
-                                cell, expr->elements_kind());
+    if (use_call_new_array) {
+      Add<HCheckValue>(function, array_function);
+      call = New<HCallNewArray>(function, argument_count, cell,
+                                expr->elements_kind());
     } else {
-      call = New<HCallNew>(constructor, argument_count);
+      call = New<HCallNew>(function, argument_count);
     }
-    Drop(argument_count);
+    PreProcessCall(call);
     return ast_context()->ReturnInstruction(call, expr->id());
   }
 }
 
 
 // Support for generating inlined runtime functions.
 
 // Lookup table for generators for runtime calls that are generated inline.
 // Elements of the table are member pointers to functions of
 // HOptimizedGraphBuilder.
@@ skipping 2517 matching lines @@
   if (ShouldProduceTraceOutput()) {
     isolate()->GetHTracer()->TraceHydrogen(name(), graph_);
   }
 
 #ifdef DEBUG
   graph_->Verify(false);  // No full verify.
 #endif
 }
 
 } }  // namespace v8::internal