Fix an bug in deoptimization after polymorphic calls in effect contexts.

For polymorphic calls (also loads and stores) we construct a type switch
graph that has a basic block merging all the variants.  There is an
environment simulation before the goto at the end of all the predecessor
blocks.  This simulation is used to define the environment on entry to the
successor block, and captures the return value of the call.  In effect
contexts, this value should not be present in the environment.

The fix is to use the AST context to decide whether to have this value in
the join node's environment at all.

BUG=1014

Review URL: http://codereview.chromium.org/6065014

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6189 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 63a651b1e9dd2ce4341c23262893ecabc40adb4a..0d92b2eebe5be196e17be4614eaeb10873ebf652 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -3103,8 +3103,15 @@ HBasicBlock* HGraphBuilder::BuildTypeSwitch(ZoneMapList* maps,
   // 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);
+    HSubgraph* subgraph = subgraphs->at(i);
+    if (subgraph->HasExit()) {
+      // In an effect context the value of the type switch is not needed.
+      // There is no need to merge it at the join block only to discard it.
+      HBasicBlock* subgraph_exit = subgraph->exit_block();
+      if (ast_context()->IsEffect()) {
+        subgraph_exit->last_environment()->Drop(1);
+      }
+      subgraph_exit->Goto(join_block);
     }
   }
 
@@ -3242,7 +3249,8 @@ void HGraphBuilder::HandlePolymorphicStoreNamedField(Assignment* expr,
     Push(value);
     instr->set_position(expr->position());
     AddInstruction(instr);
-    if (instr->HasSideEffects()) AddSimulate(expr->id());
+    if (instr->HasSideEffects()) AddSimulate(expr->AssignmentId());
+    ast_context()->ReturnValue(Pop());
   } else {
     // Build subgraph for generic store through IC.
     {
@@ -3260,11 +3268,14 @@ void HGraphBuilder::HandlePolymorphicStoreNamedField(Assignment* expr,
     }
 
     HBasicBlock* new_exit_block =
-        BuildTypeSwitch(&maps, &subgraphs, object, expr->AssignmentId());
+        BuildTypeSwitch(&maps, &subgraphs, object, expr->id());
     subgraph()->set_exit_block(new_exit_block);
+    // In an effect context, we did not materialized the value in the
+    // predecessor environments so there's no need to handle it here.
+    if (subgraph()->HasExit() && !ast_context()->IsEffect()) {
+      ast_context()->ReturnValue(Pop());
+    }
   }
-
-  if (subgraph()->HasExit()) ast_context()->ReturnValue(Pop());
 }
 
 
@@ -3548,8 +3559,7 @@ void HGraphBuilder::HandlePolymorphicLoadNamedField(Property* expr,
   if (maps.length() == 0) {
     HInstruction* instr = BuildLoadNamedGeneric(object, expr);
     instr->set_position(expr->position());
-    PushAndAdd(instr);
-    if (instr->HasSideEffects()) AddSimulate(expr->id());
+    ast_context()->ReturnInstruction(instr, expr->id());
   } else {
     // Build subgraph for generic load through IC.
     {
@@ -3568,9 +3578,12 @@ void HGraphBuilder::HandlePolymorphicLoadNamedField(Property* expr,
     HBasicBlock* new_exit_block =
         BuildTypeSwitch(&maps, &subgraphs, object, expr->id());
     subgraph()->set_exit_block(new_exit_block);
+    // In an effect context, we did not materialized the value in the
+    // predecessor environments so there's no need to handle it here.
+    if (subgraph()->HasExit() && !ast_context()->IsEffect()) {
+      ast_context()->ReturnValue(Pop());
+    }
   }
-
-  if (subgraph()->HasExit()) ast_context()->ReturnValue(Pop());
 }
 
 
@@ -3856,7 +3869,11 @@ void HGraphBuilder::HandlePolymorphicCallNamed(Call* expr,
     HBasicBlock* new_exit_block =
         BuildTypeSwitch(&maps, &subgraphs, receiver, expr->id());
     subgraph()->set_exit_block(new_exit_block);
-    if (new_exit_block != NULL) ast_context()->ReturnValue(Pop());
+    // In an effect context, we did not materialized the value in the
+    // predecessor environments so there's no need to handle it here.
+    if (new_exit_block != NULL && !ast_context()->IsEffect()) {
+      ast_context()->ReturnValue(Pop());
+    }
   }
 }