Revision: 6049
Author: [email protected]
Date: Thu Dec 16 05:13:36 2010
Log: Fix issue 977, occasional failure of the DeltaBlue benchmark.

Before, when we deoptimized after a branch we jumped to before the branch
was taken in the unoptimized code with a token value that indicated when
edge to take.  There was a lot of machinery to track this value through the
short-circuit logical operations and logical negation, and to handle it
properly at inline function return sites.  There was also machinery to
prevent incorrectly seeing this environment with the extra value never
actually materialized in the unoptimized code.

Instead, now we deoptimize directly to one of the targets of the branch.
Much but not yet all of the extra machinery has been removed or simplified.
The cost is that branching control structures (the looping statements, if
statements, conditional expressions, and the short-circuit binary logical
operations) need extra AST IDs to identify the branch targets.

Review URL: http://codereview.chromium.org/5908001
http://code.google.com/p/v8/source/detail?r=6049

Modified:
 /branches/bleeding_edge/src/ast-inl.h
 /branches/bleeding_edge/src/ast.h
 /branches/bleeding_edge/src/full-codegen.cc
 /branches/bleeding_edge/src/hydrogen-instructions.h
 /branches/bleeding_edge/src/hydrogen.cc
 /branches/bleeding_edge/src/hydrogen.h
 /branches/bleeding_edge/src/ia32/lithium-ia32.cc

=======================================
--- /branches/bleeding_edge/src/ast-inl.h       Mon Dec 13 08:29:47 2010
+++ /branches/bleeding_edge/src/ast-inl.h       Thu Dec 16 05:13:36 2010
@@ -71,14 +71,16 @@
     : IterationStatement(labels),
       cond_(NULL),
       condition_position_(-1),
-      next_id_(GetNextId()) {
+      continue_id_(GetNextId()),
+      back_edge_id_(GetNextId()) {
 }


 WhileStatement::WhileStatement(ZoneStringList* labels)
     : IterationStatement(labels),
       cond_(NULL),
-      may_have_function_literal_(true) {
+      may_have_function_literal_(true),
+      body_id_(GetNextId()) {
 }


@@ -89,7 +91,8 @@
       next_(NULL),
       may_have_function_literal_(true),
       loop_variable_(NULL),
-      next_id_(GetNextId()) {
+      continue_id_(GetNextId()),
+      body_id_(GetNextId()) {
 }


=======================================
--- /branches/bleeding_edge/src/ast.h   Mon Dec 13 08:29:47 2010
+++ /branches/bleeding_edge/src/ast.h   Thu Dec 16 05:13:36 2010
@@ -476,12 +476,14 @@
   void set_condition_position(int pos) { condition_position_ = pos; }

   // Bailout support.
-  virtual int ContinueId() const { return next_id_; }
+  virtual int ContinueId() const { return continue_id_; }
+  int BackEdgeId() const { return back_edge_id_; }

  private:
   Expression* cond_;
   int condition_position_;
-  int next_id_;
+  int continue_id_;
+  int back_edge_id_;
 };


@@ -506,11 +508,13 @@

   // Bailout support.
   virtual int ContinueId() const { return EntryId(); }
+  int BodyId() const { return body_id_; }

  private:
   Expression* cond_;
   // True if there is a function literal subexpression in the condition.
   bool may_have_function_literal_;
+  int body_id_;
 };


@@ -542,7 +546,8 @@
   }

   // Bailout support.
-  virtual int ContinueId() const { return next_id_; }
+  virtual int ContinueId() const { return continue_id_; }
+  int BodyId() const { return body_id_; }

   bool is_fast_smi_loop() { return loop_variable_ != NULL; }
   Variable* loop_variable() { return loop_variable_; }
@@ -555,7 +560,8 @@
   // True if there is a function literal subexpression in the condition.
   bool may_have_function_literal_;
   Variable* loop_variable_;
-  int next_id_;
+  int continue_id_;
+  int body_id_;
 };


@@ -735,7 +741,10 @@
               Statement* else_statement)
       : condition_(condition),
         then_statement_(then_statement),
-        else_statement_(else_statement) { }
+        else_statement_(else_statement),
+        then_id_(GetNextId()),
+        else_id_(GetNextId()) {
+  }

   DECLARE_NODE_TYPE(IfStatement)

