Implement inlining of constructor calls.

src/hydrogen.cc

Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index a27360a3fd868b428ccbe4d7388f083cb914a18a..c8b607ea8058c6ce922f4cefc29628ba0073e432 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -543,7 +543,7 @@ HConstant* HGraph::GetConstantHole() {
 HGraphBuilder::HGraphBuilder(CompilationInfo* info,
                              TypeFeedbackOracle* oracle)
     : function_state_(NULL),
-      initial_function_state_(this, info, oracle, false),
+      initial_function_state_(this, info, oracle, false, false),

[Vyacheslav Egorov (Chromium), 2012/02/13 15:01:39]

I think we should come up with enum at least for is_construct.

Call-site gets unreadable.

[Michael Starzinger, 2012/02/27 14:16:32]

Done. Merged drop_extra and is_construct into one flag.

       ast_context_(NULL),
       break_scope_(NULL),
       graph_(NULL),
@@ -2026,12 +2026,14 @@ void HGraph::ComputeMinusZeroChecks() {
 FunctionState::FunctionState(HGraphBuilder* owner,
                              CompilationInfo* info,
                              TypeFeedbackOracle* oracle,
-                             bool drop_extra)
+                             bool drop_extra,
+                             bool is_construct)
     : owner_(owner),
       compilation_info_(info),
       oracle_(oracle),
       call_context_(NULL),
       drop_extra_(drop_extra),
+      is_construct_(is_construct),
       function_return_(NULL),
       test_context_(NULL),
       outer_(owner->function_state()) {
@@ -2074,7 +2076,7 @@ AstContext::AstContext(HGraphBuilder* owner, Expression::Context kind)
       for_typeof_(false) {
   owner->set_ast_context(this);  // Push.
 #ifdef DEBUG
-  ASSERT(!owner->environment()->is_arguments_adaptor());
+  ASSERT(owner->environment()->frame_type() == JS_FUNCTION);
   original_length_ = owner->environment()->length();
 #endif
 }
@@ -2089,7 +2091,7 @@ EffectContext::~EffectContext() {
   ASSERT(owner()->HasStackOverflow() ||
          owner()->current_block() == NULL ||
          (owner()->environment()->length() == original_length_ &&
-          !owner()->environment()->is_arguments_adaptor()));
+          owner()->environment()->frame_type() == JS_FUNCTION));
 }
 
 
@@ -2097,7 +2099,7 @@ ValueContext::~ValueContext() {
   ASSERT(owner()->HasStackOverflow() ||
          owner()->current_block() == NULL ||
          (owner()->environment()->length() == original_length_ + 1 &&
-          !owner()->environment()->is_arguments_adaptor()));
+          owner()->environment()->frame_type() == JS_FUNCTION));
 }
 
 
@@ -2672,7 +2674,38 @@ void HGraphBuilder::VisitReturnStatement(ReturnStatement* stmt) {
     CHECK_ALIVE(VisitForValue(stmt->expression()));
     HValue* result = environment()->Pop();
     current_block()->FinishExit(new(zone()) HReturn(result));
-    set_current_block(NULL);
+  } else if (function_state()->is_construct()) {
+    // Return from an inlined construct call.  In a test context the return
+    // value will always evaluate to true, in a value context the return value
+    // needs to be a JSObject.
+    if (context->IsTest()) {
+      TestContext* test = TestContext::cast(context);
+      CHECK_ALIVE(VisitForEffect(stmt->expression()));
+      current_block()->Goto(test->if_true(), function_state()->drop_extra());
+    } else if (context->IsEffect()) {
+      CHECK_ALIVE(VisitForEffect(stmt->expression()));
+      current_block()->Goto(function_return(), function_state()->drop_extra());
+    } else {
+      ASSERT(context->IsValue());
+      CHECK_ALIVE(VisitForValue(stmt->expression()));
+      HValue* return_value = Pop();
+      HValue* receiver = environment()->Lookup(0);
+      HHasInstanceTypeAndBranch* typecheck =
+          new(zone()) HHasInstanceTypeAndBranch(return_value,
+                                                FIRST_SPEC_OBJECT_TYPE,
+                                                LAST_SPEC_OBJECT_TYPE);
+      HBasicBlock* if_spec_object = graph()->CreateBasicBlock();
+      HBasicBlock* not_spec_object = graph()->CreateBasicBlock();
+      typecheck->SetSuccessorAt(0, if_spec_object);
+      typecheck->SetSuccessorAt(1, not_spec_object);
+      current_block()->Finish(typecheck);
+      if_spec_object->AddLeaveInlined(return_value,
+                                      function_return(),
+                                      function_state()->drop_extra());
+      not_spec_object->AddLeaveInlined(receiver,
+                                       function_return(),
+                                       function_state()->drop_extra());
+    }
   } else {
     // Return from an inlined function, visit the subexpression in the
     // expression context of the call.
@@ -2687,13 +2720,13 @@ void HGraphBuilder::VisitReturnStatement(ReturnStatement* stmt) {
     } else {
       ASSERT(context->IsValue());
       CHECK_ALIVE(VisitForValue(stmt->expression()));
-      HValue* return_value = environment()->Pop();
+      HValue* return_value = Pop();
       current_block()->AddLeaveInlined(return_value,
                                        function_return(),
                                        function_state()->drop_extra());
     }
-    set_current_block(NULL);
   }
