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

Unified Diff: src/hydrogen.cc

Issue 12957005: Fix bug in previous_ast_id tracking and generated stores (Closed) Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge
Patch Set: Remove unrelated change Created 7 years, 9 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 | « src/hydrogen.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 6fac3021038d6f061df07f6c1e6b3460b3d1846c..bd9863f6ab7661bdbb54de132053a7b32b85090e 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -113,9 +113,11 @@ void HBasicBlock::AddInstruction(HInstruction* instr) {
ASSERT(!IsStartBlock() || !IsFinished());
ASSERT(!instr->IsLinked());
ASSERT(!IsFinished());
+ // Make sure that we never add instructions without knowing
+ // what the previous ast id is.
+ ASSERT(instr->IsSimulate() || instr->IsGoto() ||
+ !last_environment()->previous_ast_id().IsNone());
if (first_ == NULL) {
- ASSERT(last_environment() != NULL);
- ASSERT(!last_environment()->ast_id().IsNone());
HBlockEntry* entry = new(zone()) HBlockEntry();
entry->InitializeAsFirst(this);
first_ = last_ = entry;
@@ -213,10 +215,12 @@ void HBasicBlock::AddLeaveInlined(HValue* return_value,
}
-void HBasicBlock::SetInitialEnvironment(HEnvironment* env) {
+void HBasicBlock::SetInitialEnvironment(HEnvironment* env,
+ BailoutId previous_ast_id) {
ASSERT(!HasEnvironment());
ASSERT(first() == NULL);
UpdateEnvironment(env);
+ env->set_previous_ast_id(previous_ast_id);
}
@@ -227,13 +231,16 @@ void HBasicBlock::SetJoinId(BailoutId ast_id) {
HBasicBlock* predecessor = predecessors_[i];
ASSERT(predecessor->end()->IsGoto());
HSimulate* simulate = HSimulate::cast(predecessor->end()->previous());
- // We only need to verify the ID once.
ASSERT(i != 0 ||
(predecessor->last_environment()->closure().is_null() ||
predecessor->last_environment()->closure()->shared()
->VerifyBailoutId(ast_id)));
simulate->set_ast_id(ast_id);
- predecessor->last_environment()->set_ast_id(ast_id);
+ }
+ HEnvironment* last_environment = this->last_environment();
+ ASSERT(last_environment || IsFinished());
+ if (last_environment != NULL) {
+ last_environment->set_previous_ast_id(ast_id);
}
}
@@ -294,7 +301,9 @@ void HBasicBlock::RegisterPredecessor(HBasicBlock* pred) {
}
} else if (!HasEnvironment() && !IsFinished()) {
ASSERT(!IsLoopHeader());
- SetInitialEnvironment(pred->last_environment()->Copy());
+ HEnvironment* new_env = pred->last_environment()->Copy();
+ SetInitialEnvironment(new_env,
+ pred->last_environment()->previous_ast_id());
}
predecessors_.Add(pred, zone());
@@ -545,18 +554,6 @@ void HGraph::Verify(bool do_full_verify) const {
HPhi* phi = block->phis()->at(j);
phi->Verify();
}
-
- // Check that all join blocks have predecessors that end with an
- // unconditional goto and agree on their environment node id.
- if (block->predecessors()->length() >= 2) {
- BailoutId id =
- block->predecessors()->first()->last_environment()->ast_id();
- for (int k = 0; k < block->predecessors()->length(); k++) {
- HBasicBlock* predecessor = block->predecessors()->at(k);
- ASSERT(predecessor->end()->IsGoto());
- ASSERT(predecessor->last_environment()->ast_id() == id);
- }
- }
}
// Check special property of first block to have no predecessors.
@@ -640,13 +637,13 @@ DEFINE_GET_CONSTANT(Hole, the_hole, HType::Tagged(), false)
#undef DEFINE_GET_CONSTANT
-HGraphBuilder::CheckBuilder::CheckBuilder(HGraphBuilder* builder, BailoutId id)
+HGraphBuilder::CheckBuilder::CheckBuilder(HGraphBuilder* builder)
: builder_(builder),
finished_(false),
- id_(id) {
+ id_(builder->current_block()->last_environment()->previous_ast_id()) {
HEnvironment* env = builder->environment();
- failure_block_ = builder->CreateBasicBlock(env->CopyWithoutHistory());
- merge_block_ = builder->CreateBasicBlock(env->CopyWithoutHistory());
+ failure_block_ = builder->CreateBasicBlock(env->CopyWithoutHistory(), id_);
+ merge_block_ = builder->CreateBasicBlock(env->CopyWithoutHistory(), id_);
}
@@ -655,9 +652,9 @@ HValue* HGraphBuilder::CheckBuilder::CheckNotUndefined(HValue* value) {
HIsNilAndBranch* compare =
new(zone()) HIsNilAndBranch(value, kStrictEquality, kUndefinedValue);
HBasicBlock* success_block =
- builder_->CreateBasicBlock(env->CopyWithoutHistory());
+ builder_->CreateBasicBlock(env->CopyWithoutHistory(), id_);
HBasicBlock* failure_block =
- builder_->CreateBasicBlock(env->CopyWithoutHistory());
+ builder_->CreateBasicBlock(env->CopyWithoutHistory(), id_);
compare->SetSuccessorAt(0, failure_block);
compare->SetSuccessorAt(1, success_block);
failure_block->Goto(failure_block_);
@@ -675,9 +672,9 @@ HValue* HGraphBuilder::CheckBuilder::CheckIntegerCompare(HValue* left,
new(zone()) HCompareIDAndBranch(left, right, op);
compare->AssumeRepresentation(Representation::Integer32());
HBasicBlock* success_block =
- builder_->CreateBasicBlock(env->CopyWithoutHistory());
+ builder_->CreateBasicBlock(env->CopyWithoutHistory(), id_);
HBasicBlock* failure_block =
- builder_->CreateBasicBlock(env->CopyWithoutHistory());
+ builder_->CreateBasicBlock(env->CopyWithoutHistory(), id_);
compare->SetSuccessorAt(0, success_block);
compare->SetSuccessorAt(1, failure_block);
failure_block->Goto(failure_block_);
@@ -696,8 +693,8 @@ HValue* HGraphBuilder::CheckBuilder::CheckIntegerEq(HValue* left,
void HGraphBuilder::CheckBuilder::End() {
ASSERT(!finished_);
builder_->current_block()->Goto(merge_block_);
- failure_block_->FinishExitWithDeoptimization(HDeoptimize::kUseAll);
failure_block_->SetJoinId(id_);
+ failure_block_->FinishExitWithDeoptimization(HDeoptimize::kUseAll);
builder_->set_current_block(merge_block_);
merge_block_->SetJoinId(id_);
finished_ = true;
@@ -709,15 +706,15 @@ HConstant* HGraph::GetInvalidContext() {
}
-HGraphBuilder::IfBuilder::IfBuilder(HGraphBuilder* builder, BailoutId id)
+HGraphBuilder::IfBuilder::IfBuilder(HGraphBuilder* builder)
: builder_(builder),
finished_(false),
did_else_(false),
- id_(id) {
+ id_(builder->current_block()->last_environment()->previous_ast_id()) {
HEnvironment* env = builder->environment();
- first_true_block_ = builder->CreateBasicBlock(env->Copy());
+ first_true_block_ = builder->CreateBasicBlock(env->Copy(), id_);
last_true_block_ = NULL;
- first_false_block_ = builder->CreateBasicBlock(env->Copy());
+ first_false_block_ = builder->CreateBasicBlock(env->Copy(), id_);
}
@@ -778,7 +775,7 @@ void HGraphBuilder::IfBuilder::End() {
ASSERT(!last_false_block->IsFinished());
HEnvironment* merge_env =
last_true_block_->last_environment()->CopyWithoutHistory();
- merge_block_ = builder_->CreateBasicBlock(merge_env);
+ merge_block_ = builder_->CreateBasicBlock(merge_env, id_);
last_true_block_->Goto(merge_block_);
last_false_block->Goto(merge_block_);
merge_block_->SetJoinId(id_);
@@ -789,14 +786,13 @@ void HGraphBuilder::IfBuilder::End() {
HGraphBuilder::LoopBuilder::LoopBuilder(HGraphBuilder* builder,
HValue* context,
- LoopBuilder::Direction direction,
- BailoutId id)
+ LoopBuilder::Direction direction)
: builder_(builder),
context_(context),
direction_(direction),
- id_(id),
+ id_(builder->current_block()->last_environment()->previous_ast_id()),
finished_(false) {
- header_block_ = builder->CreateLoopHeaderBlock();
+ header_block_ = builder->CreateLoopHeaderBlock(id_);
body_block_ = NULL;
exit_block_ = NULL;
}
@@ -817,8 +813,8 @@ HValue* HGraphBuilder::LoopBuilder::BeginBody(
HEnvironment* body_env = env->Copy();
HEnvironment* exit_env = env->Copy();
- body_block_ = builder_->CreateBasicBlock(body_env);
- exit_block_ = builder_->CreateBasicBlock(exit_env);
+ body_block_ = builder_->CreateBasicBlock(body_env, id_);
+ exit_block_ = builder_->CreateBasicBlock(exit_env, id_);
// Remove the phi from the expression stack
body_env->Pop();
@@ -899,7 +895,7 @@ void HGraphBuilder::AddSimulate(BailoutId id,
RemovableSimulate removable) {
ASSERT(current_block() != NULL);
current_block()->AddSimulate(id, removable);
- environment()->set_ast_id(id);
+ environment()->set_previous_ast_id(id);
}
@@ -934,17 +930,18 @@ HReturn* HGraphBuilder::AddReturn(HValue* value) {
}
-HBasicBlock* HGraphBuilder::CreateBasicBlock(HEnvironment* env) {
+HBasicBlock* HGraphBuilder::CreateBasicBlock(HEnvironment* env,
+ BailoutId previous_ast_id) {
HBasicBlock* b = graph()->CreateBasicBlock();
- b->SetInitialEnvironment(env);
+ b->SetInitialEnvironment(env, previous_ast_id);
return b;
}
-HBasicBlock* HGraphBuilder::CreateLoopHeaderBlock() {
+HBasicBlock* HGraphBuilder::CreateLoopHeaderBlock(BailoutId previous_ast_id) {
HBasicBlock* header = graph()->CreateBasicBlock();
HEnvironment* entry_env = environment()->CopyAsLoopHeader(header);
- header->SetInitialEnvironment(entry_env);
+ header->SetInitialEnvironment(entry_env, previous_ast_id);
header->AttachLoopInformation();
return header;
}
@@ -1044,16 +1041,16 @@ HValue* HGraphBuilder::BuildCheckForCapacityGrow(HValue* object,
HValue* length,
HValue* key,
bool is_js_array) {
- BailoutId ast_id = environment()->ast_id();
+ BailoutId ast_id = current_block()->last_environment()->previous_ast_id();
Zone* zone = this->zone();
- IfBuilder length_checker(this, ast_id);
+ IfBuilder length_checker(this);
length_checker.BeginIf(length, key, Token::EQ);
HValue* current_capacity =
AddInstruction(new(zone) HFixedArrayBaseLength(elements));
- IfBuilder capacity_checker(this, ast_id);
+ IfBuilder capacity_checker(this);
capacity_checker.BeginIf(length, current_capacity, Token::EQ);
@@ -1064,7 +1061,7 @@ HValue* HGraphBuilder::BuildCheckForCapacityGrow(HValue* object,
HValue* new_elements = BuildGrowElementsCapacity(object, elements,
kind, length,
- new_capacity, ast_id);
+ new_capacity);
environment()->Push(new_elements);
capacity_checker.BeginElse();
@@ -1104,11 +1101,10 @@ HValue* HGraphBuilder::BuildCopyElementsOnWrite(HValue* object,
HValue* elements,
ElementsKind kind,
HValue* length) {
- BailoutId ast_id = environment()->ast_id();
Zone* zone = this->zone();
Heap* heap = isolate()->heap();
- IfBuilder cow_checker(this, ast_id);
+ IfBuilder cow_checker(this);
cow_checker.BeginIfMapEquals(elements,
Handle<Map>(heap->fixed_cow_array_map()));
@@ -1118,7 +1114,7 @@ HValue* HGraphBuilder::BuildCopyElementsOnWrite(HValue* object,
HValue* new_elements = BuildGrowElementsCapacity(object, elements,
kind, length,
- capacity, ast_id);
+ capacity);
environment()->Push(new_elements);
@@ -1180,11 +1176,9 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
HLoadExternalArrayPointer* external_elements =
new(zone) HLoadExternalArrayPointer(elements);
AddInstruction(external_elements);
- BailoutId previous_id = environment()->ast_id();
- ASSERT(!previous_id.IsNone());
- IfBuilder length_checker(this, previous_id);
+ IfBuilder length_checker(this);
length_checker.BeginIf(key, length, Token::LT);
- CheckBuilder negative_checker(this, previous_id);
+ CheckBuilder negative_checker(this);
HValue* bounds_check = negative_checker.CheckIntegerCompare(
key, graph()->GetConstant0(), Token::GTE);
negative_checker.End();
@@ -1243,8 +1237,8 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
HValue* HGraphBuilder::BuildAllocateElements(HValue* context,
ElementsKind kind,
- HValue* capacity,
- BailoutId ast_id) {
+ HValue* capacity) {
+ BailoutId ast_id = current_block()->last_environment()->previous_ast_id();
Zone* zone = this->zone();
int elements_size = IsFastDoubleElementsKind(kind)
@@ -1371,19 +1365,18 @@ HValue* HGraphBuilder::BuildGrowElementsCapacity(HValue* object,
HValue* elements,
ElementsKind kind,
HValue* length,
- HValue* new_capacity,
- BailoutId ast_id) {
+ HValue* new_capacity) {
Zone* zone = this->zone();
HValue* context = environment()->LookupContext();
BuildNewSpaceArrayCheck(new_capacity, kind);
HValue* new_elements =
- BuildAllocateElements(context, kind, new_capacity, ast_id);
+ BuildAllocateElements(context, kind, new_capacity);
BuildCopyElements(context, elements, kind,
new_elements, kind,
- length, new_capacity, ast_id);
+ length, new_capacity);
Factory* factory = isolate()->factory();
HInstruction* elements_store = AddInstruction(new(zone) HStoreNamedField(
@@ -1401,8 +1394,8 @@ void HGraphBuilder::BuildFillElementsWithHole(HValue* context,
HValue* elements,
ElementsKind elements_kind,
HValue* from,
- HValue* to,
- BailoutId ast_id) {
+ HValue* to) {
+ BailoutId ast_id = current_block()->last_environment()->previous_ast_id();
// Fast elements kinds need to be initialized in case statements below cause
// a garbage collection.
Factory* factory = isolate()->factory();
@@ -1415,7 +1408,7 @@ void HGraphBuilder::BuildFillElementsWithHole(HValue* context,
: AddInstruction(new(zone) HConstant(nan_double,
Representation::Double()));
- LoopBuilder builder(this, context, LoopBuilder::kPostIncrement, ast_id);
+ LoopBuilder builder(this, context, LoopBuilder::kPostIncrement);
HValue* key = builder.BeginBody(from, to, Token::LT);
@@ -1432,8 +1425,8 @@ void HGraphBuilder::BuildCopyElements(HValue* context,
HValue* to_elements,
ElementsKind to_elements_kind,
HValue* length,
- HValue* capacity,
- BailoutId ast_id) {
+ HValue* capacity) {
+ BailoutId ast_id = current_block()->last_environment()->previous_ast_id();
bool pre_fill_with_holes =
IsFastDoubleElementsKind(from_elements_kind) &&
IsFastObjectElementsKind(to_elements_kind);
@@ -1443,10 +1436,10 @@ void HGraphBuilder::BuildCopyElements(HValue* context,
// pre-initialized with holes to make sure that it's always in a consistent
// state.
BuildFillElementsWithHole(context, to_elements, to_elements_kind,
- graph()->GetConstant0(), capacity, ast_id);
+ graph()->GetConstant0(), capacity);
}
- LoopBuilder builder(this, context, LoopBuilder::kPostIncrement, ast_id);
+ LoopBuilder builder(this, context, LoopBuilder::kPostIncrement);
HValue* key = builder.BeginBody(graph()->GetConstant0(), length, Token::LT);
@@ -1464,7 +1457,7 @@ void HGraphBuilder::BuildCopyElements(HValue* context,
if (!pre_fill_with_holes && length != capacity) {
// Fill unused capacity with the hole.
BuildFillElementsWithHole(context, to_elements, to_elements_kind,
- key, capacity, ast_id);
+ key, capacity);
}
}
@@ -1562,9 +1555,9 @@ HGraph::HGraph(CompilationInfo* info)
start_environment_ =
new(zone_) HEnvironment(NULL, info->scope(), info->closure(), zone_);
}
- start_environment_->set_ast_id(BailoutId::FunctionEntry());
entry_block_ = CreateBasicBlock();
- entry_block_->SetInitialEnvironment(start_environment_);
+ entry_block_->SetInitialEnvironment(start_environment_,
+ BailoutId::FunctionEntry());
}
@@ -4149,7 +4142,8 @@ bool HOptimizedGraphBuilder::BuildGraph() {
// values (but not instructions), present in the initial environment and
// not replayed by the Lithium translation.
HEnvironment* initial_env = environment()->CopyWithoutHistory();
- HBasicBlock* body_entry = CreateBasicBlock(initial_env);
+ HBasicBlock* body_entry = CreateBasicBlock(initial_env,
+ BailoutId::FunctionEntry());
current_block()->Goto(body_entry);
body_entry->SetJoinId(BailoutId::FunctionEntry());
set_current_block(body_entry);
@@ -5407,7 +5401,7 @@ void HOptimizedGraphBuilder::VisitDoWhileStatement(DoWhileStatement* stmt) {
ASSERT(current_block()->HasPredecessor());
ASSERT(current_block() != NULL);
bool osr_entry = PreProcessOsrEntry(stmt);
- HBasicBlock* loop_entry = CreateLoopHeaderBlock();
+ HBasicBlock* loop_entry = CreateLoopHeaderBlock(stmt->EntryId());
current_block()->Goto(loop_entry);
set_current_block(loop_entry);
if (osr_entry) graph()->set_osr_loop_entry(loop_entry);
@@ -5450,7 +5444,7 @@ void HOptimizedGraphBuilder::VisitWhileStatement(WhileStatement* stmt) {
ASSERT(current_block()->HasPredecessor());
ASSERT(current_block() != NULL);
bool osr_entry = PreProcessOsrEntry(stmt);
- HBasicBlock* loop_entry = CreateLoopHeaderBlock();
+ HBasicBlock* loop_entry = CreateLoopHeaderBlock(stmt->EntryId());
current_block()->Goto(loop_entry);
set_current_block(loop_entry);
if (osr_entry) graph()->set_osr_loop_entry(loop_entry);
@@ -5497,7 +5491,7 @@ void HOptimizedGraphBuilder::VisitForStatement(ForStatement* stmt) {
}
ASSERT(current_block() != NULL);
bool osr_entry = PreProcessOsrEntry(stmt);
- HBasicBlock* loop_entry = CreateLoopHeaderBlock();
+ HBasicBlock* loop_entry = CreateLoopHeaderBlock(stmt->EntryId());
current_block()->Goto(loop_entry);
set_current_block(loop_entry);
if (osr_entry) graph()->set_osr_loop_entry(loop_entry);
@@ -5592,7 +5586,7 @@ void HOptimizedGraphBuilder::VisitForInStatement(ForInStatement* stmt) {
HForInCacheArray::cast(index_cache));
bool osr_entry = PreProcessOsrEntry(stmt);
- HBasicBlock* loop_entry = CreateLoopHeaderBlock();
+ HBasicBlock* loop_entry = CreateLoopHeaderBlock(stmt->EntryId());
current_block()->Goto(loop_entry);
set_current_block(loop_entry);
if (osr_entry) graph()->set_osr_loop_entry(loop_entry);
@@ -8086,7 +8080,7 @@ bool HOptimizedGraphBuilder::TryInline(CallKind call_kind,
AddSimulate(return_id);
current_block()->UpdateEnvironment(inner_env);
-
+ inner_env->set_previous_ast_id(BailoutId::FunctionEntry());
ZoneList<HValue*>* arguments_values = NULL;
// If the function uses arguments copy current arguments values
@@ -10609,6 +10603,7 @@ HEnvironment::HEnvironment(HEnvironment* outer,
pop_count_(0),
push_count_(0),
ast_id_(BailoutId::None()),
+ previous_ast_id_(BailoutId::None()),
zone_(zone) {
Initialize(scope->num_parameters() + 1, scope->num_stack_slots(), 0);
}
@@ -10625,6 +10620,7 @@ HEnvironment::HEnvironment(Zone* zone, int parameter_count)
pop_count_(0),
push_count_(0),
ast_id_(BailoutId::None()),
+ previous_ast_id_(BailoutId::None()),
zone_(zone) {
Initialize(parameter_count, 0, 0);
}
@@ -10641,6 +10637,7 @@ HEnvironment::HEnvironment(const HEnvironment* other, Zone* zone)
pop_count_(0),
push_count_(0),
ast_id_(other->ast_id()),
+ previous_ast_id_(BailoutId::None()),
zone_(zone) {
Initialize(other);
}
@@ -10661,6 +10658,7 @@ HEnvironment::HEnvironment(HEnvironment* outer,
pop_count_(0),
push_count_(0),
ast_id_(BailoutId::None()),
+ previous_ast_id_(BailoutId::None()),
zone_(zone) {
}
« no previous file with comments | « src/hydrogen.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698