@@ -747,11 +756,16 @@
   Expression* condition() const { return condition_; }
   Statement* then_statement() const { return then_statement_; }
   Statement* else_statement() const { return else_statement_; }
+
+  int ThenId() const { return then_id_; }
+  int ElseId() const { return else_id_; }

  private:
   Expression* condition_;
   Statement* then_statement_;
   Statement* else_statement_;
+  int then_id_;
+  int else_id_;
 };


@@ -1376,6 +1390,9 @@
                   int pos)
: op_(op), left_(left), right_(right), pos_(pos), is_smi_only_(false) {
     ASSERT(Token::IsBinaryOp(op));
+    right_id_ = (op == Token::AND || op == Token::OR)
+        ? GetNextId()
+        : AstNode::kNoNumber;
   }

   // Create the binary operation corresponding to a compound assignment.
@@ -1395,6 +1412,9 @@
   // Type feedback information.
   void RecordTypeFeedback(TypeFeedbackOracle* oracle);
   bool IsSmiOnly() const { return is_smi_only_; }
+
+  // Bailout support.
+  int RightId() const { return right_id_; }

  private:
   Token::Value op_;
@@ -1402,6 +1422,9 @@
   Expression* right_;
   int pos_;
   bool is_smi_only_;
+  // The short-circuit logical operations have an AST ID for their
+  // right-hand subexpression.
+  int right_id_;
 };


@@ -1526,7 +1549,10 @@
         then_expression_(then_expression),
         else_expression_(else_expression),
         then_expression_position_(then_expression_position),
-        else_expression_position_(else_expression_position) { }
+        else_expression_position_(else_expression_position),
+        then_id_(GetNextId()),
+        else_id_(GetNextId()) {
+  }

   DECLARE_NODE_TYPE(Conditional)

@@ -1536,8 +1562,11 @@
   Expression* then_expression() const { return then_expression_; }
   Expression* else_expression() const { return else_expression_; }

-  int then_expression_position() { return then_expression_position_; }
-  int else_expression_position() { return else_expression_position_; }
+ int then_expression_position() const { return then_expression_position_; } + int else_expression_position() const { return else_expression_position_; }
+
+  int ThenId() const { return then_id_; }
+  int ElseId() const { return else_id_; }

  private:
   Expression* condition_;
@@ -1545,6 +1574,8 @@
   Expression* else_expression_;
   int then_expression_position_;
   int else_expression_position_;
+  int then_id_;
+  int else_id_;
 };


=======================================
--- /branches/bleeding_edge/src/full-codegen.cc Wed Dec 15 08:14:29 2010
+++ /branches/bleeding_edge/src/full-codegen.cc Thu Dec 16 05:13:36 2010
@@ -761,6 +761,7 @@

   context()->EmitLogicalLeft(expr, &eval_right, &done);

+  PrepareForBailoutForId(expr->RightId(), NO_REGISTERS);
   __ bind(&eval_right);
   if (context()->IsTest()) ForwardBailoutToChild(expr);
   context()->HandleExpression(expr->right());
@@ -925,16 +926,21 @@

   if (stmt->HasElseStatement()) {
     VisitForControl(stmt->condition(), &then_part, &else_part, &then_part);
+    PrepareForBailoutForId(stmt->ThenId(), NO_REGISTERS);
     __ bind(&then_part);
     Visit(stmt->then_statement());
     __ jmp(&done);

+    PrepareForBailoutForId(stmt->ElseId(), NO_REGISTERS);
     __ bind(&else_part);
     Visit(stmt->else_statement());
   } else {
     VisitForControl(stmt->condition(), &then_part, &done, &then_part);
+    PrepareForBailoutForId(stmt->ThenId(), NO_REGISTERS);
     __ bind(&then_part);
     Visit(stmt->then_statement());
+
+    PrepareForBailoutForId(stmt->ElseId(), NO_REGISTERS);
   }
   __ bind(&done);
   PrepareForBailoutForId(stmt->id(), NO_REGISTERS);
@@ -1053,12 +1059,13 @@
                   &stack_check);

   // Check stack before looping.
+  PrepareForBailoutForId(stmt->BackEdgeId(), NO_REGISTERS);
   __ bind(&stack_check);
   EmitStackCheck(stmt);
   __ jmp(&body);