+  set_current_block(NULL);
 }
 
 
@@ -4711,7 +4744,7 @@ void HGraphBuilder::HandlePolymorphicCallNamed(Call* expr,
         PrintF("Trying to inline the polymorphic call to %s\n",
                *name->ToCString());
       }
-      if (FLAG_polymorphic_inlining && TryInline(expr)) {
+      if (FLAG_polymorphic_inlining && TryInlineCall(expr)) {
         // Trying to inline will signal that we should bailout from the
         // entire compilation by setting stack overflow on the visitor.
         if (HasStackOverflow()) return;
@@ -4781,19 +4814,19 @@ void HGraphBuilder::TraceInline(Handle<JSFunction> target,
 }
 
 
-bool HGraphBuilder::TryInline(Call* expr, bool drop_extra) {
+bool HGraphBuilder::TryInline(CallKind call_kind,
+                              Handle<JSFunction> target,
+                              ZoneList<Expression*>* arguments,
+                              HValue* receiver,
+                              int ast_id,
+                              int return_id,
+                              bool drop_extra,

[Vyacheslav Egorov (Chromium), 2012/02/13 15:01:39]

Consider introducing enums

[Michael Starzinger, 2012/02/27 14:16:32]

Done. Merged drop_extra and is_construct into one flag.

+                              bool is_construct) {
   if (!FLAG_use_inlining) return false;
 
-  // The function call we are inlining is a method call if the call
-  // is a property call.
-  CallKind call_kind = (expr->expression()->AsProperty() == NULL)
-      ? CALL_AS_FUNCTION
-      : CALL_AS_METHOD;
-
   // Precondition: call is monomorphic and we have found a target with the
   // appropriate arity.
   Handle<JSFunction> caller = info()->closure();
-  Handle<JSFunction> target = expr->target();
   Handle<SharedFunctionInfo> target_shared(target->shared());
 
   // Do a quick check on source code length to avoid parsing large
@@ -4830,7 +4863,7 @@ bool HGraphBuilder::TryInline(Call* expr, bool drop_extra) {
       TraceInline(target, caller, "inline depth limit reached");
       return false;
     }
-    if (!env->outer()->is_arguments_adaptor()) {
+    if (env->outer()->frame_type() == JS_FUNCTION) {
       current_level++;
     }
     env = env->outer();
@@ -4941,16 +4974,17 @@ bool HGraphBuilder::TryInline(Call* expr, bool drop_extra) {
       isolate());
   // The function state is new-allocated because we need to delete it
   // in two different places.
-  FunctionState* target_state =
-      new FunctionState(this, &target_info, &target_oracle, drop_extra);
+  FunctionState* target_state = new FunctionState(
+      this, &target_info, &target_oracle, drop_extra, is_construct);
 
   HConstant* undefined = graph()->GetConstantUndefined();
   HEnvironment* inner_env =
       environment()->CopyForInlining(target,
-                                     expr->arguments()->length(),
+                                     arguments->length(),
                                      function,
                                      undefined,
-                                     call_kind);
+                                     call_kind,
+                                     is_construct);
 #ifdef V8_TARGET_ARCH_IA32
   // IA32 only, overwrite the caller's context in the deoptimization
   // environment with the correct one.
@@ -4964,12 +4998,13 @@ bool HGraphBuilder::TryInline(Call* expr, bool drop_extra) {
 #endif
   HBasicBlock* body_entry = CreateBasicBlock(inner_env);
   current_block()->Goto(body_entry);
-  body_entry->SetJoinId(expr->ReturnId());
+  body_entry->SetJoinId(return_id);
   set_current_block(body_entry);
   AddInstruction(new(zone()) HEnterInlined(target,
-                                           expr->arguments()->length(),
+                                           arguments->length(),
                                            function,
-                                           call_kind));
+                                           call_kind,
+                                           is_construct));
   VisitDeclarations(target_info.scope()->declarations());
   VisitStatements(function->body());
   if (HasStackOverflow()) {
@@ -4988,15 +5023,17 @@ bool HGraphBuilder::TryInline(Call* expr, bool drop_extra) {
   TraceInline(target, caller, NULL);
 
   if (current_block() != NULL) {
-    // Add a return of undefined if control can fall off the body.  In a
-    // test context, undefined is false.
+    // Add default return value (i.e. undefined for normals calls or the newly
+    // allocated receiver for construct calls) if control can fall off the
+    // body.  In a test context, undefined is false and any JSObject is true.
+    HValue* return_value = is_construct ? receiver : undefined;
     if (inlined_test_context() == NULL) {
       ASSERT(function_return() != NULL);
       ASSERT(call_context()->IsEffect() || call_context()->IsValue());
       if (call_context()->IsEffect()) {
         current_block()->Goto(function_return(), drop_extra);
       } else {
-        current_block()->AddLeaveInlined(undefined,
+        current_block()->AddLeaveInlined(return_value,
                                          function_return(),
                                          drop_extra);
       }
@@ -5009,7 +5046,8 @@ bool HGraphBuilder::TryInline(Call* expr, bool drop_extra) {
       ASSERT(call_context()->IsTest());
       HBasicBlock* empty_true = graph()->CreateBasicBlock();
       HBasicBlock* empty_false = graph()->CreateBasicBlock();
-      HBranch* test = new(zone()) HBranch(undefined, empty_true, empty_false);
+      HBranch* test =
+          new(zone()) HBranch(return_value, empty_true, empty_false);
       current_block()->Finish(test);
 
       empty_true->Goto(inlined_test_context()->if_true(), drop_extra);
@@ -5029,12 +5067,12 @@ bool HGraphBuilder::TryInline(Call* expr, bool drop_extra) {
 
     // Forward to the real test context.
     if (if_true->HasPredecessor()) {
-      if_true->SetJoinId(expr->id());
+      if_true->SetJoinId(ast_id);
       HBasicBlock* true_target = TestContext::cast(ast_context())->if_true();
       if_true->Goto(true_target, function_state()->drop_extra());
     }
     if (if_false->HasPredecessor()) {
-      if_false->SetJoinId(expr->id());
+      if_false->SetJoinId(ast_id);
       HBasicBlock* false_target = TestContext::cast(ast_context())->if_false();
       if_false->Goto(false_target, function_state()->drop_extra());
     }
@@ -5042,7 +5080,7 @@ bool HGraphBuilder::TryInline(Call* expr, bool drop_extra) {
     return true;
 
   } else if (function_return()->HasPredecessor()) {
-    function_return()->SetJoinId(expr->id());
+    function_return()->SetJoinId(ast_id);
     set_current_block(function_return());
   } else {
     set_current_block(NULL);
@@ -5052,6 +5090,36 @@ bool HGraphBuilder::TryInline(Call* expr, bool drop_extra) {
 }
 
 
+bool HGraphBuilder::TryInlineCall(Call* expr, bool drop_extra) {
+  Handle<JSFunction> target = expr->target();
+  ZoneList<Expression*>* arguments = expr->arguments();
+  HValue* receiver = NULL;
+  int return_id = expr->ReturnId();
+  int ast_id = expr->id();
+
+  // The function call we are inlining is a method call if the call
+  // is a property call.
+  CallKind call_kind = (expr->expression()->AsProperty() == NULL)
+      ? CALL_AS_FUNCTION
+      : CALL_AS_METHOD;
+
+  return TryInline(call_kind, target, arguments, receiver, ast_id, return_id,
+                   drop_extra, false);
+}
+
+
+bool HGraphBuilder::TryInlineConstruct(CallNew* expr, HValue* receiver) {
+  Handle<JSFunction> target = expr->target();
+  ZoneList<Expression*>* arguments = expr->arguments();
+  CallKind call_kind = CALL_AS_FUNCTION;
+  int return_id = expr->ReturnId();
+  int ast_id = expr->id();
+
+  return TryInline(call_kind, target, arguments, receiver, ast_id, return_id,
+                   false, true);
+}
+
+
 bool HGraphBuilder::TryInlineBuiltinFunction(Call* expr,
                                              HValue* receiver,
                                              Handle<Map> receiver_map,
@@ -5335,7 +5403,7 @@ void HGraphBuilder::VisitCall(Call* expr) {
       } else {
         AddCheckConstantFunction(expr, receiver, receiver_map, true);
 
-        if (TryInline(expr)) return;
+        if (TryInlineCall(expr)) return;
         call = PreProcessCall(
             new(zone()) HCallConstantFunction(expr->target(),
                                               argument_count));
@@ -5391,7 +5459,7 @@ void HGraphBuilder::VisitCall(Call* expr) {
                IsGlobalObject());
         environment()->SetExpressionStackAt(receiver_index, global_receiver);
 
-        if (TryInline(expr)) return;
+        if (TryInlineCall(expr)) return;
         call = PreProcessCall(new(zone()) HCallKnownGlobal(expr->target(),
                                                            argument_count));
       } else {
@@ -5417,7 +5485,7 @@ void HGraphBuilder::VisitCall(Call* expr) {
       PushAndAdd(receiver);
       CHECK_ALIVE(VisitExpressions(expr->arguments()));
       AddInstruction(new(zone()) HCheckFunction(function, expr->target()));
-      if (TryInline(expr, true)) {   // Drop function from environment.
+      if (TryInlineCall(expr, true)) {  // Drop function from environment.
         return;
       } else {
         call = PreProcessCall(new(zone()) HInvokeFunction(context,
@@ -5447,25 +5515,75 @@ void HGraphBuilder::VisitCall(Call* expr) {
 }
 
 
+// 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;
+}
+
+
 void HGraphBuilder::VisitCallNew(CallNew* expr) {
   ASSERT(!HasStackOverflow());
   ASSERT(current_block() != NULL);
   ASSERT(current_block()->HasPredecessor());
-  // The constructor function is also used as the receiver argument to the
-  // JS construct call builtin.
-  HValue* constructor = NULL;
-  CHECK_ALIVE(constructor = VisitArgument(expr->expression()));
-  CHECK_ALIVE(VisitArgumentList(expr->arguments()));
-
+  expr->RecordTypeFeedback(oracle());
+  int argument_count = expr->arguments()->length() + 1;  // Plus constructor.
   HValue* context = environment()->LookupContext();
 
-  // The constructor is both an operand to the instruction and an argument
-  // to the construct call.
-  int arg_count = expr->arguments()->length() + 1;  // Plus constructor.
-  HCallNew* call = new(zone()) HCallNew(context, constructor, arg_count);
-  call->set_position(expr->position());
-  Drop(arg_count);
-  return ast_context()->ReturnInstruction(call, expr->id());
+  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();
+    AddInstruction(new(zone()) HCheckFunction(function, constructor));
+
+    // Replace the constructor function with a newly allocated receiver.
+    HInstruction* receiver = new(zone()) HAllocateObject(context,
+                                                         function,
+                                                         constructor);
+    // Index of the receiver from the top of the expression stack.
+    const int receiver_index = argument_count - 1;
+    AddInstruction(receiver);
+    ASSERT(environment()->ExpressionStackAt(receiver_index) == function);
+    environment()->SetExpressionStackAt(receiver_index, receiver);
+
+    if (TryInlineConstruct(expr, receiver)) return;
+    HInstruction* call = PreProcessCall(

[Vyacheslav Egorov (Chromium), 2012/02/13 15:01:39]

Is lazy bailout handled correctly on this call-site? We need to replace
non-object result value with receiver when we bailout lazily here.

The same for stack inspection at this call-site? (e.g. %_IsConstructCall or
debugger). 

Please ensure that these cases are covered by your test case.

[It seems for simplicity we might want to avoid partial constructor inlining
like this in the initial CL version.]

[Michael Starzinger, 2012/02/27 14:16:32]

Done. I temporarily disabled partial inlining as suggested, by just undoing most
of the changes that have been applied to the hydrogen graph before inlining was
attempted.

It's not pretty and I will try to make partial inlining work with lazy bailout
in a separate CL.

+        new(zone()) HInvokeFunction(context, function, argument_count));
+    PushAndAdd(call);
+    AddSimulate(expr->id());
+
+    // Check whether return value of constructor is a JSObject.
+    HValue* return_value = Pop();
+    HHasInstanceTypeAndBranch* typecheck =

[Vyacheslav Egorov (Chromium), 2012/02/13 15:01:39]

Branch construction code seems to be duplicated with another place
(VisitReturnStatement). Can it be shared?

[Michael Starzinger, 2012/02/27 14:16:32]

Comment no longer applies after addressing previous comment.

+        new(zone()) HHasInstanceTypeAndBranch(return_value,
+                                              FIRST_SPEC_OBJECT_TYPE,
+                                              LAST_SPEC_OBJECT_TYPE);
+    HBasicBlock* if_spec_object = graph()->CreateBasicBlock();
+    HBasicBlock* not_spec_object = graph()->CreateBasicBlock();
+    typecheck->SetSuccessorAt(0, if_spec_object);
+    typecheck->SetSuccessorAt(1, not_spec_object);
+    current_block()->Finish(typecheck);
+    if_spec_object->last_environment()->Push(return_value);
+    not_spec_object->last_environment()->Push(receiver);
+    set_current_block(CreateJoin(if_spec_object, not_spec_object, expr->id()));
+    return ast_context()->ReturnValue(Pop());
+  } else {
+    // The constructor function is both an operand to the instruction and an
+    // argument to the construct call.
+    HValue* constructor = NULL;
+    CHECK_ALIVE(constructor = VisitArgument(expr->expression()));
+    CHECK_ALIVE(VisitArgumentList(expr->arguments()));
+    HInstruction* call =
+        new(zone()) HCallNew(context, constructor, argument_count);
+    Drop(argument_count);
+    call->set_position(expr->position());
+    return ast_context()->ReturnInstruction(call, expr->id());
+  }
 }
 
 
@@ -6510,10 +6628,11 @@ void HGraphBuilder::GenerateIsStringWrapperSafeForDefaultValueOf(
 void HGraphBuilder::GenerateIsConstructCall(CallRuntime* call) {
   ASSERT(call->arguments()->length() == 0);
   if (function_state()->outer() != NULL) {
-    // We are generating graph for inlined function. Currently
-    // constructor inlining is not supported and we can just return
-    // false from %_IsConstructCall().
-    return ast_context()->ReturnValue(graph()->GetConstantFalse());
+    // We are generating graph for inlined function.
+    HValue* value = function_state()->is_construct()
+        ? graph()->GetConstantTrue()
+        : graph()->GetConstantFalse();
+    return ast_context()->ReturnValue(value);
   } else {
     return ast_context()->ReturnControl(new(zone()) HIsConstructCallAndBranch,
                                         call->id());
@@ -6901,14 +7020,14 @@ HEnvironment::HEnvironment(HEnvironment* outer,
     : closure_(closure),
       values_(0),
       assigned_variables_(4),
+      frame_type_(JS_FUNCTION),
       parameter_count_(0),
       specials_count_(1),
       local_count_(0),
       outer_(outer),
       pop_count_(0),
       push_count_(0),
-      ast_id_(AstNode::kNoNumber),
-      arguments_adaptor_(false) {
+      ast_id_(AstNode::kNoNumber) {
   Initialize(scope->num_parameters() + 1, scope->num_stack_slots(), 0);
 }
 
@@ -6916,14 +7035,14 @@ HEnvironment::HEnvironment(HEnvironment* outer,
 HEnvironment::HEnvironment(const HEnvironment* other)
     : values_(0),
       assigned_variables_(0),
+      frame_type_(JS_FUNCTION),
       parameter_count_(0),
       specials_count_(1),
       local_count_(0),
       outer_(NULL),
       pop_count_(0),
       push_count_(0),
-      ast_id_(other->ast_id()),
-      arguments_adaptor_(false) {
+      ast_id_(other->ast_id()) {
   Initialize(other);
 }
 
@@ -6934,13 +7053,28 @@ HEnvironment::HEnvironment(HEnvironment* outer,
     : closure_(closure),
       values_(arguments),
       assigned_variables_(0),
+      frame_type_(ARGUMENTS_ADAPTOR),
       parameter_count_(arguments),
       local_count_(0),
       outer_(outer),
       pop_count_(0),
       push_count_(0),
-      ast_id_(AstNode::kNoNumber),
-      arguments_adaptor_(true) {
+      ast_id_(AstNode::kNoNumber) {
+}
+
+
+HEnvironment::HEnvironment(HEnvironment* outer,
+                           Handle<JSFunction> closure)
+    : closure_(closure),
+      values_(0),
+      assigned_variables_(0),
+      frame_type_(JS_CONSTRUCT),
+      parameter_count_(0),
+      local_count_(0),
+      outer_(outer),
+      pop_count_(0),
+      push_count_(0),
+      ast_id_(AstNode::kNoNumber) {
 }
 
 
@@ -6961,13 +7095,13 @@ void HEnvironment::Initialize(const HEnvironment* other) {
   closure_ = other->closure();
   values_.AddAll(other->values_);
   assigned_variables_.AddAll(other->assigned_variables_);
+  frame_type_ = other->frame_type_;
   parameter_count_ = other->parameter_count_;
   local_count_ = other->local_count_;
   if (other->outer_ != NULL) outer_ = other->outer_->Copy();  // Deep copy.
   pop_count_ = other->pop_count_;
   push_count_ = other->push_count_;
   ast_id_ = other->ast_id_;
-  arguments_adaptor_ = other->arguments_adaptor_;
 }
 
 
@@ -7074,8 +7208,9 @@ HEnvironment* HEnvironment::CopyForInlining(
     int arguments,
     FunctionLiteral* function,
     HConstant* undefined,
-    CallKind call_kind) const {
-  ASSERT(!is_arguments_adaptor());
+    CallKind call_kind,
+    bool is_construct) const {
+  ASSERT(frame_type() == JS_FUNCTION);
 
   Zone* zone = closure()->GetIsolate()->zone();
 
@@ -7086,6 +7221,17 @@ HEnvironment* HEnvironment::CopyForInlining(
   outer->Drop(arguments + 1);  // Including receiver.
   outer->ClearHistory();
 
+  if (is_construct) {
+    // Create artificial constructor stub environment.  The receiver should
+    // actually be the constructor function, but we pass the newly allocated
+    // object instead, DoComputeConstructStubFrame() relies on that.
+    outer = new(zone) HEnvironment(outer, target);
+    for (int i = 0; i <= arguments; ++i) {  // Include receiver.
+      outer->Push(ExpressionStackAt(arguments - i));
+    }
+    outer->ClearHistory();
+  }
+
   if (arity != arguments) {
     // Create artificial arguments adaptation environment.
     outer = new(zone) HEnvironment(outer, target, arguments + 1);

[Vyacheslav Egorov (Chromium), 2012/02/13 15:01:39]

I think HEnvironent constructor for the "artificial" environments can be unified
into one if you start passing frame_type into it.

Then this code can also be moved into something like CreateArtificialEnvironment
(or CreateStubEnvironment or smth).

[Michael Starzinger, 2012/02/27 14:16:32]

Done.