Shorten live ranges of argument subexpressions.

src/hydrogen.cc

Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index eccb489f468558ed4b60ba9976682a33435d68bd..cf21a0001ef8e1b8845da2ca092df7cfcfd2adca 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -53,7 +53,7 @@ HBasicBlock::HBasicBlock(HGraph* graph)
       graph_(graph),
       phis_(4),
       first_(NULL),
-      last_(NULL),
+      cached_(NULL),
       end_(NULL),
       loop_information_(NULL),
       predecessors_(2),
@@ -113,9 +113,11 @@ void HBasicBlock::AddInstruction(HInstruction* instr) {
 HInstruction* HBasicBlock::GetLastInstruction() {

[fschneider, 2010/12/20 16:13:02]

I'd strongly favor removing this function together with the cached_ instruction
in a separate change. I can't see a real benefit. start() and end() should be
enough, I think.

If a block is finished, we get get end_->previous(). As long as a block is
unfinished, we can update end_ along the way when adding instructions.

[Kevin Millikin (Chromium), 2010/12/21 11:16:37]

Strongly agreed.

   if (end_ != NULL) return end_->previous();
   if (first_ == NULL) return NULL;
-  if (last_ == NULL) last_ = first_;
-  while (last_->next() != NULL) last_ = last_->next();
-  return last_;
+  // Search starting from cached_ if we have it and it hasn't been removed
+  // from this block.
+  if (cached_ == NULL || cached_->block() != this) cached_ = first_;
+  while (cached_->next() != NULL) cached_ = cached_->next();
+  return cached_;
 }
 
 
@@ -2174,17 +2176,20 @@ void HGraphBuilder::VisitForControl(Expression* expr,
 }
 
 
-HValue* HGraphBuilder::VisitArgument(Expression* expr) {
-  VisitForValue(expr);
-  if (HasStackOverflow() || !subgraph()->HasExit()) return NULL;
-  return environment()->Top();
+void HGraphBuilder::VisitArgument(Expression* expr) {
+  VISIT_FOR_VALUE(expr);
+  if (!subgraph()->HasExit()) return;
+  HValue* value = Pop();
+  HPushArgument* instr = new HPushArgument(value);
+  AddInstruction(instr);
+  Push(instr);
 }
 
 
 void HGraphBuilder::VisitArgumentList(ZoneList<Expression*>* arguments) {
   for (int i = 0; i < arguments->length(); i++) {
     VisitArgument(arguments->at(i));
-    if (HasStackOverflow() || !current_subgraph_->HasExit()) return;
+    if (HasStackOverflow() || !subgraph()->HasExit()) return;
   }
 }
 
@@ -2295,33 +2300,6 @@ void HGraphBuilder::PushAndAdd(HInstruction* instr) {
 }
 
 
-void HGraphBuilder::PushArgumentsForStubCall(int argument_count) {
-  const int kMaxStubArguments = 4;
-  ASSERT_GE(kMaxStubArguments, argument_count);
-  // Push the arguments on the stack.
-  HValue* arguments[kMaxStubArguments];
-  for (int i = argument_count - 1; i >= 0; i--) {
-    arguments[i] = Pop();
-  }
-  for (int i = 0; i < argument_count; i++) {
-    AddInstruction(new HPushArgument(arguments[i]));
-  }
-}
-
-
-void HGraphBuilder::ProcessCall(HCall* call) {
-  for (int i = call->argument_count() - 1; i >= 0; --i) {
-    HValue* value = Pop();
-    HPushArgument* push = new HPushArgument(value);
-    call->SetArgumentAt(i, push);
-  }
-
-  for (int i = 0; i < call->argument_count(); ++i) {
-    AddInstruction(call->PushArgumentAt(i));
-  }
-}
-
-
 void HGraphBuilder::SetupScope(Scope* scope) {
   // We don't yet handle the function name for named function expressions.
   if (scope->function() != NULL) BAILOUT("named function expression");
@@ -2376,10 +2354,11 @@ HBasicBlock* HGraphBuilder::CreateBasicBlock(HEnvironment* env) {
 
 HSubgraph* HGraphBuilder::CreateInlinedSubgraph(HEnvironment* outer,
                                                 Handle<JSFunction> target,
-                                                FunctionLiteral* function) {
+                                                FunctionLiteral* function,
+                                                HGlobalReceiver* receiver) {
   HConstant* undefined = graph()->GetConstantUndefined();
   HEnvironment* inner =
-      outer->CopyForInlining(target, function, true, undefined);
+      outer->CopyForInlining(target, function, true, undefined, receiver);
   HSubgraph* subgraph = new HSubgraph(graph());
   subgraph->Initialize(CreateBasicBlock(inner));
   return subgraph;
@@ -3067,50 +3046,51 @@ void HGraphBuilder::VisitCatchExtensionObject(CatchExtensionObject* expr) {
 
 
 HBasicBlock* HGraphBuilder::BuildTypeSwitch(ZoneMapList* maps,
-                                            ZoneList<HSubgraph*>* subgraphs,
+                                            ZoneList<HSubgraph*>* body_graphs,
+                                            HSubgraph* default_graph,
                                             HValue* receiver,
                                             int join_id) {
-  ASSERT(subgraphs->length() == (maps->length() + 1));
-
-  // Build map compare subgraphs for all but the first map.
-  ZoneList<HSubgraph*> map_compare_subgraphs(maps->length() - 1);
-  for (int i = maps->length() - 1; i > 0; --i) {
-    HSubgraph* subgraph = CreateBranchSubgraph(environment());
-    SubgraphScope scope(this, subgraph);
-    HSubgraph* else_subgraph =
-        (i == (maps->length() - 1))
-        ? subgraphs->last()
-        : map_compare_subgraphs.last();
-    current_subgraph_->exit_block()->Finish(
-        new HCompareMapAndBranch(receiver,
-                                 maps->at(i),
-                                 subgraphs->at(i)->entry_block(),
-                                 else_subgraph->entry_block()));
-    map_compare_subgraphs.Add(subgraph);
-  }
-
-  // Generate first map check to end the current block.
+  ASSERT(body_graphs->length() == maps->length());
+  // Create one block to join them all.
+  HBasicBlock* join_block = graph()->CreateBasicBlock();
+
+  // Add a smi check before the first map check.
   AddInstruction(new HCheckNonSmi(receiver));
-  HSubgraph* else_subgraph =
-      (maps->length() == 1) ? subgraphs->at(1) : map_compare_subgraphs.last();
-  current_subgraph_->exit_block()->Finish(
-      new HCompareMapAndBranch(receiver,
-                               Handle<Map>(maps->first()),
-                               subgraphs->first()->entry_block(),
-                               else_subgraph->entry_block()));
-
-  // Join all the call subgraphs in a new basic block and make
-  // this basic block the current basic block.
-  HBasicBlock* join_block = graph_->CreateBasicBlock();
-  for (int i = 0; i < subgraphs->length(); ++i) {
-    if (subgraphs->at(i)->HasExit()) {
-      subgraphs->at(i)->exit_block()->Goto(join_block);
+
+  for (int i = 0; i < maps->length(); ++i) {
+    // Add a branch to the current subgraph.  Use a fresh false subgraph for
+    // each compare except the last, which uses the default.
+    HSubgraph* true_subgraph = body_graphs->at(i);
+    HSubgraph* false_subgraph = (i == maps->length() - 1)
+        ? default_graph
+        : CreateBranchSubgraph(environment());
+    HCompareMapAndBranch* compare =
+        new HCompareMapAndBranch(receiver, maps->at(i),
+                                 true_subgraph->entry_block(),
+                                 false_subgraph->entry_block());
+    subgraph()->exit_block()->Finish(compare);
+
+    // Add a jump from the true subgraph's exit to the join block.
+    if (true_subgraph->HasExit()) {
+      true_subgraph->exit_block()->Goto(join_block);
     }
+
+    // Set the current graph cursor to the false graph.
+    subgraph()->set_exit_block(false_subgraph->exit_block());
   }
 
-  if (join_block->predecessors()->is_empty()) return NULL;
-  join_block->SetJoinId(join_id);
-  return join_block;
+  // Add a jump from the default to the join block if necessary.
+  if (subgraph()->HasExit()) {
+    subgraph()->exit_block()->Goto(join_block);
+  }
+
+  // Return the join block if it is reachable.
+  if (join_block->predecessors()->is_empty()) {
+    return NULL;
+  } else {
+    join_block->SetJoinId(join_id);
+    return join_block;
+  }
 }
 
 
@@ -3209,61 +3189,67 @@ void HGraphBuilder::HandlePolymorphicStoreNamedField(Assignment* expr,
                                                      Handle<String> name) {
   int number_of_types = Min(types->length(), kMaxStorePolymorphism);
   ZoneMapList maps(number_of_types);
-  ZoneList<HSubgraph*> subgraphs(number_of_types + 1);
+  ZoneList<HSubgraph*> store_graphs(number_of_types);
   bool needs_generic = (types->length() > kMaxStorePolymorphism);
 
   // Build subgraphs for each of the specific maps.
   //
-  // TODO(ager): We should recognize when the prototype chains for
-  // different maps are identical. In that case we can avoid
-  // repeatedly generating the same prototype map checks.
+  // TODO(ager): We should recognize when the prototype chains for different
+  // maps are identical. In that case we can avoid repeatedly generating the
+  // same prototype map checks.
   for (int i = 0; i < number_of_types; ++i) {
     Handle<Map> map = types->at(i);
     LookupResult lookup;
     if (ComputeStoredField(map, name, &lookup)) {
-      maps.Add(map);
-      HSubgraph* subgraph = CreateBranchSubgraph(environment());
-      SubgraphScope scope(this, subgraph);
+      HSubgraph* store_graph = CreateBranchSubgraph(environment());
+      SubgraphScope scope(this, store_graph);
       HInstruction* instr =
           BuildStoreNamedField(object, name, value, map, &lookup, false);
       Push(value);
       instr->set_position(expr->position());
       AddInstruction(instr);
-      subgraphs.Add(subgraph);
+      // BuildTypeSwitch will add an HGoto at the end of the store subgraph,
+      // which will emit a simulate capturing the side effect of the store.
+      maps.Add(map);
+      store_graphs.Add(store_graph);
     } else {
+      // We could not resolve the field statically so we need an IC store.
       needs_generic = true;
     }
   }
 
-  // If none of the properties were named fields we generate a
+  // If none of the properties were resolved statically we generate a
   // generic store.
-  if (maps.length() == 0) {
+  if (maps.is_empty()) {
     HInstruction* instr = new HStoreNamedGeneric(object, name, value);
     Push(value);
     instr->set_position(expr->position());
     AddInstruction(instr);
     if (instr->HasSideEffects()) AddSimulate(expr->id());
-  } else {
-    // Build subgraph for generic store through IC.
-    {
-      HSubgraph* subgraph = CreateBranchSubgraph(environment());
-      SubgraphScope scope(this, subgraph);
-      if (!needs_generic && FLAG_deoptimize_uncommon_cases) {
-        subgraph->FinishExit(new HDeoptimize());
-      } else {
-        HInstruction* instr = new HStoreNamedGeneric(object, name, value);
-        Push(value);
-        instr->set_position(expr->position());
-        AddInstruction(instr);
-      }
-      subgraphs.Add(subgraph);
-    }
+    ast_context()->ReturnValue(Pop());
+    return;
+  }
 
-    HBasicBlock* new_exit_block =
-        BuildTypeSwitch(&maps, &subgraphs, object, expr->AssignmentId());
-    subgraph()->set_exit_block(new_exit_block);
+  // Build subgraph for generic store through IC.
+  HSubgraph* default_graph = CreateBranchSubgraph(environment());
+  {
+    SubgraphScope scope(this, default_graph);
+    if (!needs_generic && FLAG_deoptimize_uncommon_cases) {
+      default_graph->FinishExit(new HDeoptimize());

[fschneider, 2010/12/20 16:13:02]

Leave out () after HDeoptimize.

[Kevin Millikin (Chromium), 2010/12/21 11:16:37]

Thanks.

+    } else {
+      HInstruction* instr = new HStoreNamedGeneric(object, name, value);
+      Push(value);
+      instr->set_position(expr->position());
+      AddInstruction(instr);
+    }
   }
 
+  HBasicBlock* new_exit_block = BuildTypeSwitch(&maps,
+                                                &store_graphs,
+                                                default_graph,
+                                                object,
+                                                expr->AssignmentId());
+  subgraph()->set_exit_block(new_exit_block);
   if (subgraph()->HasExit()) ast_context()->ReturnValue(Pop());
 }
 
@@ -3516,59 +3502,65 @@ void HGraphBuilder::HandlePolymorphicLoadNamedField(Property* expr,
                                                     Handle<String> name) {
   int number_of_types = Min(types->length(), kMaxLoadPolymorphism);
   ZoneMapList maps(number_of_types);
-  ZoneList<HSubgraph*> subgraphs(number_of_types + 1);
+  ZoneList<HSubgraph*> load_graphs(number_of_types);
   bool needs_generic = (types->length() > kMaxLoadPolymorphism);
 
   // Build subgraphs for each of the specific maps.
   //
-  // TODO(ager): We should recognize when the prototype chains for
-  // different maps are identical. In that case we can avoid
-  // repeatedly generating the same prototype map checks.
+  // TODO(ager): We should recognize when the prototype chains for different
+  // maps are identical. In that case we can avoid repeatedly generating the
+  // same prototype map checks.
   for (int i = 0; i < number_of_types; ++i) {
     Handle<Map> map = types->at(i);
     LookupResult lookup;
     map->LookupInDescriptors(NULL, *name, &lookup);
     if (lookup.IsProperty() && lookup.type() == FIELD) {
-      maps.Add(map);
-      HSubgraph* subgraph = CreateBranchSubgraph(environment());
-      SubgraphScope scope(this, subgraph);
+      HSubgraph* load_graph = CreateBranchSubgraph(environment());
+      SubgraphScope scope(this, load_graph);
       HInstruction* instr =
           BuildLoadNamedField(object, expr, map, &lookup, false);
       instr->set_position(expr->position());
       PushAndAdd(instr);
-      subgraphs.Add(subgraph);
+      // BuildTypeSwitch will add an HGoto at the end of the load subgraph,
+      // which will emit a simulate if the load has side effects.
+      maps.Add(map);
+      load_graphs.Add(load_graph);
     } else {
+      // We could not resolve the field statically so we need an IC load.
       needs_generic = true;
     }
   }
 
-  // If none of the properties were named fields we generate a
+  // If none of the properties were resolved statically we generate a
   // generic load.
-  if (maps.length() == 0) {
+  if (maps.is_empty()) {
     HInstruction* instr = BuildLoadNamedGeneric(object, expr);
     instr->set_position(expr->position());
     PushAndAdd(instr);
     if (instr->HasSideEffects()) AddSimulate(expr->id());
-  } else {
-    // Build subgraph for generic load through IC.
-    {
-      HSubgraph* subgraph = CreateBranchSubgraph(environment());
-      SubgraphScope scope(this, subgraph);
-      if (!needs_generic && FLAG_deoptimize_uncommon_cases) {
-        subgraph->FinishExit(new HDeoptimize());
-      } else {
-        HInstruction* instr = BuildLoadNamedGeneric(object, expr);
-        instr->set_position(expr->position());
-        PushAndAdd(instr);
-      }
-      subgraphs.Add(subgraph);
-    }
+    ast_context()->ReturnValue(Pop());
+    return;
+  }
 
-    HBasicBlock* new_exit_block =
-        BuildTypeSwitch(&maps, &subgraphs, object, expr->id());
-    subgraph()->set_exit_block(new_exit_block);
+  // Build subgraph for generic load through IC.
+  HSubgraph* default_graph = CreateBranchSubgraph(environment());
+  {
+    SubgraphScope scope(this, default_graph);
+    if (!needs_generic && FLAG_deoptimize_uncommon_cases) {
+      default_graph->FinishExit(new HDeoptimize());

[fschneider, 2010/12/20 16:13:02]

Leave out () after HDeoptimize.

+    } else {
+      HInstruction* instr = BuildLoadNamedGeneric(object, expr);
+      instr->set_position(expr->position());
+      PushAndAdd(instr);
+    }
   }
 
+  HBasicBlock* new_exit_block = BuildTypeSwitch(&maps,
+                                                &load_graphs,
+                                                default_graph,
+                                                object,
+                                                expr->id());
+  subgraph()->set_exit_block(new_exit_block);
   if (subgraph()->HasExit()) ast_context()->ReturnValue(Pop());
 }
 
@@ -3779,10 +3771,32 @@ void HGraphBuilder::HandlePolymorphicCallNamed(Call* expr,
                                                Handle<String> name) {
   int argument_count = expr->arguments()->length() + 1;  // Plus receiver.
   int number_of_types = Min(types->length(), kMaxCallPolymorphism);
-  ZoneMapList maps(number_of_types);
-  ZoneList<HSubgraph*> subgraphs(number_of_types + 1);
+  ZoneMapList inlined_maps(number_of_types);
+  ZoneMapList non_inlined_maps(number_of_types);
+  ZoneList<HSubgraph*> inlined_graphs(number_of_types);
+  ZoneList<HSubgraph*> non_inlined_graphs(number_of_types);
   bool needs_generic = (types->length() > kMaxCallPolymorphism);
 
+  // All the gymnastics with the pushed arguments are to cope with
+  // assumptions about the graph construction order (e.g, we must have a
+  // fully determined environment before emitting any instruction in a
+  // block, and it's not simple to patch a subset of the uses of a value).
+  //
+  // Remove the pushed function arguments from the graph before handling the
+  // variants.  Record the actual argument values before removing the
+  // PushArgument instructions because that will clear their operands.
+  ZoneList<HValue*> arguments(argument_count);
+  for (int i = argument_count - 1; i >= 0; --i) {
+    HPushArgument* push_argument =
+        HPushArgument::cast(environment()->ExpressionStackAt(i));
+    arguments.Add(push_argument->argument());
+  }
+  ReplaceAndDeleteArguments(environment(), argument_count);
+  Drop(argument_count);
+  for (int i = 0; i < argument_count; ++i) {
+    Push(arguments[i]);
+  }
+
   // Build subgraphs for each of the specific maps.
   //
   // TODO(ager): We should recognize when the prototype chains for different
@@ -3791,57 +3805,114 @@ void HGraphBuilder::HandlePolymorphicCallNamed(Call* expr,
   for (int i = 0; i < number_of_types; ++i) {
     Handle<Map> map = types->at(i);
     if (expr->ComputeTarget(map, name)) {
-      maps.Add(map);
-      HSubgraph* subgraph = CreateBranchSubgraph(environment());
-      SubgraphScope scope(this, subgraph);
+      HSubgraph* call_graph = CreateBranchSubgraph(environment());
+      SubgraphScope scope(this, call_graph);
+      // On entry to each call subgraph, push the arguments in case a call
+      // is made.  If the call is inlined, these pushes will be deleted.
+      Drop(argument_count);
+      for (int i = 0; i < argument_count; ++i) {
+        PushAndAdd(new HPushArgument(arguments[i]));
+      }
       AddCheckConstantFunction(expr, receiver, map, false);
       if (FLAG_trace_inlining && FLAG_polymorphic_inlining) {
         PrintF("Trying to inline the polymorphic call to %s\n",
                *name->ToCString());
       }
-      if (!FLAG_polymorphic_inlining || !TryInline(expr)) {
+      HEnvironment* original_env = environment()->Copy();
+      const bool kIsKnownGlobal = true;
+      if (FLAG_polymorphic_inlining && TryInline(expr, !kIsKnownGlobal)) {
+        // Delete the pushed arguments.
+        ReplaceAndDeleteArguments(original_env, argument_count);
+        inlined_maps.Add(map);
+        inlined_graphs.Add(call_graph);
+      } else {
         // Check for bailout, as trying to inline might fail due to bailout
         // during hydrogen processing.
         CHECK_BAILOUT;
-        HCall* call = new HCallConstantFunction(expr->target(), argument_count);
+        HCall* call =
+            new HCallConstantFunction(expr->target(), argument_count);
         call->set_position(expr->position());
-        ProcessCall(call);
+        Drop(argument_count);
         PushAndAdd(call);
+        non_inlined_maps.Add(map);
+        non_inlined_graphs.Add(call_graph);
       }
-      subgraphs.Add(subgraph);
+
     } else {
+      // We could not resolve the target statically so we need an IC call.
       needs_generic = true;
     }
   }
 
   // If we couldn't compute the target for any of the maps just perform an
   // IC call.
-  if (maps.length() == 0) {
+  if (inlined_maps.is_empty() && non_inlined_maps.is_empty()) {
+    Drop(argument_count);
+    for (int i = 0; i < argument_count; ++i) {
+      PushAndAdd(new HPushArgument(arguments[i]));
+    }
     HCall* call = new HCallNamed(name, argument_count);
     call->set_position(expr->position());
-    ProcessCall(call);
+    Drop(argument_count);
     ast_context()->ReturnInstruction(call, expr->id());
-  } else {
-    // Build subgraph for generic call through IC.
-    {
-      HSubgraph* subgraph = CreateBranchSubgraph(environment());
-      SubgraphScope scope(this, subgraph);
-      if (!needs_generic && FLAG_deoptimize_uncommon_cases) {
-        subgraph->FinishExit(new HDeoptimize());
-      } else {
-        HCall* call = new HCallNamed(name, argument_count);
-        call->set_position(expr->position());
-        ProcessCall(call);
-        PushAndAdd(call);
+    return;
+  }
+
+  // Build subgraph for generic call through IC.  With
+  // --deoptimize-uncommon-cases we will deoptimize instead of calling the
+  // IC unless there were more maps than the polymorphic limit.
+  HSubgraph* default_graph = CreateBranchSubgraph(environment());
+  needs_generic = needs_generic || !FLAG_deoptimize_uncommon_cases;
+  {
+    SubgraphScope scope(this, default_graph);
+    if (needs_generic) {
+      Drop(argument_count);
+      for (int i = 0; i < argument_count; ++i) {
+        PushAndAdd(new HPushArgument(arguments[i]));
       }
-      subgraphs.Add(subgraph);
+      HCall* call = new HCallNamed(name, argument_count);
+      call->set_position(expr->position());
+      Drop(argument_count);
+      PushAndAdd(call);
+    } else {
+      default_graph->FinishExit(new HDeoptimize());
     }
+  }
 
-    HBasicBlock* new_exit_block =
-        BuildTypeSwitch(&maps, &subgraphs, receiver, expr->id());
+  // If there were no inlined variants, build a simple type switch.
+  if (inlined_maps.is_empty()) {
+    HBasicBlock* new_exit_block = BuildTypeSwitch(&non_inlined_maps,
+                                                  &non_inlined_graphs,
+                                                  default_graph,
+                                                  receiver,
+                                                  expr->id());
     subgraph()->set_exit_block(new_exit_block);
-    if (new_exit_block != NULL) ast_context()->ReturnValue(Pop());
+    if (subgraph()->HasExit()) ast_context()->ReturnValue(Pop());
+    return;
   }
+
+  // We have at least one inlined variant.  If there are any non-inlined
+  // variants, build a type switch of them to use as the default for the
+  // inlined variants (if any).  Otherwise use the default.

[fschneider, 2010/12/20 16:13:02]

Remove a space after .

+  HSubgraph* non_inlined_subgraph = default_graph;
+  if (!non_inlined_maps.is_empty()) {
+    non_inlined_subgraph = CreateBranchSubgraph(environment());
+    SubgraphScope scope(this, non_inlined_subgraph);
+    HBasicBlock* non_inlined_exit = BuildTypeSwitch(&non_inlined_maps,
+                                                    &non_inlined_graphs,
+                                                    default_graph,
+                                                    receiver,
+                                                    expr->ReturnId());
+    subgraph()->set_exit_block(non_inlined_exit);
+  }
+
+  HBasicBlock* new_exit_block = BuildTypeSwitch(&inlined_maps,
+                                                &inlined_graphs,
+                                                non_inlined_subgraph,
+                                                receiver,
+                                                expr->id());
+  subgraph()->set_exit_block(new_exit_block);
+  if (subgraph()->HasExit()) ast_context()->ReturnValue(Pop());
 }
 
 
@@ -3857,7 +3928,7 @@ void HGraphBuilder::TraceInline(Handle<JSFunction> target, bool result) {
 }
 
 
-bool HGraphBuilder::TryInline(Call* expr) {
+bool HGraphBuilder::TryInline(Call* expr, bool is_known_global) {
   if (!FLAG_use_inlining) return false;
 
   // Precondition: call is monomorphic and we have found a target with the
@@ -3980,7 +4051,9 @@ bool HGraphBuilder::TryInline(Call* expr) {
   oracle_ = &new_oracle;
   graph()->info()->SetOsrAstId(AstNode::kNoNumber);
 
-  HSubgraph* body = CreateInlinedSubgraph(env, target, function);
+  HGlobalReceiver* global_receiver = NULL;
+  if (is_known_global) global_receiver = new HGlobalReceiver;
+  HSubgraph* body = CreateInlinedSubgraph(env, target, function, global_receiver);

[fschneider, 2010/12/20 16:13:02]

Long line.

[Kevin Millikin (Chromium), 2010/12/21 11:16:37]

Thanks.

   body->exit_block()->AddInstruction(new HEnterInlined(target, function));
   AddToSubgraph(body, function->body());
   if (HasStackOverflow()) {
@@ -4025,11 +4098,12 @@ bool HGraphBuilder::TryInline(Call* expr) {
     body->set_exit_block(NULL);
   }
 
-  // Record the environment at the inlined function call.
-  AddSimulate(expr->ReturnId());
+  // Insert the global receiver if it was a global function.
+  if (is_known_global) AddInstruction(global_receiver);
 
-  // Jump to the function entry (without re-recording the environment).
-  subgraph()->exit_block()->Finish(new HGoto(body->entry_block()));
+  // Jump to the function entry.
+  subgraph()->exit_block()->Goto(body->entry_block(), false);
+  body->entry_block()->SetJoinId(expr->ReturnId());
 
   // Fix up the function exits.
   if (test_context != NULL) {
@@ -4085,6 +4159,15 @@ void HBasicBlock::AddLeaveInlined(HValue* return_value, HBasicBlock* target) {
 }
 
 
+void HGraphBuilder::ReplaceAndDeleteArguments(HEnvironment* env, int count) {
+  for (int i = 0; i < count; ++i) {
+    HPushArgument* push = HPushArgument::cast(env->ExpressionStackAt(i));
+    HValue* value = push->argument();
+    push->ReplaceAndDelete(value);
+  }
+}
+
+
 bool HGraphBuilder::TryMathFunctionInline(Call* expr) {
   // Try to inline calls like Math.* as operations in the calling function.
   if (!expr->target()->shared()->IsBuiltinMathFunction()) return false;
@@ -4097,8 +4180,13 @@ bool HGraphBuilder::TryMathFunctionInline(Call* expr) {
     case kMathSqrt:
     case kMathLog:
       if (argument_count == 2) {
-        HValue* argument = Pop();
-        Drop(1);  // Receiver.
+        // Since we won't be making a call we should not push the argument
+        // subexpressions.  Instead of deleting them from the graph we could
+        // make an earlier decision that we have an inlined math function
+        // (i.e., before the argument subexpressions are translated).
+        HValue* argument = HPushArgument::cast(Top())->argument();
+        ReplaceAndDeleteArguments(environment(), 2);  // Including receiver.
+        Drop(2);  // Receiver.
         HUnaryMathOperation* op = new HUnaryMathOperation(argument, id);
         op->set_position(expr->position());
         ast_context()->ReturnInstruction(op, expr->id());
@@ -4107,9 +4195,12 @@ bool HGraphBuilder::TryMathFunctionInline(Call* expr) {
       break;
     case kMathPow:
       if (argument_count == 3) {
-        HValue* right = Pop();
-        HValue* left = Pop();
-        Pop();  // Pop receiver.
+        HValue* push_left = environment()->ExpressionStackAt(1);

[fschneider, 2010/12/20 16:13:02]

Can you write the following here instead?

ReplaceAndDeleteArguments(environment(), 3);
HValue* right = Pop();
HValue* left = Pop();
Pop();  // Pop receiver.

[Kevin Millikin (Chromium), 2010/12/21 11:16:37]

Not really, right now.  ReplaceAndDeleteArguments does not currently change the
environment.

I considered changing the environment, or having it auto drop the pushed
arguments from the environment to make sure that they weren't accidentally used.
 Nothing seems completely satisfying.

+        HValue* push_right = environment()->ExpressionStackAt(0);
+        HValue* left = HPushArgument::cast(push_left)->argument();
+        HValue* right = HPushArgument::cast(push_right)->argument();
+        ReplaceAndDeleteArguments(environment(), 3);  // Including receiver.
+        Drop(3);
         HInstruction* result = NULL;
         // Use sqrt() if exponent is 0.5 or -0.5.
         if (right->IsConstant() && HConstant::cast(right)->HasDoubleValue()) {
@@ -4212,7 +4303,8 @@ void HGraphBuilder::VisitCall(Call* expr) {
       CHECK_BAILOUT;
 
       VISIT_FOR_VALUE(prop->key());
-      // Push receiver and key like the non-optimized code generator expects it.
+      // The unoptimized code expects the key below the receiver when
+      // evaluating the argument subexpressions.
       HValue* key = Pop();
       HValue* receiver = Pop();
       Push(key);
@@ -4223,8 +4315,7 @@ void HGraphBuilder::VisitCall(Call* expr) {
 
       call = new HCallKeyed(key, argument_count);
       call->set_position(expr->position());
-      ProcessCall(call);
-      Drop(1);  // Key.
+      Drop(argument_count + 1);  // 1 is the key.
       ast_context()->ReturnInstruction(call, expr->id());
       return;
     }
@@ -4235,8 +4326,14 @@ void HGraphBuilder::VisitCall(Call* expr) {
     if (TryCallApply(expr)) return;
     CHECK_BAILOUT;
 
-    HValue* receiver = VisitArgument(prop->obj());
-    CHECK_BAILOUT;
+    // The receiver is pushed as an argument and also used as a value to
+    // check that we're calling the expected function.
+    VISIT_FOR_VALUE(prop->obj());
+    HValue* receiver = Pop();
+    HPushArgument* push_receiver = new HPushArgument(receiver);
+    AddInstruction(push_receiver);

[fschneider, 2010/12/20 16:13:02]

PushAndAdd(push_receiver);

[Kevin Millikin (Chromium), 2010/12/21 11:16:37]

I like to keep them separate.  PushAndAdd is considered harmful :)

I actually don't like it because adding a value to the bailout environment and
inserting an instruction into the Hydrogen instruction graph are nearly two
separate things.  Writing it as a single operation makes it seem like that's
just the way we do it.

I've reviewed code from several different authors that did something like:

PushAndAdd(instruction);
HValue* value = Pop();

+    Push(push_receiver);
+
     VisitArgumentList(expr->arguments());
     CHECK_BAILOUT;
 
@@ -4248,9 +4345,12 @@ void HGraphBuilder::VisitCall(Call* expr) {
     if (expr->IsMonomorphic()) {
       AddCheckConstantFunction(expr, receiver, types->first(), true);
 
-      if (TryMathFunctionInline(expr)) {
-        return;
-      } else if (TryInline(expr)) {
+      if (TryMathFunctionInline(expr)) return;
+
+      HEnvironment* original_env = environment()->Copy();
+      const bool kIsKnownGlobal = true;
+      if (TryInline(expr, !kIsKnownGlobal)) {  // Not a known global.
+        ReplaceAndDeleteArguments(original_env, argument_count);
         if (subgraph()->HasExit()) {
           HValue* return_value = Pop();
           // If we inlined a function in a test context then we need to emit
@@ -4262,13 +4362,13 @@ void HGraphBuilder::VisitCall(Call* expr) {
           ast_context()->ReturnValue(return_value);
         }
         return;
-      } else {
-        // Check for bailout, as the TryInline call in the if condition above
-        // might return false due to bailout during hydrogen processing.
-        CHECK_BAILOUT;
-        call = new HCallConstantFunction(expr->target(), argument_count);
       }
 
+      // Check for bailout, as the TryInline call in the if condition above
+      // might return false due to bailout during hydrogen processing.
+      CHECK_BAILOUT;
+      call = new HCallConstantFunction(expr->target(), argument_count);
+
     } else if (types != NULL && types->length() > 1) {
       HandlePolymorphicCallNamed(expr, receiver, types, name);
       return;
@@ -4281,12 +4381,6 @@ void HGraphBuilder::VisitCall(Call* expr) {
     Variable* var = expr->expression()->AsVariableProxy()->AsVariable();
     bool global_call = (var != NULL) && var->is_global() && !var->is_this();
 
-    if (!global_call) {
-      ++argument_count;
-      VisitArgument(expr->expression());
-      CHECK_BAILOUT;
-    }
-
     if (global_call) {
       // If there is a global property cell for the name at compile time and
       // access check is not enabled we assume that the function will not change
@@ -4297,9 +4391,15 @@ void HGraphBuilder::VisitCall(Call* expr) {
           expr->ComputeGlobalTarget(Handle<GlobalObject>(info->global_object()),
                                     var->name());
       if (known_global_function) {
-        // Push the global object instead of the global receiver because
-        // code generated by the full code generator expects it.
-        PushAndAdd(new HGlobalObject);
+        // Push the global object instead of the global receiver because the
+        // unoptimized code expects it.  The call instruction will patch to
+        // the global receiver.
+        HGlobalObject* global_object = new HGlobalObject;
+        HPushArgument* push_global = new HPushArgument(global_object);
+        AddInstruction(global_object);
+        AddInstruction(push_global);
+        Push(push_global);
+
         VisitArgumentList(expr->arguments());
         CHECK_BAILOUT;
 
@@ -4307,16 +4407,10 @@ void HGraphBuilder::VisitCall(Call* expr) {
         HValue* function = Pop();
         AddInstruction(new HCheckFunction(function, expr->target()));
 
-        // Replace the global object with the global receiver.
-        HGlobalReceiver* global_receiver = new HGlobalReceiver;
-        // Index of the receiver from the top of the expression stack.
-        const int receiver_index = argument_count - 1;
-        AddInstruction(global_receiver);
-        ASSERT(environment()->ExpressionStackAt(receiver_index)->
-               IsGlobalObject());
-        environment()->SetExpressionStackAt(receiver_index, global_receiver);
-
-        if (TryInline(expr)) {
+        const bool is_known_global = true;
+        HEnvironment* original_env = environment()->Copy();
+        if (TryInline(expr, is_known_global)) {
+          ReplaceAndDeleteArguments(original_env, argument_count);
           if (subgraph()->HasExit()) {
             HValue* return_value = Pop();
             // If we inlined a function in a test context then we need to
@@ -4333,9 +4427,18 @@ void HGraphBuilder::VisitCall(Call* expr) {
         // during hydrogen processing.
         CHECK_BAILOUT;
 
-        call = new HCallKnownGlobal(expr->target(), argument_count);
+        HGlobalReceiver* global_receiver = new HGlobalReceiver;
+        AddInstruction(global_receiver);
+        call = new HCallKnownGlobal(global_receiver,
+                                    expr->target(),
+                                    argument_count);
       } else {
-        PushAndAdd(new HGlobalObject);
+        HGlobalObject* receiver = new HGlobalObject;
+        HPushArgument* push_receiver = new HPushArgument(receiver);
+        AddInstruction(receiver);
+        AddInstruction(push_receiver);
+        Push(push_receiver);
+
         VisitArgumentList(expr->arguments());
         CHECK_BAILOUT;
 
@@ -4343,16 +4446,24 @@ void HGraphBuilder::VisitCall(Call* expr) {
       }
 
     } else {
-      PushAndAdd(new HGlobalReceiver);
+      // The function to call is passed as an extra argument.
+      VisitArgument(expr->expression());
+      CHECK_BAILOUT;
+      HGlobalReceiver* receiver = new HGlobalReceiver;
+      HPushArgument* push_receiver = new HPushArgument(receiver);
+      AddInstruction(receiver);
+      AddInstruction(push_receiver);
+      Push(push_receiver);
+
       VisitArgumentList(expr->arguments());
       CHECK_BAILOUT;
 
-      call = new HCallFunction(argument_count);
+      call = new HCallFunction(argument_count + 1);
     }
   }
 
   call->set_position(expr->position());
-  ProcessCall(call);
+  Drop(call->argument_count());
   ast_context()->ReturnInstruction(call, expr->id());
 }
 
@@ -4360,15 +4471,19 @@ void HGraphBuilder::VisitCall(Call* expr) {
 void HGraphBuilder::VisitCallNew(CallNew* expr) {
   // The constructor function is also used as the receiver argument to the
   // JS construct call builtin.
-  VisitArgument(expr->expression());
-  CHECK_BAILOUT;
+  VISIT_FOR_VALUE(expr->expression());
+  HValue* constructor = Pop();
+  HPushArgument* push_constructor = new HPushArgument(constructor);
+  AddInstruction(push_constructor);
+  Push(push_constructor);

[fschneider, 2010/12/20 16:13:02]

could be shortened to:

PushAndAdd(new HPushArgument(constructor));

+
   VisitArgumentList(expr->arguments());
   CHECK_BAILOUT;
 
   int argument_count = expr->arguments()->length() + 1;  // Plus constructor.
-  HCall* call = new HCallNew(argument_count);
+  HCall* call = new HCallNew(constructor, argument_count);
   call->set_position(expr->position());
-  ProcessCall(call);
+  Drop(argument_count);
   ast_context()->ReturnInstruction(call, expr->id());
 }
 
@@ -4401,10 +4516,6 @@ void HGraphBuilder::VisitCallRuntime(CallRuntime* expr) {
   }
   ASSERT(function != NULL);
 
-  VisitArgumentList(expr->arguments());
-  CHECK_BAILOUT;
-
-  int argument_count = expr->arguments()->length();
   if (function->intrinsic_type == Runtime::INLINE) {
     ASSERT(name->length() > 0);
     ASSERT(name->Get(0) == '_');
@@ -4417,12 +4528,15 @@ void HGraphBuilder::VisitCallRuntime(CallRuntime* expr) {
     InlineFunctionGenerator generator = kInlineFunctionGenerators[lookup_index];
 
     // Call the inline code generator using the pointer-to-member.
-    (this->*generator)(argument_count, expr->id());
+    (this->*generator)(expr);
   } else {
     ASSERT(function->intrinsic_type == Runtime::RUNTIME);
+    VisitArgumentList(expr->arguments());
+    CHECK_BAILOUT;
+    int argument_count = expr->arguments()->length();
     HCall* call = new HCallRuntime(name, expr->function(), argument_count);
     call->set_position(RelocInfo::kNoPosition);
-    ProcessCall(call);
+    Drop(argument_count);
     ast_context()->ReturnInstruction(call, expr->id());
   }
 }
@@ -4597,8 +4711,13 @@ void HGraphBuilder::VisitCountOperation(CountOperation* expr) {
       // Overwrite the receiver in the bailout environment with the result
       // of the operation, and the placeholder with the original value if
       // necessary.
-      environment()->SetExpressionStackAt(0, after);
-      if (has_extra) environment()->SetExpressionStackAt(1, before);
+      if (has_extra) {
+        Drop(2);
+        Push(before);
+      } else {
+        Drop(1);
+      }
+      Push(after);
       if (store->HasSideEffects()) AddSimulate(expr->AssignmentId());
       Drop(has_extra ? 2 : 1);
 
@@ -4640,9 +4759,13 @@ void HGraphBuilder::VisitCountOperation(CountOperation* expr) {
       // Drop the key from the bailout environment.  Overwrite the receiver
       // with the result of the operation, and the placeholder with the
       // original value if necessary.
-      Drop(1);
-      environment()->SetExpressionStackAt(0, after);
-      if (has_extra) environment()->SetExpressionStackAt(1, before);
+      if (has_extra) {
+        Drop(3);
+        Push(before);
+      } else {
+        Drop(2);
+      }
+      Push(after);
       if (store->HasSideEffects()) AddSimulate(expr->AssignmentId());
       Drop(has_extra ? 2 : 1);
 
@@ -4903,309 +5026,342 @@ void HGraphBuilder::VisitDeclaration(Declaration* decl) {
 
 // Generators for inline runtime functions.
 // Support for types.
-void HGraphBuilder::GenerateIsSmi(int argument_count, int ast_id) {
-  ASSERT(argument_count == 1);
+void HGraphBuilder::GenerateIsSmi(CallRuntime* call) {
+  ASSERT(call->arguments()->length() == 1);
+  VISIT_FOR_VALUE(call->arguments()->at(0));
   HValue* value = Pop();
   HIsSmi* result = new HIsSmi(value);
-  ast_context()->ReturnInstruction(result, ast_id);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
-void HGraphBuilder::GenerateIsSpecObject(int argument_count, int ast_id) {
-  ASSERT(argument_count == 1);
+void HGraphBuilder::GenerateIsSpecObject(CallRuntime* call) {
+  ASSERT(call->arguments()->length() == 1);
+  VISIT_FOR_VALUE(call->arguments()->at(0));
   HValue* value = Pop();
   HHasInstanceType* result =
       new HHasInstanceType(value, FIRST_JS_OBJECT_TYPE, LAST_TYPE);
-  ast_context()->ReturnInstruction(result, ast_id);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
-void HGraphBuilder::GenerateIsFunction(int argument_count, int ast_id) {
-  ASSERT(argument_count == 1);
+void HGraphBuilder::GenerateIsFunction(CallRuntime* call) {
+  ASSERT(call->arguments()->length() == 1);
+  VISIT_FOR_VALUE(call->arguments()->at(0));
   HValue* value = Pop();
   HHasInstanceType* result = new HHasInstanceType(value, JS_FUNCTION_TYPE);
-  ast_context()->ReturnInstruction(result, ast_id);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
-void HGraphBuilder::GenerateHasCachedArrayIndex(int argument_count,
-                                                int ast_id) {
-  ASSERT(argument_count == 1);
+void HGraphBuilder::GenerateHasCachedArrayIndex(CallRuntime* call) {
+  ASSERT(call->arguments()->length() == 1);
+  VISIT_FOR_VALUE(call->arguments()->at(0));
   HValue* value = Pop();
   HHasCachedArrayIndex* result = new HHasCachedArrayIndex(value);
-  ast_context()->ReturnInstruction(result, ast_id);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
-void HGraphBuilder::GenerateIsArray(int argument_count, int ast_id) {
-  ASSERT(argument_count == 1);
+void HGraphBuilder::GenerateIsArray(CallRuntime* call) {
+  ASSERT(call->arguments()->length() == 1);
+  VISIT_FOR_VALUE(call->arguments()->at(0));
   HValue* value = Pop();
   HHasInstanceType* result = new HHasInstanceType(value, JS_ARRAY_TYPE);
-  ast_context()->ReturnInstruction(result, ast_id);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
-void HGraphBuilder::GenerateIsRegExp(int argument_count, int ast_id) {
-  ASSERT(argument_count == 1);
+void HGraphBuilder::GenerateIsRegExp(CallRuntime* call) {
+  ASSERT(call->arguments()->length() == 1);
+  VISIT_FOR_VALUE(call->arguments()->at(0));
   HValue* value = Pop();
   HHasInstanceType* result = new HHasInstanceType(value, JS_REGEXP_TYPE);
-  ast_context()->ReturnInstruction(result, ast_id);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
-void HGraphBuilder::GenerateIsObject(int argument_count, int ast_id) {
-  ASSERT(argument_count == 1);
-
+void HGraphBuilder::GenerateIsObject(CallRuntime* call) {
+  ASSERT(call->arguments()->length() == 1);
+  VISIT_FOR_VALUE(call->arguments()->at(0));
   HValue* value = Pop();
   HIsObject* test = new HIsObject(value);
-  ast_context()->ReturnInstruction(test, ast_id);
+  ast_context()->ReturnInstruction(test, call->id());
 }
 
 
-void HGraphBuilder::GenerateIsNonNegativeSmi(int argument_count,
-                                             int ast_id) {
+void HGraphBuilder::GenerateIsNonNegativeSmi(CallRuntime* call) {
   BAILOUT("inlined runtime function: IsNonNegativeSmi");
 }
 
 
-void HGraphBuilder::GenerateIsUndetectableObject(int argument_count,
-                                                 int ast_id) {
+void HGraphBuilder::GenerateIsUndetectableObject(CallRuntime* call) {
   BAILOUT("inlined runtime function: IsUndetectableObject");
 }
 
 
 void HGraphBuilder::GenerateIsStringWrapperSafeForDefaultValueOf(
-    int argument_count,
-    int ast_id) {
+    CallRuntime* call) {
   BAILOUT("inlined runtime function: IsStringWrapperSafeForDefaultValueOf");
 }
 
 
-  // Support for construct call checks.
-void HGraphBuilder::GenerateIsConstructCall(int argument_count, int ast_id) {
+// Support for construct call checks.
+void HGraphBuilder::GenerateIsConstructCall(CallRuntime* call) {
   BAILOUT("inlined runtime function: IsConstructCall");
 }
 
 
 // Support for arguments.length and arguments[?].
-void HGraphBuilder::GenerateArgumentsLength(int argument_count, int ast_id) {
-  ASSERT(argument_count == 0);
+void HGraphBuilder::GenerateArgumentsLength(CallRuntime* call) {
+  ASSERT(call->arguments()->length() == 0);
   HInstruction* elements = AddInstruction(new HArgumentsElements);
   HArgumentsLength* result = new HArgumentsLength(elements);
-  ast_context()->ReturnInstruction(result, ast_id);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
-void HGraphBuilder::GenerateArguments(int argument_count, int ast_id) {
-  ASSERT(argument_count == 1);
+void HGraphBuilder::GenerateArguments(CallRuntime* call) {
+  ASSERT(call->arguments()->length() == 1);
+  VISIT_FOR_VALUE(call->arguments()->at(0));
   HValue* index = Pop();
   HInstruction* elements = AddInstruction(new HArgumentsElements);
   HInstruction* length = AddInstruction(new HArgumentsLength(elements));
   HAccessArgumentsAt* result = new HAccessArgumentsAt(elements, length, index);
-  ast_context()->ReturnInstruction(result, ast_id);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
 // Support for accessing the class and value fields of an object.
-void HGraphBuilder::GenerateClassOf(int argument_count, int ast_id) {
+void HGraphBuilder::GenerateClassOf(CallRuntime* call) {
   // The special form detected by IsClassOfTest is detected before we get here
   // and does not cause a bailout.
   BAILOUT("inlined runtime function: ClassOf");
 }
 
 
-void HGraphBuilder::GenerateValueOf(int argument_count, int ast_id) {
-  ASSERT(argument_count == 1);
+void HGraphBuilder::GenerateValueOf(CallRuntime* call) {
+  ASSERT(call->arguments()->length() == 1);
+  VISIT_FOR_VALUE(call->arguments()->at(0));
   HValue* value = Pop();
   HValueOf* result = new HValueOf(value);
-  ast_context()->ReturnInstruction(result, ast_id);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
-void HGraphBuilder::GenerateSetValueOf(int argument_count, int ast_id) {
+void HGraphBuilder::GenerateSetValueOf(CallRuntime* call) {
   BAILOUT("inlined runtime function: SetValueOf");
 }
 
 
 // Fast support for charCodeAt(n).
-void HGraphBuilder::GenerateStringCharCodeAt(int argument_count, int ast_id) {
+void HGraphBuilder::GenerateStringCharCodeAt(CallRuntime* call) {
   BAILOUT("inlined runtime function: StringCharCodeAt");
 }
 
 
 // Fast support for string.charAt(n) and string[n].
-void HGraphBuilder::GenerateStringCharFromCode(int argument_count,
-                                               int ast_id) {
+void HGraphBuilder::GenerateStringCharFromCode(CallRuntime* call) {
   BAILOUT("inlined runtime function: StringCharFromCode");
 }
 
 
 // Fast support for string.charAt(n) and string[n].
-void HGraphBuilder::GenerateStringCharAt(int argument_count, int ast_id) {
-  ASSERT_EQ(2, argument_count);
-  PushArgumentsForStubCall(argument_count);
-  HCallStub* result = new HCallStub(CodeStub::StringCharAt, argument_count);
-  ast_context()->ReturnInstruction(result, ast_id);
+void HGraphBuilder::GenerateStringCharAt(CallRuntime* call) {
+  const int kArgumentCount = 2;
+  ASSERT(call->arguments()->length() == kArgumentCount);
+  VisitArgumentList(call->arguments());

[fschneider, 2010/12/20 16:13:02]

I'd replace all VisitArgumentList followed by a check for bailout with a macro
like VISIT_ARGUMENTS that includes the if-statement below.

+  if (HasStackOverflow() || !subgraph()->HasExit()) return;
+  HCallStub* result = new HCallStub(CodeStub::StringCharAt, kArgumentCount);
+  Drop(kArgumentCount);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
 // Fast support for object equality testing.
-void HGraphBuilder::GenerateObjectEquals(int argument_count, int ast_id) {
-  ASSERT(argument_count == 2);
+void HGraphBuilder::GenerateObjectEquals(CallRuntime* call) {
+  ASSERT(call->arguments()->length() == 2);
+  VISIT_FOR_VALUE(call->arguments()->at(0));
+  VISIT_FOR_VALUE(call->arguments()->at(1));
   HValue* right = Pop();
   HValue* left = Pop();
   HCompareJSObjectEq* result = new HCompareJSObjectEq(left, right);
-  ast_context()->ReturnInstruction(result, ast_id);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
-void HGraphBuilder::GenerateLog(int argument_count, int ast_id) {
+void HGraphBuilder::GenerateLog(CallRuntime* call) {
   UNREACHABLE();  // We caught this in VisitCallRuntime.
 }
 
 
 // Fast support for Math.random().
-void HGraphBuilder::GenerateRandomHeapNumber(int argument_count, int ast_id) {
+void HGraphBuilder::GenerateRandomHeapNumber(CallRuntime* call) {
   BAILOUT("inlined runtime function: RandomHeapNumber");
 }
 
 
 // Fast support for StringAdd.
-void HGraphBuilder::GenerateStringAdd(int argument_count, int ast_id) {
-  ASSERT_EQ(2, argument_count);
-  PushArgumentsForStubCall(argument_count);
-  HCallStub* result = new HCallStub(CodeStub::StringAdd, argument_count);
-  ast_context()->ReturnInstruction(result, ast_id);
+void HGraphBuilder::GenerateStringAdd(CallRuntime* call) {
+  const int kArgumentCount = 2;
+  ASSERT(call->arguments()->length() == kArgumentCount);
+  VisitArgumentList(call->arguments());
+  if (HasStackOverflow() || !subgraph()->HasExit()) return;
+  HCallStub* result = new HCallStub(CodeStub::StringAdd, kArgumentCount);
+  Drop(kArgumentCount);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
 // Fast support for SubString.
-void HGraphBuilder::GenerateSubString(int argument_count, int ast_id) {
-  ASSERT_EQ(3, argument_count);
-  PushArgumentsForStubCall(argument_count);
-  HCallStub* result = new HCallStub(CodeStub::SubString, argument_count);
-  ast_context()->ReturnInstruction(result, ast_id);
+void HGraphBuilder::GenerateSubString(CallRuntime* call) {
+  const int kArgumentCount = 3;
+  ASSERT(call->arguments()->length() == kArgumentCount);
+  VisitArgumentList(call->arguments());
+  if (HasStackOverflow() || !subgraph()->HasExit()) return;
+  HCallStub* result = new HCallStub(CodeStub::SubString, kArgumentCount);
+  Drop(kArgumentCount);
+  ast_context()->ReturnInstruction(result, call->id());

[fschneider, 2010/12/20 16:13:02]

There seems a lot of duplicated code for all runtime-functions that call a stub
- and there will be more added. Maybe add a helper like BuildStubCall?

[Kevin Millikin (Chromium), 2010/12/21 11:16:37]

Maybe.  It seems premature.  Refactoring is easier if things are explicit at
every site rather than layers and layers of abstractions.

Also we're inconsistent right now about what is "BuildXXX", what is "HandleXXX",
what is "VisitXXX", and what is "AddXXX".  I'd rather not introduce more at this
point.

 }
 
 
 // Fast support for StringCompare.
-void HGraphBuilder::GenerateStringCompare(int argument_count, int ast_id) {
-  ASSERT_EQ(2, argument_count);
-  PushArgumentsForStubCall(argument_count);
-  HCallStub* result = new HCallStub(CodeStub::StringCompare, argument_count);
-  ast_context()->ReturnInstruction(result, ast_id);
+void HGraphBuilder::GenerateStringCompare(CallRuntime* call) {
+  const int kArgumentCount = 2;
+  ASSERT(call->arguments()->length() == kArgumentCount);
+  VisitArgumentList(call->arguments());
+  if (HasStackOverflow() || !subgraph()->HasExit()) return;
+  HCallStub* result = new HCallStub(CodeStub::StringCompare, kArgumentCount);
+  Drop(kArgumentCount);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
 // Support for direct calls from JavaScript to native RegExp code.
-void HGraphBuilder::GenerateRegExpExec(int argument_count, int ast_id) {
-  ASSERT_EQ(4, argument_count);
-  PushArgumentsForStubCall(argument_count);
-  HCallStub* result = new HCallStub(CodeStub::RegExpExec, argument_count);
-  ast_context()->ReturnInstruction(result, ast_id);
+void HGraphBuilder::GenerateRegExpExec(CallRuntime* call) {
+  const int kArgumentCount = 4;
+  ASSERT(call->arguments()->length() == kArgumentCount);
+  VisitArgumentList(call->arguments());
+  if (HasStackOverflow() || !subgraph()->HasExit()) return;
+  HCallStub* result = new HCallStub(CodeStub::RegExpExec, kArgumentCount);
+  Drop(kArgumentCount);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
 // Construct a RegExp exec result with two in-object properties.
-void HGraphBuilder::GenerateRegExpConstructResult(int argument_count,
-                                                  int ast_id) {
-  ASSERT_EQ(3, argument_count);
-  PushArgumentsForStubCall(argument_count);
+void HGraphBuilder::GenerateRegExpConstructResult(CallRuntime* call) {
+  const int kArgumentCount = 3;
+  ASSERT(call->arguments()->length() == kArgumentCount);
+  VisitArgumentList(call->arguments());
+  if (HasStackOverflow() || !subgraph()->HasExit()) return;
   HCallStub* result =
-      new HCallStub(CodeStub::RegExpConstructResult, argument_count);
-  ast_context()->ReturnInstruction(result, ast_id);
+      new HCallStub(CodeStub::RegExpConstructResult, kArgumentCount);
+  Drop(kArgumentCount);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
 // Support for fast native caches.
-void HGraphBuilder::GenerateGetFromCache(int argument_count, int ast_id) {
+void HGraphBuilder::GenerateGetFromCache(CallRuntime* call) {
   BAILOUT("inlined runtime function: GetFromCache");
 }
 
 
 // Fast support for number to string.
-void HGraphBuilder::GenerateNumberToString(int argument_count, int ast_id) {
-  ASSERT_EQ(1, argument_count);
-  PushArgumentsForStubCall(argument_count);
-  HCallStub* result = new HCallStub(CodeStub::NumberToString, argument_count);
-  ast_context()->ReturnInstruction(result, ast_id);
+void HGraphBuilder::GenerateNumberToString(CallRuntime* call) {
+  const int kArgumentCount = 1;
+  ASSERT(call->arguments()->length() == kArgumentCount);
+  VisitArgumentList(call->arguments());
+  if (HasStackOverflow() || !subgraph()->HasExit()) return;
+  HCallStub* result = new HCallStub(CodeStub::NumberToString, kArgumentCount);
+  Drop(kArgumentCount);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
 // Fast swapping of elements. Takes three expressions, the object and two
 // indices. This should only be used if the indices are known to be
 // non-negative and within bounds of the elements array at the call site.
-void HGraphBuilder::GenerateSwapElements(int argument_count, int ast_id) {
+void HGraphBuilder::GenerateSwapElements(CallRuntime* call) {
   BAILOUT("inlined runtime function: SwapElements");
 }
 
 
 // Fast call for custom callbacks.
-void HGraphBuilder::GenerateCallFunction(int argument_count, int ast_id) {
+void HGraphBuilder::GenerateCallFunction(CallRuntime* call) {
   BAILOUT("inlined runtime function: CallFunction");
 }
 
 
 // Fast call to math functions.
-void HGraphBuilder::GenerateMathPow(int argument_count, int ast_id) {
-  ASSERT_EQ(2, argument_count);
+void HGraphBuilder::GenerateMathPow(CallRuntime* call) {
+  ASSERT(call->arguments()->length() == 2);
+  VISIT_FOR_VALUE(call->arguments()->at(0));
+  VISIT_FOR_VALUE(call->arguments()->at(1));
   HValue* right = Pop();
   HValue* left = Pop();
   HPower* result = new HPower(left, right);
-  ast_context()->ReturnInstruction(result, ast_id);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
-void HGraphBuilder::GenerateMathSin(int argument_count, int ast_id) {
-  ASSERT_EQ(1, argument_count);
-  PushArgumentsForStubCall(argument_count);
+void HGraphBuilder::GenerateMathSin(CallRuntime* call) {
+  const int kArgumentCount = 1;
+  ASSERT(call->arguments()->length() == kArgumentCount);
+  VisitArgumentList(call->arguments());
+  if (HasStackOverflow() || !subgraph()->HasExit()) return;
   HCallStub* result =
-      new HCallStub(CodeStub::TranscendentalCache, argument_count);
+      new HCallStub(CodeStub::TranscendentalCache, kArgumentCount);
   result->set_transcendental_type(TranscendentalCache::SIN);
-  ast_context()->ReturnInstruction(result, ast_id);
+  Drop(kArgumentCount);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
-void HGraphBuilder::GenerateMathCos(int argument_count, int ast_id) {
-  ASSERT_EQ(1, argument_count);
-  PushArgumentsForStubCall(argument_count);
+void HGraphBuilder::GenerateMathCos(CallRuntime* call) {
+  const int kArgumentCount = 1;
+  ASSERT(call->arguments()->length() == kArgumentCount);
+  VisitArgumentList(call->arguments());
+  if (HasStackOverflow() || !subgraph()->HasExit()) return;
   HCallStub* result =
-      new HCallStub(CodeStub::TranscendentalCache, argument_count);
+      new HCallStub(CodeStub::TranscendentalCache, kArgumentCount);
   result->set_transcendental_type(TranscendentalCache::COS);
-  ast_context()->ReturnInstruction(result, ast_id);
+  Drop(kArgumentCount);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
-void HGraphBuilder::GenerateMathLog(int argument_count, int ast_id) {
-  ASSERT_EQ(1, argument_count);
-  PushArgumentsForStubCall(argument_count);
+void HGraphBuilder::GenerateMathLog(CallRuntime* call) {
+  const int kArgumentCount = 1;
+  ASSERT(call->arguments()->length() == kArgumentCount);
+  VisitArgumentList(call->arguments());
+  if (HasStackOverflow() || !subgraph()->HasExit()) return;
   HCallStub* result =
-      new HCallStub(CodeStub::TranscendentalCache, argument_count);
+      new HCallStub(CodeStub::TranscendentalCache, kArgumentCount);
   result->set_transcendental_type(TranscendentalCache::LOG);
-  ast_context()->ReturnInstruction(result, ast_id);
+  Drop(kArgumentCount);
+  ast_context()->ReturnInstruction(result, call->id());
 }
 
 
-void HGraphBuilder::GenerateMathSqrt(int argument_count, int ast_id) {
+void HGraphBuilder::GenerateMathSqrt(CallRuntime* call) {
   BAILOUT("inlined runtime function: MathSqrt");
 }
 
 
 // Check whether two RegExps are equivalent
-void HGraphBuilder::GenerateIsRegExpEquivalent(int argument_count,
-                                               int ast_id) {
+void HGraphBuilder::GenerateIsRegExpEquivalent(CallRuntime* call) {
   BAILOUT("inlined runtime function: IsRegExpEquivalent");
 }
 
 
-void HGraphBuilder::GenerateGetCachedArrayIndex(int argument_count,
-                                                int ast_id) {
+void HGraphBuilder::GenerateGetCachedArrayIndex(CallRuntime* call) {
   BAILOUT("inlined runtime function: GetCachedArrayIndex");
 }
 
 
-void HGraphBuilder::GenerateFastAsciiArrayJoin(int argument_count,
-                                               int ast_id) {
+void HGraphBuilder::GenerateFastAsciiArrayJoin(CallRuntime* call) {
   BAILOUT("inlined runtime function: FastAsciiArrayJoin");
 }
 
@@ -5214,6 +5370,7 @@ void HGraphBuilder::GenerateFastAsciiArrayJoin(int argument_count,
 #undef CHECK_BAILOUT
 #undef VISIT_FOR_EFFECT
 #undef VISIT_FOR_VALUE
+#undef VISIT_FOR_CONTROL
 #undef ADD_TO_SUBGRAPH
 
 
@@ -5341,7 +5498,8 @@ HEnvironment* HEnvironment::CopyAsLoopHeader(HBasicBlock* loop_header) const {
 HEnvironment* HEnvironment::CopyForInlining(Handle<JSFunction> target,
                                             FunctionLiteral* function,
                                             bool is_speculative,
-                                            HConstant* undefined) const {
+                                            HConstant* undefined,
+                                            HGlobalReceiver* receiver) const {
   // Outer environment is a copy of this one without the arguments.
   int arity = function->scope()->num_parameters();
   HEnvironment* outer = Copy();
@@ -5351,8 +5509,12 @@ HEnvironment* HEnvironment::CopyForInlining(Handle<JSFunction> target,
   // Get the argument values from the original environment.
   if (is_speculative) {
     for (int i = 0; i <= arity; ++i) {  // Include receiver.
-      HValue* push = ExpressionStackAt(arity - i);
-      inner->SetValueAt(i, push);
+      HPushArgument* push = HPushArgument::cast(ExpressionStackAt(arity - i));
+      if (i == 0 && receiver != NULL) {
+        inner->SetValueAt(i, receiver);
+      } else {
+        inner->SetValueAt(i, push->argument());
+      }
     }
   } else {
     for (int i = 0; i <= arity; ++i) {  // Include receiver.