-  __ bind(loop_statement.break_target());
   PrepareForBailoutForId(stmt->ExitId(), NO_REGISTERS);
+  __ bind(loop_statement.break_target());
   decrement_loop_depth();
 }

@@ -1073,6 +1080,7 @@
   // Emit the test at the bottom of the loop.
   __ jmp(&test);

+  PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
   __ bind(&body);
   Visit(stmt->body());

@@ -1090,8 +1098,8 @@
                   loop_statement.break_target(),
                   loop_statement.break_target());

-  __ bind(loop_statement.break_target());
   PrepareForBailoutForId(stmt->ExitId(), NO_REGISTERS);
+  __ bind(loop_statement.break_target());
   decrement_loop_depth();
 }

@@ -1109,12 +1117,12 @@
   // Emit the test at the bottom of the loop (even if empty).
   __ jmp(&test);

+  PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
   __ bind(&body);
   Visit(stmt->body());

-  __ bind(loop_statement.continue_target());
   PrepareForBailoutForId(stmt->ContinueId(), NO_REGISTERS);
-
+  __ bind(loop_statement.continue_target());
   SetStatementPosition(stmt);
   if (stmt->next() != NULL) {
     Visit(stmt->next());
@@ -1137,8 +1145,8 @@
     __ jmp(&body);
   }

-  __ bind(loop_statement.break_target());
   PrepareForBailoutForId(stmt->ExitId(), NO_REGISTERS);
+  __ bind(loop_statement.break_target());
   decrement_loop_depth();
 }

@@ -1269,6 +1277,7 @@
   Label true_case, false_case, done;
   VisitForControl(expr->condition(), &true_case, &false_case, &true_case);

+  PrepareForBailoutForId(expr->ThenId(), NO_REGISTERS);
   __ bind(&true_case);
   SetExpressionPosition(expr->then_expression(),
                         expr->then_expression_position());
@@ -1283,6 +1292,7 @@
     __ jmp(&done);
   }

