Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(892)

Unified Diff: src/hydrogen.h

Issue 14696015: Verify that no-side-effects scope does not add unsafe phis and does not change push-pop balance of … (Closed) Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge
Patch Set: Add test Created 7 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | src/hydrogen.cc » ('j') | src/hydrogen.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/hydrogen.h
diff --git a/src/hydrogen.h b/src/hydrogen.h
index a95424a1c9e455ffa4a9664770844aee75b28b7d..8443e928684be6c41c41712f0c914a97a61a65b7 100644
--- a/src/hydrogen.h
+++ b/src/hydrogen.h
@@ -934,11 +934,17 @@ class HGraphBuilder {
: info_(info),
graph_(NULL),
current_block_(NULL),
+ no_side_effects_scope_environment_push_pop_delta_(0),
no_side_effects_scope_count_(0) {}
virtual ~HGraphBuilder() {}
HBasicBlock* current_block() const { return current_block_; }
- void set_current_block(HBasicBlock* block) { current_block_ = block; }
+ void set_current_block(HBasicBlock* block) {
+ // Conservatively disalow adding phis by disallowing changing the block
+ // as this class has no other bottlenecks for adding phis.
+ ASSERT(SafeToAddPhiInNoSideEffectsScope());
+ current_block_ = block;
+ }
HEnvironment* environment() const {
return current_block()->last_environment();
}
@@ -965,11 +971,31 @@ class HGraphBuilder {
HReturn* AddReturn(HValue* value);
void IncrementInNoSideEffectsScope() {
+ if (no_side_effects_scope_count_ == 0) {
+ no_side_effects_scope_environment_push_pop_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_push_pop_delta_,
+ environment()->push_count() - environment()->pop_count());
+ no_side_effects_scope_environment_push_pop_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_push_pop_delta_ >= 0;
}
protected:
@@ -1332,6 +1358,7 @@ class HGraphBuilder {
CompilationInfo* info_;
HGraph* graph_;
HBasicBlock* current_block_;
+ int no_side_effects_scope_environment_push_pop_delta_;
int no_side_effects_scope_count_;
};
« no previous file with comments | « no previous file | src/hydrogen.cc » ('j') | src/hydrogen.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698