Revision: 14610
Author:   [email protected]
Date:     Fri May 10 02:26:53 2013
Log: Verify that no-side-effects scope does not add unsafe phis and does not change push-pop balance of the environment.

[email protected]
BUG=v8:2671

Review URL: https://chromiumcodereview.appspot.com/14696015
http://code.google.com/p/v8/source/detail?r=14610

Modified:
 /branches/bleeding_edge/src/hydrogen.cc
 /branches/bleeding_edge/src/hydrogen.h

=======================================
--- /branches/bleeding_edge/src/hydrogen.cc     Fri May 10 01:58:58 2013
+++ /branches/bleeding_edge/src/hydrogen.cc     Fri May 10 02:26:53 2013
@@ -752,6 +752,7 @@
       compare->SetSuccessorAt(1, split_edge);
     }
     split_edge->GotoNoSimulate(split_edge_merge_block_);
+    ASSERT(builder_->SafeToAddPhiInNoSideEffectsScope());
   } else {
     compare->SetSuccessorAt(0, first_true_block_);
     compare->SetSuccessorAt(1, first_false_block_);
@@ -769,6 +770,7 @@
     split_edge_merge_block_ =
         builder_->CreateBasicBlock(env->Copy());
     first_true_block_->GotoNoSimulate(split_edge_merge_block_);
+    ASSERT(builder_->SafeToAddPhiInNoSideEffectsScope());
     first_true_block_ = split_edge_merge_block_;
   }
   builder_->set_current_block(first_false_block_);
@@ -783,6 +785,7 @@
   if (split_edge_merge_block_ == NULL) {
     split_edge_merge_block_ = builder_->CreateBasicBlock(env->Copy());
     first_false_block_->GotoNoSimulate(split_edge_merge_block_);
+    ASSERT(builder_->SafeToAddPhiInNoSideEffectsScope());
     first_false_block_ = split_edge_merge_block_;
   }
   builder_->set_current_block(first_true_block_);
@@ -814,6 +817,7 @@
// Handle if's without any expressions, they jump directly to the "else"
     // branch.
     builder_->current_block()->GotoNoSimulate(first_false_block_);
+    ASSERT(builder_->SafeToAddPhiInNoSideEffectsScope());
     first_true_block_ = NULL;
   }
   builder_->set_current_block(first_true_block_);
@@ -876,6 +880,7 @@
       ASSERT(!last_false_block->IsFinished());
       last_true_block_->GotoNoSimulate(merge_block_);
       last_false_block->GotoNoSimulate(merge_block_);
+      ASSERT(builder_->SafeToAddPhiInNoSideEffectsScope());
       builder_->set_current_block(merge_block_);
     }
   }
@@ -908,6 +913,7 @@
   phi_->ChangeRepresentation(Representation::Integer32());
   env->Push(initial);
   builder_->current_block()->GotoNoSimulate(header_block_);
+  ASSERT(builder_->SafeToAddPhiInNoSideEffectsScope());

   HEnvironment* body_env = env->Copy();
   HEnvironment* exit_env = env->Copy();
@@ -962,6 +968,7 @@
// Push the new increment value on the expression stack to merge into the phi.
   builder_->environment()->Push(increment_);
   builder_->current_block()->GotoNoSimulate(header_block_);
+  ASSERT(builder_->SafeToAddPhiInNoSideEffectsScope());
   header_block_->loop_information()->RegisterBackEdge(body_block_);

   builder_->set_current_block(exit_block_);
@@ -8181,6 +8188,7 @@
           access->set_position(position);
         }
         if_jsarray->GotoNoSimulate(join);
+        ASSERT(SafeToAddPhiInNoSideEffectsScope());

         set_current_block(if_fastobject);
length = AddInstruction(new(zone()) HFixedArrayBaseLength(elements));
@@ -8207,6 +8215,7 @@
         Push(access);
       }
       current_block()->GotoNoSimulate(join);
+      ASSERT(SafeToAddPhiInNoSideEffectsScope());
       set_current_block(if_false);
     }
   }
=======================================
--- /branches/bleeding_edge/src/hydrogen.h      Tue May  7 14:01:53 2013
+++ /branches/bleeding_edge/src/hydrogen.h      Fri May 10 02:26:53 2013
@@ -934,6 +934,7 @@
       : info_(info),
         graph_(NULL),
         current_block_(NULL),
+        no_side_effects_scope_environment_delta_(0),
         no_side_effects_scope_count_(0) {}
   virtual ~HGraphBuilder() {}

@@ -965,11 +966,31 @@
   HReturn* AddReturn(HValue* value);

   void IncrementInNoSideEffectsScope() {
+    if (no_side_effects_scope_count_ == 0) {
+      no_side_effects_scope_environment_delta_ =
+          environment()->push_count() - environment()->pop_count();
+    }
     no_side_effects_scope_count_++;
   }

   void DecrementInNoSideEffectsScope() {
     no_side_effects_scope_count_--;
+    if (no_side_effects_scope_count_ == 0) {
+      // No-side-effects scope should not change push-pop delta.
+      ASSERT_EQ(no_side_effects_scope_environment_delta_,
+                environment()->push_count() - environment()->pop_count());
+      no_side_effects_scope_environment_delta_ = 0;
+    }
+  }
+
+  bool SafeToAddPhiInNoSideEffectsScope() {
+    // Pops and pushes after a simulate are not visible in LChunkBuilder.
+    // If the number of pops is greater than the number pushes then the
+    // environment in HGraphBuilder is shorter then the corresponding
+    // environment in LChunkBuilder. This causes non-observable phis
+    // to be pushed in the environment, which breaks deoptimization.
+    return no_side_effects_scope_count_ == 0 ||
+           no_side_effects_scope_environment_delta_ >= 0;
   }

  protected:
@@ -1332,6 +1353,7 @@
   CompilationInfo* info_;
   HGraph* graph_;
   HBasicBlock* current_block_;
+  int no_side_effects_scope_environment_delta_;
   int no_side_effects_scope_count_;
 };

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to