+  PrepareForBailoutForId(expr->ElseId(), NO_REGISTERS);
   __ bind(&false_case);
   if (context()->IsTest()) ForwardBailoutToChild(expr);
   SetExpressionPosition(expr->else_expression(),
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.h Wed Dec 15 04:32:19 2010 +++ /branches/bleeding_edge/src/hydrogen-instructions.h Thu Dec 16 05:13:36 2010
@@ -94,13 +94,13 @@
 //     HCallStub
 //     HConstant
 //     HControlInstruction
+//       HDeoptimize
 //       HGoto
 //       HUnaryControlInstruction
 //         HBranch
 //         HCompareMapAndBranch
 //         HReturn
 //         HThrow
-//     HDeoptimize
 //     HEnterInlined
 //     HFunctionLiteral
 //     HGlobalObject
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc     Wed Dec 15 09:11:11 2010
+++ /branches/bleeding_edge/src/hydrogen.cc     Thu Dec 16 05:13:36 2010
@@ -64,9 +64,7 @@
       first_instruction_index_(-1),
       last_instruction_index_(-1),
       deleted_phis_(4),
-      is_inline_return_target_(false),
-      inverted_(false),
-      deopt_predecessor_(NULL) {
+      is_inline_return_target_(false) {
 }


@@ -1292,15 +1290,15 @@
   for (int i = 0; i < graph_->blocks()->length(); i++) {
     HBasicBlock* block = graph_->blocks()->at(i);
     if (block->IsLoopHeader()) {
-      HBasicBlock* backedge = block->loop_information()->GetLastBackEdge();
-      HBasicBlock* dominator = backedge;
-      bool backedge_dominated_by_call = false;
-      while (dominator != block && !backedge_dominated_by_call) {
+ HBasicBlock* back_edge = block->loop_information()->GetLastBackEdge();
+      HBasicBlock* dominator = back_edge;
+      bool back_edge_dominated_by_call = false;
+      while (dominator != block && !back_edge_dominated_by_call) {
         HInstruction* instr = dominator->first();
-        while (instr != NULL && !backedge_dominated_by_call) {
+        while (instr != NULL && !back_edge_dominated_by_call) {
           if (instr->IsCall()) {
-            RemoveStackCheck(backedge);
-            backedge_dominated_by_call = true;
+            RemoveStackCheck(back_edge);
+            back_edge_dominated_by_call = true;
           }
           instr = instr->next();
         }
@@ -2053,38 +2051,29 @@


 void TestContext::BuildBranch(HValue* value) {
+  // We expect the graph to be in edge-split form: there is no edge that
+  // connects a branch node to a join node.  We conservatively ensure that
+  // property by always adding an empty block on the outgoing edges of this
+  // branch.
   HGraphBuilder* builder = owner();
-  HBasicBlock* materialize_true = builder->graph()->CreateBasicBlock();
-  HBasicBlock* materialize_false = builder->graph()->CreateBasicBlock();
- HBranch* branch = new HBranch(materialize_true, materialize_false, value);
+  HBasicBlock* empty_true = builder->graph()->CreateBasicBlock();
+  HBasicBlock* empty_false = builder->graph()->CreateBasicBlock();
+  HBranch* branch = new HBranch(empty_true, empty_false, value);
   builder->CurrentBlock()->Finish(branch);

-  HBasicBlock* true_block = if_true();
-  HValue* true_value = invert_true()
-      ? builder->graph()->GetConstantFalse()
-      : builder->graph()->GetConstantTrue();
-  materialize_true->set_inverted(invert_true());
-  true_block->set_deopt_predecessor(materialize_true);
-
-  if (true_block->IsInlineReturnTarget()) {
-    materialize_true->AddLeaveInlined(true_value, true_block);
+  HValue* const no_return_value = NULL;
+  HBasicBlock* true_target = if_true();
+  if (true_target->IsInlineReturnTarget()) {
+    empty_true->AddLeaveInlined(no_return_value, true_target);
   } else {
-    materialize_true->last_environment()->Push(true_value);
-    materialize_true->Goto(true_block);
+    empty_true->Goto(true_target);
   }

-  HBasicBlock* false_block = if_false();
-  HValue* false_value = invert_false()
-      ? builder->graph()->GetConstantTrue()
-      : builder->graph()->GetConstantFalse();
-  materialize_false->set_inverted(invert_false());
-  false_block->set_deopt_predecessor(materialize_false);
-
-  if (false_block->IsInlineReturnTarget()) {
-    materialize_false->AddLeaveInlined(false_value, false_block);
+  HBasicBlock* false_target = if_false();
+  if (false_target->IsInlineReturnTarget()) {
+    empty_false->AddLeaveInlined(no_return_value, false_target);
   } else {
-    materialize_false->last_environment()->Push(false_value);
-    materialize_false->Goto(false_block);
+    empty_false->Goto(false_target);
   }
   builder->subgraph()->set_exit_block(NULL);
 }
@@ -2118,6 +2107,13 @@
   } while (false)


+#define VISIT_FOR_CONTROL(expr, true_block, false_block)        \
+  do {                                                          \
+    VisitForControl(expr, true_block, false_block);             \
+    if (HasStackOverflow()) return;                             \
+  } while (false)
+
+
 // 'thing' could be an expression, statement, or list of statements.
 #define ADD_TO_SUBGRAPH(graph, thing)       \
   do {                                      \
@@ -2168,6 +2164,14 @@
   ValueContext for_value(this);
   Visit(expr);
 }
+
+
+void HGraphBuilder::VisitForControl(Expression* expr,
+                                    HBasicBlock* true_block,
+                                    HBasicBlock* false_block) {
+  TestContext for_test(this, true_block, false_block);
+  Visit(expr);
+}


 HValue* HGraphBuilder::VisitArgument(Expression* expr) {
@@ -2257,52 +2261,6 @@
   SubgraphScope scope(this, graph);
   VisitForValue(expr);
 }
-
-
-void HGraphBuilder::VisitCondition(Expression* expr,
-                                   HBasicBlock* true_block,
-                                   HBasicBlock* false_block,
-                                   bool invert_true,
-                                   bool invert_false) {
- VisitForControl(expr, true_block, false_block, invert_true, invert_false);
-  CHECK_BAILOUT;
-#ifdef DEBUG
- HValue* value = true_block->predecessors()->at(0)->last_environment()->Top();
-  true_block->set_cond(HConstant::cast(value)->handle());
-
-  value = false_block->predecessors()->at(0)->last_environment()->Top();
-  false_block->set_cond(HConstant::cast(value)->handle());
-#endif
-
-  true_block->SetJoinId(expr->id());
-  false_block->SetJoinId(expr->id());
-  true_block->last_environment()->Pop();
-  false_block->last_environment()->Pop();
-}
-
-
-void HGraphBuilder::AddConditionToSubgraph(HSubgraph* subgraph,
-                                           Expression* expr,
-                                           HSubgraph* true_graph,
-                                           HSubgraph* false_graph) {
-  SubgraphScope scope(this, subgraph);
-  VisitCondition(expr,
-                 true_graph->entry_block(),
-                 false_graph->entry_block(),
-                 false,
-                 false);
-}
-
-
-void HGraphBuilder::VisitForControl(Expression* expr,
-                                    HBasicBlock* true_block,
-                                    HBasicBlock* false_block,
-                                    bool invert_true,
-                                    bool invert_false) {
-  TestContext for_test(this, true_block, false_block,
-                       invert_true, invert_false);
-  Visit(expr);
-}


 void HGraphBuilder::AddToSubgraph(HSubgraph* graph,
@@ -2484,19 +2442,24 @@

 void HGraphBuilder::VisitIfStatement(IfStatement* stmt) {
   if (stmt->condition()->ToBooleanIsTrue()) {
+    AddSimulate(stmt->ThenId());
     Visit(stmt->then_statement());
   } else if (stmt->condition()->ToBooleanIsFalse()) {
+    AddSimulate(stmt->ElseId());
     Visit(stmt->else_statement());
   } else {
     HSubgraph* then_graph = CreateEmptySubgraph();
     HSubgraph* else_graph = CreateEmptySubgraph();
-    VisitCondition(stmt->condition(),
-                   then_graph->entry_block(),
-                   else_graph->entry_block(),
-                   false, false);
-    if (HasStackOverflow()) return;
+    VISIT_FOR_CONTROL(stmt->condition(),
+                      then_graph->entry_block(),
+                      else_graph->entry_block());
+
+    then_graph->entry_block()->SetJoinId(stmt->ThenId());
     ADD_TO_SUBGRAPH(then_graph, stmt->then_statement());
+
+    else_graph->entry_block()->SetJoinId(stmt->ElseId());
     ADD_TO_SUBGRAPH(else_graph, stmt->else_statement());
+
     current_subgraph_->AppendJoin(then_graph, else_graph, stmt);
   }
 }
@@ -2526,9 +2489,7 @@
       TestContext* test = TestContext::cast(context);
       VisitForControl(stmt->expression(),
                       test->if_true(),
-                      test->if_false(),
-                      false,
-                      false);
+                      test->if_false());
     } else {
       HValue* return_value = NULL;
       if (context->IsEffect()) {
@@ -2771,8 +2732,14 @@
   } else {
     HSubgraph* go_back = CreateEmptySubgraph();
     HSubgraph* exit = CreateEmptySubgraph();
-    AddConditionToSubgraph(body_graph, stmt->cond(), go_back, exit);
-    if (HasStackOverflow()) return;
+    {
+      SubgraphScope scope(this, body_graph);
+      VISIT_FOR_CONTROL(stmt->cond(),
+                        go_back->entry_block(),
+                        exit->entry_block());
+      go_back->entry_block()->SetJoinId(stmt->BackEdgeId());
+      exit->entry_block()->SetJoinId(stmt->ExitId());
+    }
     current_subgraph_->AppendDoWhile(body_graph, stmt, go_back, exit);
   }
 }
@@ -2799,8 +2766,14 @@
     cond_graph = CreateLoopHeaderSubgraph(environment());
     body_graph = CreateEmptySubgraph();
     exit_graph = CreateEmptySubgraph();
- AddConditionToSubgraph(cond_graph, stmt->cond(), body_graph, exit_graph);
-    if (HasStackOverflow()) return;
+    {
+      SubgraphScope scope(this, cond_graph);
+      VISIT_FOR_CONTROL(stmt->cond(),
+                        body_graph->entry_block(),
+                        exit_graph->entry_block());
+      body_graph->entry_block()->SetJoinId(stmt->BodyId());
+      exit_graph->entry_block()->SetJoinId(stmt->ExitId());
+    }
     ADD_TO_SUBGRAPH(body_graph, stmt->body());
   }

@@ -2850,13 +2823,18 @@
     cond_graph = CreateLoopHeaderSubgraph(environment());
     body_graph = CreateEmptySubgraph();
     exit_graph = CreateEmptySubgraph();
- AddConditionToSubgraph(cond_graph, stmt->cond(), body_graph, exit_graph);
-    if (HasStackOverflow()) return;
-    ADD_TO_SUBGRAPH(body_graph, stmt->body());
+    {
+      SubgraphScope scope(this, cond_graph);
+      VISIT_FOR_CONTROL(stmt->cond(),
+                        body_graph->entry_block(),
+                        exit_graph->entry_block());
+      body_graph->entry_block()->SetJoinId(stmt->BodyId());
+      exit_graph->entry_block()->SetJoinId(stmt->ExitId());
+    }
   } else {
     body_graph = CreateLoopHeaderSubgraph(environment());
-    ADD_TO_SUBGRAPH(body_graph, stmt->body());
-  }
+  }
+  ADD_TO_SUBGRAPH(body_graph, stmt->body());

   HSubgraph* next_graph = NULL;
   body_graph->ResolveContinue(stmt);
@@ -2915,13 +2893,16 @@
 void HGraphBuilder::VisitConditional(Conditional* expr) {
   HSubgraph* then_graph = CreateEmptySubgraph();
   HSubgraph* else_graph = CreateEmptySubgraph();
-  VisitCondition(expr->condition(),
-                 then_graph->entry_block(),
-                 else_graph->entry_block(),
-                 false, false);
-  if (HasStackOverflow()) return;
+  VISIT_FOR_CONTROL(expr->condition(),
+                    then_graph->entry_block(),
+                    else_graph->entry_block());
+
+  then_graph->entry_block()->SetJoinId(expr->ThenId());
   ADD_TO_SUBGRAPH(then_graph, expr->then_expression());
+
+  else_graph->entry_block()->SetJoinId(expr->ElseId());
   ADD_TO_SUBGRAPH(else_graph, expr->else_expression());
+
   current_subgraph_->AppendJoin(then_graph, else_graph, expr);
   ast_context()->ReturnValue(Pop());
 }
@@ -3979,10 +3960,7 @@
     if_true->MarkAsInlineReturnTarget();
     if_false->MarkAsInlineReturnTarget();
     // AstContext constructor pushes on the context stack.
-    bool invert_true = TestContext::cast(ast_context())->invert_true();
-    bool invert_false = TestContext::cast(ast_context())->invert_false();
-    test_context = new TestContext(this, if_true, if_false,
-                                   invert_true, invert_false);
+    test_context = new TestContext(this, if_true, if_false);
     function_return_ = NULL;
   } else {
// Inlined body is treated as if it occurs in the original call context.
@@ -4026,16 +4004,15 @@
       // simply jumping to the false target.
       //
       // TODO(3168478): refactor to avoid this.
-      HBasicBlock* materialize_true = graph()->CreateBasicBlock();
-      HBasicBlock* materialize_false = graph()->CreateBasicBlock();
+      HBasicBlock* empty_true = graph()->CreateBasicBlock();
+      HBasicBlock* empty_false = graph()->CreateBasicBlock();
       HBranch* branch =
-          new HBranch(materialize_true, materialize_false, return_value);
+          new HBranch(empty_true, empty_false, return_value);
       body->exit_block()->Finish(branch);

-      materialize_true->AddLeaveInlined(graph()->GetConstantTrue(),
-                                        test_context->if_true());
-      materialize_false->AddLeaveInlined(graph()->GetConstantFalse(),
-                                         test_context->if_false());
+      HValue* const no_return_value = NULL;
+ empty_true->AddLeaveInlined(no_return_value, test_context->if_true()); + empty_false->AddLeaveInlined(no_return_value, test_context->if_false());
     }
     body->set_exit_block(NULL);
   }
@@ -4054,35 +4031,20 @@
     if_false->SetJoinId(expr->id());
     ASSERT(ast_context() == test_context);
     delete test_context;  // Destructor pops from expression context stack.
-    // Forward to the real test context.
-
-    // Discard the lingering branch value (which may be true or false,
- // depending on whether the final condition was negated) and jump to the
-    // true target with a true branch value.
+
+    // Forward to the real test context.
+    HValue* const no_return_value = NULL;
     HBasicBlock* true_target = TestContext::cast(ast_context())->if_true();
-    bool invert_true = TestContext::cast(ast_context())->invert_true();
-    HValue* true_value = invert_true
-        ? graph()->GetConstantFalse()
-        : graph()->GetConstantTrue();
-    if_true->last_environment()->Pop();
     if (true_target->IsInlineReturnTarget()) {
-      if_true->AddLeaveInlined(true_value, true_target);
+      if_true->AddLeaveInlined(no_return_value, true_target);
     } else {
-      if_true->last_environment()->Push(true_value);
       if_true->Goto(true_target);
     }

-    // Do the same for the false target.
HBasicBlock* false_target = TestContext::cast(ast_context())->if_false();
-    bool invert_false = TestContext::cast(ast_context())->invert_false();
-    HValue* false_value = invert_false
-        ? graph()->GetConstantTrue()
-        : graph()->GetConstantFalse();
-    if_false->last_environment()->Pop();
     if (false_target->IsInlineReturnTarget()) {
-      if_false->AddLeaveInlined(false_value, false_target);
+      if_false->AddLeaveInlined(no_return_value, false_target);
     } else {
-      if_false->last_environment()->Push(false_value);
       if_false->Goto(false_target);
     }

@@ -4109,7 +4071,7 @@
   ASSERT(target->IsInlineReturnTarget());
   AddInstruction(new HLeaveInlined);
   HEnvironment* outer = last_environment()->outer();
-  outer->Push(return_value);
+  if (return_value != NULL) outer->Push(return_value);
   UpdateEnvironment(outer);
   Goto(target);
 }
@@ -4495,19 +4457,19 @@
       TestContext* context = TestContext::cast(ast_context());
       VisitForControl(expr->expression(),
                       context->if_false(),
-                      context->if_true(),
-                      !context->invert_false(),
-                      !context->invert_true());
+                      context->if_true());
     } else {
       HSubgraph* true_graph = CreateEmptySubgraph();
       HSubgraph* false_graph = CreateEmptySubgraph();
-      VisitCondition(expr->expression(),
-                     false_graph->entry_block(),
-                     true_graph->entry_block(),
-                     true, true);
-      if (HasStackOverflow()) return;
+      VISIT_FOR_CONTROL(expr->expression(),
+                        false_graph->entry_block(),
+                        true_graph->entry_block());
+      true_graph->entry_block()->SetJoinId(expr->expression()->id());
       true_graph->environment()->Push(graph_->GetConstantTrue());
+
+      false_graph->entry_block()->SetJoinId(expr->expression()->id());
       false_graph->environment()->Push(graph_->GetConstantFalse());
+
       current_subgraph_->AppendJoin(true_graph, false_graph, expr);
       ast_context()->ReturnValue(Pop());
     }
@@ -4771,18 +4733,14 @@
       // Translate left subexpression.
       HBasicBlock* eval_right = graph()->CreateBasicBlock();
       if (is_logical_and) {
-        VisitForControl(expr->left(), eval_right, context->if_false(),
-                        false, context->invert_false());
+        VISIT_FOR_CONTROL(expr->left(), eval_right, context->if_false());
       } else {
-        VisitForControl(expr->left(), context->if_true(), eval_right,
-                        context->invert_true(), false);
-      }
-      if (HasStackOverflow()) return;
-      eval_right->SetJoinId(expr->left()->id());
+        VISIT_FOR_CONTROL(expr->left(), context->if_true(), eval_right);
+      }
+      eval_right->SetJoinId(expr->RightId());

       // Translate right subexpression by visiting it in the same AST
       // context as the entire expression.
-      eval_right->last_environment()->Pop();
       subgraph()->set_exit_block(eval_right);
       Visit(expr->right());

=======================================
--- /branches/bleeding_edge/src/hydrogen.h      Thu Dec  9 04:49:53 2010
+++ /branches/bleeding_edge/src/hydrogen.h      Thu Dec 16 05:13:36 2010
@@ -135,14 +135,6 @@
   // Goto (target block)
   bool IsInlineReturnTarget() const { return is_inline_return_target_; }
   void MarkAsInlineReturnTarget() { is_inline_return_target_ = true; }
-
-  // If this block is a successor of a branch, his flags tells whether the
-  // preceding branch was inverted or not.
-  bool inverted() { return inverted_; }
-  void set_inverted(bool b) { inverted_ = b; }
-
-  HBasicBlock* deopt_predecessor() { return deopt_predecessor_; }
- void set_deopt_predecessor(HBasicBlock* block) { deopt_predecessor_ = block; }

   Handle<Object> cond() { return cond_; }
   void set_cond(Handle<Object> value) { cond_ = value; }
@@ -176,8 +168,6 @@
   ZoneList<int> deleted_phis_;
   SetOncePointer<HBasicBlock> parent_loop_header_;
   bool is_inline_return_target_;
-  bool inverted_;
-  HBasicBlock* deopt_predecessor_;
   Handle<Object> cond_;
 };

@@ -615,14 +605,10 @@
  public:
   TestContext(HGraphBuilder* owner,
               HBasicBlock* if_true,
-              HBasicBlock* if_false,
-              bool invert_true,
-              bool invert_false)
+              HBasicBlock* if_false)
       : AstContext(owner, Expression::kTest),
         if_true_(if_true),
-        if_false_(if_false),
-        invert_true_(invert_true),
-        invert_false_(invert_false) {
+        if_false_(if_false) {
   }

   virtual void ReturnValue(HValue* value);
@@ -635,9 +621,6 @@

   HBasicBlock* if_true() const { return if_true_; }
   HBasicBlock* if_false() const { return if_false_; }
-
-  bool invert_true() { return invert_true_; }
-  bool invert_false() { return invert_false_; }

  private:
   // Build the shared core part of the translation unpacking a value into
@@ -646,8 +629,6 @@

   HBasicBlock* if_true_;
   HBasicBlock* if_false_;
-  bool invert_true_;
-  bool invert_false_;
 };


@@ -723,10 +704,6 @@
   void AddToSubgraph(HSubgraph* graph, ZoneList<Statement*>* stmts);
   void AddToSubgraph(HSubgraph* graph, Statement* stmt);
   void AddToSubgraph(HSubgraph* graph, Expression* expr);
-  void AddConditionToSubgraph(HSubgraph* subgraph,
-                              Expression* expr,
-                              HSubgraph* true_graph,
-                              HSubgraph* false_graph);

   HValue* Top() const { return environment()->Top(); }
   void Drop(int n) { environment()->Drop(n); }
@@ -736,17 +713,8 @@
   void VisitForEffect(Expression* expr);
   void VisitForControl(Expression* expr,
                        HBasicBlock* true_block,
-                       HBasicBlock* false_block,
-                       bool invert_true,
-                       bool invert_false);
-
-  // Visit an expression in a 'condition' context, i.e., in a control
-  // context but not a subexpression of logical and, or, or not.
-  void VisitCondition(Expression* expr,
-                      HBasicBlock* true_graph,
-                      HBasicBlock* false_graph,
-                      bool invert_true,
-                      bool invert_false);
+                       HBasicBlock* false_block);
+
   // Visit an argument and wrap it in a PushArgument instruction.
   HValue* VisitArgument(Expression* expr);
   void VisitArgumentList(ZoneList<Expression*>* arguments);
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-ia32.cc Thu Dec 16 00:58:42 2010 +++ /branches/bleeding_edge/src/ia32/lithium-ia32.cc Thu Dec 16 05:13:36 2010
@@ -881,19 +881,6 @@


 LInstruction* LChunkBuilder::DoBlockEntry(HBlockEntry* instr) {
-  HBasicBlock* deopt_predecessor = instr->block()->deopt_predecessor();
-  if (deopt_predecessor != NULL &&
-      deopt_predecessor->inverted()) {
-    HEnvironment* env = current_block_->last_environment();
-    HValue* value = env->Pop();
-    ASSERT(value->IsConstant());
-    Handle<Object> obj = HConstant::cast(value)->handle();
- ASSERT(*obj == *Factory::true_value() || *obj == *Factory::false_value());
-    env->Push(*obj == *Factory::true_value()
-              ? current_block_->graph()->GetConstantFalse()
-              : current_block_->graph()->GetConstantTrue());
-  }
-
   return new LLabel(instr->block());
 }

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to